-
Notifications
You must be signed in to change notification settings - Fork 415
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
[opentracing] add Compatibility Layer #126
Conversation
…cer must use the version available from the user environment
… with options are missing
…g Tracer with the OpenTracing interface
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a couple of nits
opentracing/config.go
Outdated
// The initialization returns a `io.Closer` that can be used to graceful | ||
// shutdown the tracer. If the configuration object defines a disabled | ||
// Tracer, a no-op implementation is returned. | ||
func NewTracer(config *Configuration) (ot.Tracer, io.Closer, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this live in tracer.go
?
opentracing/config_test.go
Outdated
assert.Equal(false, config.Debug) | ||
assert.Equal("opentracing.test", config.ServiceName) | ||
assert.Equal("localhost", config.AgentHostname) | ||
assert.Equal("8126", config.AgentPort) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
config.SampleRate
is missing
opentracing/config_test.go
Outdated
assert.Nil(err) | ||
} | ||
|
||
func TestTracerConstructor(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This basically does all that TestConfiguration
seems to do, let's merge the two?
opentracing/span.go
Outdated
// TODO: implementation missing | ||
func (s *Span) FinishWithOptions(options ot.FinishOptions) { | ||
if options.FinishTime.IsZero() { | ||
options.FinishTime = time.Now().UTC() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reason why this is the only place where timezone is explicitly set to UTC?
Fix race condition in the test
Overview
Compatibility layer for OpenTracing specification. The PR implements:
Tracer
Configuration
struct to configure a Datadog TracerSpan
andSpanContext
implementationTags
This PR is an initial work to support entirely OpenTracing. In the current state, the OT-compatible
Tracer
wraps our previous implementation, so internals are the same. To set the Datadog Tracer as aGlobalTracer
, you should add the following code to initialize theTracer
:Default values for
Configuration
are provided, and if you don't set aServiceName
, the binary name is used.