From 230bdd10f0d3a2d00eaebcb153a5341f0882548f Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Thu, 22 Oct 2020 05:30:28 +0800 Subject: [PATCH] Add SpanContextFromContext() (#1255) * SpanFromContext returns nil if span not exists * Add tests for SpanContextFromContext * Update CHANGELOG * Update trace.go Co-authored-by: Tyler Yahn * SpanFromContext() continue to return a noopSpan{} Co-authored-by: Tyler Yahn --- CHANGELOG.md | 1 + bridge/opentracing/mix_test.go | 2 +- internal/trace/parent/parent.go | 2 +- oteltest/config.go | 2 +- oteltest/tracer.go | 4 +- propagators/trace_context.go | 2 +- trace.go | 10 +++- trace_test.go | 86 +++++++++++++++++++++++---------- 8 files changed, 76 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8c1b4480a5..394c013067b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - `EventOption` and the related `NewEventConfig` function are added to the `go.opentelemetry.io/otel` package to configure Span events. (#1254) - A `TextMapPropagator` and associated `TextMapCarrier` are added to the `go.opentelemetry.io/otel/oteltest` package to test TextMap type propagators and their use. (#1259) +- `SpanContextFromContext` returns `SpanContext` from context. (#1255) ### Changed diff --git a/bridge/opentracing/mix_test.go b/bridge/opentracing/mix_test.go index f099283050d..15c9e03c626 100644 --- a/bridge/opentracing/mix_test.go +++ b/bridge/opentracing/mix_test.go @@ -229,7 +229,7 @@ func (cast *currentActiveSpanTest) runOTOtelOT(t *testing.T, ctx context.Context } func (cast *currentActiveSpanTest) recordSpans(t *testing.T, ctx context.Context) context.Context { - spanID := otel.SpanFromContext(ctx).SpanContext().SpanID + spanID := otel.SpanContextFromContext(ctx).SpanID cast.recordedCurrentOtelSpanIDs = append(cast.recordedCurrentOtelSpanIDs, spanID) spanID = otel.SpanID{} diff --git a/internal/trace/parent/parent.go b/internal/trace/parent/parent.go index 991443e1a76..27d65579a6a 100644 --- a/internal/trace/parent/parent.go +++ b/internal/trace/parent/parent.go @@ -22,7 +22,7 @@ import ( ) func GetSpanContextAndLinks(ctx context.Context, ignoreContext bool) (otel.SpanContext, bool, []otel.Link) { - lsctx := otel.SpanFromContext(ctx).SpanContext() + lsctx := otel.SpanContextFromContext(ctx) rsctx := otel.RemoteSpanContextFromContext(ctx) if ignoreContext { diff --git a/oteltest/config.go b/oteltest/config.go index 27344c0e61f..92e7e7e02a3 100644 --- a/oteltest/config.go +++ b/oteltest/config.go @@ -28,7 +28,7 @@ func defaultSpanContextFunc() func(context.Context) otel.SpanContext { var traceID, spanID uint64 = 1, 1 return func(ctx context.Context) otel.SpanContext { var sc otel.SpanContext - if lsc := otel.SpanFromContext(ctx).SpanContext(); lsc.IsValid() { + if lsc := otel.SpanContextFromContext(ctx); lsc.IsValid() { sc = lsc } else if rsc := otel.RemoteSpanContextFromContext(ctx); rsc.IsValid() { sc = rsc diff --git a/oteltest/tracer.go b/oteltest/tracer.go index 24de6d6cedc..1bcdd6d3dbd 100644 --- a/oteltest/tracer.go +++ b/oteltest/tracer.go @@ -55,7 +55,7 @@ func (t *Tracer) Start(ctx context.Context, name string, opts ...otel.SpanOption span.spanContext = otel.SpanContext{} iodKey := label.Key("ignored-on-demand") - if lsc := otel.SpanFromContext(ctx).SpanContext(); lsc.IsValid() { + if lsc := otel.SpanContextFromContext(ctx); lsc.IsValid() { span.links[lsc] = []label.KeyValue{iodKey.String("current")} } if rsc := otel.RemoteSpanContextFromContext(ctx); rsc.IsValid() { @@ -63,7 +63,7 @@ func (t *Tracer) Start(ctx context.Context, name string, opts ...otel.SpanOption } } else { span.spanContext = t.config.SpanContextFunc(ctx) - if lsc := otel.SpanFromContext(ctx).SpanContext(); lsc.IsValid() { + if lsc := otel.SpanContextFromContext(ctx); lsc.IsValid() { span.spanContext.TraceID = lsc.TraceID span.parentSpanID = lsc.SpanID } else if rsc := otel.RemoteSpanContextFromContext(ctx); rsc.IsValid() { diff --git a/propagators/trace_context.go b/propagators/trace_context.go index 1d3d7769ce1..f4fe36a29d5 100644 --- a/propagators/trace_context.go +++ b/propagators/trace_context.go @@ -56,7 +56,7 @@ func (tc TraceContext) Inject(ctx context.Context, carrier otel.TextMapCarrier) carrier.Set(tracestateHeader, state) } - sc := otel.SpanFromContext(ctx).SpanContext() + sc := otel.SpanContextFromContext(ctx) if !sc.IsValid() { return } diff --git a/trace.go b/trace.go index edb95efa7f0..f047367ab7e 100644 --- a/trace.go +++ b/trace.go @@ -205,7 +205,7 @@ func ContextWithSpan(parent context.Context, span Span) context.Context { return context.WithValue(parent, currentSpanKey, span) } -// SpanFromContext returns the current span from ctx, or nil if none set. +// SpanFromContext returns the current span from ctx, or noop span if none set. func SpanFromContext(ctx context.Context) Span { if span, ok := ctx.Value(currentSpanKey).(Span); ok { return span @@ -213,6 +213,14 @@ func SpanFromContext(ctx context.Context) Span { return noopSpan{} } +// SpanContextFromContext returns the current SpanContext from ctx, or an empty SpanContext if none set. +func SpanContextFromContext(ctx context.Context) SpanContext { + if span := SpanFromContext(ctx); span != nil { + return span.SpanContext() + } + return SpanContext{} +} + // ContextWithRemoteSpanContext returns a copy of parent with a remote set as // the remote span context. func ContextWithRemoteSpanContext(parent context.Context, remote SpanContext) context.Context { diff --git a/trace_test.go b/trace_test.go index 246bbd5354f..09f28286bdd 100644 --- a/trace_test.go +++ b/trace_test.go @@ -17,43 +17,52 @@ package otel import ( "context" "testing" + + "github.com/stretchr/testify/assert" ) type testSpan struct { noopSpan - ID int8 + ID byte } -func TestContextSpan(t *testing.T) { - ctx := context.Background() - got, empty := SpanFromContext(ctx), noopSpan{} - if got != empty { - t.Errorf("SpanFromContext returned %v from an empty context, want %v", got, empty) - } +func (s testSpan) SpanContext() SpanContext { return SpanContext{SpanID: [8]byte{s.ID}} } - want := testSpan{ID: 0} - ctx = ContextWithSpan(ctx, want) - if got, ok := ctx.Value(currentSpanKey).(testSpan); !ok { - t.Errorf("failed to set context with %#v", want) - } else if got != want { - t.Errorf("got %#v from context with current set, want %#v", got, want) +func TestContextSpan(t *testing.T) { + testCases := []struct { + name string + context context.Context + expectedSpan Span + }{ + { + name: "empty context", + context: context.Background(), + expectedSpan: noopSpan{}, + }, + { + name: "span 0", + context: ContextWithSpan(context.Background(), testSpan{ID: 0}), + expectedSpan: testSpan{ID: 0}, + }, + { + name: "span 1", + context: ContextWithSpan(context.Background(), testSpan{ID: 1}), + expectedSpan: testSpan{ID: 1}, + }, } - if got := SpanFromContext(ctx); got != want { - t.Errorf("SpanFromContext returned %v from a set context, want %v", got, want) - } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + span := SpanFromContext(tc.context) + assert.Equal(t, tc.expectedSpan, span) - want = testSpan{ID: 1} - ctx = ContextWithSpan(ctx, want) - if got, ok := ctx.Value(currentSpanKey).(testSpan); !ok { - t.Errorf("failed to set context with %#v", want) - } else if got != want { - t.Errorf("got %#v from context with current overridden, want %#v", got, want) - } - - if got := SpanFromContext(ctx); got != want { - t.Errorf("SpanFromContext returned %v from a set context, want %v", got, want) + if _, ok := tc.expectedSpan.(noopSpan); !ok { + span, ok := tc.context.Value(currentSpanKey).(testSpan) + assert.True(t, ok) + assert.Equal(t, tc.expectedSpan.(testSpan), span) + } + }) } } @@ -403,3 +412,28 @@ func TestSpanKindString(t *testing.T) { } } } + +func TestSpanContextFromContext(t *testing.T) { + testCases := []struct { + name string + context context.Context + expectedSpanContext SpanContext + }{ + { + name: "empty context", + context: context.Background(), + }, + { + name: "span 1", + context: ContextWithSpan(context.Background(), testSpan{ID: 1}), + expectedSpanContext: SpanContext{SpanID: [8]byte{1}}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + spanContext := SpanContextFromContext(tc.context) + assert.Equal(t, tc.expectedSpanContext, spanContext) + }) + } +}