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

Overriding group_by to an empty list is ignored #4221

Open
dougmeredith opened this issue Jan 28, 2025 · 1 comment
Open

Overriding group_by to an empty list is ignored #4221

dougmeredith opened this issue Jan 28, 2025 · 1 comment
Labels

Comments

@dougmeredith
Copy link

If I specify group_by on the root route, it is inherited by child routes, as expected. If I specify group_by on a child route, it correctly overrides the inherited group_by. However, if a child attempts to override using group_by: [], this is ignored.

@grobinson-grafana
Copy link
Collaborator

grobinson-grafana commented Mar 9, 2025

This feels like a bug, as I can find no documentation or tests that show this to be intentional behavior.

What is happening is that r.GroupBy is initialized when there are one or more labels in group_by, however is not initialized when group_by: []. This happens because r.GroupBy is only initialized inside the range of r.GroupByStr:

for _, l := range r.GroupByStr {
	if l == "..." {
		r.GroupByAll = true
	} else {
		labelName := model.LabelName(l)
		if !compat.IsValidLabelName(labelName) {
			return fmt.Errorf("invalid label name %q in group_by list", l)
		}
		r.GroupBy = append(r.GroupBy, labelName)
	}
}

config.go#L918-L922

I think the fix looks something like this, but it needs tests:

diff --git a/config/config.go b/config/config.go
index 6f54c523..0f43c8e5 100644
--- a/config/config.go
+++ b/config/config.go
@@ -923,6 +923,10 @@ func (r *Route) UnmarshalYAML(unmarshal func(interface{}) error) error {
                }
        }

+       if r.GroupByStr != nil && len(r.GroupByStr) == 0 {
+               r.GroupBy = make([]model.LabelName, 0)
+       }
+
        if len(r.GroupBy) > 0 && r.GroupByAll {
                return errors.New("cannot have wildcard group_by (`...`) and other other labels at the same time")
        }

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

No branches or pull requests

2 participants