fix: using instanceName instead of volName in fabricpool perf (#3175)

* fix: using instanceName instead of volName in fabricpool perf * fix: redcue iteration with constituents cache map
NetApp · Sep 26, 2024 · d364bf9 · d364bf9
1 parent e93adb9
commit d364bf9
Show file tree

Hide file tree

Showing 3 changed files with 81 additions and 54 deletions.
diff --git a/cmd/collectors/fabricpool.go b/cmd/collectors/fabricpool.go
@@ -4,17 +4,27 @@ import (
 	"github.com/netapp/harvest/v2/pkg/matrix"
 	"log/slog"
 	"maps"
+	"regexp"
 	"strings"
 )
 
+var fabricpoolRegex = regexp.MustCompile(`^(.*)__(\d{4})_bin.*$`)
+
+type constituentData struct {
+	instance     *matrix.Instance
+	flexgroupKey string
+}
+
 func GetFlexGroupFabricPoolMetrics(dataMap map[string]*matrix.Matrix, object string, opName string, includeConstituents bool, l *slog.Logger) (*matrix.Matrix, error) {
 	var (
-		err                 error
-		latencyCacheMetrics []string
+		err                      error
+		latencyCacheMetrics      []string
+		flexgroupConstituentsMap map[string]constituentData
 	)
 
 	data := dataMap[object]
 	opsKeyPrefix := "temp_"
+	flexgroupConstituentsMap = make(map[string]constituentData)
 
 	cache := data.Clone(matrix.With{Data: false, Metrics: true, Instances: false, ExportInstances: true})
 	cache.UUID += ".FabricPool"
@@ -27,88 +37,89 @@ func GetFlexGroupFabricPoolMetrics(dataMap map[string]*matrix.Matrix, object str
 	}
 
 	// create flexgroup instance cache
-	for _, i := range data.GetInstances() {
+	for iKey, i := range data.GetInstances() {
 		if !i.IsExportable() {
 			continue
 		}
-		if match := flexgroupRegex.FindStringSubmatch(i.GetLabel("volume")); len(match) == 3 {
-			key := i.GetLabel("svm") + "." + match[1] + i.GetLabel("cloud_target")
+
+		if match := fabricpoolRegex.FindStringSubmatch(fetchVolumeName(i)); len(match) == 3 {
+			key := i.GetLabel("svm") + "." + match[1] + "." + i.GetLabel("cloud_target")
 			if cache.GetInstance(key) == nil {
 				fg, _ := cache.NewInstance(key)
 				fg.SetLabels(maps.Clone(i.GetLabels()))
 				fg.SetLabel("volume", match[1])
 			}
 			i.SetExportable(includeConstituents)
+			flexgroupConstituentsMap[iKey] = constituentData{flexgroupKey: key, instance: i}
 		}
 	}
 
 	l.Debug("extracted  flexgroup volumes", slog.Int("size", len(cache.GetInstances())))
 
 	// create summary
-	for _, i := range data.GetInstances() {
-		if match := flexgroupRegex.FindStringSubmatch(i.GetLabel("volume")); len(match) == 3 {
-			// instance key is svm.flexgroup-volume.cloud-target-name
-			key := i.GetLabel("svm") + "." + match[1] + i.GetLabel("cloud_target")
-
-			fg := cache.GetInstance(key)
-			if fg == nil {
-				l.Error("instance not in local cache", slog.String("key", key))
+	for _, constituent := range flexgroupConstituentsMap {
+		// instance key is svm.flexgroup-volume.cloud-target-name
+		key := constituent.flexgroupKey
+		i := constituent.instance
+
+		fg := cache.GetInstance(key)
+		if fg == nil {
+			l.Error("instance not in local cache", slog.String("key", key))
+			continue
+		}
+
+		for mkey, m := range data.GetMetrics() {
+			if !m.IsExportable() && m.GetType() != "float64" {
 				continue
 			}
 
-			for mkey, m := range data.GetMetrics() {
-				if !m.IsExportable() && m.GetType() != "float64" {
-					continue
-				}
-
-				fgm := cache.GetMetric(mkey)
-				if fgm == nil {
-					l.Error("metric not in local cache", slog.String("key", mkey))
-					continue
-				}
+			fgm := cache.GetMetric(mkey)
+			if fgm == nil {
+				l.Error("metric not in local cache", slog.String("key", mkey))
+				continue
+			}
 
-				if value, ok := m.GetValueFloat64(i); ok {
-					fgv, _ := fgm.GetValueFloat64(fg)
+			if value, ok := m.GetValueFloat64(i); ok {
+				fgv, _ := fgm.GetValueFloat64(fg)
 
-					// non-latency metrics: simple sum
-					if !strings.HasPrefix(mkey, "cloud_bin_op_latency_average") {
-						err := fgm.SetValueFloat64(fg, fgv+value)
-						if err != nil {
-							l.Error("error", slog.Any("err", err))
-						}
-						continue
+				// non-latency metrics: simple sum
+				if !strings.HasPrefix(mkey, "cloud_bin_op_latency_average") {
+					err := fgm.SetValueFloat64(fg, fgv+value)
+					if err != nil {
+						l.Error("error", slog.Any("err", err))
 					}
+					continue
+				}
 
-					// latency metric: weighted sum
-					opsKey := strings.Replace(mkey, "cloud_bin_op_latency_average", opName, 1)
+				// latency metric: weighted sum
+				opsKey := strings.Replace(mkey, "cloud_bin_op_latency_average", opName, 1)
 
-					if ops := data.GetMetric(opsKey); ops != nil {
-						if opsValue, ok := ops.GetValueFloat64(i); ok {
-							var tempOpsV float64
+				if ops := data.GetMetric(opsKey); ops != nil {
+					if opsValue, ok := ops.GetValueFloat64(i); ok {
+						var tempOpsV float64
 
-							prod := value * opsValue
-							tempOpsKey := opsKeyPrefix + opsKey
-							tempOps := cache.GetMetric(tempOpsKey)
+						prod := value * opsValue
+						tempOpsKey := opsKeyPrefix + opsKey
+						tempOps := cache.GetMetric(tempOpsKey)
 
-							if tempOps == nil {
-								if tempOps, err = cache.NewMetricFloat64(tempOpsKey); err != nil {
-									return nil, err
-								}
-								tempOps.SetExportable(false)
-							} else {
-								tempOpsV, _ = tempOps.GetValueFloat64(fg)
-							}
-							if value != 0 {
-								err = tempOps.SetValueFloat64(fg, tempOpsV+opsValue)
-								if err != nil {
-									l.Error("error", slog.Any("err", err))
-								}
+						if tempOps == nil {
+							if tempOps, err = cache.NewMetricFloat64(tempOpsKey); err != nil {
+								return nil, err
 							}
-							err = fgm.SetValueFloat64(fg, fgv+prod)
+							tempOps.SetExportable(false)
+						} else {
+							tempOpsV, _ = tempOps.GetValueFloat64(fg)
+						}
+						if value != 0 {
+							err = tempOps.SetValueFloat64(fg, tempOpsV+opsValue)
 							if err != nil {
 								l.Error("error", slog.Any("err", err))
 							}
 						}
+						err = fgm.SetValueFloat64(fg, fgv+prod)
+						if err != nil {
+							l.Error("error", slog.Any("err", err))
+						}
 					}
 				}
 			}
@@ -143,3 +154,14 @@ func GetFlexGroupFabricPoolMetrics(dataMap map[string]*matrix.Matrix, object str
 	}
 	return cache, nil
 }
+
+func fetchVolumeName(i *matrix.Instance) string {
+	// zapiperf name: test1_fg2__0001_bin_0_cfg_id_0
+	// restperf name: test1_fg2__0002_bin_1_cfg_id_1
+	instanceName := i.GetLabel("fabricpool")
+	compAggrName := i.GetLabel("comp_aggr_name")
+	if names := strings.Split(instanceName, compAggrName+"_"); len(names) == 2 {
+		return names[1]
+	}
+	return ""
+}
diff --git a/conf/restperf/9.12.0/wafl_comp_aggr_vol_bin.yaml b/conf/restperf/9.12.0/wafl_comp_aggr_vol_bin.yaml
@@ -4,13 +4,17 @@ object:                   fabricpool
 
 counters:
   - ^^id
+  - ^aggregate.name              => comp_aggr_name
   - ^cloud_target.name           => cloud_target
   - ^svm.name                    => svm
   - ^volume.name                 => volume
   - cloud_bin_op                 => cloud_bin_operation
   - cloud_bin_op_latency_average
 
 plugins:
+  - LabelAgent:
+    split:
+      - id `:` ,fabricpool
   - FabricPool:
       - include_constituents: false
 

diff --git a/conf/zapiperf/cdot/9.8.0/wafl_comp_aggr_vol_bin.yaml b/conf/zapiperf/cdot/9.8.0/wafl_comp_aggr_vol_bin.yaml
@@ -7,6 +7,7 @@ instance_key:             uuid
 counters:
   - cloud_bin_op_latency_average
   - cloud_bin_operation
+  - comp_aggr_name
   - instance_name
   - object_store_name       => cloud_target
   - vol_name                => volume