Skip to content

Commit

Permalink
PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Steven Karis committed Oct 16, 2019
1 parent b5e2c33 commit b99e571
Show file tree
Hide file tree
Showing 12 changed files with 320 additions and 330 deletions.
7 changes: 4 additions & 3 deletions api/propagation/noop_propagator.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"

"go.opentelemetry.io/api/core"
dctx "go.opentelemetry.io/api/distributedcontext"
)

// NoopTextFormatPropagator implements TextFormatPropagator that does nothing.
Expand All @@ -26,12 +27,12 @@ type NoopTextFormatPropagator struct{}
var _ TextFormatPropagator = NoopTextFormatPropagator{}

// Inject does nothing.
func (np NoopTextFormatPropagator) Inject(ctx context.Context, supplier Supplier) {
func (np NoopTextFormatPropagator) Inject(ctx context.Context, correlationCtx dctx.Map, supplier Supplier) {
}

// Extract does nothing and returns an empty SpanContext
func (np NoopTextFormatPropagator) Extract(ctx context.Context, supplier Supplier) core.SpanContext {
return core.EmptySpanContext()
func (np NoopTextFormatPropagator) Extract(ctx context.Context, supplier Supplier) (core.SpanContext, dctx.Map) {
return core.EmptySpanContext(), dctx.NewEmptyMap()
}

// GetAllKeys returns empty list of strings.
Expand Down
15 changes: 9 additions & 6 deletions api/propagation/propagator.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,25 @@ import (
"context"

"go.opentelemetry.io/api/core"
dctx "go.opentelemetry.io/api/distributedcontext"
)

// TextFormatPropagator is an interface that specifies methods to inject and extract SpanContext
// into/from a carrier using Supplier interface.
// and distributed context into/from a carrier using Supplier interface.
// For example, HTTP Trace Context propagator would encode SpanContext into W3C Trace
// Context Header and set the header into HttpRequest.
type TextFormatPropagator interface {
// Inject method retrieves current SpanContext from the ctx, encodes it into propagator
// specific format and then injects the encoded SpanContext using supplier into a carrier
// associated with the supplier.
Inject(ctx context.Context, supplier Supplier)
// associated with the supplier. It also takes a correlationCtx whose values will be
// injected into a carrier using the supplier.
Inject(ctx context.Context, correlationCtx dctx.Map, supplier Supplier)

// Extract method retrieves encoded SpanContext using supplier from the associated carrier.
// It decodes the SpanContext and returns it. If no SpanContext was retrieved OR
// if the retrieved SpanContext is invalid then an empty SpanContext is returned.
Extract(ctx context.Context, supplier Supplier) core.SpanContext
// It decodes the SpanContext and returns it and a dctx of correlated context.
// If no SpanContext was retrieved OR if the retrieved SpanContext is invalid then
// an empty SpanContext is returned.
Extract(ctx context.Context, supplier Supplier) (core.SpanContext, dctx.Map)

// GetAllKeys returns all the keys that this propagator injects/extracts into/from a
// carrier. The use cases for this are
Expand Down
13 changes: 11 additions & 2 deletions plugin/httptrace/httptrace.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"net/http"

"go.opentelemetry.io/api/core"
dctx "go.opentelemetry.io/api/distributedcontext"
"go.opentelemetry.io/api/key"
"go.opentelemetry.io/propagation"
)
Expand All @@ -36,16 +37,24 @@ var (

// Returns the Attributes, Context Entries, and SpanContext that were encoded by Inject.
func Extract(ctx context.Context, req *http.Request) ([]core.KeyValue, []core.KeyValue, core.SpanContext) {
sc := propagator.Extract(ctx, req.Header)
sc, correlationCtx := propagator.Extract(ctx, req.Header)

attrs := []core.KeyValue{
URLKey.String(req.URL.String()),
// Etc.
}
correlationCtx.Foreach(func(kv core.KeyValue) bool {
attrs = append(attrs, kv)
return true
})

return attrs, nil, sc
}

func Inject(ctx context.Context, req *http.Request) {
propagator.Inject(ctx, req.Header)
propagator.Inject(ctx, dctx.NewEmptyMap(), req.Header)
}

func InjectWithCorrelationCtx(ctx context.Context, correlationCtx dctx.Map, req *http.Request) {
propagator.Inject(ctx, correlationCtx, req.Header)
}
6 changes: 4 additions & 2 deletions plugin/othttp/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"net/http"

"go.opentelemetry.io/api/core"
dctx "go.opentelemetry.io/api/distributedcontext"
"go.opentelemetry.io/api/propagation"
"go.opentelemetry.io/api/trace"
prop "go.opentelemetry.io/propagation"
Expand Down Expand Up @@ -129,7 +130,8 @@ func NewHandler(handler http.Handler, operation string, opts ...HandlerOption) h
func (h *HTTPHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
opts := append([]trace.SpanOption{}, h.spanOptions...) // start with the configured options

sc := h.prop.Extract(r.Context(), r.Header)
// TODO: do something with the correlation context
sc, _ := h.prop.Extract(r.Context(), r.Header)
if sc.IsValid() { // not a valid span context, so no link / parent relationship to establish
var opt trace.SpanOption
if h.public {
Expand Down Expand Up @@ -178,7 +180,7 @@ func (h *HTTPHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
setBeforeServeAttributes(span, r.Host, r.Method, r.URL.Path, r.URL.String(), r.UserAgent())
// inject the response header before calling ServeHTTP because a Write in
// ServeHTTP will cause all headers to be written out.
h.prop.Inject(ctx, rww.Header())
h.prop.Inject(ctx, dctx.NewEmptyMap(), rww.Header())

h.handler.ServeHTTP(rww, r.WithContext(ctx))
setAfterServeAttributes(span, bw.read, rww.written, int64(rww.statusCode), bw.err, rww.err)
Expand Down
9 changes: 5 additions & 4 deletions propagation/http_b3_propagator.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"go.opentelemetry.io/api/trace"

"go.opentelemetry.io/api/core"
dctx "go.opentelemetry.io/api/distributedcontext"
apipropagation "go.opentelemetry.io/api/propagation"
)

Expand Down Expand Up @@ -65,7 +66,7 @@ func HttpB3Propagator(singleHeader bool) httpB3Propagator {
return httpB3Propagator{singleHeader}
}

func (b3 httpB3Propagator) Inject(ctx context.Context, supplier apipropagation.Supplier) {
func (b3 httpB3Propagator) Inject(ctx context.Context, correlationCtx dctx.Map, supplier apipropagation.Supplier) {
sc := trace.CurrentSpan(ctx).SpanContext()
if sc.IsValid() {
if b3.singleHeader {
Expand All @@ -90,11 +91,11 @@ func (b3 httpB3Propagator) Inject(ctx context.Context, supplier apipropagation.S
}

// Extract retrieves B3 Headers from the supplier
func (b3 httpB3Propagator) Extract(ctx context.Context, supplier apipropagation.Supplier) core.SpanContext {
func (b3 httpB3Propagator) Extract(ctx context.Context, supplier apipropagation.Supplier) (core.SpanContext, dctx.Map) {
if b3.singleHeader {
return b3.extractSingleHeader(supplier)
return b3.extractSingleHeader(supplier), dctx.NewEmptyMap()
}
return b3.extract(supplier)
return b3.extract(supplier), dctx.NewEmptyMap()
}

func (b3 httpB3Propagator) GetAllKeys() []string {
Expand Down
5 changes: 3 additions & 2 deletions propagation/http_b3_propagator_benchmark_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"net/http"
"testing"

dctx "go.opentelemetry.io/api/distributedcontext"
"go.opentelemetry.io/api/trace"
mocktrace "go.opentelemetry.io/internal/trace"
"go.opentelemetry.io/propagation"
Expand Down Expand Up @@ -65,7 +66,7 @@ func BenchmarkExtractB3(b *testing.B) {
b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
_ = propagator.Extract(ctx, req.Header)
_, _ = propagator.Extract(ctx, req.Header)
}
})
}
Expand Down Expand Up @@ -112,7 +113,7 @@ func BenchmarkInjectB3(b *testing.B) {
b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
propagator.Inject(ctx, req.Header)
propagator.Inject(ctx, dctx.NewEmptyMap(), req.Header)
}
})
}
Expand Down
5 changes: 3 additions & 2 deletions propagation/http_b3_propagator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (

"github.com/google/go-cmp/cmp"

dctx "go.opentelemetry.io/api/distributedcontext"
"go.opentelemetry.io/api/trace"
mocktrace "go.opentelemetry.io/internal/trace"
"go.opentelemetry.io/propagation"
Expand Down Expand Up @@ -65,7 +66,7 @@ func TestExtractB3(t *testing.T) {
}

ctx := context.Background()
gotSc := propagator.Extract(ctx, req.Header)
gotSc, _ := propagator.Extract(ctx, req.Header)
if diff := cmp.Diff(gotSc, tt.wantSc); diff != "" {
t.Errorf("%s: %s: -got +want %s", tg.name, tt.name, diff)
}
Expand Down Expand Up @@ -111,7 +112,7 @@ func TestInjectB3(t *testing.T) {
} else {
ctx, _ = mockTracer.Start(ctx, "inject")
}
propagator.Inject(ctx, req.Header)
propagator.Inject(ctx, dctx.NewEmptyMap(), req.Header)

for h, v := range tt.wantHeaders {
got, want := req.Header.Get(h), v
Expand Down
104 changes: 0 additions & 104 deletions propagation/http_correlation_context_propagator.go

This file was deleted.

Loading

0 comments on commit b99e571

Please sign in to comment.