opentelemetry-go
opentelemetry-go copied to clipboard
Noop tracer panics when calling `.Start()` with `nil` context
Description
Currently the .Start method of the noop tracer looks like this
// Start carries forward a non-recording Span, if one is present in the context, otherwise it
// creates a no-op Span.
func (t noopTracer) Start(ctx context.Context, name string, _ ...SpanStartOption) (context.Context, Span) {
span := SpanFromContext(ctx)
if _, ok := span.(nonRecordingSpan); !ok {
// span is likely already a noopSpan, but let's be sure
span = noopSpanInstance
}
return ContextWithSpan(ctx, span), span
}
Which throws a panic if a nil context is provided
Comparing it to the .Start method of the SDK tracer
func (tr *tracer) Start(ctx context.Context, name string, options ...trace.SpanStartOption) (context.Context, trace.Span) {
config := trace.NewSpanStartConfig(options...)
if ctx == nil {
// Prevent trace.ContextWithSpan from panicking.
ctx = context.Background()
}
...
return trace.ContextWithSpan(ctx, s), s
}
We can see that there's a safeguard for a nil context as agreed here
Expected behavior
Do not throw panic
This seems like standard Go behavior. The context package explicitly states nil context should not be used:
This seems like standard Go behavior. The
contextpackage explicitly statesnilcontext should not be used:
This was previously discussed and agreed upon here
Here's a link to the spec, that says:
API methods MUST NOT throw unhandled exceptions when used incorrectly by end users. The API and SDK SHOULD provide safe defaults for missing or invalid arguments. For instance, a name like empty may be used if the user passes in null as the span name argument during Span construction.
