feat: support comma separated aggrs in perf metrics (#2376)

* feat: support comma separated aggrs in perf metrics * feat: handle sorting in config side * feat: handled dependent labels in plugin * feat: handled dependent labels in gjson itself * feat: handle review comments
NetApp · Oct 4, 2023 · c8a6abe · c8a6abe
1 parent 231a02c
commit c8a6abe
Show file tree

Hide file tree

Showing 7 changed files with 85 additions and 68 deletions.
diff --git a/cmd/collectors/rest/plugins/netroute/netroute.go b/cmd/collectors/rest/plugins/netroute/netroute.go
@@ -8,6 +8,7 @@ import (
 	"github.com/netapp/harvest/v2/cmd/poller/plugin"
 	"github.com/netapp/harvest/v2/pkg/matrix"
 	"github.com/netapp/harvest/v2/pkg/tree/node"
+	"github.com/tidwall/gjson"
 	"strconv"
 	"strings"
 )
@@ -69,29 +70,29 @@ func (n *NetRoute) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matrix, err
 	for key, instance := range data.GetInstances() {
 		cluster := data.GetGlobalLabels()["cluster"]
 		routeID := instance.GetLabel("uuid")
-		interfaceName := instance.GetLabel("interface_name")
-		interfaceAddress := instance.GetLabel("interface_address")
-		if interfaceName != "" && interfaceAddress != "" {
-			names := strings.Split(interfaceName, ",")
-			address := strings.Split(interfaceAddress, ",")
-			if len(names) == len(address) {
-				for i, name := range names {
-					index := strings.Join([]string{cluster, strconv.Itoa(count)}, "_")
-					interfaceInstance, err := n.data.NewInstance(index)
-					if err != nil {
-						n.Logger.Error().Err(err).Str("add instance failed for instance key", key).Send()
-						return nil, err
-					}
-
-					for _, l := range instanceLabels {
-						interfaceInstance.SetLabel(l, instance.GetLabel(l))
-					}
-					interfaceInstance.SetLabel("index", index)
-					interfaceInstance.SetLabel("address", address[i])
-					interfaceInstance.SetLabel("name", name)
-					interfaceInstance.SetLabel("route_uuid", routeID)
-					count++
+		interfaces := instance.GetLabel("interfaces")
+
+		interfacesList := gjson.Result{Type: gjson.JSON, Raw: interfaces}
+		names := interfacesList.Get("name").Array()
+		address := interfacesList.Get("address").Array()
+
+		if len(names) == len(address) {
+			for i, name := range names {
+				index := strings.Join([]string{cluster, strconv.Itoa(count)}, "_")
+				interfaceInstance, err := n.data.NewInstance(index)
+				if err != nil {
+					n.Logger.Error().Err(err).Str("add instance failed for instance key", key).Send()
+					return nil, err
 				}
+
+				for _, l := range instanceLabels {
+					interfaceInstance.SetLabel(l, instance.GetLabel(l))
+				}
+				interfaceInstance.SetLabel("index", index)
+				interfaceInstance.SetLabel("address", address[i].String())
+				interfaceInstance.SetLabel("name", name.String())
+				interfaceInstance.SetLabel("route_uuid", routeID)
+				count++
 			}
 		}
 	}

diff --git a/cmd/collectors/rest/rest.go b/cmd/collectors/rest/rest.go
@@ -502,6 +502,7 @@ func (r *Rest) HandleResults(result []gjson.Result, prop *prop, isEndPoint bool)
 						labelString := r.String()
 						labelArray = append(labelArray, labelString)
 					}
+					sort.Strings(labelArray)
 					instance.SetLabel(display, strings.Join(labelArray, ","))
 				} else {
 					instance.SetLabel(display, value.String())

diff --git a/cmd/collectors/restperf/plugins/volume/volume.go b/cmd/collectors/restperf/plugins/volume/volume.go
@@ -45,6 +45,7 @@ func (v *Volume) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matrix, error
 
 	re := regexp.MustCompile(`^(.*)__(\d{4})$`)
 
+	fgAggrMap := make(map[string]*set.Set)
 	flexgroupAggrsMap := make(map[string]*set.Set)
 	// volume_aggr_labels metric is deprecated now and will be removed later.
 	metricName := "labels"
@@ -70,10 +71,10 @@ func (v *Volume) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matrix, error
 				fg, _ := cache.NewInstance(key)
 				fg.SetLabels(maps.Clone(i.GetLabels()))
 				fg.SetLabel("volume", match[1])
-				// Flexgroup don't show any aggregate, node
-				fg.SetLabel("aggr", "")
+				// Flexgroup don't show any node
 				fg.SetLabel("node", "")
 				fg.SetLabel(style, "flexgroup")
+				fgAggrMap[key] = set.New()
 			}
 
 			if volumeAggrmetric.GetInstance(key) == nil {
@@ -88,6 +89,7 @@ func (v *Volume) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matrix, error
 					v.Logger.Error().Err(err).Str("metric", metricName).Msg("Unable to set value on metric")
 				}
 			}
+			fgAggrMap[key].Add(i.GetLabel("aggr"))
 			flexgroupAggrsMap[key].Add(i.GetLabel("aggr"))
 			i.SetLabel(style, "flexgroup_constituent")
 			i.SetExportable(false)
@@ -132,6 +134,11 @@ func (v *Volume) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matrix, error
 				continue
 			}
 
