From 23eaf76ef7260d16d054f60521836a0e8202b123 Mon Sep 17 00:00:00 2001 From: Joel Takvorian Date: Mon, 11 Dec 2023 12:05:21 +0100 Subject: [PATCH] NETOBSERV-1422: fix missing "buckets" in generated promql Add test; also improve tests to be more resilient on dashboard changes --- .../monitoring/monitoring_controller_test.go | 5 +- pkg/dashboards/dashboard.go | 2 +- pkg/dashboards/dashboard_test.go | 142 ++++++++++-------- pkg/dashboards/model.go | 27 +++- pkg/test/dashboards.go | 33 ---- 5 files changed, 107 insertions(+), 102 deletions(-) delete mode 100644 pkg/test/dashboards.go diff --git a/controllers/monitoring/monitoring_controller_test.go b/controllers/monitoring/monitoring_controller_test.go index fb26d2d88..9f81615de 100644 --- a/controllers/monitoring/monitoring_controller_test.go +++ b/controllers/monitoring/monitoring_controller_test.go @@ -12,6 +12,7 @@ import ( flowslatest "github.com/netobserv/network-observability-operator/api/v1beta2" . "github.com/netobserv/network-observability-operator/controllers/controllerstest" + "github.com/netobserv/network-observability-operator/pkg/dashboards" "github.com/netobserv/network-observability-operator/pkg/test" ) @@ -76,7 +77,7 @@ func ControllerSpecs() { }, &cm); err != nil { return err } - d, err := test.DashboardFromBytes([]byte(cm.Data["netobserv-health-metrics.json"])) + d, err := dashboards.FromBytes([]byte(cm.Data["netobserv-health-metrics.json"])) if err != nil { return err } @@ -118,7 +119,7 @@ func ControllerSpecs() { }, &cm); err != nil { return err } - d, err := test.DashboardFromBytes([]byte(cm.Data["netobserv-health-metrics.json"])) + d, err := dashboards.FromBytes([]byte(cm.Data["netobserv-health-metrics.json"])) if err != nil { return err } diff --git a/pkg/dashboards/dashboard.go b/pkg/dashboards/dashboard.go index c87383c79..db4e9e88a 100644 --- a/pkg/dashboards/dashboard.go +++ b/pkg/dashboards/dashboard.go @@ -169,7 +169,7 @@ func histogramPanels(scope *metricScope, metric, labels, legend, scaler string) } } // No split - rateExpr := fmt.Sprintf("rate(netobserv_%s[2m])", metric) + rateExpr := fmt.Sprintf("rate(netobserv_%s_bucket[2m])", metric) return []Panel{{ Targets: []Target{ histogramTarget(scope, "0.99", rateExpr, labels, legend, scaler), diff --git a/pkg/dashboards/dashboard_test.go b/pkg/dashboards/dashboard_test.go index 85f4973cb..60194ddaf 100644 --- a/pkg/dashboards/dashboard_test.go +++ b/pkg/dashboards/dashboard_test.go @@ -4,7 +4,6 @@ import ( "testing" "github.com/netobserv/network-observability-operator/pkg/metrics" - "github.com/netobserv/network-observability-operator/pkg/test" "github.com/stretchr/testify/assert" ) @@ -14,45 +13,62 @@ func TestCreateFlowMetricsDashboard_All(t *testing.T) { js, err := CreateFlowMetricsDashboard("netobserv", metrics.GetAllNames()) assert.NoError(err) - d, err := test.DashboardFromBytes([]byte(js)) + d, err := FromBytes([]byte(js)) assert.NoError(err) assert.Equal("NetObserv", d.Title) assert.Len(d.Rows, 27) - // First row - row := 0 - assert.Equal("Byte rate sent per node", d.Rows[row].Title) - assert.Len(d.Rows[row].Panels, 1) - assert.Equal("", d.Rows[row].Panels[0].Title) - assert.Len(d.Rows[row].Panels[0].Targets, 1) - assert.Contains(d.Rows[row].Panels[0].Targets[0].Expr, "label_replace(label_replace(topk(10,sum(rate(netobserv_node_egress_bytes_total[2m])) by (SrcK8S_HostName,DstK8S_HostName))") - - // 11th row - row = 10 - assert.Equal("Byte rate received per namespace", d.Rows[row].Title) - assert.Len(d.Rows[row].Panels, 2) - assert.Equal("Applications", d.Rows[row].Panels[0].Title) - assert.Equal("Infrastructure", d.Rows[row].Panels[1].Title) - assert.Len(d.Rows[row].Panels[0].Targets, 1) - assert.Contains(d.Rows[row].Panels[0].Targets[0].Expr, + row := d.FindRow("Byte rate sent per node") + assert.NotNil(row) + assert.Len(row.Panels, 1) + assert.Equal("", row.Panels[0].Title) + assert.Len(row.Panels[0].Targets, 1) + assert.Contains(row.Panels[0].Targets[0].Expr, "label_replace(label_replace(topk(10,sum(rate(netobserv_node_egress_bytes_total[2m])) by (SrcK8S_HostName,DstK8S_HostName))") + + row = d.FindRow("DNS latency per node") + assert.NotNil(row) + assert.Len(row.Panels, 1) + assert.Equal("", row.Panels[0].Title) + assert.Len(row.Panels[0].Targets, 2) + assert.Contains(row.Panels[0].Targets[0].Expr, "histogram_quantile(0.99, sum(rate(netobserv_node_dns_latency_seconds_bucket[2m])) by (le,SrcK8S_HostName,DstK8S_HostName))") + + row = d.FindRow("Byte rate received per namespace") + assert.NotNil(row) + assert.Len(row.Panels, 2) + assert.Equal("Applications", row.Panels[0].Title) + assert.Equal("Infrastructure", row.Panels[1].Title) + assert.Len(row.Panels[0].Targets, 1) + assert.Contains(row.Panels[0].Targets[0].Expr, `label_replace(label_replace(topk(10,sum(rate(netobserv_namespace_ingress_bytes_total{SrcK8S_Namespace!~"|netobserv|openshift.*"}[2m]) or rate(netobserv_namespace_ingress_bytes_total{SrcK8S_Namespace=~"netobserv|openshift.*",DstK8S_Namespace!~"|netobserv|openshift.*"}[2m])) by (SrcK8S_Namespace,DstK8S_Namespace))`, ) - assert.Contains(d.Rows[row].Panels[1].Targets[0].Expr, + assert.Contains(row.Panels[1].Targets[0].Expr, `label_replace(label_replace(topk(10,sum(rate(netobserv_namespace_ingress_bytes_total{SrcK8S_Namespace=~"netobserv|openshift.*"}[2m]) or rate(netobserv_namespace_ingress_bytes_total{SrcK8S_Namespace!~"netobserv|openshift.*",DstK8S_Namespace=~"netobserv|openshift.*"}[2m])) by (SrcK8S_Namespace,DstK8S_Namespace))`, ) - // 23th row - row = 22 - assert.Equal("Packet rate received per workload", d.Rows[row].Title) - assert.Len(d.Rows[row].Panels, 2) - assert.Equal("Applications", d.Rows[row].Panels[0].Title) - assert.Equal("Infrastructure", d.Rows[row].Panels[1].Title) - assert.Len(d.Rows[row].Panels[0].Targets, 1) - assert.Contains(d.Rows[row].Panels[0].Targets[0].Expr, + row = d.FindRow("Round-trip time per namespace") + assert.NotNil(row) + assert.Len(row.Panels, 2) + assert.Equal("Applications", row.Panels[0].Title) + assert.Equal("Infrastructure", row.Panels[1].Title) + assert.Len(row.Panels[0].Targets, 2) + assert.Contains(row.Panels[0].Targets[0].Expr, + `histogram_quantile(0.99, sum(rate(netobserv_namespace_rtt_seconds_bucket{SrcK8S_Namespace!~"|netobserv|openshift.*"}[2m]) or rate(netobserv_namespace_rtt_seconds_bucket{SrcK8S_Namespace=~"netobserv|openshift.*",DstK8S_Namespace!~"|netobserv|openshift.*"}[2m])) by (le,SrcK8S_Namespace,DstK8S_Namespace))`, + ) + assert.Contains(row.Panels[1].Targets[1].Expr, + `histogram_quantile(0.50, sum(rate(netobserv_namespace_rtt_seconds_bucket{SrcK8S_Namespace=~"netobserv|openshift.*"}[2m]) or rate(netobserv_namespace_rtt_seconds_bucket{SrcK8S_Namespace!~"netobserv|openshift.*",DstK8S_Namespace=~"netobserv|openshift.*"}[2m])) by (le,SrcK8S_Namespace,DstK8S_Namespace))`, + ) + + row = d.FindRow("Packet rate received per workload") + assert.NotNil(row) + assert.Len(row.Panels, 2) + assert.Equal("Applications", row.Panels[0].Title) + assert.Equal("Infrastructure", row.Panels[1].Title) + assert.Len(row.Panels[0].Targets, 1) + assert.Contains(row.Panels[0].Targets[0].Expr, `label_replace(label_replace(topk(10,sum(rate(netobserv_workload_ingress_packets_total{SrcK8S_Namespace!~"|netobserv|openshift.*"}[2m]) or rate(netobserv_workload_ingress_packets_total{SrcK8S_Namespace=~"netobserv|openshift.*",DstK8S_Namespace!~"|netobserv|openshift.*"}[2m])) by (SrcK8S_Namespace,SrcK8S_OwnerName,DstK8S_Namespace,DstK8S_OwnerName))`, ) - assert.Contains(d.Rows[row].Panels[1].Targets[0].Expr, + assert.Contains(row.Panels[1].Targets[0].Expr, `label_replace(label_replace(topk(10,sum(rate(netobserv_workload_ingress_packets_total{SrcK8S_Namespace=~"netobserv|openshift.*"}[2m]) or rate(netobserv_workload_ingress_packets_total{SrcK8S_Namespace!~"netobserv|openshift.*",DstK8S_Namespace=~"netobserv|openshift.*"}[2m])) by (SrcK8S_Namespace,SrcK8S_OwnerName,DstK8S_Namespace,DstK8S_OwnerName))`, ) } @@ -63,19 +79,18 @@ func TestCreateFlowMetricsDashboard_OnlyNodeIngressBytes(t *testing.T) { js, err := CreateFlowMetricsDashboard("netobserv", []string{"node_ingress_bytes_total"}) assert.NoError(err) - d, err := test.DashboardFromBytes([]byte(js)) + d, err := FromBytes([]byte(js)) assert.NoError(err) assert.Equal("NetObserv", d.Title) assert.Len(d.Rows, 1) - // First row - row := 0 - assert.Equal("Byte rate received per node", d.Rows[row].Title) - assert.Len(d.Rows[row].Panels, 1) - assert.Equal("", d.Rows[row].Panels[0].Title) - assert.Len(d.Rows[row].Panels[0].Targets, 1) - assert.Contains(d.Rows[row].Panels[0].Targets[0].Expr, "label_replace(label_replace(topk(10,sum(rate(netobserv_node_ingress_bytes_total[2m])) by (SrcK8S_HostName,DstK8S_HostName))") + row := d.FindRow("Byte rate received per node") + assert.NotNil(row) + assert.Len(row.Panels, 1) + assert.Equal("", row.Panels[0].Title) + assert.Len(row.Panels[0].Targets, 1) + assert.Contains(row.Panels[0].Targets[0].Expr, "label_replace(label_replace(topk(10,sum(rate(netobserv_node_ingress_bytes_total[2m])) by (SrcK8S_HostName,DstK8S_HostName))") } func TestCreateFlowMetricsDashboard_DefaultList(t *testing.T) { @@ -84,46 +99,43 @@ func TestCreateFlowMetricsDashboard_DefaultList(t *testing.T) { js, err := CreateFlowMetricsDashboard("netobserv", metrics.DefaultIncludeList) assert.NoError(err) - d, err := test.DashboardFromBytes([]byte(js)) + d, err := FromBytes([]byte(js)) assert.NoError(err) assert.Equal("NetObserv", d.Title) assert.Len(d.Rows, 7) - // First row - row := 0 - assert.Equal("Byte rate received per node", d.Rows[row].Title) - assert.Len(d.Rows[row].Panels, 1) - assert.Equal("", d.Rows[row].Panels[0].Title) - assert.Len(d.Rows[row].Panels[0].Targets, 1) - assert.Contains(d.Rows[row].Panels[0].Targets[0].Expr, "label_replace(label_replace(topk(10,sum(rate(netobserv_node_ingress_bytes_total[2m])) by (SrcK8S_HostName,DstK8S_HostName))") - - // 2nd row - row = 1 - assert.Equal("Byte rate received per namespace", d.Rows[row].Title) - assert.Len(d.Rows[row].Panels, 2) - assert.Equal("Applications", d.Rows[row].Panels[0].Title) - assert.Equal("Infrastructure", d.Rows[row].Panels[1].Title) - assert.Len(d.Rows[row].Panels[0].Targets, 1) + row := d.FindRow("Byte rate received per node") + assert.NotNil(row) + assert.Len(row.Panels, 1) + assert.Equal("", row.Panels[0].Title) + assert.Len(row.Panels[0].Targets, 1) + assert.Contains(row.Panels[0].Targets[0].Expr, "label_replace(label_replace(topk(10,sum(rate(netobserv_node_ingress_bytes_total[2m])) by (SrcK8S_HostName,DstK8S_HostName))") + + row = d.FindRow("Byte rate received per namespace") + assert.NotNil(row) + assert.Len(row.Panels, 2) + assert.Equal("Applications", row.Panels[0].Title) + assert.Equal("Infrastructure", row.Panels[1].Title) + assert.Len(row.Panels[0].Targets, 1) // Make sure netobserv_namespace_ingress_bytes_total was replaced with netobserv_workload_ingress_bytes_total - assert.Contains(d.Rows[row].Panels[0].Targets[0].Expr, + assert.Contains(row.Panels[0].Targets[0].Expr, `label_replace(label_replace(topk(10,sum(rate(netobserv_workload_ingress_bytes_total{SrcK8S_Namespace!~"|netobserv|openshift.*"}[2m]) or rate(netobserv_workload_ingress_bytes_total{SrcK8S_Namespace=~"netobserv|openshift.*",DstK8S_Namespace!~"|netobserv|openshift.*"}[2m])) by (SrcK8S_Namespace,DstK8S_Namespace))`, ) - assert.Contains(d.Rows[row].Panels[1].Targets[0].Expr, + assert.Contains(row.Panels[1].Targets[0].Expr, `label_replace(label_replace(topk(10,sum(rate(netobserv_workload_ingress_bytes_total{SrcK8S_Namespace=~"netobserv|openshift.*"}[2m]) or rate(netobserv_workload_ingress_bytes_total{SrcK8S_Namespace!~"netobserv|openshift.*",DstK8S_Namespace=~"netobserv|openshift.*"}[2m])) by (SrcK8S_Namespace,DstK8S_Namespace))`, ) - // 7th row - row = 6 - assert.Equal("Byte rate received per workload", d.Rows[row].Title) - assert.Len(d.Rows[row].Panels, 2) - assert.Equal("Applications", d.Rows[row].Panels[0].Title) - assert.Equal("Infrastructure", d.Rows[row].Panels[1].Title) - assert.Len(d.Rows[row].Panels[0].Targets, 1) - assert.Contains(d.Rows[row].Panels[0].Targets[0].Expr, + row = d.FindRow("Byte rate received per workload") + assert.NotNil(row) + assert.Len(row.Panels, 2) + assert.Equal("Applications", row.Panels[0].Title) + assert.Equal("Infrastructure", row.Panels[1].Title) + assert.Len(row.Panels[0].Targets, 1) + assert.Contains(row.Panels[0].Targets[0].Expr, `label_replace(label_replace(topk(10,sum(rate(netobserv_workload_ingress_bytes_total{SrcK8S_Namespace!~"|netobserv|openshift.*"}[2m]) or rate(netobserv_workload_ingress_bytes_total{SrcK8S_Namespace=~"netobserv|openshift.*",DstK8S_Namespace!~"|netobserv|openshift.*"}[2m])) by (SrcK8S_Namespace,SrcK8S_OwnerName,DstK8S_Namespace,DstK8S_OwnerName))`, ) - assert.Contains(d.Rows[row].Panels[1].Targets[0].Expr, + assert.Contains(row.Panels[1].Targets[0].Expr, `label_replace(label_replace(topk(10,sum(rate(netobserv_workload_ingress_bytes_total{SrcK8S_Namespace=~"netobserv|openshift.*"}[2m]) or rate(netobserv_workload_ingress_bytes_total{SrcK8S_Namespace!~"netobserv|openshift.*",DstK8S_Namespace=~"netobserv|openshift.*"}[2m])) by (SrcK8S_Namespace,SrcK8S_OwnerName,DstK8S_Namespace,DstK8S_OwnerName))`, ) } @@ -134,7 +146,7 @@ func TestCreateHealthDashboard_Default(t *testing.T) { js, err := CreateHealthDashboard("netobserv", metrics.DefaultIncludeList) assert.NoError(err) - d, err := test.DashboardFromBytes([]byte(js)) + d, err := FromBytes([]byte(js)) assert.NoError(err) assert.Equal("NetObserv / Health", d.Title) @@ -169,7 +181,7 @@ func TestCreateHealthDashboard_NoFlowMetric(t *testing.T) { js, err := CreateHealthDashboard("netobserv", []string{}) assert.NoError(err) - d, err := test.DashboardFromBytes([]byte(js)) + d, err := FromBytes([]byte(js)) assert.NoError(err) assert.Equal("NetObserv / Health", d.Title) diff --git a/pkg/dashboards/model.go b/pkg/dashboards/model.go index fb9016113..ef53c99e5 100644 --- a/pkg/dashboards/model.go +++ b/pkg/dashboards/model.go @@ -1,12 +1,14 @@ package dashboards import ( + "encoding/json" "fmt" "strings" ) type Dashboard struct { - Rows []*Row + Rows []*Row + Title string } type Row struct { @@ -31,6 +33,29 @@ var formatCleaner = strings.NewReplacer( "\n", "", ) +func FromBytes(b []byte) (*Dashboard, error) { + var d Dashboard + err := json.Unmarshal(b, &d) + return &d, err +} + +func (d *Dashboard) Titles() []string { + var titles []string + for _, r := range d.Rows { + titles = append(titles, r.Title) + } + return titles +} + +func (d *Dashboard) FindRow(titleSubstr string) *Row { + for _, r := range d.Rows { + if strings.Contains(r.Title, titleSubstr) { + return r + } + } + return nil +} + func (d *Dashboard) ToGrafanaJSON(netobsNs string) string { // return empty if dashboard doesn't contains rows if len(d.Rows) == 0 { diff --git a/pkg/test/dashboards.go b/pkg/test/dashboards.go deleted file mode 100644 index 30445320e..000000000 --- a/pkg/test/dashboards.go +++ /dev/null @@ -1,33 +0,0 @@ -package test - -import ( - "encoding/json" -) - -type Dashboard struct { - Rows []struct { - Panels []struct { - Targets []struct { - Expr string `json:"expr"` - LegendFormat string `json:"legendFormat"` - } `json:"targets"` - Title string `json:"title"` - } `json:"panels"` - Title string `json:"title"` - } `json:"rows"` - Title string `json:"title"` -} - -func DashboardFromBytes(b []byte) (*Dashboard, error) { - var d Dashboard - err := json.Unmarshal(b, &d) - return &d, err -} - -func (d *Dashboard) Titles() []string { - var titles []string - for _, r := range d.Rows { - titles = append(titles, r.Title) - } - return titles -}