Skip to content

Commit

Permalink
Add tests and fix opentracing bridge defer warning (#3029)
Browse files Browse the repository at this point in the history
* add tests and fix opentracing bridge defer warning

* add changelog entry

* Update CHANGELOG.md

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

* Update bridge/opentracing/bridge_test.go

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
  • Loading branch information
dmathieu and MrAlias committed Jul 22, 2022
1 parent 096a162 commit 05c5509
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 2 deletions.
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)
})
}
}

0 comments on commit 05c5509

Please sign in to comment.