From ff17aa6f6ec3317060323344ae7f0f4c1b59db9f Mon Sep 17 00:00:00 2001 From: hardikl Date: Mon, 3 Mar 2025 15:11:31 +0530 Subject: [PATCH 1/2] fix: handled empty scanner and export false case for vscan --- cmd/collectors/commonutils.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/cmd/collectors/commonutils.go b/cmd/collectors/commonutils.go index 59600aee9..c33dc8d50 100644 --- a/cmd/collectors/commonutils.go +++ b/cmd/collectors/commonutils.go @@ -346,11 +346,14 @@ func AggregatePerScanner(logger *slog.Logger, data *matrix.Matrix, latencyKey st continue } scanner := i.GetLabel("scanner") + // scanner is key for cache matrix, skip when the scanner would be empty + if scanner == "" { + continue + } if cache.GetInstance(scanner) == nil { s, _ := cache.NewInstance(scanner) s.SetLabel("scanner", scanner) } - i.SetExportable(false) } // aggregate per scanner @@ -360,7 +363,12 @@ func AggregatePerScanner(logger *slog.Logger, data *matrix.Matrix, latencyKey st if !i.IsExportable() { continue } + i.SetExportable(false) scanner := i.GetLabel("scanner") + // scanner is key for cache matrix, skip when the scanner would be empty + if scanner == "" { + continue + } ps := cache.GetInstance(scanner) if ps == nil { logger.Error("Failed to find scanner instance in cache", slog.String("scanner", scanner)) From 145b16131070b2e956d18da44355079c48bd8b43 Mon Sep 17 00:00:00 2001 From: hardikl Date: Wed, 5 Mar 2025 15:05:52 +0530 Subject: [PATCH 2/2] fix: add test cases --- .../plugins/vscan/test_nonvalid_vscan.json | 104 +++++++++ .../plugins/vscan/test_valid_vscan.json | 198 ++++++++++++++++++ .../restperf/plugins/vscan/vscan_test.go | 109 ++++++++++ 3 files changed, 411 insertions(+) create mode 100644 cmd/collectors/restperf/plugins/vscan/test_nonvalid_vscan.json create mode 100644 cmd/collectors/restperf/plugins/vscan/test_valid_vscan.json create mode 100644 cmd/collectors/restperf/plugins/vscan/vscan_test.go diff --git a/cmd/collectors/restperf/plugins/vscan/test_nonvalid_vscan.json b/cmd/collectors/restperf/plugins/vscan/test_nonvalid_vscan.json new file mode 100644 index 000000000..a00379f07 --- /dev/null +++ b/cmd/collectors/restperf/plugins/vscan/test_nonvalid_vscan.json @@ -0,0 +1,104 @@ +{ + "records": [ + { + "counter_table": { + "name": "vscan" + }, + "id": "dcfast11:tstsvm01", + "properties": [ + { + "name": "node.name", + "value": "dcfast11" + }, + { + "name": "server.id", + "value": "tstsvm01:10.10.33.118" + } + ], + "counters": [ + { + "name": "scan.requests", + "value": 719 + }, + { + "name": "scan.latency", + "value": 15034279 + }, + { + "name": "scan.request_dispatched_rate", + "value": 719 + }, + { + "name": "scanner.stats_percent_cpu_used", + "value": 1 + }, + { + "name": "scanner.stats_percent_mem_used", + "value": 48 + }, + { + "name": "scanner.stats_percent_network_used", + "value": 0 + } + ], + "_links": { + "self": { + "href": "/api/cluster/counter/tables/vscan/rows/dcfast11%3Atstsvm01%3A10.10.33.118" + } + } + }, + { + "counter_table": { + "name": "vscan" + }, + "id": "dcfast11", + "properties": [ + { + "name": "node.name", + "value": "dcfast11" + }, + { + "name": "server.id", + "value": "tstsvm01:10.10.33.128" + } + ], + "counters": [ + { + "name": "scan.requests", + "value": 720 + }, + { + "name": "scan.latency", + "value": 15001939 + }, + { + "name": "scan.request_dispatched_rate", + "value": 720 + }, + { + "name": "scanner.stats_percent_cpu_used", + "value": 0 + }, + { + "name": "scanner.stats_percent_mem_used", + "value": 42 + }, + { + "name": "scanner.stats_percent_network_used", + "value": 1 + } + ], + "_links": { + "self": { + "href": "/api/cluster/counter/tables/vscan/rows/dcfast11%3Atstsvm01%3A10.10.33.128" + } + } + } + ], + "num_records": 2, + "_links": { + "self": { + "href": "/api/cluster/counter/tables/vscan/rows?return_records=true&fields=*" + } + } +} diff --git a/cmd/collectors/restperf/plugins/vscan/test_valid_vscan.json b/cmd/collectors/restperf/plugins/vscan/test_valid_vscan.json new file mode 100644 index 000000000..af80b3ff7 --- /dev/null +++ b/cmd/collectors/restperf/plugins/vscan/test_valid_vscan.json @@ -0,0 +1,198 @@ +{ + "records": [ + { + "counter_table": { + "name": "vscan" + }, + "id": "dcfast11:tstsvm01:10.10.33.118", + "properties": [ + { + "name": "node.name", + "value": "dcfast11" + }, + { + "name": "server.id", + "value": "tstsvm01:10.10.33.118" + } + ], + "counters": [ + { + "name": "scan.requests", + "value": 719 + }, + { + "name": "scan.latency", + "value": 15034279 + }, + { + "name": "scan.request_dispatched_rate", + "value": 719 + }, + { + "name": "scanner.stats_percent_cpu_used", + "value": 1 + }, + { + "name": "scanner.stats_percent_mem_used", + "value": 48 + }, + { + "name": "scanner.stats_percent_network_used", + "value": 0 + } + ], + "_links": { + "self": { + "href": "/api/cluster/counter/tables/vscan/rows/dcfast11%3Atstsvm01%3A10.10.33.118" + } + } + }, + { + "counter_table": { + "name": "vscan" + }, + "id": "dcfast11:tstsvm01:10.10.33.128", + "properties": [ + { + "name": "node.name", + "value": "dcfast11" + }, + { + "name": "server.id", + "value": "tstsvm01:10.10.33.128" + } + ], + "counters": [ + { + "name": "scan.requests", + "value": 720 + }, + { + "name": "scan.latency", + "value": 15001939 + }, + { + "name": "scan.request_dispatched_rate", + "value": 720 + }, + { + "name": "scanner.stats_percent_cpu_used", + "value": 0 + }, + { + "name": "scanner.stats_percent_mem_used", + "value": 42 + }, + { + "name": "scanner.stats_percent_network_used", + "value": 1 + } + ], + "_links": { + "self": { + "href": "/api/cluster/counter/tables/vscan/rows/dcfast11%3Atstsvm01%3A10.10.33.128" + } + } + }, + { + "counter_table": { + "name": "vscan" + }, + "id": "dcfast11:tstsvm02:10.10.33.118", + "properties": [ + { + "name": "node.name", + "value": "dcfast11" + }, + { + "name": "server.id", + "value": "tstsvm02:10.10.33.118" + } + ], + "counters": [ + { + "name": "scan.requests", + "value": 0 + }, + { + "name": "scan.latency", + "value": 0 + }, + { + "name": "scan.request_dispatched_rate", + "value": 0 + }, + { + "name": "scanner.stats_percent_cpu_used", + "value": 0 + }, + { + "name": "scanner.stats_percent_mem_used", + "value": 0 + }, + { + "name": "scanner.stats_percent_network_used", + "value": 0 + } + ], + "_links": { + "self": { + "href": "/api/cluster/counter/tables/vscan/rows/dcfast11%3Atstsvm02%3A10.10.33.118" + } + } + }, + { + "counter_table": { + "name": "vscan" + }, + "id": "dcfast11:tstsvm02:10.10.33.128", + "properties": [ + { + "name": "node.name", + "value": "dcfast11" + }, + { + "name": "server.id", + "value": "tstsvm02:10.10.33.128" + } + ], + "counters": [ + { + "name": "scan.requests", + "value": 0 + }, + { + "name": "scan.latency", + "value": 0 + }, + { + "name": "scan.request_dispatched_rate", + "value": 0 + }, + { + "name": "scanner.stats_percent_cpu_used", + "value": 0 + }, + { + "name": "scanner.stats_percent_mem_used", + "value": 0 + }, + { + "name": "scanner.stats_percent_network_used", + "value": 0 + } + ], + "_links": { + "self": { + "href": "/api/cluster/counter/tables/vscan/rows/dcfast11%3Atstsvm02%3A10.10.33.128" + } + } + } + ], + "num_records": 4, + "_links": { + "self": { + "href": "/api/cluster/counter/tables/vscan/rows?return_records=true&fields=*" + } + } +} diff --git a/cmd/collectors/restperf/plugins/vscan/vscan_test.go b/cmd/collectors/restperf/plugins/vscan/vscan_test.go new file mode 100644 index 000000000..1376ef12d --- /dev/null +++ b/cmd/collectors/restperf/plugins/vscan/vscan_test.go @@ -0,0 +1,109 @@ +package vscan + +import ( + "encoding/json" + "github.com/netapp/harvest/v2/cmd/poller/options" + "github.com/netapp/harvest/v2/cmd/poller/plugin" + "github.com/netapp/harvest/v2/pkg/matrix" + "github.com/netapp/harvest/v2/pkg/tree/node" + "log/slog" + "os" + "testing" +) + +type VscanRecords struct { + VscanRecords []VscanRecord `json:"records"` +} + +type VscanRecord struct { + ID string `json:"id"` +} + +func runTest(t *testing.T, createRestVscan func(params *node.Node) plugin.Plugin, testFile string, metricsPerScanner string, cacheExportedInstanceCount int, dataExportedInstanceCount int) { + params := node.NewS("Vscan") + params.NewChildS("metricsPerScanner", metricsPerScanner) + v := createRestVscan(params) + data := readTestFile(testFile) + + dataMap := map[string]*matrix.Matrix{ + data.Object: data, + } + // run the plugin + results, _, err := v.Run(dataMap) + if err != nil { + t.Fatal(err) + } + + if metricsPerScanner == "false" { + if results != nil { + t.Fatalf("result should be nil") + } else { + return + } + } + + // Verify the cacheData + cacheData := results[0] + if len(cacheData.GetInstances()) != cacheExportedInstanceCount { + t.Fatalf("expected %d cacheExportedInstance count, got %d", cacheExportedInstanceCount, len(cacheData.GetInstances())) + } + + exportedInstance := 0 + for _, instance := range data.GetInstances() { + if instance.IsExportable() { + exportedInstance++ + } + } + if exportedInstance != dataExportedInstanceCount { + t.Fatalf("expected %d dataExportedInstance count, got %d", dataExportedInstanceCount, len(data.GetInstances())) + } +} + +func TestRunForAllImplementations(t *testing.T) { + type test struct { + name string + testFilePath string + metricsPerScanner string + cacheExportedInstanceCount int + dataExportedInstanceCount int + } + + tests := []test{ + {name: "TestMetricScannerTrueValidData", metricsPerScanner: "true", testFilePath: "test_valid_vscan.json", cacheExportedInstanceCount: 2, dataExportedInstanceCount: 0}, + {name: "TestMetricScannerTrueNonValidData", metricsPerScanner: "true", testFilePath: "test_nonvalid_vscan.json", cacheExportedInstanceCount: 0, dataExportedInstanceCount: 0}, + {name: "TestMetricScannerFalseValidData", metricsPerScanner: "false", testFilePath: "test_valid_vscan.json", cacheExportedInstanceCount: 0, dataExportedInstanceCount: 4}, + {name: "TestMetricScannerFalseNonValidData", metricsPerScanner: "false", testFilePath: "test_nonvalid_vscan.json", cacheExportedInstanceCount: 0, dataExportedInstanceCount: 2}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + runTest(t, createRestVscan, tt.testFilePath, tt.metricsPerScanner, tt.cacheExportedInstanceCount, tt.dataExportedInstanceCount) + }) + } +} + +func createRestVscan(params *node.Node) plugin.Plugin { + o := options.Options{IsTest: true} + v := &Vscan{AbstractPlugin: plugin.New("vscan", &o, params, nil, "vscan", nil)} + v.SLogger = slog.Default() + return v +} + +func readTestFile(testFilePath string) *matrix.Matrix { + data := matrix.New("vscan", "vscan", "vscan") + var vscanRecords VscanRecords + + file, _ := os.Open(testFilePath) + err := json.NewDecoder(file).Decode(&vscanRecords) + if err != nil { + return nil + } + + for i := range len(vscanRecords.VscanRecords) { + id := vscanRecords.VscanRecords[i].ID + instance, _ := data.NewInstance(id) + instance.SetLabel("id", id) + } + + return data +}