trace: add OpenTelemetry backend, deprecate OpenTracing#19619
Conversation
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #19619 +/- ##
===========================================
- Coverage 69.67% 43.68% -25.99%
===========================================
Files 1614 9 -1605
Lines 216793 396 -216397
===========================================
- Hits 151044 173 -150871
+ Misses 65749 223 -65526
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
📝 Documentation updates detected! New suggestion: Document OpenTelemetry tracing backend and deprecate OpenTracing Tip: Tag @Promptless in GitHub PR comments to guide documentation changes during code review 🐙 |
There was a problem hiding this comment.
Pull request overview
Adds an OpenTelemetry tracing backend to go/trace (OTLP/gRPC exporter) and begins deprecating the existing OpenTracing-based backends, aligning Vitess tracing with current ecosystem standards.
Changes:
- Introduces a new
--tracer opentelemetrybackend using the OTel SDK + OTLP/gRPC exporter and custom gRPC interceptors for context propagation. - Moves
--tracing-sampling-rateregistration totrace.goso all tracing backends share it, and adds OTel-specific flags (--otel-endpoint,--otel-insecure). - Deprecates
opentracing-jaegerandopentracing-datadogwith startup warnings and documents the migration path in the v24 changelog.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| go/trace/trace.go | Centralizes shared tracing flags (incl. sampling rate) for all backends. |
| go/trace/plugin_otel.go | Registers the new opentelemetry backend and exposes OTel exporter flags. |
| go/trace/otel.go | Implements tracingService for OTel and provides custom gRPC interceptors + context parsing helpers. |
| go/trace/otel_test.go | Adds unit tests for OTel carrier decoding, span plumbing, and attribute conversion. |
| go/trace/plugin_jaeger.go | Removes duplicate sampling-rate flag registration and adds deprecation warning/docs for Jaeger backend. |
| go/trace/plugin_datadog.go | Adds deprecation warning/docs for Datadog OpenTracing backend. |
| go/flags/endtoend/vttablet.txt | Updates generated flag output to include OTel flags and updated sampling-rate help. |
| go/flags/endtoend/vtgate.txt | Updates generated flag output to include OTel flags and updated sampling-rate help. |
| go/flags/endtoend/vtctld.txt | Updates generated flag output to include OTel flags and updated sampling-rate help. |
| go/flags/endtoend/vtctlclient.txt | Updates generated flag output to include OTel flags and updated sampling-rate help. |
| go/flags/endtoend/vtcombo.txt | Updates generated flag output to include OTel flags and updated sampling-rate help. |
| go/flags/endtoend/vtclient.txt | Updates generated flag output to include OTel flags and updated sampling-rate help. |
| go.mod | Adds OTLP trace exporter deps and bumps some related indirect deps. |
| go.sum | Records new module checksums for updated/added deps. |
| changelog/24.0/24.0.0/summary.md | Documents the new OTel backend, deprecations, and migration steps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
|
@mattlord ready for a re-review now. Thanks! |
mattlord
left a comment
There was a problem hiding this comment.
LGTM! Nice work on this, @timvaillancourt ! ❤️
nickvanw
left a comment
There was a problem hiding this comment.
Leaving one correctness follow-up.
I verified this locally and left the details inline on the NewFromString path.
…n fix - Add resource.WithFromEnv() to pick up OTEL_RESOURCE_ATTRIBUTES env var - Fix NewFromString to extract into a clean context so a pre-existing span cannot mask an invalid carrier payload - Add regression test for the extraction bug Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Description
To address #17341 and #19621, this PR adds a new
--tracer opentelemetrybackend that exports traces via OTLP/gRPC. The existingopentracing-jaegerandopentracing-datadogbackends are deprecated and will be removed in v25.Basically, the world is consolidating on OpenTelemetry. This is great for Vitess, because we can eventually (v25+) run a single tracing implementation.
The motivation here is that
jaeger-client-gohas been archived for a while now and the Jaeger project recommends migrating to OpenTelemetry. Since our Datadog plugin uses the OpenTracing bridge indd-trace-go, it makes sense to deprecate both at onceBoth old backends continue to work in v24, they just log a deprecation warning at startup. This gives users a release cycle to migrate
What changed
otel.goimplements thetracingServiceinterface using the OTel SDKplugin_otel.goregisters the"opentelemetry"factory with an OTLP/gRPC exporterotelgrpclibrary removed its interceptor API in v0.63.0, so we write thin wrappers that fit the existingtracingServiceinterface)--tracing-sampling-rateflag moved fromplugin_jaeger.gototrace.goso all backends can reference it without duplicate flag registrationplugin_jaeger.goandplugin_datadog.golog a deprecation warningNew flags
--otel-endpoint(defaultlocalhost:4317) - collector endpoint (gRPC)--otel-insecure(defaultfalse) - use insecure connection to collectorv25 cleanup
After the deprecation period we can delete
opentracing.go,plugin_jaeger.go,plugin_datadog.go,logger.goand dropopentracing-go,opentracing-contrib/go-grpc,uber/jaeger-client-go, anddd-trace-go.v1from go.mod.vtadmin/grpcserver/server.goalso has direct OpenTracing usage that will need migratingRelated Issue(s)
Resolves: #17341 and #19621
Checklist
Deployment Notes
No impact for existing deployments. The new
opentelemetrybackend is opt-in. Existingopentracing-jaegerandopentracing-datadogusers will see a deprecation warning in their logs but nothing else changesAI Disclosure
Written primarily by Claude Code - I provided direction on the design (coexistence vs replacement, flag naming, deprecation strategy)