Merge pull request #6958 from Kunalbehbud/fix/otel-env-override

oci: don't override build-provided OpenTelemetry env vars

executor/oci/spec.go

@@ -46,6 +46,29 @@ var tracingEnvVars = []string{
 	"OTEL_EXPORTER_OTLP_TRACES_PROTOCOL=grpc",
 }
 
+// envName returns the variable name of a "NAME=VALUE" environment entry.
+func envName(env string) string {
+	name, _, _ := strings.Cut(env, "=")
+	return name
+}
+
+// appendMissingTracingEnv appends the OpenTelemetry trace-exporter variables
+// that the build has not already set. This lets a build override or opt out of
+// them (for example with `--opt env.OTEL_TRACES_EXPORTER=none`) instead of
+// BuildKit unconditionally forcing its own values.
+func appendMissingTracingEnv(env []string) []string {
+	existing := make(map[string]struct{}, len(env))
+	for _, e := range env {
+		existing[envName(e)] = struct{}{}
+	}
+	for _, e := range tracingEnvVars {
+		if _, ok := existing[envName(e)]; !ok {
+			env = append(env, e)
+		}
+	}
+	return env
+}
+
 func (pm ProcessMode) String() string {
 	switch pm {
 	case ProcessSandbox:
@@ -125,7 +148,9 @@ func GenerateSpec(ctx context.Context, meta executor.Meta, mounts []executor.Mou
 
 	if tracingSocket != "" {
 		// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md
-		meta.Env = append(meta.Env, tracingEnvVars...)
+		// Only inject the trace-exporter variables the build has not already set,
+		// so it can override or opt out of them (e.g. `--opt env.OTEL_TRACES_EXPORTER=none`).
+		meta.Env = appendMissingTracingEnv(meta.Env)
 		meta.Env = append(meta.Env, childprocess.Environ(ctx)...)
 	}
 

executor/oci/spec_test.go

@@ -0,0 +1,41 @@
+package oci
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestEnvName(t *testing.T) {
+	require.Equal(t, "FOO", envName("FOO=bar"))
+	require.Equal(t, "FOO", envName("FOO="))
+	require.Equal(t, "FOO", envName("FOO"))
+	require.Equal(t, "", envName("=bar"))
+}
+
+func TestAppendMissingTracingEnv(t *testing.T) {
+	t.Run("appends all tracing vars when none are set", func(t *testing.T) {
+		got := appendMissingTracingEnv([]string{"PATH=/usr/bin"})
+		require.Equal(t, append([]string{"PATH=/usr/bin"}, tracingEnvVars...), got)
+	})
+
+	t.Run("keeps a user-provided OTEL_TRACES_EXPORTER (e.g. none)", func(t *testing.T) {
+		got := appendMissingTracingEnv([]string{"OTEL_TRACES_EXPORTER=none"})
+		require.Contains(t, got, "OTEL_TRACES_EXPORTER=none")
+		require.NotContains(t, got, "OTEL_TRACES_EXPORTER=otlp")
+		// the remaining tracing vars are still injected
+		for _, e := range tracingEnvVars {
+			if envName(e) != "OTEL_TRACES_EXPORTER" {
+				require.Contains(t, got, e)
+			}
+		}
+	})
+
+	t.Run("does not override any user-provided tracing var", func(t *testing.T) {
+		var env []string
+		for _, e := range tracingEnvVars {
+			env = append(env, envName(e)+"=custom")
+		}
+		require.Equal(t, env, appendMissingTracingEnv(env))
+	})
+}