From cfe06056165eb5716586b2fb2e368acf26f756e5 Mon Sep 17 00:00:00 2001 From: "Jonathan A. Sternberg" Date: Tue, 18 Feb 2025 12:06:46 -0600 Subject: [PATCH 1/2] cli-plugins: merge OTEL_RESOURCE_ATTRIBUTES environment variable Merge `OTEL_RESOURCE_ATTRIBUTES` when there is one already in the environment. This allows user-specified resource attributes to be passed on to CLI plugins while still allowing the extra attributes added for telemetry information. This was the original intended use-case but it seems to have never made it in. The reason `OTEL_RESOURCE_ATTRIBUTES` was used is because we could combine it with user-centric ones. Signed-off-by: Jonathan A. Sternberg --- cli-plugins/manager/cobra.go | 40 +++++++++++++++++++++++++++---- cli-plugins/manager/cobra_test.go | 26 ++++++++++++++++++++ 2 files changed, 61 insertions(+), 5 deletions(-) create mode 100644 cli-plugins/manager/cobra_test.go diff --git a/cli-plugins/manager/cobra.go b/cli-plugins/manager/cobra.go index 4bfa06fa5c0a..08a4de7ed998 100644 --- a/cli-plugins/manager/cobra.go +++ b/cli-plugins/manager/cobra.go @@ -2,14 +2,15 @@ package manager import ( "fmt" - "net/url" "os" "strings" "sync" "github.com/docker/cli/cli/command" "github.com/spf13/cobra" + "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/baggage" ) const ( @@ -136,12 +137,41 @@ func appendPluginResourceAttributesEnvvar(env []string, cmd *cobra.Command, plug if attrs := getPluginResourceAttributes(cmd, plugin); attrs.Len() > 0 { // values in environment variables need to be in baggage format // otel/baggage package can be used after update to v1.22, currently it encodes incorrectly - attrsSlice := make([]string, attrs.Len()) + // Construct baggage members for each of the attributes. + // Ignore any failures as these aren't significant and + // represent an internal issue. + var b baggage.Baggage for iter := attrs.Iter(); iter.Next(); { - i, v := iter.IndexedAttribute() - attrsSlice[i] = string(v.Key) + "=" + url.PathEscape(v.Value.AsString()) + attr := iter.Attribute() + m, err := baggage.NewMemberRaw(string(attr.Key), attr.Value.AsString()) + if err != nil { + otel.Handle(err) + continue + } + + newB, err := b.SetMember(m) + if err != nil { + otel.Handle(err) + continue + } + b = newB + } + + // Combine plugin added resource attributes with ones found in the environment + // variable. Our own attributes should be namespaced so there shouldn't be a + // conflict. We do not parse the environment variable because we do not want + // to handle errors in user configuration. + attrsSlice := make([]string, 0, 2) + if v := strings.TrimSpace(os.Getenv(ResourceAttributesEnvvar)); v != "" { + attrsSlice = append(attrsSlice, v) + } + if v := b.String(); v != "" { + attrsSlice = append(attrsSlice, v) + } + + if len(attrsSlice) > 0 { + env = append(env, ResourceAttributesEnvvar+"="+strings.Join(attrsSlice, ",")) } - env = append(env, ResourceAttributesEnvvar+"="+strings.Join(attrsSlice, ",")) } return env } diff --git a/cli-plugins/manager/cobra_test.go b/cli-plugins/manager/cobra_test.go new file mode 100644 index 000000000000..207b536f1faf --- /dev/null +++ b/cli-plugins/manager/cobra_test.go @@ -0,0 +1,26 @@ +package manager + +import ( + "testing" + + "github.com/spf13/cobra" + "gotest.tools/v3/assert" +) + +func TestPluginResourceAttributesEnvvar(t *testing.T) { + cmd := &cobra.Command{ + Annotations: map[string]string{ + cobra.CommandDisplayNameAnnotation: "docker", + }, + } + + // Ensure basic usage is fine. + env := appendPluginResourceAttributesEnvvar(nil, cmd, Plugin{Name: "compose"}) + assert.DeepEqual(t, []string{"OTEL_RESOURCE_ATTRIBUTES=docker.cli.cobra.command_path=docker%20compose"}, env) + + // Add a user-based environment variable to OTEL_RESOURCE_ATTRIBUTES. + t.Setenv("OTEL_RESOURCE_ATTRIBUTES", "a.b.c=foo") + + env = appendPluginResourceAttributesEnvvar(nil, cmd, Plugin{Name: "compose"}) + assert.DeepEqual(t, []string{"OTEL_RESOURCE_ATTRIBUTES=a.b.c=foo,docker.cli.cobra.command_path=docker%20compose"}, env) +} From 8890a1c9292ed5f2bf45df44a0e57d345f563d31 Mon Sep 17 00:00:00 2001 From: "Jonathan A. Sternberg" Date: Wed, 19 Feb 2025 10:19:00 -0600 Subject: [PATCH 2/2] cli-plugins: remove docker.cli specific otel attributes after usage Remove the `docker.cli` prefixed attributes from `OTEL_RESOURCE_ATTRIBUTES` after the telemetry provider has been created within a plugin. This prevents accidentally sending the attributes to something downstream for the user. This also fixes an issue with compose where the self-injected `OTEL_RESOURCE_ATTRIBUTES` would override an existing attribute in the environment file because the "user environment" overrode the environment file, but the "user environment" was created by the `docker` tool rather than by the user's environment. When `OTEL_RESOURCE_ATTRIBUTES` is empty after pruning, the environment variable is unset. Signed-off-by: Jonathan A. Sternberg --- cli-plugins/manager/cobra.go | 22 ++++++----------- cli-plugins/manager/manager.go | 2 +- cli/command/cli.go | 45 ++++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 15 deletions(-) diff --git a/cli-plugins/manager/cobra.go b/cli-plugins/manager/cobra.go index 08a4de7ed998..ec45045b0651 100644 --- a/cli-plugins/manager/cobra.go +++ b/cli-plugins/manager/cobra.go @@ -107,7 +107,7 @@ func AddPluginCommandStubs(dockerCli command.Cli, rootCmd *cobra.Command) (err e } const ( - dockerCliAttributePrefix = attribute.Key("docker.cli") + dockerCliAttributePrefix = command.DockerCliAttributePrefix cobraCommandPath = attribute.Key("cobra.command_path") ) @@ -126,7 +126,7 @@ func getPluginResourceAttributes(cmd *cobra.Command, plugin Plugin) attribute.Se for iter := attrSet.Iter(); iter.Next(); { attr := iter.Attribute() kvs = append(kvs, attribute.KeyValue{ - Key: dockerCliAttributePrefix + "." + attr.Key, + Key: dockerCliAttributePrefix + attr.Key, Value: attr.Value, }) } @@ -135,12 +135,10 @@ func getPluginResourceAttributes(cmd *cobra.Command, plugin Plugin) attribute.Se func appendPluginResourceAttributesEnvvar(env []string, cmd *cobra.Command, plugin Plugin) []string { if attrs := getPluginResourceAttributes(cmd, plugin); attrs.Len() > 0 { - // values in environment variables need to be in baggage format - // otel/baggage package can be used after update to v1.22, currently it encodes incorrectly // Construct baggage members for each of the attributes. // Ignore any failures as these aren't significant and // represent an internal issue. - var b baggage.Baggage + members := make([]baggage.Member, 0, attrs.Len()) for iter := attrs.Iter(); iter.Next(); { attr := iter.Attribute() m, err := baggage.NewMemberRaw(string(attr.Key), attr.Value.AsString()) @@ -148,13 +146,7 @@ func appendPluginResourceAttributesEnvvar(env []string, cmd *cobra.Command, plug otel.Handle(err) continue } - - newB, err := b.SetMember(m) - if err != nil { - otel.Handle(err) - continue - } - b = newB + members = append(members, m) } // Combine plugin added resource attributes with ones found in the environment @@ -165,8 +157,10 @@ func appendPluginResourceAttributesEnvvar(env []string, cmd *cobra.Command, plug if v := strings.TrimSpace(os.Getenv(ResourceAttributesEnvvar)); v != "" { attrsSlice = append(attrsSlice, v) } - if v := b.String(); v != "" { - attrsSlice = append(attrsSlice, v) + if b, err := baggage.New(members...); err != nil { + otel.Handle(err) + } else if b.Len() > 0 { + attrsSlice = append(attrsSlice, b.String()) } if len(attrsSlice) > 0 { diff --git a/cli-plugins/manager/manager.go b/cli-plugins/manager/manager.go index 9f795bc4987b..09b5b14e734a 100644 --- a/cli-plugins/manager/manager.go +++ b/cli-plugins/manager/manager.go @@ -26,7 +26,7 @@ const ( // ResourceAttributesEnvvar is the name of the envvar that includes additional // resource attributes for OTEL. - ResourceAttributesEnvvar = "OTEL_RESOURCE_ATTRIBUTES" + ResourceAttributesEnvvar = command.ResourceAttributesEnvvar ) // errPluginNotFound is the error returned when a plugin could not be found. diff --git a/cli/command/cli.go b/cli/command/cli.go index 227720fa07d8..1452e1a240c1 100644 --- a/cli/command/cli.go +++ b/cli/command/cli.go @@ -11,6 +11,7 @@ import ( "path/filepath" "runtime" "strconv" + "strings" "sync" "time" @@ -292,6 +293,7 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions, ops ...CLIOption) if cli.enableGlobalTracer { cli.createGlobalTracerProvider(cli.baseCtx) } + filterResourceAttributesEnvvar() return nil } @@ -591,3 +593,46 @@ func DefaultContextStoreConfig() store.Config { defaultStoreEndpoints..., ) } + +const ( + // ResourceAttributesEnvvar is the name of the envvar that includes additional + // resource attributes for OTEL. + ResourceAttributesEnvvar = "OTEL_RESOURCE_ATTRIBUTES" + + // DockerCliAttributePrefix is the prefix for any docker cli OTEL attributes. + DockerCliAttributePrefix = "docker.cli." +) + +func filterResourceAttributesEnvvar() { + if v := os.Getenv(ResourceAttributesEnvvar); v != "" { + if filtered := filterResourceAttributes(v); filtered != "" { + os.Setenv(ResourceAttributesEnvvar, filtered) + } else { + os.Unsetenv(ResourceAttributesEnvvar) + } + } +} + +func filterResourceAttributes(s string) string { + if trimmed := strings.TrimSpace(s); trimmed == "" { + return trimmed + } + + pairs := strings.Split(s, ",") + elems := make([]string, 0, len(pairs)) + for _, p := range pairs { + k, _, found := strings.Cut(p, "=") + if !found { + // Do not interact with invalid otel resources. + elems = append(elems, p) + continue + } + + // Skip attributes that have our docker.cli prefix. + if strings.HasPrefix(k, DockerCliAttributePrefix) { + continue + } + elems = append(elems, p) + } + return strings.Join(elems, ",") +}