propagation: extract of multiple header values (#5973)

* Add `ValuesGetter` interface, an optional `TextMapCarrier` feature
that adds `Values(string) []string`.
* `HeaderCarrier` implements `ValuesGetter`.
* Change `Baggage` to use the `Values()` method if it's implemented.

Notable comment:
https://github.com/open-telemetry/opentelemetry-go/pull/5973#discussion_r2074894706

Adds tests extracting requests with multiple 'baggage' headers set.
Does not introduce any breaking changes or alter any existing tests.

Spec issue:
https://github.com/open-telemetry/opentelemetry-specification/issues/433
Corresponding Java prototype:
https://github.com/open-telemetry/opentelemetry-java/pull/6852

---------

Co-authored-by: Robert Pająk <pellared@hotmail.com>
Co-authored-by: dmathieu <42@dmathieu.com>

propagation/baggage.go

@@ -28,7 +28,21 @@ func (b Baggage) Inject(ctx context.Context, carrier TextMapCarrier) {
 }
 
 // Extract returns a copy of parent with the baggage from the carrier added.
+// If carrier implements [ValuesGetter] (e.g. [HeaderCarrier]), Values is invoked
+// for multiple values extraction. Otherwise, Get is called.
 func (b Baggage) Extract(parent context.Context, carrier TextMapCarrier) context.Context {
+	if multiCarrier, ok := carrier.(ValuesGetter); ok {
+		return extractMultiBaggage(parent, multiCarrier)
+	}
+	return extractSingleBaggage(parent, carrier)
+}
+
+// Fields returns the keys who's values are set with Inject.
+func (b Baggage) Fields() []string {
+	return []string{baggageHeader}
+}
+
+func extractSingleBaggage(parent context.Context, carrier TextMapCarrier) context.Context {
 	bStr := carrier.Get(baggageHeader)
 	if bStr == "" {
 		return parent
@@ -41,7 +55,23 @@ func (b Baggage) Extract(parent context.Context, carrier TextMapCarrier) context
 	return baggage.ContextWithBaggage(parent, bag)
 }
 
-// Fields returns the keys who's values are set with Inject.
-func (b Baggage) Fields() []string {
-	return []string{baggageHeader}
+func extractMultiBaggage(parent context.Context, carrier ValuesGetter) context.Context {
+	bVals := carrier.Values(baggageHeader)
+	if len(bVals) == 0 {
+		return parent
+	}
+	var members []baggage.Member
+	for _, bStr := range bVals {
+		currBag, err := baggage.Parse(bStr)
+		if err != nil {
+			continue
+		}
+		members = append(members, currBag.Members()...)
+	}
+
+	b, err := baggage.New(members...)
+	if err != nil || b.Len() == 0 {
+		return parent
+	}
+	return baggage.ContextWithBaggage(parent, b)
 }

propagation/baggage_test.go

@@ -56,7 +56,7 @@ func (m members) Baggage(t *testing.T) baggage.Baggage {
 	return bag
 }
 
-func TestExtractValidBaggageFromHTTPReq(t *testing.T) {
+func TestExtractValidBaggage(t *testing.T) {
 	prop := propagation.TextMapPropagator(propagation.Baggage{})
 	tests := []struct {
 		name   string
@@ -113,12 +113,78 @@ func TestExtractValidBaggageFromHTTPReq(t *testing.T) {
 				{Key: "key1", Value: "val%2"},
 			},
 		},
+		{
+			name:   "empty header",
+			header: "",
+			want:   members{},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mapCarr := propagation.MapCarrier{}
+			mapCarr["baggage"] = tt.header
+			req, _ := http.NewRequest(http.MethodGet, "http://example.com", nil)
+			req.Header.Set("baggage", tt.header)
+
+			// test with http header carrier (which implements ValuesGetter)
+			ctx := prop.Extract(context.Background(), propagation.HeaderCarrier(req.Header))
+			expected := tt.want.Baggage(t)
+			assert.Equal(t, expected, baggage.FromContext(ctx), "should extract baggage for HeaderCarrier")
+
+			// test with map carrier (which does not implement ValuesGetter)
+			ctx = prop.Extract(context.Background(), mapCarr)
+			expected = tt.want.Baggage(t)
+			assert.Equal(t, expected, baggage.FromContext(ctx), "should extract baggage for MapCarrier")
+		})
+	}
+}
+
+func TestExtractValidMultipleBaggageHeaders(t *testing.T) {
+	prop := propagation.TextMapPropagator(propagation.Baggage{})
+	tests := []struct {
+		name    string
+		headers []string
+		want    members
+	}{
+		{
+			name:    "non conflicting headers",
+			headers: []string{"key1=val1", "key2=val2"},
+			want: members{
+				{Key: "key1", Value: "val1"},
+				{Key: "key2", Value: "val2"},
+			},
+		},
+		{
+			name:    "conflicting keys, uses last val",
+			headers: []string{"key1=val1", "key1=val2"},
+			want: members{
+				{Key: "key1", Value: "val2"},
+			},
+		},
+		{
+			name:    "single empty",
+			headers: []string{"", "key1=val1"},
+			want: members{
+				{Key: "key1", Value: "val1"},
+			},
+		},
+		{
+			name:    "all empty",
+			headers: []string{"", ""},
+			want:    members{},
+		},
+		{
+			name:    "none",
+			headers: []string{},
+			want:    members{},
+		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			req, _ := http.NewRequest(http.MethodGet, "http://example.com", nil)
-			req.Header.Set("baggage", tt.header)
+			req.Header["Baggage"] = tt.headers
 
 			ctx := context.Background()
 			ctx = prop.Extract(ctx, propagation.HeaderCarrier(req.Header))

propagation/propagation.go

@@ -9,6 +9,7 @@ import (
 )
 
 // TextMapCarrier is the storage medium used by a TextMapPropagator.
+// See ValuesGetter for how a TextMapCarrier can get multiple values for a key.
 type TextMapCarrier interface {
 	// DO NOT CHANGE: any modification will not be backwards compatible and
 	// must never be done outside of a new major release.
@@ -29,6 +30,18 @@ type TextMapCarrier interface {
 	// must never be done outside of a new major release.
 }
 
+// ValuesGetter can return multiple values for a single key,
+// with contrast to TextMapCarrier.Get which returns a single value.
+type ValuesGetter interface {
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
+
+	// Values returns all values associated with the passed key.
+	Values(key string) []string
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
+}
+
 // MapCarrier is a TextMapCarrier that uses a map held in memory as a storage
 // medium for propagated key-value pairs.
 type MapCarrier map[string]string
@@ -55,14 +68,25 @@ func (c MapCarrier) Keys() []string {
 	return keys
 }
 
-// HeaderCarrier adapts http.Header to satisfy the TextMapCarrier interface.
+// HeaderCarrier adapts http.Header to satisfy the TextMapCarrier and ValuesGetter interfaces.
 type HeaderCarrier http.Header
 
-// Get returns the value associated with the passed key.
+// Compile time check that HeaderCarrier implements ValuesGetter.
+var _ TextMapCarrier = HeaderCarrier{}
+
+// Compile time check that HeaderCarrier implements TextMapCarrier.
+var _ ValuesGetter = HeaderCarrier{}
+
+// Get returns the first value associated with the passed key.
 func (hc HeaderCarrier) Get(key string) string {
 	return http.Header(hc).Get(key)
 }
 
+// Values returns all values associated with the passed key.
+func (hc HeaderCarrier) Values(key string) []string {
+	return http.Header(hc).Values(key)
+}
+
 // Set stores the key-value pair.
 func (hc HeaderCarrier) Set(key string, value string) {
 	http.Header(hc).Set(key, value)
@@ -89,6 +113,8 @@ type TextMapPropagator interface {
 	// must never be done outside of a new major release.
 
 	// Extract reads cross-cutting concerns from the carrier into a Context.
+	// Implementations may check if the carrier implements ValuesGetter,
+	// to support extraction of multiple values per key.
 	Extract(ctx context.Context, carrier TextMapCarrier) context.Context
 	// DO NOT CHANGE: any modification will not be backwards compatible and
 	// must never be done outside of a new major release.