Make it clear that Exemplar is opt-in#2105
Make it clear that Exemplar is opt-in#2105cijothomas wants to merge 2 commits intoopen-telemetry:mainfrom
Conversation
|
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#exemplar-defaults This section feels like the intent was to have this as the default behavior. Hoping to get clarification here. |
| on its associated metric data point should result in the full set of attributes | ||
| from the original sample measurement. | ||
|
|
||
| The `ExemplarReservoir` SHOULD avoid allocations when sampling exemplars. |
There was a problem hiding this comment.
is deleting this line relevant to PR's intent?
There was a problem hiding this comment.
It was unrelated, yes. I mentioned it in the description.
There was a problem hiding this comment.
This was here intentionally. Allocations can really hurt performance and it's an optional reequirement.
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
| on its associated metric data point should result in the full set of attributes | ||
| from the original sample measurement. | ||
|
|
||
| The `ExemplarReservoir` SHOULD avoid allocations when sampling exemplars. |
There was a problem hiding this comment.
This was here intentionally. Allocations can really hurt performance and it's an optional reequirement.
| A Metric SDK MUST provide a mechanism to sample `Exemplar`s from measurements. | ||
|
|
||
| A Metric SDK MUST allow `Exemplar` sampling to be disabled. In this instance the SDK SHOULD not have overhead related to exemplar sampling. | ||
| `Exemplar` sampling MUST be an opt-in feature. When not enabled, the SDK SHOULD NOT have overhead related to exemplar sampling. |
There was a problem hiding this comment.
Exemplar sampling is meant to be default-on, but only with sampled traces. Default is documented here.
This means you only get exemplar sampling if you also have trace instrumentation.
|
This was not the intention of the specification. |
| A Metric SDK MUST allow `Exemplar` sampling to be disabled. In this instance the SDK SHOULD not have overhead related to exemplar sampling. | ||
| `Exemplar` sampling MUST be an opt-in feature. When not enabled, the SDK SHOULD NOT have overhead related to exemplar sampling. | ||
|
|
||
| A Metric SDK MUST sample `Exemplar`s only from measurements within the context of a sampled trace BY DEFAULT. |
There was a problem hiding this comment.
This is the line that contradicts the PR. By default exemplar sampling should following the tracing configuration.
reyang
added
area:sdk
|
Fyi: Prometheus' Exemplars support is opt-in too. I have a slightly connected question: should OTel Metrics support any Exemplar source or only OTel Tracing? If so, I would introduce an Also, should the |
|
The original intention of this PR was to make the wording clear that Exemplar is an opt-in feature. But @jsuereth has clarified that the intention was to have it on-by-default. Closing this PR as no longer relevant. Will open a separate issue to ask opinion on making this opt-in/other qns. |
As this PR is closed, could you open this in a separate issue, to ensure its not lost. Thanks for reviewing! |
|
@jonatan-ivanov @cijothomas I think there's an important distinction here that's possibly missing. By Default, Exemplar sampling is on only if there exists a Sampled Trace. The default requires:
Exemplar sampling, by default, is controlled via Trace sampling. The ExemplarFilter/ExemplarResorvoir hooks are only for advanced Exemplar usage. This is unlike other systems where Exemplar sampling may or may not correlate with the existence of Traces. I'd expect a user of just the otel metrics SDK to never see an exemplar (by default). It's only once they've also instrumented traces that they see exemplars be generated.
An Exemplar != a trace. The exemplar is the measurement, we just attach the trace_id to it. OTel is the only library (I know of) where attaching traces is first-class, e.g. in prometheus exemplars just attach labels to measurements. That said, to the extent there's another system instrumenting traces, I'd hope they expose OTEL-friendly hooks for interacting with trace. I think it's out-of-scope for OTEL (which already provides pure-apis for tracing) to support backends which don't leverage those APIs. Instead we should just provide a bridge (similar to what we've done for OpenTracing + OpenCensus). |
|
I understand the behavior of Exemplar sampling (only sample if there is a sampled span), and I think everything should behave like that but what I'm saying is if the user configured OTel tracing and they have a sampled span, sampling an Exemplar can be useless if the backend does not support them and as far as I know only Prometheus supports Exemplars and this feature is off by default. I did not say Exemplar == a trace but I think this answers my question:
If this means that another tracing libraries can implement a public interface that has a goal of providing the Exemplar data and OTel Metrics would call this during sampling an Exemplar, I think we should be able to plug-in any tracer implementation to provide the data for Exemplars. |
…#2105) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
6 participants
Its not clear if the Exemplar feature is enabled by default or disabled by default. Trying to make it obvious.
I assume this is an opt-in feature. If that's the not the case, happy to reword it accordingly.
Only goal of this PR is to make it very clear whether its enabled/disabled by default.
Also removed a statement about "allocations" as thats implementation details, and not required in spec.