Skip to content

feat: add basic support for metrics exemplar#1609

Merged
kaylareopelle merged 35 commits intoopen-telemetry:mainfrom
xuan-cao-swi:metrics-exemplar
Apr 7, 2026
Merged

feat: add basic support for metrics exemplar#1609
kaylareopelle merged 35 commits intoopen-telemetry:mainfrom
xuan-cao-swi:metrics-exemplar

Conversation

@xuan-cao-swi
Copy link
Copy Markdown
Contributor

@xuan-cao-swi xuan-cao-swi commented Feb 27, 2024

Description

Exemplar is useful for trace-metric correlation and I'd like to make a contribute (based on otel ticket and otel spec).

Exemplars in Grafana and Tempo (trace)

Screenshot 2026-03-12 at 9 35 21 AM

Exemplars in Prometheus

Screenshot 2026-03-12 at 9 35 48 AM

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 2, 2024

👋 This pull request has been marked as stale because it has been open with no activity. You can: comment on the issue or remove the stale label to hold stale off for a while, add the keep label to hold stale off permanently, or do nothing. If you do nothing this pull request will be closed eventually by the stale bot

@xuan-cao-swi xuan-cao-swi marked this pull request as ready for review June 30, 2025 15:19
@kaylareopelle kaylareopelle moved this from In progress to Ready for Review in Ruby - Metrics Aug 15, 2025
@xuan-cao-swi xuan-cao-swi marked this pull request as ready for review January 2, 2026 19:12
@kaylareopelle
Copy link
Copy Markdown
Contributor

Hi @xuan-cao-swi, this is a big PR and I'm about 40% of the way through it. I'm going to continue my review tomorrow, but I have one small request in the meantime.

Could you add documentation to the create_#{instrument} API methods for the new exemplar_filter and exemplar_reservoir arguments?

@xuan-cao-swi
Copy link
Copy Markdown
Contributor Author

@kaylareopelle Thanks! Sorry about this huge PR. I added the doc for exemplar-related params in api.

@kaylareopelle
Copy link
Copy Markdown
Contributor

@kaylareopelle Thanks! Sorry about this huge PR. I added the doc for exemplar-related params in api.

@xuan-cao-swi Thanks for the updates! No problem about the size, just takes a little more time :) The upside to big PRs is that they're usually feature-complete, so you can see how everything fits together. Thanks for your patience!

Copy link
Copy Markdown
Contributor

@kaylareopelle kaylareopelle left a comment

Choose a reason for hiding this comment

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

Great job on this! I'm not finished with my review yet. I'm sharing some comments now so you can look at them. I need more time to understand the rest of the PR and run some tests. I will continue the review tomorrow. Thank you!

Comment thread exporter/otlp-metrics/lib/opentelemetry/exporter/otlp/metrics/metrics_exporter.rb Outdated
bucket_counts: hdp.bucket_counts,
explicit_bounds: hdp.explicit_bounds,
exemplars: hdp.exemplars,
exemplars: hdp.exemplars&.map { |ex| as_otlp_exemplar(ex) } || [],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I get a little worried sometimes about calling methods within argument assignments. It might throw off an entire export instead of just impacting that particular key. What do you think about moving the &.map { |ex| as_otlp_exemplar(ex) } || [] logic to a method and passing hdp.exemplars as an arg?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Refactored to a separate function as_otlp_exemplars.

ndp.start_time_unix_nano = start_time
ndp.time_unix_nano = end_time
reservoir = @exemplar_reservoir_storage[ndp.attributes]
ndp.exemplars = reservoir&.collect(attributes: ndp.attributes, aggregation_temporality: :delta)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we want to force the aggregation_temporality to be :delta here? Do we have access to @aggregation_temporality like in the sum aggregation?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think we should force :delta here as last_value implements the gauge aggregation which has no aggregation temporality.

Comment thread metrics_sdk/lib/opentelemetry/sdk/metrics/aggregation/sum.rb
Comment thread metrics_sdk/lib/opentelemetry/sdk/metrics/exemplar/always_on_exemplar_filter.rb Outdated
Comment on lines +150 to +156
# Adds a new exemplar_filter to replace exist exemplar_filter
# Default to TraceBasedExemplarFilter
#
# @param exemplar_filter the new ExemplarFilter to be added.
def exemplar_filter_on(exemplar_filter: Exemplar::TraceBasedExemplarFilter)
@exemplar_filter = exemplar_filter
end
Copy link
Copy Markdown
Contributor

