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.

Sign up for GitHub

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

Jump to bottom

Simplify how experimental PromQL functions are enabled #10660

Merged
merged 2 commits into from
Feb 20, 2025
Merged

Simplify how experimental PromQL functions are enabled #10660

merged 2 commits into from
Feb 20, 2025

Conversation

56quarters
Copy link
Contributor

@56quarters 56quarters commented Feb 14, 2025 •
edited
Loading

What this PR does

Remove experimental configuration flags querier.promql-experimental-functions-enabled and query-frontend.block-promql-experimental-functions used for controlling access to experimental PromQL features. Instead enable experimental PromQL functions at the engine level by default, block access to them by default, and selectively enable them at a per-user level using the middleware introduced in #9798.

Justification for this change:

  • There are currently 3 different settings that need to changed to allow customers to make use experimental functions in PromQL that need to be applied 3 different places (queriers, query-frontend, and per-tenant limits).
  • The performance impact from always enabling but blocking via middleware experimental PromQL functions (what this PR does) is so small that it does not show up in profiling from what I can tell.

Which issue(s) this PR fixes or relates to

Related #9798

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.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 14, 2025 •
edited
Loading

💻 Deploy preview deleted.

@56quarters 56quarters force-pushed the 56quarters/exp-config-cleanup branch 4 times, most recently from cb77f3b to 8a7d444 Compare February 19, 2025 15:28
@56quarters
Simplify how experimental PromQL functions are enabled
f603de2 
Remove experimental configuration flags
`querier.promql-experimental-functions-enabled` and
`query-frontend.block-promql-experimental-functions` used for
controlling access to experimental PromQL features. Instead enable
experimental PromQL functions at the engine level by default, block
access to them by default, and selectively enable them at a per-user
level using the middleware introduced in #9798.

Justification for this change:
* There are currently 3 different settings that need to changed to
  allow customers to make use experimental functions in PromQL that
  need to be applied 3 different places (queriers, query-frontend, and
  per-tenant limits).
* The performance impact from always enabling but blocking via
  middleware experimental PromQL functions (what this PR does) is
  so small that it does not show up in profiling from what I can
  tell.

Signed-off-by: Nick Pillitteri <nick.pillitteri@grafana.com>
@56quarters 56quarters force-pushed the 56quarters/exp-config-cleanup branch from 8a7d444 to f603de2 Compare February 19, 2025 15:33
@56quarters 56quarters marked this pull request as ready for review February 19, 2025 15:41
@56quarters 56quarters requested review from tacole02 and a team as code owners February 19, 2025 15:41
dimitarvdimitrov
dimitarvdimitrov approved these changes Feb 19, 2025
View reviewed changes
Copy link
Contributor

@dimitarvdimitrov dimitarvdimitrov left a comment

Choose a reason for hiding this comment

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

changes seem good.

Do I have it right that if someone wants to enable experimental promQL functions for all tenants, then they just need to set -query-frontend.enabled-promql-experimental-functions=all?

@56quarters
Copy link
Contributor Author

changes seem good.

Do I have it right that if someone wants to enable experimental promQL functions for all tenants, then they just need to set -query-frontend.enabled-promql-experimental-functions=all?

Yep, exactly.

@56quarters
Copy link
Contributor Author

changes seem good.
Do I have it right that if someone wants to enable experimental promQL functions for all tenants, then they just need to set -query-frontend.enabled-promql-experimental-functions=all?

Yep, exactly.

We're notably missing any non-reference documentation about this. I'll open an issue to add some after merging this PR (maybe under docs/sources/mimir/configure/?)

@dimitarvdimitrov
Copy link
Contributor

either configure or near the experimental features docs in about-versioning.md

tacole02
tacole02 approved these changes Feb 20, 2025
View reviewed changes
@tacole02
Copy link
Contributor

Thanks for updating this! And please feel free to ping me with any questions on the subsequent docs PR you plan to open. 😄

@56quarters @tacole02
Update CHANGELOG.md
396d02d 
Co-authored-by: Taylor C <41653732+tacole02@users.noreply.github.com>
@56quarters 56quarters merged commit 5963193 into main Feb 20, 2025
30 checks passed
@56quarters 56quarters deleted the 56quarters/exp-config-cleanup branch February 20, 2025 19:25
56quarters added a commit that referenced this pull request Feb 20, 2025
@56quarters
Docs: Explain how to enable experimental PromQL functions
91a134b 
Related to #10660

Signed-off-by: Nick Pillitteri <nick.pillitteri@grafana.com>
@56quarters 56quarters mentioned this pull request Feb 20, 2025
Open
4 tasks
56quarters added a commit that referenced this pull request Feb 20, 2025
@56quarters
Docs: Explain how to enable experimental PromQL functions
6a4a433 
Related to #10660

Signed-off-by: Nick Pillitteri <nick.pillitteri@grafana.com>
56quarters added a commit that referenced this pull request Feb 20, 2025
@56quarters
Docs: Explain how to enable experimental PromQL functions
c7a7d51 
Related to #10660

Signed-off-by: Nick Pillitteri <nick.pillitteri@grafana.com>
duricanikolic added a commit that referenced this pull request Feb 21, 2025
@duricanikolic
Revert "Simplify how experimental PromQL functions are enabled (#10660)"
a6ef9c1 
This reverts commit 5963193.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dimitarvdimitrov dimitarvdimitrov dimitarvdimitrov approved these changes

@tacole02 tacole02 tacole02 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@56quarters @dimitarvdimitrov @tacole02