envcar: ignore non-normalized env names on Get and Keys

CHANGELOG.md

@@ -15,6 +15,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 ### Changed
 
+- Use direct normalized-key lookups in `Carrier.Get` and `Carrier.Keys` in `go.opentelemetry.io/contrib/propagators/envcar`. (#9112)
 - Update log bridge conversions to use attribute key-values instead of the removed log key-values in `go.opentelemetry.io/contrib/bridges/otellogr`, `go.opentelemetry.io/contrib/bridges/otellogrus`, `go.opentelemetry.io/contrib/bridges/otelslog`, and `go.opentelemetry.io/contrib/bridges/otelzap`. (#9180)
 - The `Version()` function in `go.opentelemetry.io/contrib/instrumentation/github.com/gorilla/mux/otelmux` has been replaced by `const Version`. (#9076)
 - Set `error.type` attribute instead of adding `exception` span events in `go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin`. (#8977)

propagators/envcar/carrier.go

@@ -6,61 +6,52 @@ package envcar // import "go.opentelemetry.io/contrib/propagators/envcar"
 import (
 	"os"
 	"strings"
-	"sync"
 
 	"go.opentelemetry.io/otel/propagation"
 )
 
-// Carrier is a TextMapCarrier that uses the environment variables as a
-// storage medium for propagated key-value pairs. The keys are normalized
-// before being used to access the environment variables.
-// This is useful for propagating values that are set in the environment
-// and need to be accessed by different processes or services.
-// The keys are uppercased to avoid case sensitivity issues across different
-// operating systems and environments.
+// Carrier is a [propagation.TextMapCarrier] for environment variables.
 //
-// If you do not set SetEnvFunc, [Carrier.Set] will do nothing.
-// Using [os.Setenv] here is discouraged as the environment should
-// be immutable:
-// https://opentelemetry.io/docs/specs/otel/context/env-carriers/#environment-variable-immutability
+// [Carrier.Get] and [Carrier.Keys] read from the current process environment.
+// [Carrier.Get] normalizes the key before lookup. [Carrier.Keys] only lists
+// environment variable names that are already normalized.
+//
+// [Carrier.Set] writes through [Carrier.SetEnvFunc] with a normalized key. If
+// SetEnvFunc is nil, [Carrier.Set] does nothing. This lets injection target the
+// environment of a child process without mutating the current process
+// environment. Using [os.Setenv] as SetEnvFunc is discouraged because
+// applications should not modify their own context-related environment
+// variables.
+//
+// Key name normalization is defined by the [OpenTelemetry specification].
+//
+// [OpenTelemetry specification]: https://opentelemetry.io/docs/specs/otel/context/env-carriers/#key-name-normalization
 type Carrier struct {
-	// SetEnvFunc is the function that sets the environment variable.
-	// Usually, you want to set the environment variables for processes
-	// that are spawned by the current process.
+	// SetEnvFunc sets an environment variable for injected context.
+	// [Carrier.Set] calls SetEnvFunc with a normalized key.
+	//
+	// Set this to update the environment that will be passed to a child
+	// process. Leave it nil for an extract-only carrier.
 	SetEnvFunc func(key, value string)
-	values     map[string]string
-	once       sync.Once
 }
 
 // Compile time check that Carrier implements the TextMapCarrier.
 var _ propagation.TextMapCarrier = (*Carrier)(nil)
 
-// fetch runs once on first access, and stores the environment in the
-// carrier.
-func (c *Carrier) fetch() {
-	c.once.Do(func() {
-		environ := os.Environ()
-		c.values = make(map[string]string, len(environ))
-		for _, kv := range environ {
-			kvPair := strings.SplitN(kv, "=", 2)
-			key := normalize(kvPair[0])
-			c.values[key] = kvPair[1]
-		}
-	})
-}
-
-// Get returns the value associated with the normalized passed key.
-// The first call to [Carrier.Get] or [Carrier.Keys] for a
-// given Carrier will read and store the values from the
-// environment and all future reads will be from that store.
-func (c *Carrier) Get(key string) string {
-	c.fetch()
-	return c.values[normalize(key)]
+// Get normalizes key and returns the corresponding value from the current
+// process environment. It returns an empty string if the normalized
+// environment variable is unset or set to an empty value.
+//
+// On platforms with case-insensitive environment lookup, such as Windows, the
+// lookup may match an environment variable whose name differs from the
+// normalized key only by case.
+func (*Carrier) Get(key string) string {
+	return os.Getenv(normalize(key))
 }
 
-// Set stores the key-value pair in the environment variable.
-// The key is normalized before being used to set the
-// environment variable.
+// Set stores the key-value pair by calling [Carrier.SetEnvFunc].
+// The key is normalized before SetEnvFunc is called.
+//
 // If SetEnvFunc is not set, this method does nothing.
 func (c *Carrier) Set(key, value string) {
 	if c.SetEnvFunc == nil {
@@ -70,16 +61,15 @@ func (c *Carrier) Set(key, value string) {
 	c.SetEnvFunc(k, value)
 }
 
-// Keys lists the keys stored in this carrier.
-// This returns all the keys in the environment variables.
-// The first call to [Carrier.Get] or [Carrier.Keys] for a
-// given Carrier will read and store the values from the
-// environment and all future reads will be from that store.
-// The keys are returned in their normalized form.
-func (c *Carrier) Keys() []string {
-	c.fetch()
-	keys := make([]string, 0, len(c.values))
-	for key := range c.values {
+// Keys returns normalized environment variable names from the current process.
+func (*Carrier) Keys() []string {
+	environ := os.Environ()
+	keys := make([]string, 0, len(environ))
+	for _, kv := range environ {
+		key, _, _ := strings.Cut(kv, "=")
+		if !normalized(key) {
+			continue
+		}
 		keys = append(keys, key)
 	}
 	return keys

propagators/envcar/carrier_example_test.go

@@ -15,11 +15,20 @@ import (
 	"go.opentelemetry.io/contrib/propagators/envcar"
 )
 
-// This example is a go program where the environment variables are carrying the
-// trace information, and we're going to pick them up into our context.
+// An example where the environment variables are carrying the trace
+// information, and we're going to pick them up into our context.
 func ExampleCarrier_extractFromParent() {
-	// Simulate environment variables set by a parent process.
-	// In practice, these would already be set when this process starts.
+	// Simulate an environment variable set by a parent process. In practice,
+	// this would already be set when this process starts, and the application
+	// would extract context during initialization.
+	orig, ok := os.LookupEnv("TRACEPARENT")
+	defer func() {
+		if ok {
+			_ = os.Setenv("TRACEPARENT", orig)
+			return
+		}
+		_ = os.Unsetenv("TRACEPARENT")
+	}()
 	_ = os.Setenv("TRACEPARENT", "00-0102030405060708090a0b0c0d0e0f10-0102030405060708-01")
 
 	// Create a carrier to read trace context from environment variables.
@@ -29,7 +38,8 @@ func ExampleCarrier_extractFromParent() {
 	prop := propagation.TraceContext{}
 	ctx := prop.Extract(context.Background(), &carrier)
 
-	// The context now contains the span context from the parent.
+	// The context now contains the span context from the parent. Keep and pass
+	// this context instead of repeatedly extracting from the environment.
 	spanCtx := trace.SpanContextFromContext(ctx)
 	fmt.Printf("Trace ID: %s\n", spanCtx.TraceID())
 	fmt.Printf("Span ID: %s\n", spanCtx.SpanID())
@@ -40,8 +50,8 @@ func ExampleCarrier_extractFromParent() {
 	// Sampled: true
 }
 
-// This example is a go program where we have a trace and we'd like to inject it
-// into a command we're going to run.
+// An example where we have a trace and we'd like to inject it into a command
+// we're going to run.
 func ExampleCarrier_childProcess() {
 	// Create a span context with a known trace ID.
 	traceID := trace.TraceID{0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10}
@@ -54,6 +64,7 @@ func ExampleCarrier_childProcess() {
 	ctx := trace.ContextWithSpanContext(context.Background(), spanCtx)
 
 	// Prepare a command that prints the TRACEPARENT environment variable.
+	// Each child process gets its own copy of the environment.
 	cmd := exec.CommandContext(context.Background(), "printenv", "TRACEPARENT")
 	cmd.Env = os.Environ()
 
@@ -65,7 +76,7 @@ func ExampleCarrier_childProcess() {
 		},
 	}
 
-	// Inject trace context into the child's environment.
+	// Inject trace context into the child's environment copy.
 	prop := propagation.TraceContext{}
 	prop.Inject(ctx, &carrier)
 

propagators/envcar/carrier_test.go

@@ -6,7 +6,7 @@ package envcar_test
 import (
 	"os"
 	"os/exec"
-	"slices"
+	"runtime"
 	"strings"
 	"sync"
 	"testing"
@@ -61,19 +61,6 @@ func TestExtractValidTraceContextEnvCarrier(t *testing.T) {
 				Remote:     true,
 			}),
 		},
-		{
-			name: "lowercase env names",
-			envs: map[string]string{
-				"traceparent": "00-000000000000007b00000000000001c8-000000000000007b-00",
-				"tracestate":  stateStr,
-			},
-			want: trace.NewSpanContext(trace.SpanContextConfig{
-				TraceID:    traceID,
-				SpanID:     spanID,
-				TraceState: state,
-				Remote:     true,
-			}),
-		},
 	}
 
 	for _, tc := range tests {
@@ -88,6 +75,21 @@ func TestExtractValidTraceContextEnvCarrier(t *testing.T) {
 	}
 }
 
+func TestExtractIgnoresNonNormalizedEnvNames(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("Windows environment variables are case-insensitive, so this test is not applicable.")
+	}
+
+	unsetenv(t, "TRACEPARENT")
+	unsetenv(t, "TRACESTATE")
+	t.Setenv("traceparent", "00-000000000000007b00000000000001c8-000000000000007b-00")
+	t.Setenv("tracestate", "key1=value1,key2=value2")
+
+	ctx := prop.Extract(t.Context(), &envcar.Carrier{})
+
+	assert.False(t, trace.SpanContextFromContext(ctx).IsValid())
+}
+
 func TestInjectTraceContextEnvCarrier(t *testing.T) {
 	stateStr := "key1=value1,key2=value2"
 	state, err := trace.ParseTraceState(stateStr)
@@ -147,13 +149,16 @@ func TestInjectTraceContextEnvCarrier(t *testing.T) {
 }
 
 func TestCarrierKeys(t *testing.T) {
-	t.Setenv("traceparent", "value")
+	t.Setenv("TRACEPARENT", "value")
+	unsetenv(t, "ENVCAR_NON_NORMALIZED_KEY")
+	t.Setenv("envcar-non-normalized-key", "ignored")
 
 	c := envcar.Carrier{}
 	keys := c.Keys()
 
 	assert.Contains(t, keys, "TRACEPARENT")
-	assert.NotContains(t, keys, "traceparent")
+	assert.NotContains(t, keys, "envcar-non-normalized-key")
+	assert.NotContains(t, keys, "ENVCAR_NON_NORMALIZED_KEY")
 }
 
 func TestCarrierSetNilFunc(_ *testing.T) {
@@ -162,13 +167,22 @@ func TestCarrierSetNilFunc(_ *testing.T) {
 }
 
 func TestCarrierGetNormalizesKey(t *testing.T) {
-	t.Setenv("traceparent", "myvalue")
+	t.Setenv("TRACEPARENT", "myvalue")
 
 	c := envcar.Carrier{}
 	assert.Equal(t, "myvalue", c.Get("traceparent"))
 	assert.Equal(t, "myvalue", c.Get("TRACEPARENT"))
 }
 
+func TestCarrierGetIgnoresNonNormalizedEnvNames(t *testing.T) {
+	unsetenv(t, "ENVCAR_GET_NON_NORMALIZED_KEY")
+	t.Setenv("envcar-get-non-normalized-key", "ignored")
+
+	c := envcar.Carrier{}
+	assert.Empty(t, c.Get("envcar_get_non_normalized_key"))
+	assert.Empty(t, c.Get("ENVCAR_GET_NON_NORMALIZED_KEY"))
+}
+
 func TestCarrierSetUppercasesUnderscoresKey(t *testing.T) {
 	var gotKey string
 	var gotValue string
@@ -192,6 +206,29 @@ func TestCarrierSetUppercasesUnderscoresKey(t *testing.T) {
 	assert.Equal(t, "🧳", gotValue)
 }
 
+func TestCarrierGetReadsDirectly(t *testing.T) {
+	t.Setenv("TRACEPARENT", "myvalue")
+	c := envcar.Carrier{}
+	require.Equal(t, "myvalue", c.Get("TRACEPARENT"))
+
+	t.Setenv("TRACEPARENT", "bad")
+	t.Setenv("ENVCAR_GET_READS_DIRECTLY_NEW", "value")
+
+	assert.Equal(t, "bad", c.Get("TRACEPARENT"))
+	assert.Contains(t, c.Keys(), "ENVCAR_GET_READS_DIRECTLY_NEW")
+}
+
+func TestCarrierKeysReadsDirectly(t *testing.T) {
+	t.Setenv("TRACEPARENT", "myvalue")
+	c := envcar.Carrier{}
+	require.Contains(t, c.Keys(), "TRACEPARENT")
+
+	t.Setenv("ENVCAR_KEYS_FETCH_ONCE_NEW", "value")
+
+	assert.Contains(t, c.Keys(), "TRACEPARENT")
+	assert.Contains(t, c.Keys(), "ENVCAR_KEYS_FETCH_ONCE_NEW")
+}
+
 func TestConcurrentChildProcesses(t *testing.T) {
 	// Test that concurrent goroutines can each spawn child processes
 	// with their own unique trace context.
@@ -261,16 +298,16 @@ func TestConcurrentChildProcesses(t *testing.T) {
 	}
 }
 
-// Ensure Get and Keys to fetch from cache.
-func TestCarrierGetFetchOnce(t *testing.T) {
-	t.Setenv("TRACEPARENT", "myvalue")
-	c := envcar.Carrier{}
-	require.Equal(t, "myvalue", c.Get("TRACEPARENT"))
-	require.NotEqual(t, -1, slices.Index(c.Keys(), "TRACEPARENT"))
-	t.Setenv("TRACEPARENT", "bad")
-	t.Setenv("SOMETHINGNEW", "bad")
-	// Assert a carrier instance reads the env vars only once:
-	assert.Equal(t, "myvalue", c.Get("TRACEPARENT"))
-	assert.NotEqual(t, -1, slices.Index(c.Keys(), "TRACEPARENT"))
-	assert.Equal(t, -1, slices.Index(c.Keys(), "SOMETHINGNEW"))
+func unsetenv(t *testing.T, key string) {
+	t.Helper()
+
+	value, ok := os.LookupEnv(key)
+	require.NoError(t, os.Unsetenv(key))
+	t.Cleanup(func() {
+		if ok {
+			require.NoError(t, os.Setenv(key, value)) //nolint:usetesting // t.Setenv cannot restore after an explicit unset.
+			return
+		}
+		require.NoError(t, os.Unsetenv(key))
+	})
 }

propagators/envcar/doc.go

@@ -1,6 +1,22 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
-// Package envcar implements the Environment Carrier specification as documented
-// here https://opentelemetry.io/docs/specs/otel/context/env-carriers/
+// Package envcar implements the
+// [Environment Variables as Context Propagation Carriers specification].
+//
+// Environment variable propagation is intended for process boundaries where
+// network protocols are not available, such as batch jobs, CI/CD, and command
+// line tools. Treat context-related environment variables as process-startup
+// input.
+//
+// Applications should extract context from the environment once during startup
+// and then use the extracted context. Repeated extraction uses the process
+// environment as a live parent-context source and may observe later environment
+// variable changes.
+//
+// Note that environment variables can be visible to code in the same process
+// and, on many systems, to other users or processes with sufficient
+// permissions. Do not use this carrier for sensitive context.
+//
+// [Environment Variables as Context Propagation Carriers specification]: https://opentelemetry.io/docs/specs/otel/context/env-carriers/
 package envcar // import "go.opentelemetry.io/contrib/propagators/envcar"