Skip to content

[FEATURE] PromQL: Implements idelta and irate with histograms#15853

Merged
beorn7 merged 4 commits intoprometheus:mainfrom
NeerajGartia21:promql/ifunc
Jan 31, 2025
Merged

[FEATURE] PromQL: Implements idelta and irate with histograms#15853
beorn7 merged 4 commits intoprometheus:mainfrom
NeerajGartia21:promql/ifunc

Conversation

@NeerajGartia21
Copy link
Contributor

@NeerajGartia21 NeerajGartia21 commented Jan 21, 2025

Part of #13934

This PR implements irate and idelta for histogram samples.

Signed-off-by: Neeraj Gartia <neerajgartia211002@gmail.com>
Signed-off-by: Neeraj Gartia <neerajgartia211002@gmail.com>
Copy link
Member

@beorn7 beorn7 left a comment

Choose a reason for hiding this comment

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

Thanks. Looks generally right. I have an idea about the initial management of the "four last samples". And maybe we should allow a result if there are two histograms with incompatible buckets in an irate calculation.

Signed-off-by: Neeraj Gartia <neerajgartia211002@gmail.com>
@NeerajGartia21 NeerajGartia21 requested a review from beorn7 January 23, 2025 20:39
@beorn7 beorn7 marked this pull request as ready for review January 28, 2025 13:47
@beorn7 beorn7 requested a review from roidelapluie as a code owner January 28, 2025 13:47
@beorn7
Copy link
Member

beorn7 commented Jan 29, 2025

To make things consistent for rate and friends, I created #15902

While working on it, I found a weird histogram formatting and created #15903

2nd level yak shaving... :)

@beorn7
Copy link
Member

beorn7 commented Jan 29, 2025

I also have to update spec and doc.

@beorn7
Copy link
Member

beorn7 commented Jan 29, 2025

Spec update is contained in prometheus/docs#2576 , and after all I think we shouldn't discuss this in the regular PromQL doc. It is super niche and would be very confusing at this place. The spec is the right place to mention it.

Signed-off-by: Neeraj Gartia <neerajgartia211002@gmail.com>
@NeerajGartia21
Copy link
Contributor Author

@beorn7 I've applied all the suggested changes and this PR is ready for another round of review.

Copy link
Member

@beorn7 beorn7 left a comment

Choose a reason for hiding this comment

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

Thank you very much, looks beautiful now.

@beorn7 beorn7 merged commit 130cd02 into prometheus:main Jan 31, 2025
26 checks passed
@NeerajGartia21 NeerajGartia21 deleted the promql/ifunc branch February 1, 2025 08:35
charleskorn added a commit to grafana/mimir that referenced this pull request Mar 12, 2025
* Upgrade mimir-prometheus

* Bring in upstream test changes

* Apply change to match prometheus/prometheus#15974

* Remove test case added in upstream in prometheus/prometheus#15975

* Add tests for case where range vector selectors and subqueries have 0 range

Brings in change from #10586

* Update `sort` and `sort_desc` behaviour to match prometheus/prometheus#15964

* Add support for native histograms to `irate` and `idelta` to match prometheus/prometheus#15853

* Adjust test cases to match support for native histograms in `irate` and `idelta`

* Change binop annotations to match behaviour in prometheus/prometheus#15895

* Ignore incompatible schemas between the first and second point in `rate` and `increase` if the second point is a reset (prometheus/prometheus#15902)

* Adjust annotation test cases to match new behaviour introduced in previous commit

* Disable tests failing due to prometheus/prometheus#16199

* Add changelog entry

* Update `TestQuerySharding_FunctionCorrectness` to reflect changes in prometheus/prometheus#15964
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.

2 participants