Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tests and fix opentracing bridge defer warning #3029

Merged
merged 4 commits into from Jul 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -16,6 +16,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
The package contains semantic conventions from the `v1.11.0` version of the OpenTelemetry specification. (#3009)
- Add http.method attribute to http server metric. (#3018)

### Fixed

- Invalid warning for context setup being deferred in OpenTracing bridge (#3029).

## [1.8.0/0.31.0] - 2022-07-08

### Added
Expand Down
5 changes: 3 additions & 2 deletions bridge/opentracing/bridge.go
Expand Up @@ -328,7 +328,8 @@ var _ ot.TracerContextWithSpanExtension = &BridgeTracer{}
func NewBridgeTracer() *BridgeTracer {
return &BridgeTracer{
setTracer: bridgeSetTracer{
otelTracer: noopTracer,
warningHandler: func(msg string) {},
otelTracer: noopTracer,
},
warningHandler: func(msg string) {},
propagator: nil,
Expand Down Expand Up @@ -434,7 +435,7 @@ func (t *BridgeTracer) StartSpan(operationName string, opts ...ot.StartSpanOptio
trace.WithLinks(links...),
trace.WithSpanKind(kind),
)
if checkCtx != checkCtx2 {
if ot.SpanFromContext(checkCtx2) != nil {
t.warnOnce.Do(func() {
t.warningHandler("SDK should have deferred the context setup, see the documentation of go.opentelemetry.io/otel/bridge/opentracing/migration\n")
})
Expand Down
65 changes: 65 additions & 0 deletions bridge/opentracing/bridge_test.go
Expand Up @@ -24,6 +24,7 @@ import (
ot "github.com/opentracing/opentracing-go"
"github.com/stretchr/testify/assert"

"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/propagation"
"go.opentelemetry.io/otel/trace"
)
Expand Down Expand Up @@ -360,3 +361,67 @@ func TestBridgeTracer_ExtractAndInject(t *testing.T) {
})
}
}

type nonDeferWrapperTracer struct {
*WrapperTracer
}

func (t *nonDeferWrapperTracer) Start(ctx context.Context, name string, opts ...trace.SpanStartOption) (context.Context, trace.Span) {
// Run start on the parent wrapper with a brand new context
// so `WithDeferredSetup` hasn't been called, and the OpenTracing context is injected.
return t.WrapperTracer.Start(context.Background(), name, opts...)
}

func TestBridgeTracer_StartSpan(t *testing.T) {
testCases := []struct {
name string
before func(*testing.T, *BridgeTracer)
expectWarnings []string
}{
{
name: "with no option set",
expectWarnings: []string{
"The OpenTelemetry tracer is not set, default no-op tracer is used! Call SetOpenTelemetryTracer to set it up.\n",
},
},
{
name: "with wrapper tracer set",
before: func(t *testing.T, bridge *BridgeTracer) {
wTracer := NewWrapperTracer(bridge, otel.Tracer("test"))
bridge.SetOpenTelemetryTracer(wTracer)
},
expectWarnings: []string(nil),
},
{
name: "with a non-defered wrapper tracer",
before: func(t *testing.T, bridge *BridgeTracer) {
wTracer := &nonDeferWrapperTracer{
NewWrapperTracer(bridge, otel.Tracer("test")),
}
bridge.SetOpenTelemetryTracer(wTracer)
},
expectWarnings: []string{
"SDK should have deferred the context setup, see the documentation of go.opentelemetry.io/otel/bridge/opentracing/migration\n",
},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
var warningMessages []string
bridge := NewBridgeTracer()
bridge.SetWarningHandler(func(msg string) {
warningMessages = append(warningMessages, msg)
})

if tc.before != nil {
tc.before(t, bridge)
}

span := bridge.StartSpan("test")
assert.NotNil(t, span)

assert.Equal(t, tc.expectWarnings, warningMessages)
})
}
}