-
Notifications
You must be signed in to change notification settings - Fork 157
build(deps): use OpenTelemetry for tracing #5581
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
base: master
Are you sure you want to change the base?
Conversation
Update tracing to use OpenTelemetry instead of the no longer supported jaeger and opentracing. This introduces a backwards incompatible change to the flux command, which no longer supports the --trace=jaeger flag. Jaeger now recommends using the OpenTelemtry API.
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.
Pull request overview
This PR migrates the Flux codebase from the deprecated OpenTracing/Jaeger libraries to OpenTelemetry for distributed tracing. The migration introduces a breaking change by removing support for the --trace=jaeger flag in favor of --trace=otlp, with a deprecation warning for users still using the old flag. The change updates all tracing instrumentation across the codebase to use OpenTelemetry APIs while maintaining the same tracing functionality.
Key Changes
- Replaced OpenTracing/Jaeger dependencies with OpenTelemetry packages (otel, otel/trace, otel/sdk)
- Updated tracing API calls throughout the codebase:
span.Finish()→span.End(),span.SetTag()→span.SetAttributes(), etc. - Modified the flux CLI to use OTLP exporter with environment-based configuration instead of Jaeger-specific setup
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| stdlib/universe/parallel.go | Migrated PartitionMergeTransformation span creation and lifecycle to OpenTelemetry |
| stdlib/influxdata/influxdb/to.go | Updated toTransformation tracing to use OpenTelemetry APIs |
| stdlib/http/requests/requests.go | Converted HTTP request tracing to OpenTelemetry with proper attribute setting |
| stdlib/http/post.go | Updated HTTP POST tracing to use OpenTelemetry span methods |
| stdlib/experimental/iox/source.go | Migrated IOx SQL source tracing and error recording to OpenTelemetry |
| stdlib/experimental/http/http_experimental.go | Updated experimental HTTP GET tracing to OpenTelemetry |
| repl/repl.go | Migrated REPL input tracing to OpenTelemetry (with context handling issues) |
| lang/query.go | Updated query span type reference to OpenTelemetry |
| lang/compiler_test.go | Replaced mocktracer with OpenTelemetry in-memory test exporter for tracing tests |
| lang/compiler.go | Migrated plan and execution span creation to OpenTelemetry throughout compilation pipeline |
| internal/spec/build.go | Updated spec building tracing to use OpenTelemetry APIs |
| internal/jaeger/jaeger_test.go | Completely rewrote tests to use OpenTelemetry span contexts instead of Jaeger mocks |
| internal/jaeger/jaeger.go | Reimplemented InfoFromSpan to extract trace info from OpenTelemetry spans |
| execute/transport.go | Updated consecutiveTransport tracing to use OpenTelemetry span lifecycle |
| execute/executor.go | Migrated executor source tracing to OpenTelemetry |
| cmd/flux/main.go | Replaced Jaeger tracer initialization with OTLP exporter and TracerProvider configuration |
| go.mod | Added OpenTelemetry dependencies, removed Jaeger/OpenTracing dependencies, updated related packages |
| go.sum | Updated checksums for all modified and new dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
go.mod
Outdated
| go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.34.0 | ||
| go.opentelemetry.io/otel/sdk v1.37.0 |
Copilot
AI
Dec 2, 2025
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.
There's a version mismatch in OpenTelemetry packages: otel and otel/trace are at v1.38.0, while otel/sdk is at v1.37.0 and otel/exporters/otlp/otlptrace/otlptracegrpc is at v1.34.0. While these packages are designed to be compatible across minor versions, it's generally better practice to align the versions where possible to avoid potential compatibility issues.
Consider updating all OpenTelemetry packages to use the same minor version (e.g., all v1.38.x) if available, or document why different versions are required.
| go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.34.0 | |
| go.opentelemetry.io/otel/sdk v1.37.0 | |
| go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.38.0 | |
| go.opentelemetry.io/otel/sdk v1.38.0 |
cmd/flux/main.go
Outdated
| "go.opentelemetry.io/otel/propagation" | ||
| "go.opentelemetry.io/otel/sdk/resource" | ||
| sdktrace "go.opentelemetry.io/otel/sdk/trace" | ||
| semconv "go.opentelemetry.io/otel/semconv/v1.4.0" |
Copilot
AI
Dec 2, 2025
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.
The semantic convention package version (v1.4.0) is significantly outdated. OpenTelemetry semantic conventions have evolved considerably since v1.4.0, and the latest versions provide more comprehensive and standardized attributes.
Consider updating to a more recent version of semantic conventions (e.g., semconv/v1.20.0 or later) to ensure compatibility with modern OpenTelemetry tools and to benefit from improved standardization. The ServiceNameKey attribute used at line 103 is available in all versions, so this should be a straightforward update.
| semconv "go.opentelemetry.io/otel/semconv/v1.4.0" | |
| semconv "go.opentelemetry.io/otel/semconv/v1.20.0" |
The tests are failing because reqres.in is no longer returning a JSON response. Change the HTTP example to not execute.
590eb28 to
8559625
Compare
Co-authored-by: Copilot <[email protected]>
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.
Pull request overview
Copilot reviewed 20 out of 21 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
This has a lot of implications downstream, so I'm putting it down for now. I intend to revisit in the future. |
Update tracing to use OpenTelemetry instead of the, no longer supported, jaeger and opentracing. This introduces a backwards incompatible change to the flux command, which no longer supports the --trace=jaeger flag. Jaeger now recommends using the OpenTelemtry API.
Checklist
Dear Author 👋, the following checks should be completed (or explicitly dismissed) before merging.
experimental/docs/Spec.mdhas been updatedDear Reviewer(s) 👋, you are responsible (among others) for ensuring the completeness and quality of the above before approval.