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 <jonathan.sternberg@docker.com>

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
 }

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)
+}