-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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 Exemplars for http_request_duration_seconds histogram #3977
Conversation
Signed-off-by: Matthias Loibl <[email protected]> A lot of these changes are actually changing the order of calling the middlwares. Going forward we need to make sure to first call the tracing middleware so that by the time our monitoring/instrumentation middleware gets hit the tracing spans are already populated.
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.
This is sooo good. Thanks @metalmatze !
// If we find a tracingID we'll expose it as Exemplar. | ||
span := opentracing.SpanFromContext(r.Context()) | ||
if span != nil { | ||
spanCtx, ok := span.Context().(jaeger.SpanContext) |
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.
Here we just support Jaeger tracer? Is it compatible with lightstep
, stackdriver
, etc?
if span != nil { | ||
spanCtx, ok := span.Context().(jaeger.SpanContext) | ||
if ok { | ||
observer.(prometheus.ExemplarObserver).ObserveWithExemplar( |
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.
Like in the weaveworks example https://github.com/weaveworks/common/pull/202/files#diff-29d7cb6219a01fc80b731ac8004ac0f5edd5a1be507b164939ba4a4d1caf9ac2R70, I think we also need to check for sampled trace ID.
Let's iterate over it @yeya24 good points. Also I would see |
Changes
The Prometheus HTTP handler now enables OpenMetrics as response format upon content negotiation.
You can test it by running
curl -s -H 'Accept: application/openmetrics-text' localhost:10904/metrics
This allows us to pass a
traceID
toObserveWithExemplar
.A lot of the other changes are actually changing the order of calling the middlewares.
Going forward we need to make sure to first call the tracing middleware so that by the time our monitoring/instrumentation middleware gets hit the tracing spans are already populated.
Verification
Tested locally with Jaeger and Prometheus running
--enable-feature=exemplar-storage
.Closes #3944