From 6dcf5d46b1145a8a9975c10154316560dfb1cb72 Mon Sep 17 00:00:00 2001 From: Alan Protasio Date: Wed, 17 Aug 2022 10:42:54 -0700 Subject: [PATCH 1/6] Fix opentracing.Bridge where it was not identifying the spanKinf correctly --- bridge/opentracing/bridge.go | 21 +++++++++----------- bridge/opentracing/bridge_test.go | 33 +++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/bridge/opentracing/bridge.go b/bridge/opentracing/bridge.go index 2922b9869d5..3b5563024da 100644 --- a/bridge/opentracing/bridge.go +++ b/bridge/opentracing/bridge.go @@ -18,7 +18,6 @@ import ( "context" "fmt" "net/http" - "strings" "sync" ot "github.com/opentracing/opentracing-go" @@ -502,17 +501,15 @@ func otTagsToOTelAttributesKindAndError(tags map[string]interface{}) ([]attribut for k, v := range tags { switch k { case string(otext.SpanKind): - if s, ok := v.(string); ok { - switch strings.ToLower(s) { - case "client": - kind = trace.SpanKindClient - case "server": - kind = trace.SpanKindServer - case "producer": - kind = trace.SpanKindProducer - case "consumer": - kind = trace.SpanKindConsumer - } + switch v { + case otext.SpanKindRPCClientEnum: + kind = trace.SpanKindClient + case otext.SpanKindRPCServerEnum: + kind = trace.SpanKindServer + case otext.SpanKindProducerEnum: + kind = trace.SpanKindProducer + case otext.SpanKindConsumerEnum: + kind = trace.SpanKindConsumer } case string(otext.Error): if b, ok := v.(bool); ok && b { diff --git a/bridge/opentracing/bridge_test.go b/bridge/opentracing/bridge_test.go index a64d6f73919..db7f62bd43c 100644 --- a/bridge/opentracing/bridge_test.go +++ b/bridge/opentracing/bridge_test.go @@ -22,7 +22,9 @@ import ( "testing" ot "github.com/opentracing/opentracing-go" + "github.com/opentracing/opentracing-go/ext" "github.com/stretchr/testify/assert" + "go.opentelemetry.io/otel/bridge/opentracing/internal" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/propagation" @@ -425,3 +427,34 @@ func TestBridgeTracer_StartSpan(t *testing.T) { }) } } + +func Test_otTagsToOTelAttributesKindAndError(t *testing.T) { + tracer := internal.NewMockTracer() + sc := &bridgeSpanContext{} + + testCases := []struct { + name string + opt []ot.StartSpanOption + expected trace.SpanKind + }{ + { + name: "client", + opt: []ot.StartSpanOption{ext.SpanKindRPCClient}, + expected: trace.SpanKindClient, + }, + { + name: "server", + opt: []ot.StartSpanOption{ext.RPCServerOption(sc)}, + expected: trace.SpanKindServer, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + b, _ := NewTracerPair(tracer) + + s := b.StartSpan(tc.name, tc.opt...) + assert.Equal(t, s.(*bridgeSpan).otelSpan.(*internal.MockSpan).SpanKind, tc.expected) + }) + } +} From d8d3d2d074e35b217e43bfd6a1ad20d38aa07006 Mon Sep 17 00:00:00 2001 From: Alan Protasio Date: Wed, 17 Aug 2022 10:58:24 -0700 Subject: [PATCH 2/6] fix test --- bridge/opentracing/mix_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bridge/opentracing/mix_test.go b/bridge/opentracing/mix_test.go index 0cd5a667ca5..304bdaae591 100644 --- a/bridge/opentracing/mix_test.go +++ b/bridge/opentracing/mix_test.go @@ -20,7 +20,7 @@ import ( "testing" ot "github.com/opentracing/opentracing-go" - + "github.com/opentracing/opentracing-go/ext" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/baggage" @@ -637,7 +637,7 @@ func runOtelOTOtel(t *testing.T, ctx context.Context, name string, callback func func runOTOtelOT(t *testing.T, ctx context.Context, name string, callback func(*testing.T, context.Context) context.Context) { tr := otel.Tracer("") - span, ctx := ot.StartSpanFromContext(ctx, fmt.Sprintf("%s_OT_OtelOT", name), ot.Tag{Key: "span.kind", Value: "client"}) + span, ctx := ot.StartSpanFromContext(ctx, fmt.Sprintf("%s_OT_OtelOT", name), ot.Tag{Key: "span.kind", Value: ext.SpanKindRPCClientEnum}) defer span.Finish() ctx = callback(t, ctx) func(ctx2 context.Context) { @@ -645,7 +645,7 @@ func runOTOtelOT(t *testing.T, ctx context.Context, name string, callback func(* defer span.End() ctx2 = callback(t, ctx2) func(ctx3 context.Context) { - span, ctx3 := ot.StartSpanFromContext(ctx3, fmt.Sprintf("%sOTOtel_OT_", name), ot.Tag{Key: "span.kind", Value: "producer"}) + span, ctx3 := ot.StartSpanFromContext(ctx3, fmt.Sprintf("%sOTOtel_OT_", name), ot.Tag{Key: "span.kind", Value: ext.SpanKindProducerEnum}) defer span.Finish() _ = callback(t, ctx3) }(ctx2) From de376addb25e18c9caceb1451eee89e0a83b3f66 Mon Sep 17 00:00:00 2001 From: Alan Protasio Date: Wed, 17 Aug 2022 11:07:19 -0700 Subject: [PATCH 3/6] changelog --- CHANGELOG.md | 2 ++ bridge/opentracing/bridge_test.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a28332f3fbc..9c7a6a06f05 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Support Go 1.19. Include compatibility testing and document support. (#3077) +### Fixed +- Fix misidentification of otel spanKind in opentracing bridge (`go.opentelemetry.io/otel/bridge/opentracing`) ## [1.9.0/0.0.3] - 2022-08-01 ### Added diff --git a/bridge/opentracing/bridge_test.go b/bridge/opentracing/bridge_test.go index db7f62bd43c..3b66767e960 100644 --- a/bridge/opentracing/bridge_test.go +++ b/bridge/opentracing/bridge_test.go @@ -24,9 +24,9 @@ import ( ot "github.com/opentracing/opentracing-go" "github.com/opentracing/opentracing-go/ext" "github.com/stretchr/testify/assert" - "go.opentelemetry.io/otel/bridge/opentracing/internal" "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/bridge/opentracing/internal" "go.opentelemetry.io/otel/propagation" "go.opentelemetry.io/otel/trace" ) From 40db81d735d9a16feba3a03cd0c228f53d2d9e18 Mon Sep 17 00:00:00 2001 From: Alan Protasio Date: Wed, 17 Aug 2022 11:31:18 -0700 Subject: [PATCH 4/6] Keeping backward comppatibillity --- bridge/opentracing/bridge.go | 7 ++++++- bridge/opentracing/bridge_test.go | 10 ++++++++++ bridge/opentracing/mix_test.go | 6 +++--- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/bridge/opentracing/bridge.go b/bridge/opentracing/bridge.go index 3b5563024da..967aaa20d4b 100644 --- a/bridge/opentracing/bridge.go +++ b/bridge/opentracing/bridge.go @@ -18,6 +18,7 @@ import ( "context" "fmt" "net/http" + "strings" "sync" ot "github.com/opentracing/opentracing-go" @@ -501,7 +502,11 @@ func otTagsToOTelAttributesKindAndError(tags map[string]interface{}) ([]attribut for k, v := range tags { switch k { case string(otext.SpanKind): - switch v { + sk := v + if s, ok := v.(string); ok { + sk = otext.SpanKindEnum(strings.ToLower(s)) + } + switch sk { case otext.SpanKindRPCClientEnum: kind = trace.SpanKindClient case otext.SpanKindRPCServerEnum: diff --git a/bridge/opentracing/bridge_test.go b/bridge/opentracing/bridge_test.go index 3b66767e960..7286bd77823 100644 --- a/bridge/opentracing/bridge_test.go +++ b/bridge/opentracing/bridge_test.go @@ -447,6 +447,16 @@ func Test_otTagsToOTelAttributesKindAndError(t *testing.T) { opt: []ot.StartSpanOption{ext.RPCServerOption(sc)}, expected: trace.SpanKindServer, }, + { + name: "client string", + opt: []ot.StartSpanOption{ot.Tag{Key: "span.kind", Value: "client"}}, + expected: trace.SpanKindClient, + }, + { + name: "server string", + opt: []ot.StartSpanOption{ot.Tag{Key: "span.kind", Value: "server"}}, + expected: trace.SpanKindServer, + }, } for _, tc := range testCases { diff --git a/bridge/opentracing/mix_test.go b/bridge/opentracing/mix_test.go index 304bdaae591..0cd5a667ca5 100644 --- a/bridge/opentracing/mix_test.go +++ b/bridge/opentracing/mix_test.go @@ -20,7 +20,7 @@ import ( "testing" ot "github.com/opentracing/opentracing-go" - "github.com/opentracing/opentracing-go/ext" + "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/baggage" @@ -637,7 +637,7 @@ func runOtelOTOtel(t *testing.T, ctx context.Context, name string, callback func func runOTOtelOT(t *testing.T, ctx context.Context, name string, callback func(*testing.T, context.Context) context.Context) { tr := otel.Tracer("") - span, ctx := ot.StartSpanFromContext(ctx, fmt.Sprintf("%s_OT_OtelOT", name), ot.Tag{Key: "span.kind", Value: ext.SpanKindRPCClientEnum}) + span, ctx := ot.StartSpanFromContext(ctx, fmt.Sprintf("%s_OT_OtelOT", name), ot.Tag{Key: "span.kind", Value: "client"}) defer span.Finish() ctx = callback(t, ctx) func(ctx2 context.Context) { @@ -645,7 +645,7 @@ func runOTOtelOT(t *testing.T, ctx context.Context, name string, callback func(* defer span.End() ctx2 = callback(t, ctx2) func(ctx3 context.Context) { - span, ctx3 := ot.StartSpanFromContext(ctx3, fmt.Sprintf("%sOTOtel_OT_", name), ot.Tag{Key: "span.kind", Value: ext.SpanKindProducerEnum}) + span, ctx3 := ot.StartSpanFromContext(ctx3, fmt.Sprintf("%sOTOtel_OT_", name), ot.Tag{Key: "span.kind", Value: "producer"}) defer span.Finish() _ = callback(t, ctx3) }(ctx2) From 1cb870de55dc854f7b17069d22ef17f0d56eb3a7 Mon Sep 17 00:00:00 2001 From: Alan Protasio Date: Wed, 17 Aug 2022 12:58:51 -0700 Subject: [PATCH 5/6] Update CHANGELOG.md Co-authored-by: Anthony Mirabella --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c7a6a06f05..9200aa0a6c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm Include compatibility testing and document support. (#3077) ### Fixed -- Fix misidentification of otel spanKind in opentracing bridge (`go.opentelemetry.io/otel/bridge/opentracing`) +- Fix misidentification of OpenTelemetry `SpanKind` in OpenTracing bridge (`go.opentelemetry.io/otel/bridge/opentracing`). (#3096) ## [1.9.0/0.0.3] - 2022-08-01 ### Added From 318b06a1180d4688de45613a2828bf1e7dc7a3f0 Mon Sep 17 00:00:00 2001 From: Chester Cheung Date: Thu, 18 Aug 2022 17:40:52 +0800 Subject: [PATCH 6/6] Update CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9200aa0a6c7..3dd5926bfda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm Include compatibility testing and document support. (#3077) ### Fixed + - Fix misidentification of OpenTelemetry `SpanKind` in OpenTracing bridge (`go.opentelemetry.io/otel/bridge/opentracing`). (#3096) + ## [1.9.0/0.0.3] - 2022-08-01 ### Added