From 9dc175d6ef7057c0e86d3a097b86012676fe4a95 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 3 Mar 2025 14:21:45 +0100 Subject: [PATCH 1/2] cli/command: un-export ResourceAttributesEnvvar, DockerCliAttributePrefix These utility functions were added in 8890a1c9292ed5f2bf45df44a0e57d345f563d31, and are all related to OTEL. The ResourceAttributesEnvvar const defines the "OTEL_RESOURCE_ATTRIBUTES" environment-variable to use, which is part of the [OpenTelemetry specification], so should be considered a well-known env-var, and not up to us to define a const for. These code-changes were not yet included in a release, so we don't have to deprecate. This patch: - Moves the utility functions to the telemetry files, so that all code related to OpenTelemetry is together. - Un-exports the ResourceAttributesEnvvar to reduce our public API. - Un-exports the DockerCliAttributePrefix to reduce depdency on cli/command in CLI-plugins, but adds a TODO to move telemetry-related code to a common (internal) package. - Deprecates the cli-plugins/manager.ResourceAttributesEnvvar const. This const has no known consumers, so we could skip deprecation, but just in case some codebase uses this. [OpenTelemetry specification]: https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/#general-sdk-configuration Signed-off-by: Sebastiaan van Stijn --- cli-plugins/manager/cobra.go | 18 +++++++++---- cli-plugins/manager/manager.go | 4 ++- cli/command/cli.go | 44 ------------------------------- cli/command/telemetry.go | 47 ++++++++++++++++++++++++++++++++++ 4 files changed, 63 insertions(+), 50 deletions(-) diff --git a/cli-plugins/manager/cobra.go b/cli-plugins/manager/cobra.go index ec45045b06..ba3fedcd9a 100644 --- a/cli-plugins/manager/cobra.go +++ b/cli-plugins/manager/cobra.go @@ -107,9 +107,17 @@ func AddPluginCommandStubs(dockerCli command.Cli, rootCmd *cobra.Command) (err e } const ( - dockerCliAttributePrefix = command.DockerCliAttributePrefix + // resourceAttributesEnvVar is the name of the envvar that includes additional + // resource attributes for OTEL as defined in the [OpenTelemetry specification]. + // + // [OpenTelemetry specification]: https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/#general-sdk-configuration + resourceAttributesEnvVar = "OTEL_RESOURCE_ATTRIBUTES" - cobraCommandPath = attribute.Key("cobra.command_path") + // dockerCLIAttributePrefix is the prefix for any docker cli OTEL attributes. + // + // It is a copy of the const defined in [command.dockerCLIAttributePrefix]. + dockerCLIAttributePrefix = "docker.cli." + cobraCommandPath = attribute.Key("cobra.command_path") ) func getPluginResourceAttributes(cmd *cobra.Command, plugin Plugin) attribute.Set { @@ -126,7 +134,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, }) } @@ -154,7 +162,7 @@ func appendPluginResourceAttributesEnvvar(env []string, cmd *cobra.Command, plug // 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 != "" { + if v := strings.TrimSpace(os.Getenv(resourceAttributesEnvVar)); v != "" { attrsSlice = append(attrsSlice, v) } if b, err := baggage.New(members...); err != nil { @@ -164,7 +172,7 @@ func appendPluginResourceAttributesEnvvar(env []string, cmd *cobra.Command, plug } 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/manager.go b/cli-plugins/manager/manager.go index 09b5b14e73..91eeed19a4 100644 --- a/cli-plugins/manager/manager.go +++ b/cli-plugins/manager/manager.go @@ -26,7 +26,9 @@ const ( // ResourceAttributesEnvvar is the name of the envvar that includes additional // resource attributes for OTEL. - ResourceAttributesEnvvar = command.ResourceAttributesEnvvar + // + // Deprecated: The "OTEL_RESOURCE_ATTRIBUTES" env-var is part of the OpenTelemetry specification; users should define their own const for this. This const will be removed in the next release. + ResourceAttributesEnvvar = "OTEL_RESOURCE_ATTRIBUTES" ) // 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 1452e1a240..1c0a9ffab0 100644 --- a/cli/command/cli.go +++ b/cli/command/cli.go @@ -11,7 +11,6 @@ import ( "path/filepath" "runtime" "strconv" - "strings" "sync" "time" @@ -593,46 +592,3 @@ 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, ",") -} diff --git a/cli/command/telemetry.go b/cli/command/telemetry.go index 9804a77a59..e8e6296b5a 100644 --- a/cli/command/telemetry.go +++ b/cli/command/telemetry.go @@ -4,6 +4,7 @@ import ( "context" "os" "path/filepath" + "strings" "sync" "time" @@ -216,3 +217,49 @@ func (r *cliReader) ForceFlush(ctx context.Context) error { func deltaTemporality(_ sdkmetric.InstrumentKind) metricdata.Temporality { return metricdata.DeltaTemporality } + +// resourceAttributesEnvVar is the name of the envvar that includes additional +// resource attributes for OTEL as defined in the [OpenTelemetry specification]. +// +// [OpenTelemetry specification]: https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/#general-sdk-configuration +const resourceAttributesEnvVar = "OTEL_RESOURCE_ATTRIBUTES" + +func filterResourceAttributesEnvvar() { + if v := os.Getenv(resourceAttributesEnvVar); v != "" { + if filtered := filterResourceAttributes(v); filtered != "" { + _ = os.Setenv(resourceAttributesEnvVar, filtered) + } else { + _ = os.Unsetenv(resourceAttributesEnvVar) + } + } +} + +// dockerCLIAttributePrefix is the prefix for any docker cli OTEL attributes. +// When updating, make sure to also update the copy in cli-plugins/manager. +// +// TODO(thaJeztah): move telemetry-related code to an (internal) package to reduce dependency on cli/command in cli-plugins, which has too many imports. +const dockerCLIAttributePrefix = "docker.cli." + +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, ",") +} From 8bedb69f2c48ff15a9d9efbb5553b52efd1518b4 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 3 Mar 2025 14:24:00 +0100 Subject: [PATCH 2/2] cli-plugins/manager: move OTEL-related code to separate file Signed-off-by: Sebastiaan van Stijn --- cli-plugins/manager/cobra.go | 76 ----------------------------- cli-plugins/manager/telemetry.go | 84 ++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 76 deletions(-) create mode 100644 cli-plugins/manager/telemetry.go diff --git a/cli-plugins/manager/cobra.go b/cli-plugins/manager/cobra.go index ba3fedcd9a..73dcc9cce1 100644 --- a/cli-plugins/manager/cobra.go +++ b/cli-plugins/manager/cobra.go @@ -3,14 +3,10 @@ package manager import ( "fmt" "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 ( @@ -105,75 +101,3 @@ func AddPluginCommandStubs(dockerCli command.Cli, rootCmd *cobra.Command) (err e }) return err } - -const ( - // resourceAttributesEnvVar is the name of the envvar that includes additional - // resource attributes for OTEL as defined in the [OpenTelemetry specification]. - // - // [OpenTelemetry specification]: https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/#general-sdk-configuration - resourceAttributesEnvVar = "OTEL_RESOURCE_ATTRIBUTES" - - // dockerCLIAttributePrefix is the prefix for any docker cli OTEL attributes. - // - // It is a copy of the const defined in [command.dockerCLIAttributePrefix]. - dockerCLIAttributePrefix = "docker.cli." - cobraCommandPath = attribute.Key("cobra.command_path") -) - -func getPluginResourceAttributes(cmd *cobra.Command, plugin Plugin) attribute.Set { - commandPath := cmd.Annotations[CommandAnnotationPluginCommandPath] - if commandPath == "" { - commandPath = fmt.Sprintf("%s %s", cmd.CommandPath(), plugin.Name) - } - - attrSet := attribute.NewSet( - cobraCommandPath.String(commandPath), - ) - - kvs := make([]attribute.KeyValue, 0, attrSet.Len()) - for iter := attrSet.Iter(); iter.Next(); { - attr := iter.Attribute() - kvs = append(kvs, attribute.KeyValue{ - Key: dockerCLIAttributePrefix + attr.Key, - Value: attr.Value, - }) - } - return attribute.NewSet(kvs...) -} - -func appendPluginResourceAttributesEnvvar(env []string, cmd *cobra.Command, plugin Plugin) []string { - if attrs := getPluginResourceAttributes(cmd, plugin); attrs.Len() > 0 { - // Construct baggage members for each of the attributes. - // Ignore any failures as these aren't significant and - // represent an internal issue. - 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()) - if err != nil { - otel.Handle(err) - continue - } - members = append(members, m) - } - - // 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 b, err := baggage.New(members...); err != nil { - otel.Handle(err) - } else if b.Len() > 0 { - attrsSlice = append(attrsSlice, b.String()) - } - - if len(attrsSlice) > 0 { - env = append(env, resourceAttributesEnvVar+"="+strings.Join(attrsSlice, ",")) - } - } - return env -} diff --git a/cli-plugins/manager/telemetry.go b/cli-plugins/manager/telemetry.go new file mode 100644 index 0000000000..3382d4d8d2 --- /dev/null +++ b/cli-plugins/manager/telemetry.go @@ -0,0 +1,84 @@ +package manager + +import ( + "fmt" + "os" + "strings" + + "github.com/spf13/cobra" + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/baggage" +) + +const ( + // resourceAttributesEnvVar is the name of the envvar that includes additional + // resource attributes for OTEL as defined in the [OpenTelemetry specification]. + // + // [OpenTelemetry specification]: https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/#general-sdk-configuration + resourceAttributesEnvVar = "OTEL_RESOURCE_ATTRIBUTES" + + // dockerCLIAttributePrefix is the prefix for any docker cli OTEL attributes. + // + // It is a copy of the const defined in [command.dockerCLIAttributePrefix]. + dockerCLIAttributePrefix = "docker.cli." + cobraCommandPath = attribute.Key("cobra.command_path") +) + +func getPluginResourceAttributes(cmd *cobra.Command, plugin Plugin) attribute.Set { + commandPath := cmd.Annotations[CommandAnnotationPluginCommandPath] + if commandPath == "" { + commandPath = fmt.Sprintf("%s %s", cmd.CommandPath(), plugin.Name) + } + + attrSet := attribute.NewSet( + cobraCommandPath.String(commandPath), + ) + + kvs := make([]attribute.KeyValue, 0, attrSet.Len()) + for iter := attrSet.Iter(); iter.Next(); { + attr := iter.Attribute() + kvs = append(kvs, attribute.KeyValue{ + Key: dockerCLIAttributePrefix + attr.Key, + Value: attr.Value, + }) + } + return attribute.NewSet(kvs...) +} + +func appendPluginResourceAttributesEnvvar(env []string, cmd *cobra.Command, plugin Plugin) []string { + if attrs := getPluginResourceAttributes(cmd, plugin); attrs.Len() > 0 { + // Construct baggage members for each of the attributes. + // Ignore any failures as these aren't significant and + // represent an internal issue. + 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()) + if err != nil { + otel.Handle(err) + continue + } + members = append(members, m) + } + + // 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 b, err := baggage.New(members...); err != nil { + otel.Handle(err) + } else if b.Len() > 0 { + attrsSlice = append(attrsSlice, b.String()) + } + + if len(attrsSlice) > 0 { + env = append(env, resourceAttributesEnvVar+"="+strings.Join(attrsSlice, ",")) + } + } + return env +}