[RudraDudhat2509]

Fixes #5844

What's wrong

create_histogram()'s validation of explicit_bucket_boundaries_advisory only checks that each element is a float/int. It never checks the values are finite or strictly increasing, so a type-valid but unsorted/non-finite advisory passes silently with no warning. The real check lives in _ExplicitBucketHistogramAggregation.__init__, which only runs lazily the first time the histogram is recorded, so it raises ValueError uncaught out of the user's record() call, and keeps raising on every later call since the failed match never gets cached.

What this does

Extends create_histogram's advisory validation to also check finite + strictly increasing (the same predicate _ExplicitBucketHistogramAggregation already enforces), folded into the existing "drop with a warning" path so a bad advisory is caught and reported at creation time instead of crashing the hot record() path later.

Tests

Added 3 cases to test_create_histogram_advisory_validation (unsorted, duplicate, NaN boundaries) and a new test_histogram_bad_advisory_does_not_crash_record that builds a histogram through a real MeterProvider/InMemoryMetricReader and records through it, proving the fix end-to-end. That required wiring a real reader since the test class's self.meter fixture uses a mocked measurement consumer that never reaches the aggregation code, so it can't actually exercise record().

[opentelemetry-pr-dashboard]

Pull request dashboard status

Waiting on reviewers · refreshed 2026-10-28 06:45 UTC

Review the latest changes.

Status above doesn't look right?

• Just replied or pushed? Anything around or after the refresh time above may not be picked up yet — give it a few minutes.
• Anything look wrong? Report it with what you expected; it helps us improve the dashboard.

[knowprasad]

Took a look through the changes and tested the edge cases against the SDK aggregation logic.

Moving the finiteness and strict-monotonicity validation directly into create_histogram() makes a lot of sense. Previously, passing an unsorted sequence (like [100, 10, 50]) passed the type guard and deferred the ValueError until the aggregation was constructed lazily on the first record() call.

Nice touch naming each test histogram distinctly (f"histogram-{idx}") so the log assertion doesn't get masked by duplicate instrument warnings, and wiring a real InMemoryMetricReader in the record test to exercise the full pipeline.

All checks look clean on my end.

[chrikrah]

@RudraDudhat2509 From my perspective this can be merged at 6f20767. On the base, record() raises out of the API for an unsorted advisory. Here it logs at creation and records.

$ python -m pytest opentelemetry-sdk/tests/metrics/test_metrics.py -q -p no:cacheprovider -k "advisory or bad_advisory"   # 6f20767, Python 3.12, api and sdk imported from the tree
4 passed, 64 deselected, 5 subtests passed in 0.13s
# metrics/_internal/__init__.py from merge base 7fd5f6b, tests kept
4 failed, 3 passed, 64 deselected, 2 subtests passed in 0.23s
E   AssertionError: no logs of level WARNING or higher triggered on root
$ python -m pytest opentelemetry-sdk/tests/metrics -q -p no:cacheprovider   # 6f20767
349 passed, 1 warning, 47 subtests passed in 8.05s
$ python probe_5845.py   # create_histogram("h", explicit_bucket_boundaries_advisory=[100, 10, 50]); h.record(5)
7fd5f6b: record raises ValueError boundaries must be strictly increasing
6f20767: record ok
# same record(5), but the boundaries come from View(aggregation=ExplicitBucketHistogramAggregation(boundaries=[100, 10, 50]))
6f20767: record raises ValueError boundaries must be strictly increasing

#5847 moves this block into _validate_explicit_bucket_boundaries_advisory in the API, so whichever lands second carries the check into that helper.

non-blocking: a bad View still raises from record(), via your #5340. The spec's error handling forbids unhandled exceptions at run time, so I'd reject the boundaries when the View gets created. Did #5340 mean to raise at record time?

[RudraDudhat2509]

no wasnt deliberate, #5340 just fixed the sort check and never thought about when _create_aggregation runs. good catch, public ExplicitBucketHistogramAggregation init does zero validation, only the internal one does and thats lazy via record(). can open a follow up for it. noted on #5847 too