From 6e184cd16f2251f377c081d5fa44ba768284adfa Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 2 Oct 2020 12:27:16 -0700 Subject: [PATCH] Update propagation to conform with OpenTelemetry specification (#1212) * Move propagation package contents to the otel package * Implement package relocation * Update propagation to OTel spec * Add changes to changelog * Add propagation tests --- CHANGELOG.md | 8 ++ api/baggage/baggage_propagator.go | 29 ++--- api/baggage/baggage_propagator_test.go | 15 ++- api/global/internal/state.go | 25 ++-- api/global/propagation.go | 17 ++- api/propagation/doc.go | 16 --- api/propagation/propagation.go | 143 ---------------------- bridge/opentracing/bridge.go | 22 ++-- example/basic/main.go | 9 +- example/otel-collector/main.go | 7 +- otel.go | 3 - propagation.go | 78 ++++++++++++ propagation_test.go | 100 +++++++++++++++ propagators/propagators.go | 25 ---- propagators/propagators_test.go | 36 +++--- propagators/trace_context.go | 31 +++-- propagators/trace_context_example_test.go | 6 +- propagators/trace_context_test.go | 21 ++-- 18 files changed, 280 insertions(+), 311 deletions(-) delete mode 100644 api/propagation/doc.go delete mode 100644 api/propagation/propagation.go create mode 100644 propagation.go create mode 100644 propagation_test.go delete mode 100644 propagators/propagators.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 4788ef29c72..c777839ff9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,14 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Changed - Set default propagator to no-op propagator. (#1184) +- The `HTTPSupplier`, `HTTPExtractor`, `HTTPInjector`, and `HTTPPropagator` from the `go.opentelemetry.io/otel/api/propagation` package were replaced with unified `TextMapCarrier` and `TextMapPropagator` in the `go.opentelemetry.io/otel` package. (#1212) +- The `New` function from the `go.opentelemetry.io/otel/api/propagation` package was replaced with `NewCompositeTextMapPropagator` in the `go.opentelemetry.io/otel` package. (#1212) + +### Removed + +- The `ExtractHTTP` and `InjectHTTP` fuctions from the `go.opentelemetry.io/otel/api/propagation` package were removed. (#1212) +- The `Propagators` interface from the `go.opentelemetry.io/otel/api/propagation` package was removed to conform to the OpenTelemetry specification. + The explicit `TextMapPropagator` type can be used in its place as this is the `Propagator` type the specification defines. (#1212) ### Removed diff --git a/api/baggage/baggage_propagator.go b/api/baggage/baggage_propagator.go index c06d3b941ec..5824b9236d3 100644 --- a/api/baggage/baggage_propagator.go +++ b/api/baggage/baggage_propagator.go @@ -19,7 +19,7 @@ import ( "net/url" "strings" - "go.opentelemetry.io/otel/api/propagation" + "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/label" ) @@ -27,21 +27,14 @@ import ( // https://github.com/open-telemetry/opentelemetry-specification/blob/18b2752ebe6c7f0cdd8c7b2bcbdceb0ae3f5ad95/specification/correlationcontext/api.md#header-name const baggageHeader = "otcorrelations" -// Baggage propagates Key:Values in W3C CorrelationContext -// format. +// Baggage propagates Key:Values in W3C CorrelationContext format. // nolint:golint type Baggage struct{} -var _ propagation.HTTPPropagator = Baggage{} +var _ otel.TextMapPropagator = Baggage{} -// DefaultHTTPPropagator returns the default context correlation HTTP -// propagator. -func DefaultHTTPPropagator() propagation.HTTPPropagator { - return Baggage{} -} - -// Inject implements HTTPInjector. -func (b Baggage) Inject(ctx context.Context, supplier propagation.HTTPSupplier) { +// Inject set baggage key-values from the Context into the carrier. +func (b Baggage) Inject(ctx context.Context, carrier otel.TextMapCarrier) { baggageMap := MapFromContext(ctx) firstIter := true var headerValueBuilder strings.Builder @@ -57,13 +50,13 @@ func (b Baggage) Inject(ctx context.Context, supplier propagation.HTTPSupplier) }) if headerValueBuilder.Len() > 0 { headerString := headerValueBuilder.String() - supplier.Set(baggageHeader, headerString) + carrier.Set(baggageHeader, headerString) } } -// Extract implements HTTPExtractor. -func (b Baggage) Extract(ctx context.Context, supplier propagation.HTTPSupplier) context.Context { - baggage := supplier.Get(baggageHeader) +// Extract reads baggage key-values from the carrier into a returned Context. +func (b Baggage) Extract(ctx context.Context, carrier otel.TextMapCarrier) context.Context { + baggage := carrier.Get(baggageHeader) if baggage == "" { return ctx } @@ -112,7 +105,7 @@ func (b Baggage) Extract(ctx context.Context, supplier propagation.HTTPSupplier) return ctx } -// GetAllKeys implements HTTPPropagator. -func (b Baggage) GetAllKeys() []string { +// Fields returns the keys who's values are set with Inject. +func (b Baggage) Fields() []string { return []string{baggageHeader} } diff --git a/api/baggage/baggage_propagator_test.go b/api/baggage/baggage_propagator_test.go index eb24f179cd4..319ca2a1214 100644 --- a/api/baggage/baggage_propagator_test.go +++ b/api/baggage/baggage_propagator_test.go @@ -22,13 +22,13 @@ import ( "github.com/google/go-cmp/cmp" + "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/api/baggage" - "go.opentelemetry.io/otel/api/propagation" "go.opentelemetry.io/otel/label" ) func TestExtractValidBaggageFromHTTPReq(t *testing.T) { - props := propagation.New(propagation.WithExtractors(baggage.Baggage{})) + prop := otel.TextMapPropagator(baggage.Baggage{}) tests := []struct { name string header string @@ -90,7 +90,7 @@ func TestExtractValidBaggageFromHTTPReq(t *testing.T) { req.Header.Set("otcorrelations", tt.header) ctx := context.Background() - ctx = propagation.ExtractHTTP(ctx, props, req.Header) + ctx = prop.Extract(ctx, req.Header) gotBaggage := baggage.MapFromContext(ctx) wantBaggage := baggage.NewMap(baggage.MapUpdate{MultiKV: tt.wantKVs}) if gotBaggage.Len() != wantBaggage.Len() { @@ -117,7 +117,7 @@ func TestExtractValidBaggageFromHTTPReq(t *testing.T) { } func TestExtractInvalidDistributedContextFromHTTPReq(t *testing.T) { - props := propagation.New(propagation.WithExtractors(baggage.Baggage{})) + prop := otel.TextMapPropagator(baggage.Baggage{}) tests := []struct { name string header string @@ -152,7 +152,7 @@ func TestExtractInvalidDistributedContextFromHTTPReq(t *testing.T) { ctx := baggage.NewContext(context.Background(), tt.hasKVs...) wantBaggage := baggage.MapFromContext(ctx) - ctx = propagation.ExtractHTTP(ctx, props, req.Header) + ctx = prop.Extract(ctx, req.Header) gotBaggage := baggage.MapFromContext(ctx) if gotBaggage.Len() != wantBaggage.Len() { t.Errorf( @@ -176,7 +176,6 @@ func TestExtractInvalidDistributedContextFromHTTPReq(t *testing.T) { func TestInjectBaggageToHTTPReq(t *testing.T) { propagator := baggage.Baggage{} - props := propagation.New(propagation.WithInjectors(propagator)) tests := []struct { name string kvs []label.KeyValue @@ -229,7 +228,7 @@ func TestInjectBaggageToHTTPReq(t *testing.T) { t.Run(tt.name, func(t *testing.T) { req, _ := http.NewRequest("GET", "http://example.com", nil) ctx := baggage.ContextWithMap(context.Background(), baggage.NewMap(baggage.MapUpdate{MultiKV: tt.kvs})) - propagation.InjectHTTP(ctx, props, req.Header) + propagator.Inject(ctx, req.Header) gotHeader := req.Header.Get("otcorrelations") wantedLen := len(strings.Join(tt.wantInHeader, ",")) @@ -252,7 +251,7 @@ func TestInjectBaggageToHTTPReq(t *testing.T) { func TestTraceContextPropagator_GetAllKeys(t *testing.T) { var propagator baggage.Baggage want := []string{"otcorrelations"} - got := propagator.GetAllKeys() + got := propagator.Fields() if diff := cmp.Diff(got, want); diff != "" { t.Errorf("GetAllKeys: -got +want %s", diff) } diff --git a/api/global/internal/state.go b/api/global/internal/state.go index d936f74604e..ecdfd75ab55 100644 --- a/api/global/internal/state.go +++ b/api/global/internal/state.go @@ -18,8 +18,8 @@ import ( "sync" "sync/atomic" + "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/api/metric" - "go.opentelemetry.io/otel/api/propagation" "go.opentelemetry.io/otel/api/trace" ) @@ -33,7 +33,7 @@ type ( } propagatorsHolder struct { - pr propagation.Propagators + tm otel.TextMapPropagator } ) @@ -90,14 +90,14 @@ func SetMeterProvider(mp metric.MeterProvider) { globalMeter.Store(meterProviderHolder{mp: mp}) } -// Propagators is the internal implementation for global.Propagators. -func Propagators() propagation.Propagators { - return globalPropagators.Load().(propagatorsHolder).pr +// TextMapPropagator is the internal implementation for global.TextMapPropagator. +func TextMapPropagator() otel.TextMapPropagator { + return globalPropagators.Load().(propagatorsHolder).tm } -// SetPropagators is the internal implementation for global.SetPropagators. -func SetPropagators(pr propagation.Propagators) { - globalPropagators.Store(propagatorsHolder{pr: pr}) +// SetTextMapPropagator is the internal implementation for global.SetTextMapPropagator. +func SetTextMapPropagator(p otel.TextMapPropagator) { + globalPropagators.Store(propagatorsHolder{tm: p}) } func defaultTracerValue() *atomic.Value { @@ -114,13 +114,14 @@ func defaultMeterValue() *atomic.Value { func defaultPropagatorsValue() *atomic.Value { v := &atomic.Value{} - v.Store(propagatorsHolder{pr: getDefaultPropagators()}) + v.Store(propagatorsHolder{tm: getDefaultTextMapPropagator()}) return v } -// getDefaultPropagators returns a default noop Propagators -func getDefaultPropagators() propagation.Propagators { - return propagation.New() +// getDefaultTextMapPropagator returns the default TextMapPropagator, +// configured with W3C trace and baggage propagation. +func getDefaultTextMapPropagator() otel.TextMapPropagator { + return otel.NewCompositeTextMapPropagator() } // ResetForTest restores the initial global state, for testing purposes. diff --git a/api/global/propagation.go b/api/global/propagation.go index e3090293dae..c5f4c2b28df 100644 --- a/api/global/propagation.go +++ b/api/global/propagation.go @@ -15,18 +15,17 @@ package global import ( + "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/api/global/internal" - "go.opentelemetry.io/otel/api/propagation" ) -// Propagators returns the registered global propagators instance. If -// none is registered then an instance of propagators.NoopPropagators -// is returned. -func Propagators() propagation.Propagators { - return internal.Propagators() +// TextMapPropagator returns the global TextMapPropagator. If none has been +// set, a No-Op TextMapPropagator is returned. +func TextMapPropagator() otel.TextMapPropagator { + return internal.TextMapPropagator() } -// SetPropagators registers `p` as the global propagators instance. -func SetPropagators(p propagation.Propagators) { - internal.SetPropagators(p) +// SetTextMapPropagator sets propagator as the global TSetTextMapPropagator. +func SetTextMapPropagator(propagator otel.TextMapPropagator) { + internal.SetTextMapPropagator(propagator) } diff --git a/api/propagation/doc.go b/api/propagation/doc.go deleted file mode 100644 index 6e5922a83f1..00000000000 --- a/api/propagation/doc.go +++ /dev/null @@ -1,16 +0,0 @@ -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -// Package propagation provides support for propagating context over HTTP. -package propagation // import "go.opentelemetry.io/otel/api/propagation" diff --git a/api/propagation/propagation.go b/api/propagation/propagation.go deleted file mode 100644 index 860fbcae68b..00000000000 --- a/api/propagation/propagation.go +++ /dev/null @@ -1,143 +0,0 @@ -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package propagation - -import ( - "context" -) - -// HTTPSupplier is an interface that specifies methods to retrieve and -// store a single value for a key to an associated carrier. It is -// implemented by http.Headers. -type HTTPSupplier interface { - // Get method retrieves a single value for a given key. - Get(key string) string - // Set method stores a single value for a given key. Note that - // this should not be appending a value to some array, but - // rather overwrite the old value. - Set(key string, value string) -} - -// HTTPExtractor extracts information from a HTTPSupplier into a -// context. -type HTTPExtractor interface { - // Extract method retrieves encoded information using supplier - // from the associated carrier, decodes it and creates a new - // context containing the decoded information. - // - // Information can be a correlation context or a remote span - // context. In case of span context, the propagator should - // store it in the context using - // trace.ContextWithRemoteSpanContext. In case of correlation - // context, the propagator should use correlation.WithMap to - // store it in the context. - Extract(ctx context.Context, supplier HTTPSupplier) context.Context -} - -// HTTPInjector injects information into a HTTPSupplier. -type HTTPInjector interface { - // Inject method retrieves information from the context, - // encodes it into propagator specific format and then injects - // the encoded information using supplier into an associated - // carrier. - Inject(ctx context.Context, supplier HTTPSupplier) -} - -// Config contains the current set of extractors and injectors. -type Config struct { - httpEx []HTTPExtractor - httpIn []HTTPInjector -} - -// Propagators is the interface to a set of injectors and extractors -// for all supported carrier formats. It can be used to chain multiple -// propagators into a single entity. -type Propagators interface { - // HTTPExtractors returns the configured extractors. - HTTPExtractors() []HTTPExtractor - - // HTTPInjectors returns the configured injectors. - HTTPInjectors() []HTTPInjector -} - -// HTTPPropagator is the interface to inject to and extract from -// HTTPSupplier. -type HTTPPropagator interface { - HTTPInjector - HTTPExtractor - - // GetAllKeys returns the HTTP header names used. - GetAllKeys() []string -} - -// Option support passing configuration parameters to New(). -type Option func(*Config) - -// propagators is the default Propagators implementation. -type propagators struct { - config Config -} - -// New returns a standard Propagators implementation. -func New(options ...Option) Propagators { - config := Config{} - for _, opt := range options { - opt(&config) - } - return &propagators{ - config: config, - } -} - -// WithInjectors appends to the optional injector set. -func WithInjectors(inj ...HTTPInjector) Option { - return func(config *Config) { - config.httpIn = append(config.httpIn, inj...) - } -} - -// WithExtractors appends to the optional extractor set. -func WithExtractors(ext ...HTTPExtractor) Option { - return func(config *Config) { - config.httpEx = append(config.httpEx, ext...) - } -} - -// HTTPExtractors implements Propagators. -func (p *propagators) HTTPExtractors() []HTTPExtractor { - return p.config.httpEx -} - -// HTTPInjectors implements Propagators. -func (p *propagators) HTTPInjectors() []HTTPInjector { - return p.config.httpIn -} - -// ExtractHTTP applies props.HTTPExtractors() to the passed context -// and the supplier and returns the combined result context. -func ExtractHTTP(ctx context.Context, props Propagators, supplier HTTPSupplier) context.Context { - for _, ex := range props.HTTPExtractors() { - ctx = ex.Extract(ctx, supplier) - } - return ctx -} - -// InjectHTTP applies props.HTTPInjectors() to the passed context and -// the supplier. -func InjectHTTP(ctx context.Context, props Propagators, supplier HTTPSupplier) { - for _, in := range props.HTTPInjectors() { - in.Inject(ctx, supplier) - } -} diff --git a/bridge/opentracing/bridge.go b/bridge/opentracing/bridge.go index 25d1402ec27..3a0d2a3bf02 100644 --- a/bridge/opentracing/bridge.go +++ b/bridge/opentracing/bridge.go @@ -25,9 +25,9 @@ import ( otext "github.com/opentracing/opentracing-go/ext" otlog "github.com/opentracing/opentracing-go/log" + "go.opentelemetry.io/otel" otelbaggage "go.opentelemetry.io/otel/api/baggage" otelglobal "go.opentelemetry.io/otel/api/global" - otelpropagation "go.opentelemetry.io/otel/api/propagation" oteltrace "go.opentelemetry.io/otel/api/trace" "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/internal/trace/noop" @@ -287,7 +287,7 @@ type BridgeTracer struct { warningHandler BridgeWarningHandler warnOnce sync.Once - propagators otelpropagation.Propagators + propagator otel.TextMapPropagator } var _ ot.Tracer = &BridgeTracer{} @@ -304,7 +304,7 @@ func NewBridgeTracer() *BridgeTracer { otelTracer: noop.Tracer, }, warningHandler: func(msg string) {}, - propagators: nil, + propagator: nil, } } @@ -322,8 +322,8 @@ func (t *BridgeTracer) SetOpenTelemetryTracer(tracer oteltrace.Tracer) { t.setTracer.isSet = true } -func (t *BridgeTracer) SetPropagators(propagators otelpropagation.Propagators) { - t.propagators = propagators +func (t *BridgeTracer) SetTextMapPropagator(propagator otel.TextMapPropagator) { + t.propagator = propagator } func (t *BridgeTracer) NewHookedContext(ctx context.Context) context.Context { @@ -614,7 +614,7 @@ func (t *BridgeTracer) Inject(sm ot.SpanContext, format interface{}, carrier int } ctx := oteltrace.ContextWithSpan(context.Background(), fs) ctx = otelbaggage.ContextWithMap(ctx, bridgeSC.baggageItems) - otelpropagation.InjectHTTP(ctx, t.getPropagators(), header) + t.getPropagator().Inject(ctx, header) return nil } @@ -631,7 +631,7 @@ func (t *BridgeTracer) Extract(format interface{}, carrier interface{}) (ot.Span return nil, ot.ErrInvalidCarrier } header := http.Header(hhcarrier) - ctx := otelpropagation.ExtractHTTP(context.Background(), t.getPropagators(), header) + ctx := t.getPropagator().Extract(context.Background(), header) baggage := otelbaggage.MapFromContext(ctx) otelSC, _, _ := otelparent.GetSpanContextAndLinks(ctx, false) bridgeSC := &bridgeSpanContext{ @@ -644,9 +644,9 @@ func (t *BridgeTracer) Extract(format interface{}, carrier interface{}) (ot.Span return bridgeSC, nil } -func (t *BridgeTracer) getPropagators() otelpropagation.Propagators { - if t.propagators != nil { - return t.propagators +func (t *BridgeTracer) getPropagator() otel.TextMapPropagator { + if t.propagator != nil { + return t.propagator } - return otelglobal.Propagators() + return otelglobal.TextMapPropagator() } diff --git a/example/basic/main.go b/example/basic/main.go index 8e343ff7da7..3de912b612f 100644 --- a/example/basic/main.go +++ b/example/basic/main.go @@ -21,7 +21,6 @@ import ( "go.opentelemetry.io/otel/api/baggage" "go.opentelemetry.io/otel/api/global" "go.opentelemetry.io/otel/api/metric" - "go.opentelemetry.io/otel/api/propagation" "go.opentelemetry.io/otel/api/trace" "go.opentelemetry.io/otel/exporters/stdout" "go.opentelemetry.io/otel/label" @@ -62,12 +61,8 @@ func main() { global.SetTracerProvider(tp) global.SetMeterProvider(pusher.MeterProvider()) - // set propagator to baggage since the default is no-op - bagPropagator := baggage.DefaultHTTPPropagator() - props := propagation.New(propagation.WithExtractors(bagPropagator), - propagation.WithInjectors(bagPropagator)) - - global.SetPropagators(props) + // set global propagator to baggage (the default is no-op). + global.SetTextMapPropagator(baggage.Baggage{}) tracer := global.Tracer("ex.com/basic") meter := global.Meter("ex.com/basic") diff --git a/example/otel-collector/main.go b/example/otel-collector/main.go index 20b2bccb113..d917f5eaf19 100644 --- a/example/otel-collector/main.go +++ b/example/otel-collector/main.go @@ -27,7 +27,6 @@ import ( "go.opentelemetry.io/otel/api/global" "go.opentelemetry.io/otel/api/metric" - "go.opentelemetry.io/otel/api/propagation" apitrace "go.opentelemetry.io/otel/api/trace" "go.opentelemetry.io/otel/exporters/otlp" "go.opentelemetry.io/otel/label" @@ -75,10 +74,8 @@ func initProvider() func() { push.WithPeriod(2*time.Second), ) - tcPropagator := propagators.TraceContext{} - props := propagation.New(propagation.WithExtractors(tcPropagator), - propagation.WithInjectors(tcPropagator)) - global.SetPropagators(props) + // set global propagator to tracecontext (the default is no-op). + global.SetTextMapPropagator(propagators.TraceContext{}) global.SetTracerProvider(tracerProvider) global.SetMeterProvider(pusher.MeterProvider()) pusher.Start() diff --git a/otel.go b/otel.go index ebaaae1610c..aaabac2dc22 100644 --- a/otel.go +++ b/otel.go @@ -16,7 +16,6 @@ package otel import ( "go.opentelemetry.io/otel/api/metric" - "go.opentelemetry.io/otel/api/propagation" "go.opentelemetry.io/otel/api/trace" ) @@ -24,8 +23,6 @@ type Tracer = trace.Tracer type Meter = metric.Meter -type Propagators = propagation.Propagators - // ErrorHandler handles irremediable events. type ErrorHandler interface { // Handle handles any error deemed irremediable by an OpenTelemetry diff --git a/propagation.go b/propagation.go new file mode 100644 index 00000000000..8e3bd8d4654 --- /dev/null +++ b/propagation.go @@ -0,0 +1,78 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package otel + +import "context" + +// TextMapCarrier is the storage medium used by a TextMapPropagator. +type TextMapCarrier interface { + // Get returns the value associated with the passed key. + Get(key string) string + // Set stores the key-value pair. + Set(key string, value string) +} + +// TextMapPropagator propagates cross-cutting concerns as key-value text +// pairs within a carrier that travels in-band across process boundaries. +type TextMapPropagator interface { + // Inject set cross-cutting concerns from the Context into the carrier. + Inject(ctx context.Context, carrier TextMapCarrier) + // Extract reads cross-cutting concerns from the carrier into a Context. + Extract(ctx context.Context, carrier TextMapCarrier) context.Context + // Fields returns the keys who's values are set with Inject. + Fields() []string +} + +type compositeTextMapPropagator []TextMapPropagator + +func (p compositeTextMapPropagator) Inject(ctx context.Context, carrier TextMapCarrier) { + for _, i := range p { + i.Inject(ctx, carrier) + } +} + +func (p compositeTextMapPropagator) Extract(ctx context.Context, carrier TextMapCarrier) context.Context { + for _, i := range p { + ctx = i.Extract(ctx, carrier) + } + return ctx +} + +func (p compositeTextMapPropagator) Fields() []string { + unique := make(map[string]struct{}) + for _, i := range p { + for _, k := range i.Fields() { + unique[k] = struct{}{} + } + } + + fields := make([]string, 0, len(unique)) + for k := range unique { + fields = append(fields, k) + } + return fields +} + +// NewCompositeTextMapPropagator returns a unified TextMapPropagator from the +// group of passed TextMapPropagator. This allows different cross-cutting +// concerns to be propagates in a unified manner. +// +// The returned TextMapPropagator will inject and extract cross-cutting +// concerns in the order the TextMapPropagators were provided. Additionally, +// the Fields method will return a de-duplicated slice of the keys that are +// set with the Inject method. +func NewCompositeTextMapPropagator(p ...TextMapPropagator) TextMapPropagator { + return compositeTextMapPropagator(p) +} diff --git a/propagation_test.go b/propagation_test.go new file mode 100644 index 00000000000..14d2058254f --- /dev/null +++ b/propagation_test.go @@ -0,0 +1,100 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package otel + +import ( + "context" + "strings" + "testing" +) + +type ctxKeyType uint + +var ( + ctxKey ctxKeyType = 0 +) + +type carrier []string + +func (c *carrier) Get(string) string { return "" } + +func (c *carrier) Set(setter, _ string) { + *c = append(*c, setter) +} + +type propagator struct { + Name string +} + +func (p propagator) Inject(ctx context.Context, carrier TextMapCarrier) { + carrier.Set(p.Name, "") +} + +func (p propagator) Extract(ctx context.Context, carrier TextMapCarrier) context.Context { + v := ctx.Value(ctxKey) + if v == nil { + ctx = context.WithValue(ctx, ctxKey, []string{p.Name}) + } else { + orig := v.([]string) + ctx = context.WithValue(ctx, ctxKey, append(orig, p.Name)) + } + return ctx +} + +func (p propagator) Fields() []string { return []string{p.Name} } + +func TestCompositeTextMapPropagatorFields(t *testing.T) { + a, b1, b2 := propagator{"a"}, propagator{"b"}, propagator{"b"} + + want := map[string]struct{}{ + "a": {}, + "b": {}, + } + got := NewCompositeTextMapPropagator(a, b1, b2).Fields() + if len(got) != len(want) { + t.Fatalf("invalid fields from composite: %v (want %v)", got, want) + } + for _, v := range got { + if _, ok := want[v]; !ok { + t.Errorf("invalid field returned from composite: %q", v) + } + } +} + +func TestCompositeTextMapPropagatorInject(t *testing.T) { + a, b := propagator{"a"}, propagator{"b"} + + c := make(carrier, 0, 2) + NewCompositeTextMapPropagator(a, b).Inject(context.Background(), &c) + + if got := strings.Join([]string(c), ","); got != "a,b" { + t.Errorf("invalid inject order: %s", got) + } +} + +func TestCompositeTextMapPropagatorExtract(t *testing.T) { + a, b := propagator{"a"}, propagator{"b"} + + ctx := context.Background() + ctx = NewCompositeTextMapPropagator(a, b).Extract(ctx, nil) + + v := ctx.Value(ctxKey) + if v == nil { + t.Fatal("no composite extraction") + } + if got := strings.Join(v.([]string), ","); got != "a,b" { + t.Errorf("invalid extract order: %s", got) + } +} diff --git a/propagators/propagators.go b/propagators/propagators.go deleted file mode 100644 index b35b2c66485..00000000000 --- a/propagators/propagators.go +++ /dev/null @@ -1,25 +0,0 @@ -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package propagators - -import "go.opentelemetry.io/otel/api/propagation" - -// DefaultHTTPPropagator returns the default OpenTelemetry HTTP propagator, -// the W3C Trace Context propagator. -func DefaultHTTPPropagator() propagation.HTTPPropagator { - // As specified here: - // https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/context/api-propagators.md#global-propagators - return TraceContext{} -} diff --git a/propagators/propagators_test.go b/propagators/propagators_test.go index 06a2f2c1142..d362a6822c8 100644 --- a/propagators/propagators_test.go +++ b/propagators/propagators_test.go @@ -21,7 +21,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "go.opentelemetry.io/otel/api/propagation" + "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/api/trace" "go.opentelemetry.io/otel/propagators" ) @@ -58,9 +58,9 @@ type outOfThinAirPropagator struct { t *testing.T } -var _ propagation.HTTPPropagator = outOfThinAirPropagator{} +var _ otel.TextMapPropagator = outOfThinAirPropagator{} -func (p outOfThinAirPropagator) Extract(ctx context.Context, supplier propagation.HTTPSupplier) context.Context { +func (p outOfThinAirPropagator) Extract(ctx context.Context, carrier otel.TextMapCarrier) context.Context { sc := trace.SpanContext{ TraceID: traceID, SpanID: spanID, @@ -70,34 +70,33 @@ func (p outOfThinAirPropagator) Extract(ctx context.Context, supplier propagatio return trace.ContextWithRemoteSpanContext(ctx, sc) } -func (outOfThinAirPropagator) Inject(context.Context, propagation.HTTPSupplier) {} +func (outOfThinAirPropagator) Inject(context.Context, otel.TextMapCarrier) {} -func (outOfThinAirPropagator) GetAllKeys() []string { +func (outOfThinAirPropagator) Fields() []string { return nil } -type nilSupplier struct{} +type nilCarrier struct{} -var _ propagation.HTTPSupplier = nilSupplier{} +var _ otel.TextMapCarrier = nilCarrier{} -func (nilSupplier) Get(key string) string { +func (nilCarrier) Get(key string) string { return "" } -func (nilSupplier) Set(key string, value string) {} +func (nilCarrier) Set(key string, value string) {} func TestMultiplePropagators(t *testing.T) { ootaProp := outOfThinAirPropagator{t: t} - ns := nilSupplier{} - testProps := []propagation.HTTPPropagator{ + ns := nilCarrier{} + testProps := []otel.TextMapPropagator{ propagators.TraceContext{}, } bg := context.Background() // sanity check of oota propagator, ensuring that it really // generates the valid span context out of thin air { - props := propagation.New(propagation.WithExtractors(ootaProp)) - ctx := propagation.ExtractHTTP(bg, props, ns) + ctx := ootaProp.Extract(bg, ns) sc := trace.RemoteSpanContextFromContext(ctx) require.True(t, sc.IsValid(), "oota prop failed sanity check") } @@ -105,19 +104,14 @@ func TestMultiplePropagators(t *testing.T) { // really are not putting any valid span context into an empty // go context in absence of the HTTP headers. for _, prop := range testProps { - props := propagation.New(propagation.WithExtractors(prop)) - ctx := propagation.ExtractHTTP(bg, props, ns) + ctx := prop.Extract(bg, ns) sc := trace.RemoteSpanContextFromContext(ctx) require.Falsef(t, sc.IsValid(), "%#v failed sanity check", prop) } for _, prop := range testProps { - props := propagation.New(propagation.WithExtractors(ootaProp, prop)) - ctx := propagation.ExtractHTTP(bg, props, ns) + props := otel.NewCompositeTextMapPropagator(ootaProp, prop) + ctx := props.Extract(bg, ns) sc := trace.RemoteSpanContextFromContext(ctx) assert.Truef(t, sc.IsValid(), "%#v clobbers span context", prop) } } - -func TestDefaultHTTPPropagator(t *testing.T) { - assert.IsType(t, propagators.TraceContext{}, propagators.DefaultHTTPPropagator()) -} diff --git a/propagators/trace_context.go b/propagators/trace_context.go index 06554781dce..4564c9b9e69 100644 --- a/propagators/trace_context.go +++ b/propagators/trace_context.go @@ -20,7 +20,7 @@ import ( "fmt" "regexp" - "go.opentelemetry.io/otel/api/propagation" + "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/api/trace" ) @@ -47,15 +47,14 @@ const ( // their proprietary information. type TraceContext struct{} -var _ propagation.HTTPPropagator = TraceContext{} +var _ otel.TextMapPropagator = TraceContext{} var traceCtxRegExp = regexp.MustCompile("^(?P[0-9a-f]{2})-(?P[a-f0-9]{32})-(?P[a-f0-9]{16})-(?P[a-f0-9]{2})(?:-.*)?$") -// Inject injects a context into the supplier as W3C Trace Context HTTP -// headers. -func (tc TraceContext) Inject(ctx context.Context, supplier propagation.HTTPSupplier) { +// Inject set tracecontext from the Context into the carrier. +func (tc TraceContext) Inject(ctx context.Context, carrier otel.TextMapCarrier) { tracestate := ctx.Value(tracestateKey) if state, ok := tracestate.(string); tracestate != nil && ok { - supplier.Set(tracestateHeader, state) + carrier.Set(tracestateHeader, state) } sc := trace.SpanFromContext(ctx).SpanContext() @@ -67,26 +66,25 @@ func (tc TraceContext) Inject(ctx context.Context, supplier propagation.HTTPSupp sc.TraceID, sc.SpanID, sc.TraceFlags&trace.FlagsSampled) - supplier.Set(traceparentHeader, h) + carrier.Set(traceparentHeader, h) } -// Extract extracts a context from the supplier if it contains W3C Trace -// Context headers. -func (tc TraceContext) Extract(ctx context.Context, supplier propagation.HTTPSupplier) context.Context { - state := supplier.Get(tracestateHeader) +// Extract reads tracecontext from the carrier into a returned Context. +func (tc TraceContext) Extract(ctx context.Context, carrier otel.TextMapCarrier) context.Context { + state := carrier.Get(tracestateHeader) if state != "" { ctx = context.WithValue(ctx, tracestateKey, state) } - sc := tc.extract(supplier) + sc := tc.extract(carrier) if !sc.IsValid() { return ctx } return trace.ContextWithRemoteSpanContext(ctx, sc) } -func (tc TraceContext) extract(supplier propagation.HTTPSupplier) trace.SpanContext { - h := supplier.Get(traceparentHeader) +func (tc TraceContext) extract(carrier otel.TextMapCarrier) trace.SpanContext { + h := carrier.Get(traceparentHeader) if h == "" { return trace.EmptySpanContext() } @@ -153,8 +151,7 @@ func (tc TraceContext) extract(supplier propagation.HTTPSupplier) trace.SpanCont return sc } -// GetAllKeys returns the HTTP header names this propagator will use when -// injecting. -func (tc TraceContext) GetAllKeys() []string { +// Fields returns the keys who's values are set with Inject. +func (tc TraceContext) Fields() []string { return []string{traceparentHeader, tracestateHeader} } diff --git a/propagators/trace_context_example_test.go b/propagators/trace_context_example_test.go index f7dbe667c32..38fb26d4c55 100644 --- a/propagators/trace_context_example_test.go +++ b/propagators/trace_context_example_test.go @@ -16,15 +16,11 @@ package propagators_test import ( "go.opentelemetry.io/otel/api/global" - "go.opentelemetry.io/otel/api/propagation" "go.opentelemetry.io/otel/propagators" ) func ExampleTraceContext() { tc := propagators.TraceContext{} // Register the TraceContext propagator globally. - global.SetPropagators(propagation.New( - propagation.WithExtractors(tc), - propagation.WithInjectors(tc), - )) + global.SetTextMapPropagator(tc) } diff --git a/propagators/trace_context_test.go b/propagators/trace_context_test.go index 8eaabb67596..27d5d1c6cc9 100644 --- a/propagators/trace_context_test.go +++ b/propagators/trace_context_test.go @@ -21,14 +21,13 @@ import ( "github.com/google/go-cmp/cmp" - "go.opentelemetry.io/otel/api/propagation" "go.opentelemetry.io/otel/api/trace" "go.opentelemetry.io/otel/api/trace/tracetest" "go.opentelemetry.io/otel/propagators" ) func TestExtractValidTraceContextFromHTTPReq(t *testing.T) { - props := propagation.New(propagation.WithExtractors(propagators.TraceContext{})) + prop := propagators.TraceContext{} tests := []struct { name string header string @@ -112,7 +111,7 @@ func TestExtractValidTraceContextFromHTTPReq(t *testing.T) { req.Header.Set("traceparent", tt.header) ctx := context.Background() - ctx = propagation.ExtractHTTP(ctx, props, req.Header) + ctx = prop.Extract(ctx, req.Header) gotSc := trace.RemoteSpanContextFromContext(ctx) if diff := cmp.Diff(gotSc, tt.wantSc); diff != "" { t.Errorf("Extract Tracecontext: %s: -got +want %s", tt.name, diff) @@ -123,7 +122,7 @@ func TestExtractValidTraceContextFromHTTPReq(t *testing.T) { func TestExtractInvalidTraceContextFromHTTPReq(t *testing.T) { wantSc := trace.EmptySpanContext() - props := propagation.New(propagation.WithExtractors(propagators.TraceContext{})) + prop := propagators.TraceContext{} tests := []struct { name string header string @@ -200,7 +199,7 @@ func TestExtractInvalidTraceContextFromHTTPReq(t *testing.T) { req.Header.Set("traceparent", tt.header) ctx := context.Background() - ctx = propagation.ExtractHTTP(ctx, props, req.Header) + ctx = prop.Extract(ctx, req.Header) gotSc := trace.RemoteSpanContextFromContext(ctx) if diff := cmp.Diff(gotSc, wantSc); diff != "" { t.Errorf("Extract Tracecontext: %s: -got +want %s", tt.name, diff) @@ -215,7 +214,7 @@ func TestInjectTraceContextToHTTPReq(t *testing.T) { Sampled: false, StartSpanID: &id, } - props := propagation.New(propagation.WithInjectors(propagators.TraceContext{})) + prop := propagators.TraceContext{} tests := []struct { name string sc trace.SpanContext @@ -261,7 +260,7 @@ func TestInjectTraceContextToHTTPReq(t *testing.T) { ctx = trace.ContextWithRemoteSpanContext(ctx, tt.sc) ctx, _ = mockTracer.Start(ctx, "inject") } - propagation.InjectHTTP(ctx, props, req.Header) + prop.Inject(ctx, req.Header) gotHeader := req.Header.Get("traceparent") if diff := cmp.Diff(gotHeader, tt.wantHeader); diff != "" { @@ -274,23 +273,23 @@ func TestInjectTraceContextToHTTPReq(t *testing.T) { func TestTraceContextPropagator_GetAllKeys(t *testing.T) { var propagator propagators.TraceContext want := []string{"traceparent", "tracestate"} - got := propagator.GetAllKeys() + got := propagator.Fields() if diff := cmp.Diff(got, want); diff != "" { t.Errorf("GetAllKeys: -got +want %s", diff) } } func TestTraceStatePropagation(t *testing.T) { - props := propagation.New(propagation.WithInjectors(propagators.TraceContext{}), propagation.WithExtractors(propagators.TraceContext{})) + prop := propagators.TraceContext{} want := "opaquevalue" headerName := "tracestate" inReq, _ := http.NewRequest(http.MethodGet, "http://example.com", nil) inReq.Header.Add(headerName, want) - ctx := propagation.ExtractHTTP(context.Background(), props, inReq.Header) + ctx := prop.Extract(context.Background(), inReq.Header) outReq, _ := http.NewRequest(http.MethodGet, "http://www.example.com", nil) - propagation.InjectHTTP(ctx, props, outReq.Header) + prop.Inject(ctx, outReq.Header) if diff := cmp.Diff(outReq.Header.Get(headerName), want); diff != "" { t.Errorf("Propagate tracestate: -got +want %s", diff)