Skip to content

Fix: Allow empty group_by to override parent route#4825

Merged
SuperQ merged 5 commits intoprometheus:mainfrom
adity-a34:fix/group-by-empty
Jan 30, 2026
Merged

Fix: Allow empty group_by to override parent route#4825
SuperQ merged 5 commits intoprometheus:mainfrom
adity-a34:fix/group-by-empty

Conversation

@adity-a34
Copy link
Contributor

Fixes #4221

Child routes with group_by: [] were incorrectly inheriting parent route's
group_by configuration instead of overriding it. This fix initializes GroupBy
as an empty slice when GroupByStr is explicitly set to empty, allowing child
routes to properly disable grouping.

Signed-off-by: ADITYATIWARI342005 <142050150+ADITYATIWARI342005@users.noreply.github.com>
Copy link
Contributor

@SoloJacobs SoloJacobs left a comment

Choose a reason for hiding this comment

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

Did you check that this fix resolves the issue completely, e.g., by creating a small test setup?

Signed-off-by: Aditya Tiwari <adityatiwari342005@gmail.com>
@adity-a34
Copy link
Contributor Author

Hi @SoloJacobs

Did you check that this fix resolves the issue completely, e.g., by creating a small test setup?

I tested for the unit test and go tests for build and gofmt etc. However I did not create a dedicated test setup/script, but since you mentioned it, I did that too

validation script confirms:

  • Parent route: group_by = [alertname cluster] (2 labels) ✅
  • Child route: group_by = [] (empty, not nil) ✅
  • Key proof: Is nil? false - the fix distinguishes empty from inheritance ✅

The fix correctly resolves #4221 by initializing GroupBy as an empty slice when group_by: [] is explicitly set, allowing child routes to override parent configuration.

I've also simplify the test config as suggested (removing unnecessary match/receiver fields).

@adity-a34 adity-a34 requested a review from SoloJacobs December 24, 2025 06:12
Copy link
Contributor

@SoloJacobs SoloJacobs left a comment

Choose a reason for hiding this comment

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

Very nice, well done 🙂 Just a reminder, due to the current time constraints of maintainers, all changes take some time to be merged.

Signed-off-by: Aditya Tiwari <adityatiwari342005@gmail.com>
Signed-off-by: Aditya Tiwari <adityatiwari342005@gmail.com>
@SuperQ SuperQ merged commit 87452e6 into prometheus:main Jan 30, 2026
7 checks passed
@SoloJacobs SoloJacobs mentioned this pull request Jan 30, 2026
SuperQ added a commit that referenced this pull request Feb 2, 2026
* [ENHANCEMENT] docs(opsgenie): Fix description of `api_url` field. #4908
* [ENHANCEMENT] docs(slack): Document missing app configs. #4871
* [ENHANCEMENT] docs: Fix `max-silence-size-bytes`. #4805
* [ENHANCEMENT] docs: Update expr for `AlertmanagerClusterFailedToSendAlerts` to exclude value 0. #4872
* [ENHANCEMENT] docs: Use matchers for inhibit rules examples. #4131
* [ENHANCEMENT] docs: add notification integrations. #4901
* [ENHANCEMENT] docs: update `slack_config` attachments documentation links. #4802
* [ENHANCEMENT] docs: update description of filter query params in openapi doc. #4810
* [ENHANCEMENT] provider: Reduce lock contention. #4809
* [FEATURE] slack: Add support for top-level text field in slack notification. #4867
* [FEATURE] smtp: Add support for authsecret from file. #3087
* [FEATURE] smtp: Customize the ssl/tls port support (#4757). #4818
* [FEATURE] smtp: Enhance email notifier configuration validation. #4826
* [FEATURE] telegram: Add `chat_id_file` configuration parameter. #4909
* [FEATURE] telegram: Support global bot token. #4823
* [FEATURE] webhook: Support templating in url fields. #4798
* [FEATURE] wechat: Add config directive to pass api secret via file. #4734
* [FEATURE] provider: Implement per alert limits. #4819
* [BUGFIX] Allow empty `group_by` to override parent route. #4825
* [BUGFIX] Set `spellcheck=false` attribute on silence filter input. #4811
* [BUGFIX] jira: Fix for handling api v3 with ADF. #4756
* [BUGFIX] jira: Prevent hostname corruption in cloud api url replacement. #4892
---------

Signed-off-by: Solomon Jacobs <solomonjacobs@protonmail.com>
Signed-off-by: Ben Kochie <superq@gmail.com>
Co-authored-by: Ben Kochie <superq@gmail.com>
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.

Overriding group_by to an empty list is ignored

4 participants