Use OTel library from tracing while keeping Jaeger exporter config #11249
Conversation
274f83b to
540a665
Compare
540a665 to
85ff702
Compare
|
💻 Deploy preview deleted. |
50710c9 to
6002a4d
Compare
8d54453 to
bc63d79
Compare
| @@ -35,6 +35,8 @@ import ( | |||
| "github.com/grafana/mimir/pkg/util/validation" | |||
| ) | |||
|
|
|||
| var tracer = otel.Tracer("pkg/blockbuilder") | |||
There was a problem hiding this comment.
Is it guaranteed that this initialization runs after Otel gets parameters from config?
Would it be worth having some kind of sync.Once thing called when it is first needed?
There was a problem hiding this comment.
I was wondering same question, because this definitely runs at package initialization time and we configure tracer for the first time from main().
So I dug the code, and found out that the default implementation of the tracer is a special one which is sync.Once-replaced to the first one you manually configure.
That's actually an inconvenience for tests, but apparently having per-package global tracers is the way to go so I won't fight it.
Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>
bc63d79 to
6bb796f
Compare
@bboreham does that count as an approval? :D |
| To enable sampling in Grafana Mimir components you can specify either `JAEGER_SAMPLER_MANAGER_HOST_PORT` for remote sampling, or `JAEGER_SAMPLER_TYPE` and `JAEGER_SAMPLER_PARAM` to manually set sampling configuration. | ||
| Refer to [Jaeger Client Go documentation](https://github.com/jaegertracing/jaeger-client-go#environment-variables)for the full list of environment variables you can configure. | ||
|
|
||
| Note that you must specify one of `JAEGER_AGENT_HOST` or `JAEGER_SAMPLER_MANAGER_HOST_PORT` in each component for Jaeger to be enabled, even if you plan to use the default values. |
There was a problem hiding this comment.
I think we can use a note admonition here.
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the tracing implementation from OpenTracing to OpenTelemetry while retaining the Jaeger exporter configuration. Key changes include:
- Replacing OpenTracing instrumentation with equivalent OpenTelemetry calls across multiple packages
- Updating HTTP and gRPC transports to use OTel libraries
- Adjusting configuration and documentation to reflect the tracing migration
Reviewed Changes
Copilot reviewed 110 out of 110 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/frontend/querymiddleware/cardinality.go | Replaced opentracing log fields with OpenTelemetry LogKV calls |
| pkg/frontend/frontend_test.go | Updated HTTP transports to otelhttp in tests |
| pkg/frontend/downstream_roundtripper.go | Replaced custom instrumentation transport with otelhttp transport |
| pkg/distributor/query.go | Changed span logging from opentracing to OTel attributes |
| pkg/distributor/push.go | Updated error header tracing from opentracing to OTel attributes |
| pkg/distributor/otel.go | Updated spanlogger usage to use tracer.Start instead of opentracing |
| pkg/distributor/influx.go | Adjusted span creation to new OTel API |
| pkg/distributor/distributor.go | Replaced opentracing span tags with OTel attributes across various spans |
| pkg/continuoustest/write_read_series.go | Updated spanlogger invocations to use tracer |
| pkg/continuoustest/client.go | Switched from custom instrumentation transport 10000 to wrapped otelhttp |
| pkg/blockbuilder/blockbuilder.go | Updated gRPC client dial options to add otelgrpc client handler |
| pkg/alertmanager/distributor.go | Changed span creation to use tracer.Start with corresponding End() calls |
| pkg/alertmanager/alertmanager_client.go | Added otelgrpc.StatsHandler to gRPC client dial options |
| pkg/alertmanager/alertmanager.go | Introduced OTel tracer for alertmanager components |
| go.mod | Updated dependency versions and added OTel libraries |
| docs/sources/mimir/configure/configure-tracing.md | Adjusted tracing configuration documentation to reflect OTel migration |
| cmd/query-tee/main.go | Switched tracing initialization to use tracing.NewOTelFromJaegerEnv |
| cmd/mimir/main.go | Updated tracing initialization to use tracing.NewOTelFromJaegerEnv |
| cmd/mimir-continuous-test/main.go | Switched tracing initialization to use tracing.NewOTelFromJaegerEnv |
| Makefile | Removed linting for opentracing-related packages |
Co-authored-by: Taylor C <41653732+tacole02@users.noreply.github.com>
Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>
Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>
|
Some screenshots of it working in development/mimir-microservices-mode
However, I don't see the same in our internal dev cluster, so I prepared a revert just in case while still investigating what's going on. |
What this PR does
Migrates Mimir to OTel tracing. This is Step 4 from #2708 (comment)
This should be a noop from a user perspective, as we still configure tracing using Jaeger's env vars and we export traces in Jaeger format.
Which issue(s) this PR fixes or relates to
Ref: #2708
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX].about-versioning.mdupdated with experimental features.