Ignore reader preferred aggregations incompatible with async instruments - #5554
Open
Eason09053360 wants to merge 2 commits into
Open
Conversation
MetricReaderStorage warns and skips a View that configures a histogram aggregation for an asynchronous instrument, but the same aggregation selected through MetricReader(preferred_aggregation=...) bypassed that check entirely. Histogram aggregations only implement the DELTA instrument temporality path, so asynchronous instruments -- which always report as CUMULATIVE -- silently produced no data points on every collection. This affected both ExplicitBucketHistogramAggregation and ExponentialBucketHistogramAggregation across all three asynchronous instrument classes. Warn and keep the instrument's default aggregation instead, so selecting an aggregation through a reader behaves the same as selecting it through a view.
Eason09053360
marked this pull request as ready for review
August 17, 2026 17:31
Pull request dashboard statusWaiting on reviewers · refreshed 2026-08-21 01:46 UTC Review the latest changes. Status above doesn't look right?
|
Eason09053360
marked this pull request as draft
August 18, 2026 02:32
Eason09053360
marked this pull request as ready for review
August 18, 2026 02:32
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No existing issue — found by reading the code.
MetricReaderStoragewarns and skips aViewthat configures a histogramaggregation for an asynchronous instrument. The same aggregation set through
MetricReader(preferred_aggregation=...)skips that check and silently producesno data at all: histogram aggregations only implement the DELTA instrument
temporality path, so asynchronous instruments (always CUMULATIVE) return
Nonefrom every
collect().Both histogram aggregations and all three asynchronous instrument classes are
affected. Synchronous instruments are not, so the OTLP exporter's
OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATIONhandling (which mapsHistogram) is unchanged.This warns and keeps the instrument's default aggregation, matching what the
Viewpath already does.Related: #5461 fixes the matching gap on the
Viewpath. Different file andfunction, so the two can be reviewed independently.
Type of change
How Has This Been Tested?
test_configure_aggregation_asynchronous_histogram_incompatibility(3 async instrument classes × both histogram aggregations) and
test_configure_aggregation_synchronous_histogram_allowed.opentelemetry-sdk/tests/metrics/— 309 passed, 1 skipped.Does This PR Require a Contrib Repo Change?
Checklist: