Skip to content

Comments

New proposal: Add support for x-functions in MQE#10305

Closed
wilfriedroset wants to merge 1 commit intografana:mainfrom
wilfriedroset:mqe-support-x-functions
Closed

New proposal: Add support for x-functions in MQE#10305
wilfriedroset wants to merge 1 commit intografana:mainfrom
wilfriedroset:mqe-support-x-functions

Conversation

@wilfriedroset
Copy link
Collaborator

What this PR does

This PR follows a discussion on slack. It introduce a new proposal regarding the support for x-functions in MQE.

Which issue(s) this PR fixes or relates to

N/A

Checklist

  • Tests updated.
  • Documentation added.
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
  • about-versioning.md updated with experimental features.

@wilfriedroset wilfriedroset requested a review from a team as a code owner December 23, 2024 08:15
@wilfriedroset
Copy link
Collaborator Author

I believe this could related to #10067 which centralize the work around MQE.

Signed-off-by: Wilfried Roset <wilfriedroset@users.noreply.github.com>
@wilfriedroset wilfriedroset force-pushed the mqe-support-x-functions branch from d0e3c9a to 12e88c7 Compare December 24, 2024 08:43
@jhesketh
Copy link
Contributor

Hi @wilfriedroset o/

Thanks for working on this and putting together the proposal! My apologies for the slow response, it's been a busy holiday period.

Firstly, I think this proposal puts forward two things that I think are better to reason about separately.

(A): That Mimir should accept custom functions, and
(B): That Mimir should implement an xrate function.

From discussions with the Mimir team there is generally no opposition to (A). That is, where it makes sense, Mimir has the option to consider functions outside of the PromQL feature set. "Where it makes sense" needs to take many things into account such as: does it fit upstream, is it a Mimir only query, does it benefit users, what is the maintenance overhead, the amount of complexity and so on.

Given the number of factors to consider it's difficult to blanket say we'll accept "x-functions" or similar. As such, I think it's okay to assume that Mimir is open to custom functions and we will discuss each one on their own merits.

So for the purposes for this PR I will consider it as a proposal to add xrate to Mimir. For now, I won't bother with the details such as the feature flags or scope etc. but more on the scope of what it would take for Mimir to accept this.

This was discussed at Grafana and had the following notes:

  • Adding it to Mimir as an extension should be a last resort.
  • At the time it was proposed to Prometheus there were no experimental functions making the bar much higher for accepting new ones.
  • It is worth having a "champion" try proposing it to Prometheus again as an experimental function meeting the objections made against it.
  • Whoever is interested in working on a design-doc for Prometheus should have a meeting with @beorn7 to discuss and share historical context.
  • After-which an updated design-doc would be a good place to start.

All that is not to say we would reject this proposal for Mimir, but I think we need to do two things first:

  1. Get an updated consensus from upstream.
  2. If it is still rejected upstream, then reworking this proposal for Mimir to focus specifically on xrate (rather than more generally alternative functions) addressing why it makes sense for Mimir and not upstream.

Hope that makes sense, and sorry for the amount of hurdles to overcome on this one!

@wilfriedroset
Copy link
Collaborator Author

thank you @jhesketh.
I will try to work on the proposal.
@beorn7 should we have a quick meeting together before I start working on the proposal?

@beorn7
Copy link
Contributor

beorn7 commented Jan 14, 2025

I added a TODO to my list to write up a braindump about the whole “xrate complex”. I wanted to do this for a long time, and maybe it's the right time now. It will be very rough, but I'll explain it in person once it is done. I'll let you know.

@github-actions
Copy link
Contributor

Thank you for your contribution. This pull request has been marked as stale because it has had no activity in the last 150 days. It will be closed in 30 days if there is no further activity. If you need more time, you can add a comment to the PR.

@github-actions github-actions bot added the stale label Jun 14, 2025
@wilfriedroset
Copy link
Collaborator Author

This is not staled please do not close it.
I believe this PR will be unblocked once a consensus has been reach in prometheus/proposals#52

WDYT @beorn7 ?

@github-actions github-actions bot removed the stale label Jun 17, 2025
@beorn7
Copy link
Contributor

beorn7 commented Jun 17, 2025

prometheus/proposals#52 is different from "add support for x-functions". It adds two new modifiers to range selectors as an attempt to distill the many proposals and thoughts of the past into something that hopefully will do the job. (It is in itself an experimental feature and will be iterated upon depending on the outcome when it is used in real life.)

With prometheus/proposals#52 implemented in Prometheus, I would assume MQE will re-implement the same feature. This will then re-establish feature parity with PromQL (rather than creating an additional deviation as proposed in this PR).

Given that prometheus/proposals#52 is well underway, I actually don't see any need to still implement "X-functions" in MQE.

@github-actions
Copy link
Contributor

Thank you for your contribution. This pull request has been marked as stale because it has had no activity in the last 150 days. It will be closed in 30 days if there is no further activity. If you need more time, you can add a comment to the PR.

@github-actions github-actions bot added the stale label Nov 15, 2025
@charleskorn
Copy link
Contributor

Closing in favour of prometheus/proposals#52, which we're actively working to bring into MQE (#13398)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants