-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Support for exemplars in prometheusexporter #13127
Support for exemplars in prometheusexporter #13127
Conversation
cc @alvinhom |
15979b4
to
3e5916f
Compare
cc @mx-psi |
05982aa
to
3ba4655
Compare
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.
Thanks for taking the time to fix the issue upstream :)
d0bf357
to
92648b0
Compare
@mx-psi so the change log check fails because we don't have an issue number in the release yaml, what are some options to fix this? should this check be skipped? |
Feel free to reference #5192 as your issue |
… histogram metrics. Also, this PR includes client_golang v1.13.0 that contains our fix for the panic issue.
652214c
to
19f1322
Compare
@dashpole it's saying |
Only maintainers can merge. |
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.
I just realized this isn't complete, but I'm OK if you want to do the remaining items as follow-ups. See https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/data-model.md#exemplars-2. In particular, we need to handle the otel exemplar's TraceID and SpanID (as trace_id
and span_id
). Also, we need to make sure we don't exceed the max number of characters in the exemplar labels.
|
That is what is being referred to, but I don't see where that is implemented in the collector's |
|
|
Ah, looks like the spanmetricsprocessor isn't implemented correctly, then. It should be using the trace/span id field instead of a named attribute. The way the OTel spec is currently written, if the exemplar is too large, only the trace id and span id attributes should be sent. Right now, it drops the metric |
Since this PR does't modify spanmetricsprocssor, I am assuming we can address this in another PR.
Does this need to be addressed in this PR? |
Right. There are three things that need to be done:
1 should definitely be done separately. 2 and 3 can be done here, or as a follow-up. |
Description:
Adding a feature to
prometheusexporter
to export exemplars along with histogram metrics. In order to add this feature to theprometheusexporter
, we made a PR (prometheus/client_golang#986, merged) to add exemplars support to the prometheus/client_golang library.Note
We fixed the
client_golang
panic that was the root cause of this issue in this PR and was part of v1.13.0 releaseExemplars only show up in Open Metrics format, so I have added a config option
EnableOpenMetrics
in prometheus request handler that can be used to toggle the output format.Testing:
Example config for testing the
prometheusexporter
inopenetelemerty-collector
:To view the exemplars from prometheus exporter, you have to set the following header in your request.
Accept: application/openmetrics-text;
Example curl command:
Sample output for one of the buckets: