Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Enhancement]: Make prometheus v2/v3 opt in #1016

Open
4 tasks done
lorenzofelletti opened this issue Jan 29, 2025 · 5 comments
Open
4 tasks done

[Enhancement]: Make prometheus v2/v3 opt in #1016

lorenzofelletti opened this issue Jan 29, 2025 · 5 comments
Labels
keepalive Use to prevent automatic closing

Comments

@lorenzofelletti
Copy link
Contributor

lorenzofelletti commented Jan 29, 2025

What's the general idea for the enhancement?

#1008 rightfully changed mixin rules not handling normalised buckets in Prometheus v3.
However the change involved switching to regex matching for bucket matching, which is more computationally intensive than the = selector.

I propose to change this by introducing a mechanism (e.g. config parameter) to target v2 or v3-style rules, thus removing the necessity to use a regular expression. This would positively impact performance for all users, as some of these expressions are already quite expensive to compute.

Please provide any helpful snippets.

What parts of the codebase does the enhancement target?

Rules

Anything else relevant to the enhancement that would help with the triage process?

No response

I agree to the following terms:

  • I agree to follow this project's Code of Conduct.
  • I have filled out all the required information above to the best of my ability.
  • I have searched the issues of this repository and believe that this is not a duplicate.
  • I have confirmed this proposal applies to the default branch of the repository, as of the latest commit at the time of submission.
@skl
Copy link
Collaborator

skl commented Jan 29, 2025

Great idea @lorenzofelletti! Couple of questions:

  1. Do you have any benchmarks/data around the impact of the different matchers? If there's evidence to suggest the impact is significant, I know to be more careful around the matcher used in future 😄
  2. Would this be just for the queries touched in fix(rules): handle apiserver normalized buckets #1008 or are there other touchpoints to consider as well that you know of?

@lorenzofelletti
Copy link
Contributor Author

Hey @skl good questions. In order

  1. I don't have any benchmark as yet.
  2. I've tried to take a look around, and seems like those are the only rules impacted, but I am not 100% sure.

@lorenzofelletti
Copy link
Contributor Author

Here is the documentation I'd used to check around for other incompatibilities: https://prometheus.io/docs/prometheus/latest/migration/#promql.

Regex: . now matches newline

These are the places in which we use regexes:

Normalised buckets

These (#1008) to me look like the only places affected by the change.

Lookback and range selectors

Lookback and range selectors are left open and right closed (previously left closed and right closed). [...] This change has likely few effects for everyday use, except for some subquery use cases. [...] Tests are more likely to affected.

As the documentation says, it is unlikely that the change would affect any query.

Copy link

github-actions bot commented Mar 1, 2025

This issue has not had any activity in the past 30 days, so the
stale label has been added to it.

  • The stale label will be removed if there is new activity
  • The issue will be closed in 7 days if there is no new activity
  • Add the keepalive label to exempt this issue from the stale check action

Thank you for your contributions!

@github-actions github-actions bot added the stale label Mar 1, 2025
@skl skl added keepalive Use to prevent automatic closing and removed stale labels Mar 1, 2025
@skl
Copy link
Collaborator

skl commented Mar 1, 2025

I'd be interested in some profiles/benchmarks regarding exact vs regex label matching.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keepalive Use to prevent automatic closing
Projects
None yet
Development

No branches or pull requests

2 participants