Add SpanContextFromContext() (open-telemetry#1255)

* SpanFromContext returns nil if span not exists * Add tests for SpanContextFromContext * Update CHANGELOG * Update trace.go Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com> * SpanFromContext() continue to return a noopSpan{} Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
atlassian-forks · Oct 21, 2020 · 230bdd1 · 230bdd1
1 parent c9bc90b
commit 230bdd1
Show file tree

Hide file tree

Showing 8 changed files with 76 additions and 33 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -55,15 +55,15 @@ 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() {
 			span.links[rsc] = []label.KeyValue{iodKey.String("remote")}
 		}
 	} 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
@@ -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
@@ -205,14 +205,22 @@ 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
 	}
 	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
@@ -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)
+		})
+	}
+}