[opentracing bridge] support setting span kind tag after the creation…

… of a span

Signed-off-by: Chen Xu <chen.x@uber.com>

CHANGELOG.md

@@ -25,6 +25,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
   - `metric.NewNoopMeterProvider` is replaced with `noop.NewMeterProvider`
 - Wrap `UploadMetrics` error in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/` to improve error message when encountering generic grpc errors. (#3974)
 - Move global metric back to `go.opentelemetry.io/otel/metric/global` from `go.opentelemetry.io/otel`. (#3986)
+- Allow to set span kind tag after a span is created in OpenTracing bridge (#3997).
 
 ### Fixed
 

bridge/opentracing/bridge.go

@@ -158,7 +158,15 @@ func (s *bridgeSpan) SetOperationName(operationName string) ot.Span {
 func (s *bridgeSpan) SetTag(key string, value interface{}) ot.Span {
 	switch key {
 	case string(otext.SpanKind):
-		// TODO: Should we ignore it?
+		type readSpanKindSpan interface {
+			SpanKind() trace.SpanKind
+		}
+		// only add span kind tag when otel span kind is unset or bridge default (internal)
+		if span, ok := s.otelSpan.(readSpanKindSpan); ok {
+			if span.SpanKind() == trace.SpanKindInternal || span.SpanKind() == trace.SpanKindInternal {
+				s.otelSpan.SetAttributes(otTagToOTelAttr(key, value))
+			}
+		}
 	case string(otext.Error):
 		if b, ok := value.(bool); ok && b {
 			s.otelSpan.SetStatus(codes.Error, "")

bridge/opentracing/bridge_test.go

@@ -510,7 +510,7 @@ func Test_otTagsToOTelAttributesKindAndError(t *testing.T) {
 			b, _ := NewTracerPair(tracer)
 
 			s := b.StartSpan(tc.name, tc.opt...)
-			assert.Equal(t, s.(*bridgeSpan).otelSpan.(*internal.MockSpan).SpanKind, tc.expected)
+			assert.Equal(t, s.(*bridgeSpan).otelSpan.(*internal.MockSpan).SpanKind(), tc.expected)
 		})
 	}
 }
@@ -546,3 +546,59 @@ func TestBridge_SpanContext_IsSampled(t *testing.T) {
 		})
 	}
 }
+
+func TestBridgeSpan_SetSpanKindTag(t *testing.T) {
+	testCases := []struct {
+		name               string
+		inputSpanKind      interface{}
+		setSpanKind        interface{}
+		expectedAttributes []attribute.KeyValue
+	}{
+		{
+			name:               "set span kind tag to client for OTel span without span kind specified",
+			inputSpanKind:      nil,
+			setSpanKind:        ext.SpanKindRPCClientEnum,
+			expectedAttributes: []attribute.KeyValue{attribute.String(string(ext.SpanKind), string(ext.SpanKindRPCClientEnum))},
+		},
+		{
+			name:               "set span kind tag to server for OTel span without span kind specified",
+			inputSpanKind:      nil,
+			setSpanKind:        ext.SpanKindRPCServerEnum,
+			expectedAttributes: []attribute.KeyValue{attribute.String(string(ext.SpanKind), string(ext.SpanKindRPCServerEnum))},
+		},
+		{
+			name:               "allow to set span kind tag to non-standard span kind",
+			inputSpanKind:      nil,
+			setSpanKind:        "non-standard span kind",
+			expectedAttributes: []attribute.KeyValue{attribute.String(string(ext.SpanKind), "non-standard span kind")},
+		},
+		{
+			name:               "set span kind tag to client for OTel span with span kind specified",
+			inputSpanKind:      ext.SpanKindRPCServerEnum,
+			setSpanKind:        ext.SpanKindRPCClientEnum,
+			expectedAttributes: nil,
+		},
+		{
+			name:               "set span kind tag to server for OTel span with span kind specified",
+			inputSpanKind:      ext.SpanKindRPCClientEnum,
+			setSpanKind:        ext.SpanKindRPCServerEnum,
+			expectedAttributes: nil,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			otelMockTracer := internal.NewMockTracer()
+			bridgeTracer, _ := NewTracerPair(otelMockTracer)
+			startOption := ot.Tags{}
+			if tc.inputSpanKind != nil {
+				startOption = ot.Tags{string(ext.SpanKind): tc.inputSpanKind}
+			}
+			span := bridgeTracer.StartSpan("abc", startOption)
+			span.SetTag(string(ext.SpanKind), tc.setSpanKind)
+			mockSpan := span.(*bridgeSpan).otelSpan.(*internal.MockSpan)
+
+			assert.Equal(t, tc.expectedAttributes, mockSpan.Attributes)
+		})
+	}
+}

bridge/opentracing/internal/mock.go

@@ -88,7 +88,7 @@ func (t *MockTracer) Start(ctx context.Context, name string, opts ...trace.SpanS
 		EndTime:        time.Time{},
 		ParentSpanID:   t.getParentSpanID(ctx, &config),
 		Events:         nil,
-		SpanKind:       trace.ValidateSpanKind(config.SpanKind()),
+		spanKind:       trace.ValidateSpanKind(config.SpanKind()),
 	}
 	if !migration.SkipContextSetup(ctx) {
 		ctx = trace.ContextWithSpan(ctx, span)
@@ -185,7 +185,7 @@ type MockSpan struct {
 	mockTracer     *MockTracer
 	officialTracer trace.Tracer
 	spanContext    trace.SpanContext
-	SpanKind       trace.SpanKind
+	spanKind       trace.SpanKind
 	recording      bool
 
 	Attributes   []attribute.KeyValue
@@ -292,3 +292,7 @@ func (s *MockSpan) OverrideTracer(tracer trace.Tracer) {
 }
 
 func (s *MockSpan) TracerProvider() trace.TracerProvider { return trace.NewNoopTracerProvider() }
+
+func (s *MockSpan) SpanKind() trace.SpanKind {
+	return s.spanKind
+}

bridge/opentracing/mix_test.go

@@ -583,8 +583,8 @@ func checkTraceAndSpans(t *testing.T, tracer *internal.MockTracer, expectedTrace
 		if span.ParentSpanID != expectedParentSpanID {
 			t.Errorf("Expected finished span %d (span ID: %d) to have parent span ID %d, but got %d", idx, sctx.SpanID(), expectedParentSpanID, span.ParentSpanID)
 		}
-		if span.SpanKind != sks[span.SpanContext().SpanID()] {
-			t.Errorf("Expected finished span %d (span ID: %d) to have span.kind to be '%v' but was '%v'", idx, sctx.SpanID(), sks[span.SpanContext().SpanID()], span.SpanKind)
+		if span.SpanKind() != sks[span.SpanContext().SpanID()] {
+			t.Errorf("Expected finished span %d (span ID: %d) to have span.kind to be '%v' but was '%v'", idx, sctx.SpanID(), sks[span.SpanContext().SpanID()], span.SpanKind())
 		}
 	}
 }