Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update propagation to conform with OpenTelemetry specification #1212

Merged
merged 7 commits into from
Oct 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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