+			// set aggrs label for fg, make sure the order of aggregate is same for each poll
+			aggrs := fgAggrMap[key].Values()
+			sort.Strings(aggrs)
+			fg.SetLabel("aggr", strings.Join(aggrs, ","))
+
 			for mkey, m := range data.GetMetrics() {
 
 				if !m.IsExportable() && m.GetType() != "float64" {

diff --git a/cmd/collectors/zapi/collector/zapi.go b/cmd/collectors/zapi/collector/zapi.go
@@ -284,7 +284,9 @@ func (z *Zapi) PollData() (map[string]*matrix.Matrix, error) {
 				// Handling array with comma separated values
 				previousValue := instance.GetLabel(label)
 				if isAppend && previousValue != "" {
-					instance.SetLabel(label, previousValue+","+value)
+					currentVal := strings.Split(previousValue+","+value, ",")
+					sort.Strings(currentVal)
+					instance.SetLabel(label, strings.Join(currentVal, ","))
 					z.Logger.Trace().Msgf(" > %slabel (%s) [%s] set value (%s)%s", color.Yellow, key, label, instance.GetLabel(label)+","+value, color.End)
 				} else {
 					instance.SetLabel(label, value)

diff --git a/cmd/collectors/zapiperf/plugins/volume/volume.go b/cmd/collectors/zapiperf/plugins/volume/volume.go
@@ -53,6 +53,7 @@ func (v *Volume) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matrix, error
 	opsKeyPrefix := "temp_"
 	re := regexp.MustCompile(`^(.*)__(\d{4})$`)
 
+	fgAggrMap := make(map[string]*set.Set)
 	flexgroupAggrsMap := make(map[string]*set.Set)
 	// volume_aggr_labels metric is deprecated now and will be removed later.
 	metricName := "labels"
@@ -81,10 +82,10 @@ func (v *Volume) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matrix, error
 				fg, _ := cache.NewInstance(key)
 				fg.SetLabels(maps.Clone(i.GetLabels()))
 				fg.SetLabel("volume", match[1])
-				// Flexgroup don't show any aggregate, node
-				fg.SetLabel("aggr", "")
+				// Flexgroup don't show any node
 				fg.SetLabel("node", "")
 				fg.SetLabel(style, "flexgroup")
+				fgAggrMap[key] = set.New()
 			}
 
 			if volumeAggrmetric.GetInstance(key) == nil {
@@ -99,6 +100,7 @@ func (v *Volume) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matrix, error
 					v.Logger.Error().Err(err).Str("metric", metricName).Msg("Unable to set value on metric")
 				}
 			}
+			fgAggrMap[key].Add(i.GetLabel("aggr"))
 			flexgroupAggrsMap[key].Add(i.GetLabel("aggr"))
 			i.SetLabel(style, "flexgroup_constituent")
 			i.SetExportable(false)
@@ -144,6 +146,11 @@ func (v *Volume) Run(dataMap map[string]*matrix.Matrix) ([]*matrix.Matrix, error
 				continue
 			}
 
+			// set aggrs label for fg, make sure the order of aggregate is same for each poll
+			aggrs := fgAggrMap[key].Values()
+			sort.Strings(aggrs)
+			fg.SetLabel("aggr", strings.Join(aggrs, ","))
+
 			for mkey, m := range data.GetMetrics() {
 
 				if !m.IsExportable() && m.GetType() != "float64" {

diff --git a/conf/rest/9.12.0/netroute.yaml b/conf/rest/9.12.0/netroute.yaml
@@ -3,21 +3,20 @@ query:                    api/network/ip/routes
 object:                   net_route
 
 counters:
-  - ^^uuid                                   => uuid
-  - ^destination.address                     => destination
-  - ^destination.family                      => family
-  - ^destination.netmask                     => netmask_length
-  - ^gateway                                 => gateway
-  - ^interfaces.#.ip.address                 => interface_address       # Added in Ontap 9.9
-  - ^interfaces.#.name                       => interface_name          # Added in Ontap 9.9
-  - ^ipspace.name                            => ipspace
-  - ^scope                                   => scope
-  - ^svm.name                                => svm
+  - ^^uuid                                          => uuid
+  - ^destination.address                            => destination
+  - ^destination.family                             => family
+  - ^destination.netmask                            => netmask_length
+  - ^gateway                                        => gateway
+  - ^ipspace.name                                   => ipspace
+  - ^scope                                          => scope
+  - ^svm.name                                       => svm
+  - ^{interfaces.#.name,interfaces.#.ip.address}    => interfaces              # Added in Ontap 9.9
   - hidden_fields:
       - interfaces
 
 plugins:
-  - NetRoute   #Creates net_route_interface_labels from interface_name and interface_address metrics collected above
+  - NetRoute   #Creates net_route_interface_labels from interfaces metrics collected above
 
 export_options:
   instance_keys: