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

CHANGELOG.md

@@ -12,6 +12,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 - Add `azurecontainerapps` resource detector for Azure Container Apps. (#8939)
 
+### Changed
+
+- Ignore non-normalized environment variable names on `Carrier.Get` and `Carrier.Keys` in `go.opentelemetry.io/contrib/propagators/envcar`. (#9112)
+
 <!-- Released section -->
 <!-- Don't change this section unless doing release -->
 

propagators/envcar/carrier.go

@@ -11,9 +11,11 @@ import (
 	"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.
+// Carrier is a TextMapCarrier that uses environment variables as a storage
+// medium for propagated key-value pairs. Keys passed to [Carrier.Get] and
+// [Carrier.Set] are normalized before lookup or write. Environment variables
+// read from the current process are stored only when their names are already
+// normalized.
 // 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
@@ -25,6 +27,7 @@ import (
 // https://opentelemetry.io/docs/specs/otel/context/env-carriers/#environment-variable-immutability
 type Carrier struct {
 	// SetEnvFunc is the function that sets the environment variable.
+	// [Carrier.Set] calls SetEnvFunc with a normalized key.
 	// Usually, you want to set the environment variables for processes
 	// that are spawned by the current process.
 	SetEnvFunc func(key, value string)
@@ -35,24 +38,28 @@ type Carrier struct {
 // 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.
+// fetch runs once on first access, and stores environment variables with
+// already-normalized names 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])
+			key := kvPair[0]
+			if !normalized(key) {
+				continue
+			}
 			c.values[key] = kvPair[1]
 		}
 	})
 }
 
-// Get returns the value associated with the normalized passed key.
+// Get returns the value associated with the normalized 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.
+// given Carrier will read and store the values from environment variables
+// whose names are already normalized, and all future reads will be from that
+// store.
 func (c *Carrier) Get(key string) string {
 	c.fetch()
 	return c.values[normalize(key)]
@@ -70,12 +77,13 @@ 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.
+// Keys lists the normalized keys stored in this carrier.
+// This returns all keys from environment variables whose names are already
+// normalized.
 // 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.
+// given Carrier will read and store the values from environment variables
+// whose names are already normalized, and all future reads will be from that
+// store.
 func (c *Carrier) Keys() []string {
 	c.fetch()
 	keys := make([]string, 0, len(c.values))

propagators/envcar/carrier_example_test.go

@@ -20,6 +20,14 @@ import (
 func ExampleCarrier_extractFromParent() {
 	// Simulate environment variables set by a parent process.
 	// In practice, these would already be set when this process starts.
+	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.

propagators/envcar/carrier_test.go

@@ -6,6 +6,7 @@ package envcar_test
 import (
 	"os"
 	"os/exec"
+	"runtime"
 	"slices"
 	"strings"
 	"sync"
@@ -61,19 +62,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 +76,23 @@ 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.")
+	}
+
+	// Guard against TRACEPARENT/TRACESTATE being set in the outer test environment.
+	t.Setenv("TRACEPARENT", "")
+	t.Setenv("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 +152,15 @@ func TestInjectTraceContextEnvCarrier(t *testing.T) {
 }
 
 func TestCarrierKeys(t *testing.T) {
-	t.Setenv("traceparent", "value")
+	t.Setenv("TRACEPARENT", "value")
+	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 +169,25 @@ 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) {
+	if runtime.GOOS == "windows" {
+		t.Skip("Windows environment variables are case-insensitive, so this test is not applicable.")
+	}
+
+	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

propagators/envcar/doc.go

@@ -2,5 +2,5 @@
 // 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/
+// here: https://opentelemetry.io/docs/specs/otel/context/env-carriers/.
 package envcar // import "go.opentelemetry.io/contrib/propagators/envcar"

propagators/envcar/normalize.go

@@ -46,3 +46,26 @@ func normalize(s string) string {
 	}
 	return string(b)
 }
+
+// normalized reports whether s is already a normalized environment variable
+// name.
+func normalized(s string) bool {
+	if s == "" {
+		return true
+	}
+	// Normalized names cannot start with a digit; normalize would prepend '_'.
+	if s[0] >= '0' && s[0] <= '9' {
+		return false
+	}
+
+	for _, r := range s {
+		switch {
+		// A-Z, 0-9, and _ are already normalized.
+		case r >= 'A' && r <= 'Z', r >= '0' && r <= '9', r == '_':
+		default:
+			// Lowercase letters and all other characters would be rewritten.
+			return false
+		}
+	}
+	return true
+}

propagators/envcar/normalize_test.go

@@ -37,6 +37,14 @@ func TestNormalize(t *testing.T) {
 	}
 }
 
+func TestNormalized(t *testing.T) {
+	for _, tc := range normalizeCases {
+		t.Run(tc.in, func(t *testing.T) {
+			assert.Equal(t, tc.in == tc.want, normalized(tc.in))
+		})
+	}
+}
+
 func BenchmarkNormalize(b *testing.B) {
 	for _, tc := range normalizeCases {
 		b.Run(tc.in, func(b *testing.B) {
@@ -47,3 +55,14 @@ func BenchmarkNormalize(b *testing.B) {
 		})
 	}
 }
+
+func BenchmarkNormalized(b *testing.B) {
+	for _, tc := range normalizeCases {
+		b.Run(tc.in, func(b *testing.B) {
+			b.ReportAllocs()
+			for b.Loop() {
+				normalized(tc.in)
+			}
+		})
+	}
+}