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.

Sign up for GitHub

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

Jump to bottom

fix: handle min-max for network dashboard #1763

Merged
merged 5 commits into from
Feb 23, 2023
Merged

fix: handle min-max for network dashboard #1763

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5eab65b
fix: handle min-max for network dashboard
Hardikl Feb 22, 2023
f104c08
fix: added ci test case for min-max in case of gradient-guage
Hardikl Feb 22, 2023
9724973
fix: handled review comments
Hardikl Feb 23, 2023
f073d69
Merge remote-tracking branch 'origin/main' into guage_changes
Hardikl Feb 23, 2023
a2127e0
fix: handled review comments
Hardikl Feb 23, 2023
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
27 changes: 22 additions & 5 deletions cmd/tools/grafana/dashboard_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func TestUnitsAndExprMatch(t *testing.T) {
mt := newMetricsTable()
visitDashboards([]string{"../../../grafana/dashboards/cmode", "../../../grafana/dashboards/storagegrid"},
func(path string, data []byte) {
checkUnits(path, mt, data)
checkUnits(t, path, mt, data)
})

// Exceptions are meant to reduce false negatives
Expand Down Expand Up @@ -161,12 +161,12 @@ func newMetricsTable() *metricsTable {
}
}

func checkUnits(dashboardPath string, mt *metricsTable, data []byte) {
func checkUnits(t *testing.T, dashboardPath string, mt *metricsTable, data []byte) {
gjson.GetBytes(data, "panels").ForEach(func(key, value gjson.Result) bool {
doPanel("", key, value, mt, dashboardPath)
doPanel(t, "", key, value, mt, dashboardPath)
value.Get("panels").ForEach(func(key2, value2 gjson.Result) bool {
pathPrefix := fmt.Sprintf("panels[%d].", key.Int())
doPanel(pathPrefix, key2, value2, mt, dashboardPath)
doPanel(t, pathPrefix, key2, value2, mt, dashboardPath)
return true
})
return true
Expand All @@ -184,7 +184,7 @@ var metricDivideMetric2 = regexp.MustCompile(`(\w+)/.*?(\w+){`)
// detects arrays
var metricWithArray = regexp.MustCompile(`metric=~*"(.*?)"`)

func doPanel(pathPrefix string, key gjson.Result, value gjson.Result, mt *metricsTable, dashboardPath string) bool {
func doPanel(t *testing.T, pathPrefix string, key gjson.Result, value gjson.Result, mt *metricsTable, dashboardPath string) bool {
kind := value.Get("type").String()
if kind == "row" {
return true
Expand All @@ -197,17 +197,22 @@ func doPanel(pathPrefix string, key gjson.Result, value gjson.Result, mt *metric
title := value.Get("title").String()
sPath := shortPath(dashboardPath)

propertiesMap := make(map[string]map[string]string)
overrides := make([]override, 0, len(overridesSlice))
expressions := make([]expression, 0)
valueToName := make(map[string]string) // only used with panels[*].transformations[*].options.renameByName

for oi, overrideN := range overridesSlice {
matcherID := overrideN.Get("matcher.id")
// make sure that mapKey is unique for each override element
propertiesMapKey := matcherID.String() + strconv.Itoa(oi)
propertiesMap[propertiesMapKey] = make(map[string]string)
matcherOptions := overrideN.Get("matcher.options")
propertiesN := overrideN.Get("properties").Array()
for pi, propN := range propertiesN {
propID := propN.Get("id").String()
propVal := propN.Get("value").String()
propertiesMap[propertiesMapKey][propID] = propVal
if propID == "unit" {
o := override{
id: matcherID.String(),
Expand All @@ -221,6 +226,18 @@ func doPanel(pathPrefix string, key gjson.Result, value gjson.Result, mt *metric
}
}

// In case of gradient-gauge and percent(0.0-1.0), we must override min and max value
for _, properties := range propertiesMap {
if properties["unit"] == "percentunit" && properties["custom.displayMode"] == "gradient-gauge" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it only apply to gradient-gauge?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked with other types which displayed value other than text.
Working as expected with:
color-background-solid
color-background

Don't work as expected with:
lcd-gauge
basic

Will add below 2 types in that condition

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

if maxVal, exist := properties["max"]; !exist || maxVal != "1" {
t.Errorf("dashboard=%s, title=%s should have max value 1", sPath, title)
}
if minVal, exist := properties["min"]; !exist || minVal != "0" {
t.Errorf("dashboard=%s, title=%s should have min value 0", sPath, title)
}
}
}

// Heatmap units are saved in a different place
if kind == "heatmap" && defaultUnit == "" {
defaultUnit = value.Get("yAxis.format").String()
Expand Down
16 changes: 16 additions & 0 deletions grafana/dashboards/7mode/network7.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,14 @@
}
]
}
},
{
"id": "max",
"value": 1
},
{
"id": "min",
"value": 0
}
]
},
Expand Down Expand Up @@ -1503,6 +1511,14 @@
}
]
}
},
{
"id": "max",
"value": 1
},
{
"id": "min",
"value": 0
}
]
},
Expand Down
65 changes: 16 additions & 49 deletions grafana/dashboards/cmode/network.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,14 @@
}
]
}
},
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you share an example of before and after. A live setup will be helpful to see.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added Network Updated dashboard for testing in 10.195.64.86 machine.

In our clusters, all the values of nic_util_percent are very less.
image

For testing purpose only, just multiply value by 100 in query #c in both the places.

  1. Screenshot from old Network dashboard where no min-max set

image

  1. Screenshot from updated network dashboard where min-max values are set

image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some of the overrides are not found. Could you check and remove stale entries

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed.

"id": "max",
"value": 1
},
{
"id": "min",
"value": 0
}
]
},
Expand Down Expand Up @@ -1530,6 +1538,14 @@
}
]
}
},
{
"id": "max",
"value": 1
},
{
"id": "min",
"value": 0
}
]
},
Expand Down Expand Up @@ -2535,54 +2551,6 @@
}
]
},
{
"matcher": {
"id": "byName",
"options": "Value #C"
},
"properties": [
{
"id": "unit",
"value": "percentunit"
},
{
"id": "custom.displayMode",
"value": "gradient-gauge"
},
{
"id": "displayName",
"value": "used %"
},
{
"id": "noValue",
"value": "n/a"
},
{
"id": "thresholds",
"value": {
"mode": "absolute",
"steps": [
{
"color": "rgb(80, 220, 20)",
"value": null
},
{
"color": "light-yellow",
"value": 50
},
{
"color": "semi-dark-orange",
"value": 75
},
{
"color": "semi-dark-red",
"value": 90
}
]
}
}
]
},
{
"matcher": {
"id": "byName",
Expand Down Expand Up @@ -2693,7 +2661,6 @@
"node",
"port",
"speed",
"Value #C",
"Value #A",
"Value #B"
]
Expand Down