ddtrace/opentracer: translate Datadog errors to OpenTracing errors #1009
Conversation
ddtrace/opentracer/tracer_test.go
Outdated
| func TestTranslateError(t *testing.T) { | ||
| t.Run("nil", func(t *testing.T) { | ||
| assert.Nil(t, translateError(nil)) | ||
| }) | ||
| t.Run("unrecognized", func(t *testing.T) { | ||
| err := errors.New("unrecognized") | ||
| assert.Equal(t, err, translateError(err)) | ||
| }) | ||
| t.Run("ErrSpanContextNotFound", func(t *testing.T) { | ||
| assert.Equal(t, opentracing.ErrSpanContextNotFound, translateError(tracer.ErrSpanContextNotFound)) | ||
| }) | ||
| t.Run("ErrInvalidCarrier", func(t *testing.T) { | ||
| assert.Equal(t, opentracing.ErrInvalidCarrier, translateError(tracer.ErrInvalidCarrier)) | ||
| }) | ||
| t.Run("ErrInvalidSpanContext", func(t *testing.T) { | ||
| assert.Equal(t, opentracing.ErrInvalidSpanContext, translateError(tracer.ErrInvalidSpanContext)) | ||
| }) | ||
| t.Run("ErrSpanContextCorrupted", func(t *testing.T) { | ||
| assert.Equal(t, opentracing.ErrSpanContextCorrupted, translateError(tracer.ErrSpanContextCorrupted)) | ||
| }) | ||
| } |
There was a problem hiding this comment.
I think this would be better as a table test (example).
That will reduce the boilerplate and make it simpler to update if we ever add more errors.
There was a problem hiding this comment.
Refactored tests to be table tests and test that Inject and Extract actually return the correct error.
This change fixes so that the OpenTracing implementation returns standard errors defined in opentracing-go, as opposed to returning errors defined in ddtrace/tracer. Fixes DataDog#998
krosen040
force-pushed
the
issue-998
branch
from
e1b14d5
to
e2c2612
Compare
There was a problem hiding this comment.
Actually, just noticed one more thing.
Tests seem to be missing ErrInvalidSpanContext. That should be a small addition to what you have to completely cover the errors.
See here for the logic that produces an ErrInvalidSpanContext (looks like just spanid = 0 || traceid == 0 || ctx.(type) != *tracer.spanContext)
dd-trace-go/ddtrace/tracer/textmap.go
Lines 217 to 221 in d62beaa
And here for the tests that generate it. We only need to do one to ensure the conversion.
dd-trace-go/ddtrace/tracer/textmap_test.go
Lines 91 to 96 in d62beaa
…s translated correctly
|
Added a test for |
This change fixes so that the OpenTracing implementation returns standard errors
defined in opentracing-go, as opposed to returning errors defined in ddtrace/tracer.
Fixes #998