Skip to content

Change tracingService methods to use contexts #19093

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.

Sign up for GitHub

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

Merged
mattlord merged 2 commits into from
Jan 22, 2026
+35 −32
Merged

Change tracingService methods to use contexts #19093

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9f7ce81
Change tracingService methods to use contexts
mcpherrinm Dec 30, 2025
9de286e
Check contexts in test
mcpherrinm Jan 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions go/trace/fake.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,13 @@ import (

type noopTracingServer struct{}

func (noopTracingServer) New(Span, string) Span { return NoopSpan{} }
func (noopTracingServer) FromContext(context.Context) (Span, bool) { return nil, false }
func (noopTracingServer) NewFromString(parent, label string) (Span, error) { return NoopSpan{}, nil }
func (noopTracingServer) New(ctx context.Context, label string) (Span, context.Context) {
return NoopSpan{}, ctx
}
func (noopTracingServer) FromContext(context.Context) (Span, bool) { return nil, false }
func (noopTracingServer) NewFromString(ctx context.Context, parent, label string) (Span, context.Context, error) {
return NoopSpan{}, ctx, nil
}
func (noopTracingServer) NewContext(parent context.Context, _ Span) context.Context { return parent }
func (noopTracingServer) AddGrpcServerOptions(addInterceptors func(s grpc.StreamServerInterceptor, u grpc.UnaryServerInterceptor)) {
}
Expand Down
3 changes: 2 additions & 1 deletion go/trace/fake_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ func TestNoopTracingServer(t *testing.T) {
tracingSvc, closer, err := factoryFunc("value")
require.NoError(t, err)
require.NoError(t, closer.Close())
span, err := tracingSvc.NewFromString("parent", "label")
span, ctx, err := tracingSvc.NewFromString(t.Context(), "parent", "label")
require.NoError(t, err)
require.Empty(t, span)
require.Equal(t, t.Context(), ctx)
}
15 changes: 9 additions & 6 deletions go/trace/opentracing.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ func (jf openTracingService) AddGrpcClientOptions(addInterceptors func(s grpc.St
}

// New is part of an interface implementation
func (jf openTracingService) New(parent Span, label string) Span {
func (jf openTracingService) New(ctx context.Context, label string) (Span, context.Context) {
parent, _ := jf.FromContext(ctx)
var innerSpan opentracing.Span
if parent == nil {
innerSpan = jf.Tracer.GetOpenTracingTracer().StartSpan(label)
Expand All @@ -76,7 +77,8 @@ func (jf openTracingService) New(parent Span, label string) Span {
span := jaegerParent.otSpan
innerSpan = jf.Tracer.GetOpenTracingTracer().StartSpan(label, opentracing.ChildOf(span.Context()))
}
return openTracingSpan{otSpan: innerSpan}
span := openTracingSpan{otSpan: innerSpan}
return span, jf.NewContext(ctx, span)
}

func extractMapFromString(in string) (opentracing.TextMapCarrier, error) {
Expand All @@ -94,17 +96,18 @@ func extractMapFromString(in string) (opentracing.TextMapCarrier, error) {
return dat, nil
}

func (jf openTracingService) NewFromString(parent, label string) (Span, error) {
func (jf openTracingService) NewFromString(ctx context.Context, parent, label string) (Span, context.Context, error) {
carrier, err := extractMapFromString(parent)
if err != nil {
return nil, err
return nil, nil, err
}
spanContext, err := jf.Tracer.GetOpenTracingTracer().Extract(opentracing.TextMap, carrier)
if err != nil {
return nil, vterrors.Wrap(err, "failed to deserialize span context")
return nil, nil, vterrors.Wrap(err, "failed to deserialize span context")
}
innerSpan := jf.Tracer.GetOpenTracingTracer().StartSpan(label, opentracing.ChildOf(spanContext))
return openTracingSpan{otSpan: innerSpan}, nil
span := openTracingSpan{otSpan: innerSpan}
return span, jf.NewContext(ctx, span), nil
}

// FromContext is part of an interface implementation
Expand Down
8 changes: 5 additions & 3 deletions go/trace/opentracing_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,17 +54,19 @@ func TestNewSpan(t *testing.T) {
svc := openTracingService{
Tracer: &fakeTracer{},
}
clientSpan := svc.New(nil, "test-label")
clientSpan, ctx := svc.New(t.Context(), "test-label")
require.NotEmpty(t, clientSpan)
require.NotNil(t, ctx)

clientSpan = svc.New(clientSpan, "client-span")
clientSpan, ctx = svc.New(ctx, "client-span")
require.NotEmpty(t, clientSpan)
require.NotNil(t, ctx)

spanFromCtx, ok := svc.FromContext(context.Background())
require.False(t, ok)
require.Nil(t, spanFromCtx)

ctx := svc.NewContext(context.TODO(), clientSpan)
ctx = svc.NewContext(t.Context(), clientSpan)
require.NotNil(t, ctx)
clientSpan.Finish()

Expand Down
19 changes: 5 additions & 14 deletions go/trace/trace.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,23 +46,14 @@ type Span interface {

// NewSpan creates a new Span with the currently installed tracing plugin.
// If no tracing plugin is installed, it returns a fake Span that does nothing.
func NewSpan(inCtx context.Context, label string) (Span, context.Context) {
parent, _ := currentTracer.FromContext(inCtx)
span := currentTracer.New(parent, label)
outCtx := currentTracer.NewContext(inCtx, span)

return span, outCtx
func NewSpan(ctx context.Context, label string) (Span, context.Context) {
Copy link
Collaborator

@mhamza15 mhamza15 Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: OpenTelemetry returns (context.Context, Span) I believe. Would it be a hassle to match it? Not a strong opinion so feel free to ignore.

Copy link
Contributor Author

@mcpherrinm mcpherrinm Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered it, but this order matches more of the existing Vitess codebase, and I didn’t want to mess about with changing that yet.

IMO the right option is going to be to add otel now, drop opentracing soon, and then do a final cleanup pass.

return currentTracer.New(ctx, label)
}
Comment on lines -49 to 51
Copy link
Contributor Author

@mcpherrinm mcpherrinm Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to flag this as the core of the change - everything else is making this change work.

NewSpan(ctx context.Context, label string) (Span, context.Context) matches what we want for Opentelemetry, so we just collapse currentTracer.FromContext -> currentTracer.New -> currentTracer.NewContext into currentTracer.New.


// NewFromString creates a new Span with the currently installed tracing plugin, extracting the span context from
// the provided string.
func NewFromString(inCtx context.Context, parent, label string) (Span, context.Context, error) {
span, err := currentTracer.NewFromString(parent, label)
if err != nil {
return nil, nil, err
}
outCtx := currentTracer.NewContext(inCtx, span)
return span, outCtx, nil
return currentTracer.NewFromString(inCtx, parent, label)
}

// AnnotateSQL annotates information about a sql query in the span. This is done in a way
Expand Down Expand Up @@ -104,10 +95,10 @@ func AddGrpcClientOptions(addInterceptors func(s grpc.StreamClientInterceptor, u
// tracingService is an interface for creating spans or extracting them from Contexts.
type tracingService interface {
// New creates a new span from an existing one, if provided. The parent can also be nil
New(parent Span, label string) Span
New(ctx context.Context, label string) (Span, context.Context)

// NewFromString creates a new span and uses the provided string to reconstitute the parent span
NewFromString(parent, label string) (Span, error)
NewFromString(ctx context.Context, parent, label string) (Span, context.Context, error)

// FromContext extracts a span from a context, making it possible to annotate the span with additional information
FromContext(ctx context.Context) (Span, bool)
Expand Down
12 changes: 7 additions & 5 deletions go/trace/trace_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,17 +134,19 @@ func (f *fakeTracer) GetOpenTracingTracer() opentracing.Tracer {
return opentracing.GlobalTracer()
}

func (f *fakeTracer) NewFromString(parent, label string) (Span, error) {
func (f *fakeTracer) NewFromString(ctx context.Context, parent, label string) (Span, context.Context, error) {
if parent == "" {
return &mockSpan{tracer: f}, errors.New("parent is empty")
return nil, nil, errors.New("parent is empty")
}
return &mockSpan{tracer: f}, nil
span := &mockSpan{tracer: f}
return span, f.NewContext(ctx, span), nil
}

func (f *fakeTracer) New(parent Span, label string) Span {
func (f *fakeTracer) New(ctx context.Context, label string) (Span, context.Context) {
f.log = append(f.log, "span started")

return &mockSpan{tracer: f}
span := &mockSpan{tracer: f}
return span, f.NewContext(ctx, span)
}

func (f *fakeTracer) FromContext(ctx context.Context) (Span, bool) {
Expand Down
Loading