Added methods for SpanID and TraceID on bridgeSpanContext (#3966)

* Added methods for SpanID and TraceID on bridgeSpanContext * changed test name * Added entry in changelog, updated readme, added comments on methods and fixed test case * updated CHANGELOG.md * promoted all methods from SpanContext to bridgeSpanContext * fixed readme and removed redudant IsSampled() from bridgeContext * fixed readme lint * Apply suggestions from code review Co-authored-by: Robert Pająk <pellared@hotmail.com> * addressed code review comment --------- Co-authored-by: Robert Pająk <pellared@hotmail.com>
open-telemetry · Apr 25, 2023 · 86f3258 · 86f3258
1 parent 6acade8
commit 86f3258
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 26 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -35,6 +35,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
   This adds an implementation requirement to set the interface default behavior for unimplemented methods. (#3916)
 - Move No-Op implementation from `go.opentelemetry.io/otel/metric` into its own package `go.opentelemetry.io/otel/metric/noop`. (#3941)
   - `metric.NewNoopMeterProvider` is replaced with `noop.NewMeterProvider`
+- Add all the methods from `"go.opentelemetry.io/otel/trace".SpanContext` to `bridgeSpanContext` by embedding `otel.SpanContext` in `bridgeSpanContext`. (#3966)
 - Wrap `UploadMetrics` error in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/` to improve error message when encountering generic grpc errors. (#3974)
 - The measurement methods for all instruments in `go.opentelemetry.io/otel/metric/instrument` accept an option instead of the variadic `"go.opentelemetry.io/otel/attribute".KeyValue`. (#3971)
   - The `Int64Counter.Add` method now accepts `...AddOption`

diff --git a/bridge/opentracing/README.md b/bridge/opentracing/README.md
@@ -43,17 +43,22 @@ When you have started an OpenTracing Span, make sure the OpenTelemetry knows abo
 
 The bridge functionality can be extended beyond the OpenTracing API.
 
-### `SpanContext.IsSampled`
-
-Return the underlying OpenTelemetry [`Span.IsSampled`](https://pkg.go.dev/go.opentelemetry.io/otel/trace#SpanContext.IsSampled) value by converting a `bridgeSpanContext`.
+Any [`trace.SpanContext`](https://pkg.go.dev/go.opentelemetry.io/otel/trace#SpanContext) method can be accessed as following:
 
 ```go
-type samplable interface {
+type spanContextProvider interface {
 	IsSampled() bool
+	TraceID() trace.TraceID
+	SpanID() trace.SpanID
+	TraceFlags() trace.TraceFlags
+	... // any other available method can be added here to access it
 }
 
 var sc opentracing.SpanContext = ...
-if s, ok := sc.(samplable); ok && s.IsSampled() {
-	// Do something with sc knowing it is sampled.
+if s, ok := sc.(spanContextProvider); ok {
+	// Use TraceID by s.TraceID()
+	// Use SpanID by s.SpanID()
+	// Use TraceFlags by s.TraceFlags()
+	...
 }
 ```
diff --git a/bridge/opentracing/bridge.go b/bridge/opentracing/bridge.go
@@ -44,16 +44,16 @@ var (
 )
 
 type bridgeSpanContext struct {
-	bag             baggage.Baggage
-	otelSpanContext trace.SpanContext
+	bag baggage.Baggage
+	trace.SpanContext
 }
 
 var _ ot.SpanContext = &bridgeSpanContext{}
 
 func newBridgeSpanContext(otelSpanContext trace.SpanContext, parentOtSpanContext ot.SpanContext) *bridgeSpanContext {
 	bCtx := &bridgeSpanContext{
-		bag:             baggage.Baggage{},
-		otelSpanContext: otelSpanContext,
+		bag:         baggage.Baggage{},
+		SpanContext: otelSpanContext,
 	}
 	if parentOtSpanContext != nil {
 		parentOtSpanContext.ForeachBaggageItem(func(key, value string) bool {
@@ -72,10 +72,6 @@ func (c *bridgeSpanContext) ForeachBaggageItem(handler func(k, v string) bool) {
 	}
 }
 
-func (c *bridgeSpanContext) IsSampled() bool {
-	return c.otelSpanContext.IsSampled()
-}
-
 func (c *bridgeSpanContext) setBaggageItem(restrictedKey, value string) {
 	crk := http.CanonicalHeaderKey(restrictedKey)
 	m, err := baggage.NewMember(crk, value)
@@ -429,7 +425,7 @@ func (t *BridgeTracer) StartSpan(operationName string, opts ...ot.StartSpanOptio
 	attributes, kind, hadTrueErrorTag := otTagsToOTelAttributesKindAndError(sso.Tags)
 	checkCtx := migration.WithDeferredSetup(context.Background())
 	if parentBridgeSC != nil {
-		checkCtx = trace.ContextWithRemoteSpanContext(checkCtx, parentBridgeSC.otelSpanContext)
+		checkCtx = trace.ContextWithRemoteSpanContext(checkCtx, parentBridgeSC.SpanContext)
 	}
 	checkCtx2, otelSpan := t.setTracer.tracer().Start(
 		checkCtx,
@@ -610,7 +606,7 @@ func otSpanReferencesToParentAndLinks(references []ot.SpanReference) (*bridgeSpa
 
 func otSpanReferenceToOTelLink(bridgeSC *bridgeSpanContext, refType ot.SpanReferenceType) trace.Link {
 	return trace.Link{
-		SpanContext: bridgeSC.otelSpanContext,
+		SpanContext: bridgeSC.SpanContext,
 		Attributes:  otSpanReferenceTypeToOTelLinkAttributes(refType),
 	}
 }
@@ -655,7 +651,7 @@ func (t *BridgeTracer) Inject(sm ot.SpanContext, format interface{}, carrier int
 	if !ok {
 		return ot.ErrInvalidSpanContext
 	}
-	if !bridgeSC.otelSpanContext.IsValid() {
+	if !bridgeSC.IsValid() {
 		return ot.ErrInvalidSpanContext
 	}
 
@@ -687,7 +683,7 @@ func (t *BridgeTracer) Inject(sm ot.SpanContext, format interface{}, carrier int
 
 	fs := fakeSpan{
 		Span: noopSpan,
-		sc:   bridgeSC.otelSpanContext,
+		sc:   bridgeSC.SpanContext,
 	}
 	ctx := trace.ContextWithSpan(context.Background(), fs)
 	ctx = baggage.ContextWithBaggage(ctx, bridgeSC.bag)
@@ -729,10 +725,10 @@ func (t *BridgeTracer) Extract(format interface{}, carrier interface{}) (ot.Span
 	ctx := t.getPropagator().Extract(context.Background(), textCarrier)
 	bag := baggage.FromContext(ctx)
 	bridgeSC := &bridgeSpanContext{
-		bag:             bag,
-		otelSpanContext: trace.SpanContextFromContext(ctx),
+		bag:         bag,
+		SpanContext: trace.SpanContextFromContext(ctx),
 	}
-	if !bridgeSC.otelSpanContext.IsValid() {
+	if !bridgeSC.IsValid() {
 		return nil, ot.ErrSpanContextNotFound
 	}
 	return bridgeSC, nil

diff --git a/bridge/opentracing/bridge_test.go b/bridge/opentracing/bridge_test.go
@@ -366,12 +366,12 @@ func TestBridgeTracer_ExtractAndInject(t *testing.T) {
 					bsc, ok := spanContext.(*bridgeSpanContext)
 					assert.True(t, ok)
 					require.NotNil(t, bsc)
-					require.NotNil(t, bsc.otelSpanContext)
-					require.NotNil(t, bsc.otelSpanContext.SpanID())
-					require.NotNil(t, bsc.otelSpanContext.TraceID())
+					require.NotNil(t, bsc.SpanContext)
+					require.NotNil(t, bsc.SpanID())
+					require.NotNil(t, bsc.TraceID())
 
-					assert.Equal(t, spanID.String(), bsc.otelSpanContext.SpanID().String())
-					assert.Equal(t, traceID.String(), bsc.otelSpanContext.TraceID().String())
+					assert.Equal(t, spanID.String(), bsc.SpanID().String())
+					assert.Equal(t, traceID.String(), bsc.TraceID().String())
 				}
 			}
 		})
@@ -546,3 +546,33 @@ func TestBridge_SpanContext_IsSampled(t *testing.T) {
 		})
 	}
 }
+
+func TestBridgeSpanContextPromotedMethods(t *testing.T) {
+	bridge := NewBridgeTracer()
+	bridge.SetTextMapPropagator(new(testTextMapPropagator))
+
+	tmc := newTextCarrier()
+
+	type spanContextProvider interface {
+		HasTraceID() bool
+		TraceID() trace.TraceID
+		HasSpanID() bool
+		SpanID() trace.SpanID
+	}
+
+	err := bridge.Inject(newBridgeSpanContext(trace.NewSpanContext(trace.SpanContextConfig{
+		TraceID: [16]byte{byte(1)},
+		SpanID:  [8]byte{byte(2)},
+	}), nil), ot.TextMap, tmc)
+	assert.NoError(t, err)
+
+	spanContext, err := bridge.Extract(ot.TextMap, tmc)
+	assert.NoError(t, err)
+
+	assert.NotPanics(t, func() {
+		assert.Equal(t, spanID.String(), spanContext.(spanContextProvider).SpanID().String())
+		assert.Equal(t, traceID.String(), spanContext.(spanContextProvider).TraceID().String())
+		assert.True(t, spanContext.(spanContextProvider).HasSpanID())
+		assert.True(t, spanContext.(spanContextProvider).HasTraceID())
+	})
+}