Skip to content

Querier: Support configuring optimizers and XFunctions #6873

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

harry671003
Copy link
Contributor

@harry671003 harry671003 commented Jul 10, 2025

What this PR does:

  • Adds new config for enabling XFunctions - xincrease(), xrate(), xdelta(), configuring optimizers etc for Thanos engine.
  • I took the liberty to refactor the engine config into it's own struct. Assuming we can make breaking changes here since Thanos engine is marked as experimental.

Which issue(s) this PR fixes:
Fixes #

Checklist

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

@harry671003 harry671003 changed the title Refactor engine config Querier: Refactor engine config Jul 10, 2025
@harry671003 harry671003 force-pushed the refactor_engine_config branch 4 times, most recently from edea965 to c160724 Compare July 10, 2025 21:33
@harry671003 harry671003 changed the title Querier: Refactor engine config Querier: Support configuring optimizers and XFunctions Jul 10, 2025
@harry671003 harry671003 marked this pull request as ready for review July 10, 2025 21:33
@harry671003 harry671003 force-pushed the refactor_engine_config branch 2 times, most recently from 099bdbf to 9052bee Compare July 10, 2025 22:08
@harry671003 harry671003 force-pushed the refactor_engine_config branch from 9052bee to 895dd48 Compare July 11, 2025 15:54
func (cfg *Config) RegisterFlags(f *flag.FlagSet) {
f.BoolVar(&cfg.EnableThanosEngine, "querier.thanos-engine", false, "Experimental. Use Thanos promql engine https://github.com/thanos-io/promql-engine rather than the Prometheus promql engine.")
f.BoolVar(&cfg.EnableXFunctions, "querier.enable-x-functions", false, "Enable xincrease, xdelta, xrate etc from Thanos engine.")
f.StringVar(&cfg.Optimizers, "querier.optimizers", "default", "Logical plan optimizers. Multiple optimizers can be provided as a comma-separated list. Supported values: "+strings.Join(supportedOptimizers, ", "))
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can accept a prefix so that the prefix can be either querier or ruler.

<prefix>.thanos-engine.enabled
<prefix>.thanos-engine.enable-x-functions
<prefix>.thanos-engine.optimizers

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should also have a small doc talking about what are these optimizers and what they do. For now, can we at least mention in the flag description that, what optimizers are included in default and what additional optimizers are included in all? And they cannot be enabled together.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the second comment, I am planning to add a String() function to the Optimizer interface in thanos engine. This will make it easier to do.

require.Equal(t, 200, res.StatusCode)

for xFunc := range parse.XFunctions {
result, err := c.Query(fmt.Sprintf(`%s(series_1{job="test"}[1m])`, xFunc), series1Timestamp)
Copy link
Contributor

Choose a reason for hiding this comment

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

I am a bit surprised that this test could pass when querying query frontend. I don't see we register those functions there. Only in querier.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe because the middlewares are disabled by default.

@harry671003 harry671003 force-pushed the refactor_engine_config branch 3 times, most recently from bdea544 to 717b8cd Compare July 14, 2025 19:35
Signed-off-by: 🌲 Harry 🌊 John 🏔 <[email protected]>
@harry671003 harry671003 force-pushed the refactor_engine_config branch from 717b8cd to 4fcd0ec Compare July 15, 2025 00:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants