diff --git a/CHANGELOG.md b/CHANGELOG.md index cf4c3abed3b..f0fedffccc5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Add `WithHTTPClient` option to configure the `http.Client` used by `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`. (#6751) - Add `WithHTTPClient` option to configure the `http.Client` used by `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`. (#6752) - Add `WithHTTPClient` option to configure the `http.Client` used by `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp`. (#6688) +- Add `ValuesGetter` in `go.opentelemetry.io/otel/propagation`, a `TextMapCarrier` that supports retrieving multiple values for a single key. (#5973) +- Add `Values` method to `HeaderCarrier` to implement the new `ValuesGetter` interface in `go.opentelemetry.io/otel/propagation`. (#5973) +- Update `Baggage` in `go.opentelemetry.io/otel/propagation` to retrieve multiple values for a key when the carrier implements `ValuesGetter`. (#5973) - Add `AssertEqual` function in `go.opentelemetry.io/otel/log/logtest`. (#6662) ### Removed diff --git a/propagation/baggage.go b/propagation/baggage.go index 552263ba734..ebda5026d6b 100644 --- a/propagation/baggage.go +++ b/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) } diff --git a/propagation/baggage_test.go b/propagation/baggage_test.go index 4559a808fe2..8ce34bdec5c 100644 --- a/propagation/baggage_test.go +++ 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,13 +113,79 @@ 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["Baggage"] = tt.headers + ctx := context.Background() ctx = prop.Extract(ctx, propagation.HeaderCarrier(req.Header)) expected := tt.want.Baggage(t) diff --git a/propagation/propagation.go b/propagation/propagation.go index 8c8286aab4d..5c8c26ea2eb 100644 --- a/propagation/propagation.go +++ b/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.