From 128bac3dafe5541c3566d72fd9f0de561b449941 Mon Sep 17 00:00:00 2001 From: mphanias Date: Wed, 30 Oct 2024 13:26:27 +0530 Subject: [PATCH] OM-209 - review feedback --- internal/pkg/statprocessors/sp_latency.go | 25 ++++++------- internal/pkg/statprocessors/sp_namespaces.go | 35 ++++++++++--------- internal/pkg/statprocessors/sp_node_stats.go | 20 ++++++----- internal/pkg/statprocessors/statsprocessor.go | 6 ++-- 4 files changed, 42 insertions(+), 44 deletions(-) diff --git a/internal/pkg/statprocessors/sp_latency.go b/internal/pkg/statprocessors/sp_latency.go index e993657..b36a875 100644 --- a/internal/pkg/statprocessors/sp_latency.go +++ b/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-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) } diff --git a/internal/pkg/statprocessors/sp_namespaces.go b/internal/pkg/statprocessors/sp_namespaces.go index 0a30039..08ed238 100644 --- a/internal/pkg/statprocessors/sp_namespaces.go +++ b/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 } diff --git a/internal/pkg/statprocessors/sp_node_stats.go b/internal/pkg/statprocessors/sp_node_stats.go index 54a9841..1f6683e 100644 --- a/internal/pkg/statprocessors/sp_node_stats.go +++ b/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) } } } diff --git a/internal/pkg/statprocessors/statsprocessor.go b/internal/pkg/statprocessors/statsprocessor.go index e8fdf75..e6a8e3b 100644 --- a/internal/pkg/statprocessors/statsprocessor.go +++ b/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