Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Set attribute on exemplar spans #3723

Closed
wants to merge 4 commits into from
Closed

Set attribute on exemplar spans #3723

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e2b6efc
Set attribute on exemplar spans
samwright Oct 17, 2023
5517978
update algorithms
samwright Oct 17, 2023
de4d465
undo changes to fixed-size reservoir
samwright Oct 17, 2023
a2d3af2
make use of otel.exemplar best-effort instead of exact
samwright Oct 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions specification/metrics/sdk.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -993,6 +993,13 @@ span context and baggage can be inspected at this point.
The "offer" method does not need to store all measurements it is given and
MAY further sample beyond the `ExemplarFilter`.

If a span might be used as an exemplar, the ExemplarReservoir MUST set the
span attribute `otel.exemplar` to the value `true`, otherwise it MUST
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only makes sense if you put a restriction to guarantee that an exemplar is preserved once added to a reservoir (which is not true at all today).

This could have poor interaction w/ cumulative streams.

I'd rather see this function exposed via a custom Exemplar Reservoir hook. Imagine this:

  • As Instrument Advice or as View configuration, you can provide an ExemplarReservoir factory for your metric.
  • You should be able to provide an ExemplarReservoir implementation that guarantees sampling once something hits the reservoir and would be able to add this flag.
  • All of that should be able to be done without official specification.

The only thing you're missing is the ability to provide custom reservoirs, which is something we should absoutely be adding to the exemplar specification.

not change the attribute. The number of spans with this annotation
that are not being used as exemplars SHOULD be kept as small as possible. This
allows tail-based span sampling to keep spans that are used as exemplars,
without unnecessarily keeping too many spans.

The "collect" method MUST return accumulated `Exemplar`s. Exemplars are expected
to abide by the `AggregationTemporality` of any metric point they are recorded
with. In other words, Exemplars reported against a metric data point SHOULD have
Expand Down Expand Up @@ -1030,6 +1037,7 @@ algorithm](https://en.wikipedia.org/wiki/Reservoir_sampling) can be used:
bucket = random_integer(0, num_measurements_seen)
if bucket < num_buckets then
reservoir[bucket] = measurement
current_span.set_attribute("otel.example", true)
end
```

Expand All @@ -1044,14 +1052,15 @@ SHOULD be used.
#### AlignedHistogramBucketExemplarReservoir

This Exemplar reservoir MUST take a configuration parameter that is the
configuration of a Histogram. This implementation MUST keep the last seen
configuration of a Histogram. This implementation MUST keep the first seen
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reservoir is designed to match Prometheus behavior of today.

Rather than changing this specification, I'd prefer instead to expose a mechanism for users to specify their own custom Exemplar Reservoirs. This is the right way going forward to allow users to better sample the exemplars for their own use case or do more intelligent per-application sampling (similarly to how we expose Samplers for tracing).

measurement that falls within a histogram bucket. The reservoir will accept
measurements using the equivalent of the following naive algorithm:

```
bucket = find_histogram_bucket(measurement)
if bucket < num_buckets then
if bucket < num_buckets && reservoir[bucket] == null then
reservoir[bucket] = measurement
current_span.set_attribute("otel.example", true)
end

def find_histogram_bucket(measurement):
Expand Down
Loading