@kaylareopelle kaylareopelle Mar 18, 2026

Choose a reason for hiding this comment

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

I'm a worried about naming this method exemplar_filter_on. Is that a name used in other implementations?

Just reading the method name, I assume it turns on the AlwaysOn filter.

What do you think about something like enable_exemplar_filter?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated the name to enable_exemplar_filter and disable_exemplar_filter

Comment thread metrics_sdk/lib/opentelemetry/sdk/metrics/meter_provider.rb Outdated
module Metrics
module Exemplar
# ExemplarFilter
class ExemplarFilter
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since the other exemplar filters inherit from this one, what do you think about changing the code structure to create a subdirectory within exemplar just for filters?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'd like to keep them flat (e.g. no subdirectory) like other languages' file structure

module Exemplar
# ExemplarReservoir base class
# Subclasses must implement offer and collect methods
class ExemplarReservoir
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Similar question as to what I had for filters -- should there be a subdirectory for the reservoirs that inherit from this one?

# TraceBasedExemplarFilter
class TraceBasedExemplarFilter < ExemplarFilter
def self.should_sample?(value, timestamp, attributes, context)
::OpenTelemetry::Trace.current_span(context).context.trace_flags.sampled?
Copy link
Copy Markdown
Contributor

@kaylareopelle kaylareopelle Mar 18, 2026

Choose a reason for hiding this comment

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

I'm a little worried that so many chained method calls could raise an error. Rather than adding safe nav operators to each one, what do you think about adding a rescue that would just catch any error and make the method return false?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated the function that separates the call chain and rescue

xuan-cao-swi and others added 2 commits March 20, 2026 13:33
Co-authored-by: Kayla Reopelle <87386821+kaylareopelle@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@kaylareopelle kaylareopelle left a comment

Choose a reason for hiding this comment

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

@xuan-cao-swi - Thank you for all your updates!

end
end

# TODO: OpenTelemetry.meter_provider.add_view
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we still need this TODO?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I missed the review. No, we don't need it. I will create another pr to clean this up

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No problem!

OpenTelemetry.logger = original_logger
end

def create_meter
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You know what, I think it's okay here. I was thinking about moving it to the test_helpers gem originally, but it doesn't seem very useful outside of this test environment on second thought.

And good to know! Thank you for clarifying!

Copy link
Copy Markdown
Contributor

@kaylareopelle kaylareopelle left a comment

Choose a reason for hiding this comment

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

Thanks for your patience and work on this feature, @xuan-cao-swi. This is a big PR and though I think I understand it now, I may have missed a few things. I'm comfortable getting this out into the wild and we can address any follow-ups after users test it out.

@kaylareopelle kaylareopelle merged commit f81fbee into open-telemetry:main Apr 7, 2026
63 checks passed
@github-project-automation github-project-automation Bot moved this from Ready for Review to Done in Ruby - Metrics Apr 7, 2026
kaylareopelle added a commit to kaylareopelle/opentelemetry-specification that referenced this pull request Apr 7, 2026
@otelbot-ruby otelbot-ruby Bot mentioned this pull request Apr 7, 2026
github-merge-queue Bot pushed a commit to open-telemetry/opentelemetry-specification that referenced this pull request Apr 8, 2026
## Changes

Ruby recently merged a PR adding support for Exemplars to our Metrics
implementation.

See: open-telemetry/opentelemetry-ruby#1609

This PR updates the spec compliance matrix to reflect the change.

For non-trivial changes, follow the [change proposal
process](https://github.com/open-telemetry/opentelemetry-specification/blob/main/CONTRIBUTING.md#proposing-a-change).

* [-] Related issues #
* [-] Related [OTEP(s)](https://github.com/open-telemetry/oteps) #
* [-] Links to the prototypes (when adding or changing features)
* [-]
[`CHANGELOG.md`](https://github.com/open-telemetry/opentelemetry-specification/blob/main/CHANGELOG.md)
file updated for non-trivial changes
* For trivial changes, include `[chore]` in the PR title to skip the
changelog check
* [X] [Spec compliance
matrix](https://github.com/open-telemetry/opentelemetry-specification/blob/main/spec-compliance-matrix/template.yaml)
updated if necessary
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants