Skip to content

promql: Check 1st histogram's CRH in sum_over_time#17331

Merged
beorn7 merged 2 commits intomainfrom
beorn7/promql
Oct 14, 2025
Merged

promql: Check 1st histogram's CRH in sum_over_time#17331
beorn7 merged 2 commits intomainfrom
beorn7/promql

Conversation

@beorn7
Copy link
Member

@beorn7 beorn7 commented Oct 13, 2025

avg_over_time already correctly checked the counter reset hint fo all
histograms, but in sum_over_time, the 1st histogram was missed. In
both cases, the 1st histogram is processed outside the loop.

Thanks to @charleskorn for spotting this.

Does this PR introduce a user-facing change?

NONE

(This does fix a bug, but this bug is in the fix of #17312, which is not yet released yet. So adding another line in the release notes for the fix of the fix would be weird.)

avg_over_time already correctly checked the counter reset hint fo all
histograms, but in sum_over_time, the 1st histogram was missed in the
loop. This commit exposes the bug in a test.

Signed-off-by: beorn7 <beorn@grafana.com>
avg_over_time already correctly checked the counter reset hint fo all
histograms, but in sum_over_time, the 1st histogram was missed. In
both cases, the 1st histogram is processed outside the loop.

Signed-off-by: beorn7 <beorn@grafana.com>
Copy link
Contributor

@charleskorn charleskorn left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for fixing this

Copy link
Member

@krajorama krajorama left a comment

Choose a reason for hiding this comment

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

Nice catch, I knew something was off, but didn't catch it :( https://github.com/prometheus/prometheus/pull/17312/files#r2417329551

@beorn7
Copy link
Member Author

beorn7 commented Oct 14, 2025

Thanks everyone.

@beorn7 beorn7 merged commit a18d18e into main Oct 14, 2025
46 checks passed
@beorn7 beorn7 deleted the beorn7/promql branch October 14, 2025 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants