diff --git a/docs/rules/panel-units-rule.md b/docs/rules/panel-units-rule.md index d68807b..cf2f88d 100644 --- a/docs/rules/panel-units-rule.md +++ b/docs/rules/panel-units-rule.md @@ -4,7 +4,11 @@ Checks that every panel has a unit specified, and that the unit is valid per the It currently only checks panels of type ["stat", "singlestat", "graph", "table", "timeseries", "gauge"]. # Best Practice -All panels should have all of their axis labeled with an apprioriate unit. +All panels should have an apprioriate unit set. # Possible exceptions -A panel may be visualizing something which does not have a predefined unit, or which is self explanatory from the vizualization title. In this case you may wish to create a lint exclusion for this rule. \ No newline at end of file +This rule is automatically excluded when: + - Value mappings are set in a panel. + - A Stat panel is configured to show non-numeric values (like label's value), for that 'Fields options' are configured to any value other than 'Numeric fields' (which is default). + +Also, a panel may be visualizing something which does not have a predefined unit, or which is self explanatory from the vizualization title. In this case you may wish to create a lint exclusion for this rule. \ No newline at end of file diff --git a/go.mod b/go.mod index ba69689..2e91c84 100644 --- a/go.mod +++ b/go.mod @@ -64,6 +64,7 @@ require ( github.com/gorilla/mux v1.8.1 // indirect github.com/grafana/dskit v0.0.0-20240905221822-931a021fb06b // indirect github.com/grafana/gomemcache v0.0.0-20240229205252-cd6a66d6fb56 // indirect + github.com/grafana/grafana-foundation-sdk/go v0.0.0-20241101005901-83e3491f2a70 // indirect github.com/grafana/jsonparser v0.0.0-20240425183733-ea80629e1a32 // indirect github.com/grafana/loki/pkg/push v0.0.0-20231124142027-e52380921608 // indirect github.com/grafana/pyroscope-go/godeltaprof v0.1.8 // indirect diff --git a/go.sum b/go.sum index 11254dd..36e9d2b 100644 --- a/go.sum +++ b/go.sum @@ -197,6 +197,8 @@ github.com/grafana/dskit v0.0.0-20240905221822-931a021fb06b h1:x2HCzk29I0o5pRPfq github.com/grafana/dskit v0.0.0-20240905221822-931a021fb06b/go.mod h1:SPLNCARd4xdjCkue0O6hvuoveuS1dGJjDnfxYe405YQ= github.com/grafana/gomemcache v0.0.0-20240229205252-cd6a66d6fb56 h1:X8IKQ0wu40wpvYcKfBcc5T4QnhdQjUhtUtB/1CY89lE= github.com/grafana/gomemcache v0.0.0-20240229205252-cd6a66d6fb56/go.mod h1:PGk3RjYHpxMM8HFPhKKo+vve3DdlPUELZLSDEFehPuU= +github.com/grafana/grafana-foundation-sdk/go v0.0.0-20241101005901-83e3491f2a70 h1:69GI3KsF851YnwYp6zHdsskcGp3ZnGsWc+ve8vMp1mc= +github.com/grafana/grafana-foundation-sdk/go v0.0.0-20241101005901-83e3491f2a70/go.mod h1:WtWosval1KCZP9BGa42b8aVoJmVXSg0EvQXi9LDSVZQ= github.com/grafana/jsonparser v0.0.0-20240425183733-ea80629e1a32 h1:NznuPwItog+rwdVg8hAuGKP29ndRSzJAwhxKldkP8oQ= github.com/grafana/jsonparser v0.0.0-20240425183733-ea80629e1a32/go.mod h1:796sq+UcONnSlzA3RtlBZ+b/hrerkZXiEmO8oMjyRwY= github.com/grafana/loki/pkg/push v0.0.0-20231124142027-e52380921608 h1:ZYk42718kSXOiIKdjZKljWLgBpzL5z1yutKABksQCMg= diff --git a/lint/lint.go b/lint/lint.go index b38df8a..686be48 100644 --- a/lint/lint.go +++ b/lint/lint.go @@ -4,6 +4,8 @@ import ( "encoding/json" "fmt" "strings" + + "github.com/grafana/grafana-foundation-sdk/go/dashboard" ) type Severity int @@ -195,55 +197,31 @@ func (a *Annotation) GetDataSource() (Datasource, error) { // Panel is a deliberately incomplete representation of the Dashboard -> Panel type in grafana. // The properties which are extracted from JSON are only those used for linting purposes. type Panel struct { - Id int `json:"id"` - Title string `json:"title"` - Description string `json:"description,omitempty"` - Targets []Target `json:"targets,omitempty"` - Datasource interface{} `json:"datasource,omitempty"` - Type string `json:"type"` - Panels []Panel `json:"panels,omitempty"` - FieldConfig *FieldConfig `json:"fieldConfig,omitempty"` + Id int `json:"id"` + Title string `json:"title"` + Description string `json:"description,omitempty"` + Targets []Target `json:"targets,omitempty"` + Datasource interface{} `json:"datasource,omitempty"` + Type string `json:"type"` + Panels []Panel `json:"panels,omitempty"` + FieldConfig *FieldConfig `json:"fieldConfig,omitempty"` + Options json.RawMessage `json:"options,omitempty"` } -type FieldConfig struct { - Defaults Defaults `json:"defaults,omitempty"` - Overrides []Override `json:"overrides,omitempty"` -} - -type Override struct { - OverrideProperties []OverrideProperty `json:"properties"` -} - -type OverrideProperty struct { - Id string `json:"id"` - Value string `json:"value"` +// Stat panel options is a deliberately incomplete representation of the stat panel options from grafana. +// The properties which are extracted from JSON are only those used for linting purposes. +type StatOptions struct { + ReduceOptions ReduceOptions `json:"reduceOptions,omitempty"` } -func (o *OverrideProperty) UnmarshalJSON(buf []byte) error { - // An override value can be of type string or int - // This function detects type mismatch and uses an int type for Value - var raw struct { - Id string `json:"id"` - Value string `json:"value"` - } - - if err := json.Unmarshal(buf, &raw); err != nil { - var raw struct { - Id string `json:"id"` - Value int `json:"value"` - } - if err := json.Unmarshal(buf, &raw); err != nil { - // Override can have varying different types int, string and arrays - // Currently only units are being checked from overrides so returning nil in case of unhandled types - return nil - } - } - - return nil +// oversimplified Reduce options +type ReduceOptions struct { + Fields string `json:"fields,omitempty"` } -type Defaults struct { - Unit string `json:"unit,omitempty"` +type FieldConfig struct { + Defaults dashboard.FieldConfig + Overrides []dashboard.DashboardFieldConfigSourceOverrides } // GetPanels returns the all panels nested inside the panel (inc the current panel) diff --git a/lint/rule_panel_units.go b/lint/rule_panel_units.go index ace2c3f..7fea86a 100644 --- a/lint/rule_panel_units.go +++ b/lint/rule_panel_units.go @@ -1,6 +1,11 @@ package lint -import "fmt" +import ( + "encoding/json" + "fmt" + + "github.com/grafana/grafana-foundation-sdk/go/dashboard" +) func NewPanelUnitsRule() *PanelRuleFunc { validUnits := []string{ @@ -70,10 +75,28 @@ func NewPanelUnitsRule() *PanelRuleFunc { r := PanelRuleResults{} switch p.Type { case panelTypeStat, panelTypeSingleStat, panelTypeGraph, panelTypeTimeTable, panelTypeTimeSeries, panelTypeGauge: + + // ignore if has reduceOptions fields (for stat panels only): + if p.Type == "stat" { + var opts StatOptions + err := json.Unmarshal(p.Options, &opts) + if err != nil { + r.AddError(d, p, err.Error()) + } + if hasReduceOptionsNonNumericFields(&opts.ReduceOptions) { + return r + } + } + + //ignore if has value mappings: + if len(getValueMappings(p)) > 0 { + return r + } + configuredUnit := getConfiguredUnit(p) if configuredUnit != "" { for _, u := range validUnits { - if u == p.FieldConfig.Defaults.Unit { + if u == configuredUnit { return r } } @@ -90,15 +113,39 @@ func getConfiguredUnit(p Panel) string { // First check if an override with unit exists - if no override then check if standard unit is present and valid if p.FieldConfig != nil && len(p.FieldConfig.Overrides) > 0 { for _, p := range p.FieldConfig.Overrides { - for _, o := range p.OverrideProperties { + for _, o := range p.Properties { if o.Id == "unit" { - configuredUnit = o.Value + configuredUnit = o.Value.(string) } } } } - if configuredUnit == "" && p.FieldConfig != nil && len(p.FieldConfig.Defaults.Unit) > 0 { - configuredUnit = p.FieldConfig.Defaults.Unit + if configuredUnit == "" && p.FieldConfig != nil && p.FieldConfig.Defaults.Unit != nil { + configuredUnit = *p.FieldConfig.Defaults.Unit } return configuredUnit } + +func getValueMappings(p Panel) []dashboard.ValueMapping { + valueMappings := make([]dashboard.ValueMapping, 0) + // First check if an override with unit exists - if no override then check if standard unit is present and valid + if p.FieldConfig != nil && len(p.FieldConfig.Overrides) > 0 { + for _, p := range p.FieldConfig.Overrides { + for _, o := range p.Properties { + if o.Id == "mappings" { + valueMappings = o.Value.([]dashboard.ValueMapping) + } + } + } + } + if len(valueMappings) == 0 && p.FieldConfig != nil && p.FieldConfig.Defaults.Mappings != nil { + valueMappings = p.FieldConfig.Defaults.Mappings + } + return valueMappings +} + +// Numeric fields are set as empty string "". Any other value means nonnumeric on grafana stat panel. +func hasReduceOptionsNonNumericFields(reduceOpts *ReduceOptions) bool { + + return reduceOpts.Fields != "" +} diff --git a/lint/rule_panel_units_test.go b/lint/rule_panel_units_test.go index 1b98fc6..21f1d84 100644 --- a/lint/rule_panel_units_test.go +++ b/lint/rule_panel_units_test.go @@ -2,11 +2,28 @@ package lint import ( "testing" + + "github.com/grafana/grafana-foundation-sdk/go/dashboard" ) +func ptr[T any](t T) *T { return &t } func TestPanelUnits(t *testing.T) { linter := NewPanelUnitsRule() + testValueMap := &dashboard.ValueMap{ + Type: "value", + Options: map[string]dashboard.ValueMappingResult{ + "1": { + Text: ptr("Ok"), + Color: ptr("green"), + }, + "2": { + Text: ptr("Down"), + Color: ptr("red"), + }, + }, + } + for _, tc := range []struct { name string result Result @@ -23,8 +40,8 @@ func TestPanelUnits(t *testing.T) { Datasource: "foo", Title: "bar", FieldConfig: &FieldConfig{ - Defaults: Defaults{ - Unit: "MyInvalidUnit", + Defaults: dashboard.FieldConfig{ + Unit: ptr("MyInvalidUnit"), }, }, }, @@ -62,8 +79,8 @@ func TestPanelUnits(t *testing.T) { Datasource: "foo", Title: "bar", FieldConfig: &FieldConfig{ - Defaults: Defaults{ - Unit: "short", + Defaults: dashboard.FieldConfig{ + Unit: ptr("short"), }, }, }, @@ -76,8 +93,92 @@ func TestPanelUnits(t *testing.T) { Datasource: "foo", Title: "bar", FieldConfig: &FieldConfig{ - Defaults: Defaults{ - Unit: "none", + Defaults: dashboard.FieldConfig{ + Unit: ptr("none"), + }, + }, + }, + }, + { + name: "has nonnumeric reduceOptions fields", + result: ResultSuccess, + panel: Panel{ + Type: "stat", + Datasource: "foo", + Title: "bar", + Options: []byte(` + { + "reduceOptions": { + "fields": "/^version$/" + } + } + + `), + }, + }, + { + name: "has empty reduceOptions fields(Numeric Fields default value)", + result: Result{ + Severity: Error, + Message: "Dashboard 'test', panel 'bar' has no or invalid units defined: ''", + }, + panel: Panel{ + Type: "stat", + Datasource: "foo", + Title: "bar", + Options: []byte(` + { + "reduceOptions": { + "fields": "" + } + } + + `), + }, + }, + { + name: "no units but have value mappings", + result: ResultSuccess, + panel: Panel{ + Type: "singlestat", + Datasource: "foo", + Title: "bar", + FieldConfig: &FieldConfig{ + Defaults: dashboard.FieldConfig{ + Mappings: []dashboard.ValueMapOrRangeMapOrRegexMapOrSpecialValueMap{ + dashboard.ValueMapOrRangeMapOrRegexMapOrSpecialValueMap{ + ValueMap: testValueMap, + }, + }, + }, + }, + }, + }, + { + name: "no units but have value mappings in overrides", + result: ResultSuccess, + panel: Panel{ + Type: "singlestat", + Datasource: "foo", + Title: "bar", + FieldConfig: &FieldConfig{ + Overrides: []dashboard.DashboardFieldConfigSourceOverrides{ + dashboard.DashboardFieldConfigSourceOverrides{ + Matcher: dashboard.MatcherConfig{ + Id: "byRegexp", + Options: "/.*/", + }, + Properties: []dashboard.DynamicConfigValue{ + dashboard.DynamicConfigValue{ + Id: "mappings", + Value: []dashboard.ValueMapOrRangeMapOrRegexMapOrSpecialValueMap{ + dashboard.ValueMapOrRangeMapOrRegexMapOrSpecialValueMap{ + ValueMap: testValueMap, + }, + }, + }, + }, + }, }, }, },