-
Notifications
You must be signed in to change notification settings - Fork 193
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
🐛: Target.GroupBy should be singular in the type declaration #180
Conversation
panel.go
Outdated
@@ -566,7 +566,7 @@ type Target struct { | |||
CrossSeriesReducer string `json:"crossSeriesReducer,omitempty"` | |||
PerSeriesAligner string `json:"perSeriesAligner,omitempty"` | |||
ValueType string `json:"valueType,omitempty"` | |||
GroupBys []string `json:"groupBys,omitempty"` | |||
GroupBy []string `json:"groupBys,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this need to be:
GroupBy []string `json:"groupBys,omitempty"` | |
GroupBy []string `json:"groupBy,omitempty"` |
? At least that's what you say in the PR description if I understood you correctly 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, that's exactly it. I knew I missed a spot somewhere. I've addressed that now.
…rafana-tools#180)" This reverts commit 6856e18.
…-tools#180) * GroupBy should be singular in the JSON * fix json declaration for groupBy Co-authored-by: Tank <[email protected]>
in Grafana 8.0.5:
Before, I was using the
Target.GroupBys
property, but it turns out that that did not have any effect. By examining other dashboards manually created, I noticed that the JSON for the Panels actually hadgroupby
in the singular. By changing the panel json for panels created by the grafana sdk and applying it, I was able to get the desired functionality.With the plural:
and with the singular:
the name of the cluster is redacted because it does display correctly.
this should restore the groupby functionality in the grafana-sdk. I did a search to see if the
GroupBys
property was ever used, and I don't see any evidence of it in the documentation.