[receiver/prometheusreceiver] Preserve bucketless histograms during NHCB conversion - #49892
Conversation
…HCB conversion Assisted-by: GitHub Copilot
|
|
|
Welcome, contributor! Thank you for your contribution to opentelemetry-collector-contrib. Important reminders:
|
Pull request dashboard statusWaiting on the author · refreshed 2026-08-02 18:42 UTC Respond to 1 review item (e.g. link a commit, explain why not, ask a follow-up):
Status above doesn't look right?
|
|
I found in a follow-up review that the fix also starts emitting semantically inconsistent _sum-only histogram families. For example, a histogram containing _sum=123.5 but no _count can now become: count=0 That is semantically questionable and differs from the classic path, which drops it. Prometheus’s conversion loses whether _count was absent, so fixing this cleanly may require more than checking Count == 0 Can you help me understand:
|
* Added retention-on/off E2E coverage, asserting one vs. two histogram representations. * Added float stale-NHCB coverage. * Clarified the changelog’s retention condition. Left sum-only semantics, stale routing, and remote-write behavior for maintainer-approved follow-ups. Validation passed: receiver tests, go vet, make lint, changelog validation, and diff checks. mint build is inapplicable because this is not a Multiproduct repository.
…ub.com/shubhgupta2104/opentelemetry-collector-contrib into fix-prometheusreceiver-bucketless-nhcb
dashpole
left a comment
There was a problem hiding this comment.
Looks correct to me. @krajorama should take a look as well
There was a problem hiding this comment.
Pull request overview
Fixes a regression in the Prometheus receiver’s classic-histogram → NHCB conversion path where “bucketless” classic histograms (only _sum/_count, no _bucket series) could be silently dropped. The change aligns NHCB conversion behavior with the existing classic-histogram handling established in prior work (#22070 / #23448), preserving the implicit +Inf bucket representation instead of returning early.
Changes:
- Stop treating NHCB histograms with empty
CustomValues(no explicit bounds) as “unsupported” intoDistributionPoint, so bucketless histograms are still emitted. - Add unit and end-to-end receiver tests covering integer/float NHCB bucketless histograms, including stale samples and scrape behavior when classic retention is toggled.
- Add a user-facing changelog entry describing the bug fix.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
receiver/prometheusreceiver/internal/metricfamily.go |
Removes early-return behavior that dropped NHCB bucketless histograms during conversion to OTLP histograms. |
receiver/prometheusreceiver/internal/metricfamily_test.go |
Adds NHCB unit tests for empty explicit bounds (bucketless) for both integer and float histograms, including stale cases. |
receiver/prometheusreceiver/metrics_receiver_test.go |
Adds an end-to-end scrape test ensuring bucketless classic histograms are preserved when converted to NHCB (with/without classic retention). |
.chloggen/fix-prometheusreceiver-bucketless-nhcb.yaml |
Adds a changelog entry documenting the fix for users. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@dashpole I think you will need to click Approve and run workflow again, because I added a merge commit. Also, any thoughts on my comment above? |
|
/workflow-approve |
It should be consistent with the behavior in the classic path. So if it is rejected in that case, i'm OK with rejecting it here as well. |
Keep NHCB conversion consistent with the classic path by dropping non-stale histograms with a zero count and nonzero sum. Cover integer, float, and scrape paths while preserving valid zero-count and stale histograms. Assisted-by: GitHub Copilot
|
/workflow-approve |
|
@krajorama Can you also take a look please? Thanks :) |
Assisted-by Github-Copilot
Description
Bucketless classic histograms contain _sum and _count series but no _bucket series.
When
convert_classic_histograms_to_nhcb: trueis enabled and classic histogram retention is disabled, Prometheus converts these histograms into an NHCB with empty CustomValues and a single implicit+Infbucket. The receiver currently returns early for this representation, silently dropping the histogram.This change preserves the implicit +Inf bucket, consistent with the behaviour established in #22070 and #23448.
There are no configuration or public API changes.
Link to tracking issue
Fixes #49893
Related to #22070, #23448 , and #43095 .
Testing
Added receiver tests covering:
Documentation
Added a user-facing changelog entry describing the fix.
Authorship