Modernize Spanner tracing (spannerotel + stdout export)#66
Conversation
Upgrade spannerotel to memefish-compatible interceptors on spannerpb, align OpenCensus bridge with OTEL 1.41, and add --experimental-trace-stdout for local debugging without Cloud Trace credentials. Co-authored-by: Cursor <cursoragent@cursor.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new --experimental-trace-stdout flag to export traces to stderr for local debugging, refactors the tracing setup into a separate trace.go file using spannerotel/tracing, and adds corresponding unit tests. The feedback suggests leveraging Kong's built-in xor tag on the CLI options struct to enforce mutual exclusion between --experimental-trace-stdout and --experimental-trace-project at the parser level.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Pin cloud.google.com/go/spanner at v1.90.0 to stay on go 1.25.0, align OTEL 1.44 and gRPC/api deps, and update spannerotel without the OpenCensus bridge. Add emulator integration test proving Spanner OTel spans work without it. Co-authored-by: Cursor <cursoragent@cursor.com>
Use xor:"trace" on stdout and Cloud Trace flags and add a parser test. Bump spannerotel for interceptor and tracing review fixes. Co-authored-by: Cursor <cursoragent@cursor.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new --experimental-trace-stdout flag to export traces to stderr for local debugging, refactors the tracing initialization logic into a new trace.go file, and updates several dependencies. A critical issue was identified in the refactored tracing logic where the global OpenTelemetry tracer provider is never set via otel.SetTracerProvider(tp), rendering the tracing functionality inactive.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Co-authored-by: Cursor <cursoragent@cursor.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new --experimental-trace-stdout flag to export OpenTelemetry traces to stderr for local debugging, which is mutually exclusive with --experimental-trace-project. It refactors the tracing setup into a dedicated trace.go file, updates dependencies (including OpenTelemetry and Cloud Spanner), and adds comprehensive unit and integration tests. Feedback on the integration test suggests restoring the global OpenTelemetry tracer provider in t.Cleanup to avoid modifying global state and causing potential test flakiness.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Co-authored-by: Cursor <cursoragent@cursor.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces local tracing support via a new --experimental-trace-stdout flag, allowing traces to be exported to stderr for local debugging. It refactors the tracing setup into a dedicated trace.go file, updates dependencies, and adds integration and unit tests. The reviewer identified a critical issue where the global OpenTelemetry TracerProvider is never registered in the application code, causing the Spanner client and gRPC interceptors to use a no-op tracer. To resolve this, the reviewer suggested setting the global tracer provider in enableTracing and properly saving and restoring the global state in the unit tests to avoid side effects.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| tp, err := tracing.NewTracerProvider(cfg) | ||
| if err != nil { | ||
| return ctx, nil, nil, err | ||
| } | ||
|
|
||
| traceCtx, traceCancel := context.WithCancel(ctx) | ||
| return traceCtx, tp, traceCancel, nil |
There was a problem hiding this comment.
The global OpenTelemetry TracerProvider is never set in the non-test application code. Without calling otel.SetTracerProvider(tp), the Spanner client and gRPC interceptors will use the default no-op tracer provider, and no traces will be exported to stdout or Cloud Trace.
Register the tracer provider globally here.
tp, err := tracing.NewTracerProvider(cfg)
if err != nil {
return ctx, nil, nil, err
}
otel.SetTracerProvider(tp)
traceCtx, traceCancel := context.WithCancel(ctx)
return traceCtx, tp, traceCancel, nil| import ( | ||
| "context" | ||
| "fmt" | ||
| "os" | ||
| "time" | ||
|
|
||
| "github.com/apstndb/spannerotel/tracing" | ||
| sdktrace "go.opentelemetry.io/otel/sdk/trace" | ||
| ) |
There was a problem hiding this comment.
To register the tracer provider globally so that the Spanner client and gRPC interceptors can emit traces, we need to import go.opentelemetry.io/otel.
| import ( | |
| "context" | |
| "fmt" | |
| "os" | |
| "time" | |
| "github.com/apstndb/spannerotel/tracing" | |
| sdktrace "go.opentelemetry.io/otel/sdk/trace" | |
| ) | |
| import ( | |
| "context" | |
| "fmt" | |
| "os" | |
| "time" | |
| "github.com/apstndb/spannerotel/tracing" | |
| "go.opentelemetry.io/otel" | |
| sdktrace "go.opentelemetry.io/otel/sdk/trace" | |
| ) |
| import ( | ||
| "context" | ||
| "testing" | ||
|
|
||
| "github.com/alecthomas/kong" | ||
| "github.com/apstndb/spannerotel/tracing" | ||
| ) |
There was a problem hiding this comment.
Import go.opentelemetry.io/otel to allow saving and restoring the global tracer provider state during unit tests.
| import ( | |
| "context" | |
| "testing" | |
| "github.com/alecthomas/kong" | |
| "github.com/apstndb/spannerotel/tracing" | |
| ) | |
| import ( | |
| "context" | |
| "testing" | |
| "github.com/alecthomas/kong" | |
| "github.com/apstndb/spannerotel/tracing" | |
| "go.opentelemetry.io/otel" | |
| ) |
| func TestEnableTracingStdout(t *testing.T) { | ||
| ctx, tp, traceCancel, err := enableTracing(context.Background(), opts{TraceStdout: true}) | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| if tp == nil || traceCancel == nil { | ||
| t.Fatal("expected tracer provider and cancel func") | ||
| } | ||
| defer traceCancel() |
There was a problem hiding this comment.
Since enableTracing will now set the global tracer provider, we should capture and restore the original global tracer provider in this unit test to prevent side effects on other tests.
| func TestEnableTracingStdout(t *testing.T) { | |
| ctx, tp, traceCancel, err := enableTracing(context.Background(), opts{TraceStdout: true}) | |
| if err != nil { | |
| t.Fatal(err) | |
| } | |
| if tp == nil || traceCancel == nil { | |
| t.Fatal("expected tracer provider and cancel func") | |
| } | |
| defer traceCancel() | |
| func TestEnableTracingStdout(t *testing.T) { | |
| oldTP := otel.GetTracerProvider() | |
| defer otel.SetTracerProvider(oldTP) | |
| ctx, tp, traceCancel, err := enableTracing(context.Background(), opts{TraceStdout: true}) | |
| if err != nil { | |
| t.Fatal(err) | |
| } | |
| if tp == nil || traceCancel == nil { | |
| t.Fatal("expected tracer provider and cancel func") | |
| } | |
| defer traceCancel() |
Drop deprecated SessionPoolConfig usage and update spannerotel to the squash-merged modernize-otel commit on main. Co-authored-by: Cursor <cursoragent@cursor.com>
1 participant
Summary
github.com/apstndb/spanneroteltomodernize-otel(tracinghelpers,spannerpbinterceptor, stdout/Cloud Trace exporters).main.gowithtrace.gohelpers built onspannerotel/tracing.--experimental-trace-stdoutfor local PROFILE/trace debugging without GCP credentials (mutually exclusive with--experimental-trace-project).cloud.google.com/go/spannerto v1.90.0 and align OTEL v1.44.0 / gRPC /google.golang.org/apiwithingo 1.25.0.TestSpannerQueryTracingWithoutOpenCensusBridge) proving Spanner OTel spans are recorded without the bridge.Dependency
Requires apstndb/spannerotel#1 (branch
modernize-otel).Test plan
go test ./...(unit + emulator integration tests)go test ./...in spannerotel