-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Add the enable tracing opt-in flag #4685
Add the enable tracing opt-in flag #4685
Conversation
Signed-off-by: albertteoh <see.kwang.teoh@gmail.com>
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #4685 +/- ##
==========================================
+ Coverage 97.02% 97.06% +0.03%
==========================================
Files 301 301
Lines 17878 17880 +2
==========================================
+ Hits 17347 17355 +8
+ Misses 425 421 -4
+ Partials 106 104 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
Signed-off-by: albertteoh <see.kwang.teoh@gmail.com>
server, err := app.NewServer(svc.Logger, queryService, metricsQueryService, queryOpts, tm, jtracer) | ||
server, err := app.NewServer(svc.Logger, queryService, metricsQueryService, queryOpts, tm, jt) |
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.
Renamed to jt
to avoid clashing names with the jtracer
package.
@@ -88,6 +88,8 @@ func initOTEL(ctx context.Context, svc string) (*sdktrace.TracerProvider, error) | |||
)) | |||
}) | |||
|
|||
otel.SetTracerProvider(tracerProvider) |
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.
Moved here so we just set the global tracer in one place, and removes the otel dependencies from the respective files that use them.
I didn't see, or could think of, a use case where we wouldn't want to set the global tracer when instantiating a jtracer.
Let me know if that's not the case.
Which problem is this PR solving?
Resolves #4680
Description of the changes
--query.enable-tracing
to enable tracing for the jaeger-query component.How was this change tested?
Confirmed that
jaeger-query
is visible and contains traces:Checklist
- [] I have added unit tests for the new functionalityjaeger
:make lint test
jaeger-ui
:yarn lint
andyarn test