Overriding group_by to an empty list is ignored

[dougmeredith]

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]

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")
        }

[ADITYATIWARI342005]

Hi @grobinson-grafana what’s the current status?Is the issue still open as their are no mentioned PRs, I’d like to work on it : I can implement the minimal fix (treat an explicit group_by: [] as an explicit empty slice and clear GroupByAll), add unit tests and a small docs note, and open a PR.

Any objections or additional guidance before I start? Thanks.