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

Add unit rule exclusions #205

Merged
merged 7 commits into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions docs/rules/panel-units-rule.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
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.
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
64 changes: 21 additions & 43 deletions lint/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"encoding/json"
"fmt"
"strings"

"github.com/grafana/grafana-foundation-sdk/go/dashboard"
)

type Severity int
Expand Down Expand Up @@ -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)
Expand Down
59 changes: 53 additions & 6 deletions lint/rule_panel_units.go
Original file line number Diff line number Diff line change
@@ -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{
Expand Down Expand Up @@ -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
}
}
Expand All @@ -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 != ""
}
113 changes: 107 additions & 6 deletions lint/rule_panel_units_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"),
},
},
},
Expand Down Expand Up @@ -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"),
},
},
},
Expand All @@ -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,
},
},
},
},
},
},
},
},
Expand Down
Loading