Skip to content

Commit

Permalink
Update propagation to conform with OpenTelemetry specification (#1212)
Browse files Browse the repository at this point in the history
* Move propagation package contents to the otel package

* Implement package relocation

* Update propagation to OTel spec

* Add changes to changelog

* Add propagation tests
  • Loading branch information
MrAlias authored Oct 2, 2020
1 parent dc79f7f commit 6e184cd
Show file tree
Hide file tree
Showing 18 changed files with 280 additions and 311 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
29 changes: 11 additions & 18 deletions api/baggage/baggage_propagator.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,29 +19,22 @@ import (
"net/url"
"strings"

"go.opentelemetry.io/otel/api/propagation"
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/label"
)

// Temporary header name until W3C finalizes format.
// 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
Expand All @@ -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
}
Expand Down Expand Up @@ -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}
}
15 changes: 7 additions & 8 deletions api/baggage/baggage_propagator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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() {
Expand All @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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
Expand Down Expand Up @@ -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, ","))
Expand All @@ -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)
}
Expand Down
25 changes: 13 additions & 12 deletions api/global/internal/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -33,7 +33,7 @@ type (
}

propagatorsHolder struct {
pr propagation.Propagators
tm otel.TextMapPropagator
}
)

Expand Down Expand Up @@ -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 {
Expand All @@ -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.
Expand Down
17 changes: 8 additions & 9 deletions api/global/propagation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
16 changes: 0 additions & 16 deletions api/propagation/doc.go

This file was deleted.

143 changes: 0 additions & 143 deletions api/propagation/propagation.go

This file was deleted.

Loading

0 comments on commit 6e184cd

Please sign in to comment.