OM-209 - review feedback

internal/pkg/statprocessors/sp_latency.go

@@ -49,24 +49,19 @@ func (lw *LatencyStatsProcessor) getLatenciesCommands(rawMetrics map[string]stri
 
 	// below latency-command are added to the auto-enabled list, i.e. latencies: command
 	// re-repl is auto-enabled, but not coming as part of latencies: list, hence we are adding it explicitly
-	//
-	// Hashmap content format := namespace_enable_<latency>=latency-command
-	// some latencies are configured and fetch for each namespace.
-	// command will be like latencies:hist={NAMESPACE}-proxy / latencies:hist={NAMESPACE}-benchmarks-read
-	//
-	for _, latencyCommand := range NamespaceLatencyBenchmarks {
-		// ns-enable-benchmarks-ops-sub or ns-enable-hist-proxy
-		histCommand := "latencies:hist=" + latencyCommand
-		log.Tracef("namespace histCommand - asinfo <-h IP> -v %s", histCommand)
-		commands = append(commands, histCommand)
+	// command will be like
+	//      latencies:hist={NAMESPACE}-proxy / latencies:hist={NAMESPACE}-benchmarks-read
+	//      latencies:hist=info
+
+	for nsName := range NamespaceLatencyBenchmarks {
+		for _, latencyCommand := range NamespaceLatencyBenchmarks[nsName] {
+			histCommand := "latencies:hist=" + latencyCommand
+			commands = append(commands, histCommand)
+		}
 	}
 
-	// some latencies are configured at service level which will come for each node
-	// command will be like latencies:hist=info
-	for _, latencyCommand := range NodeLatencyBenchmarks {
-		// enable-benchmarks-fabric or enable-hist-info
+	for _, latencyCommand := range ServiceLatencyBenchmarks {
 		histCommand := "latencies:hist=" + latencyCommand
-		log.Tracef("node histCommand - asinfo <-h IP>  -v %s", histCommand)
 		commands = append(commands, histCommand)
 	}
 

internal/pkg/statprocessors/sp_namespaces.go

@@ -50,14 +50,17 @@ func (nw *NamespaceStatsProcessor) PassOneKeys() []string {
 
 func (nw *NamespaceStatsProcessor) PassTwoKeys(rawMetrics map[string]string) []string {
 	s := rawMetrics[KEY_NS_METADATA]
-	list := strings.Split(s, ";")
+	nsList := strings.Split(s, ";")
 
 	log.Tracef("namespaces:%s", s)
 
 	var infoKeys []string
-	for _, k := range list {
+	for _, ns := range nsList {
 		// infoKey ==> namespace/test, namespace/bar
-		infoKeys = append(infoKeys, KEY_NS_NAMESPACE+"/"+k)
+		infoKeys = append(infoKeys, KEY_NS_NAMESPACE+"/"+ns)
+		if NamespaceLatencyBenchmarks[ns] == nil {
+			NamespaceLatencyBenchmarks[ns] = make(map[string]string)
+		}
 	}
 
 	if nw.canSendIndexPressureInfoKey() {
@@ -228,28 +231,28 @@ func (nw *NamespaceStatsProcessor) refreshNamespaceStats(singleInfoKey string, i
 			nsMetricsToSend = append(nsMetricsToSend, asMetric)
 		}
 
-		// below code section is to ensure ns+latencies combination is handled during LatencyWatcher
-		//
-		// check and if latency benchmarks stat - is it enabled (bool true==1 and false==0 after conversion)
+		// below code section is to ensure latencies combination is handled during LatencyWatcher
 		if isStatLatencyHistRelated(stat) {
-			// remove old value as microbenchmark may get enabled / disable on-the-fly at server so we cannot rely on value
-			delete(NamespaceLatencyBenchmarks, nsName+"_"+stat)
-
+			// pv==1 means histogram is enabled
 			if pv == 1 {
-				latencyOption := "{" + nsName + "}-" + stat
-				if strings.Contains(latencyOption, "enable-") {
-					latencyOption = strings.ReplaceAll(latencyOption, "enable-", "")
+				latencySubcommand := "{" + nsName + "}-" + stat
+				if strings.Contains(latencySubcommand, "enable-") {
+					latencySubcommand = strings.ReplaceAll(latencySubcommand, "enable-", "")
 				}
-				if strings.Contains(latencyOption, "hist-") {
-					latencyOption = strings.ReplaceAll(latencyOption, "hist-", "")
+				// some histogram command has 'hist-' prefix but the latency command does not expect hist- when issue the command
+				if strings.Contains(latencySubcommand, "hist-") {
+					latencySubcommand = strings.ReplaceAll(latencySubcommand, "hist-", "")
 				}
-				NamespaceLatencyBenchmarks[nsName+"_"+stat] = latencyOption
+				NamespaceLatencyBenchmarks[nsName][stat] = latencySubcommand
+			} else {
+				// pv==0 means histogram is disabled
+				delete(NamespaceLatencyBenchmarks[nsName], stat)
 			}
 		}
 
 	}
 	// append default re-repl, as this auto-enabled, but not coming as part of latencies, we need this as namespace is available only here
-	NamespaceLatencyBenchmarks[nsName+"_re-repl"] = "{" + nsName + "}-" + "re-repl"
+	NamespaceLatencyBenchmarks[nsName]["re-repl"] = "{" + nsName + "}-" + "re-repl"
 
 	return nsMetricsToSend
 }

internal/pkg/statprocessors/sp_node_stats.go

@@ -89,19 +89,21 @@ func (sw *NodeStatsProcessor) handleRefresh(nodeRawMetrics string) []AerospikeSt
 		// check and if latency benchmarks stat, is it enabled (bool true==1 and false==0 after conversion)
 		if isStatLatencyHistRelated(stat) {
 
-			// remove old value as microbenchmark may get enabled / disable on-the-fly at server so we cannot rely on value
-			delete(NodeLatencyBenchmarks, stat)
-
+			// pv==1 means histogram is enabled
 			if pv == 1 {
-				latencyOption := stat
-				if strings.Contains(latencyOption, "enable-") {
-					latencyOption = strings.ReplaceAll(latencyOption, "enable-", "")
+				latencySubcommand := stat
+				if strings.Contains(latencySubcommand, "enable-") {
+					latencySubcommand = strings.ReplaceAll(latencySubcommand, "enable-", "")
 				}
-				if strings.Contains(latencyOption, "hist-") {
-					latencyOption = strings.ReplaceAll(latencyOption, "hist-", "")
+				// some histogram command has 'hist-' prefix but the latency command does not expect hist- when issue the command
+				if strings.Contains(latencySubcommand, "hist-") {
+					latencySubcommand = strings.ReplaceAll(latencySubcommand, "hist-", "")
 				}
 
-				NodeLatencyBenchmarks[stat] = latencyOption
+				ServiceLatencyBenchmarks[stat] = latencySubcommand
+			} else {
+				// pv==0 means histogram is disabled
+				delete(ServiceLatencyBenchmarks, stat)
 			}
 		}
 	}

internal/pkg/statprocessors/statsprocessor.go

@@ -5,10 +5,8 @@ var (
 	Service, ClusterName, Build string
 )
 
-var NodeLatencyBenchmarks = make(map[string]string)
-var NamespaceLatencyBenchmarks = make(map[string]string)
-
-// var NamespaceLatencyBenchmarks = make(map[string]map[string]string)
+var ServiceLatencyBenchmarks = make(map[string]string)
+var NamespaceLatencyBenchmarks = make(map[string]map[string]string)
 
 type StatProcessor interface {
 	PassOneKeys() []string