Merge pull request #17688 from vespa-engine/freva/filter-cc

Do not process metrics we dont care about

3 files changed, 36 insertions, 19 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/ClusterDeploymentMetricsRetriever.java b/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/ClusterDeploymentMetricsRetriever.java
index 0c8b33b9002..e1135063f97 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/ClusterDeploymentMetricsRetriever.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/ClusterDeploymentMetricsRetriever.java
@@ -22,11 +22,13 @@ import java.time.Duration;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
+import java.util.OptionalDouble;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
+import java.util.function.Supplier;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
@@ -110,40 +112,40 @@ public class ClusterDeploymentMetricsRetriever {
 
     private static void parseService(Inspector service, Map<ClusterInfo, DeploymentMetricsAggregator> clusterMetricsMap) {
         String serviceName = service.field("name").asString();
+        if (!WANTED_METRIC_SERVICES.contains(serviceName)) return;
         service.field("metrics").traverse((ArrayTraverser) (i, metric) ->
-                addMetricsToAggeregator(serviceName, metric, clusterMetricsMap)
+                addMetricsToAggregator(serviceName, metric, clusterMetricsMap)
         );
     }
 
-    private static void addMetricsToAggeregator(String serviceName, Inspector metric, Map<ClusterInfo, DeploymentMetricsAggregator> clusterMetricsMap) {
-        if (!WANTED_METRIC_SERVICES.contains(serviceName)) return;
+    private static void addMetricsToAggregator(String serviceName, Inspector metric, Map<ClusterInfo, DeploymentMetricsAggregator> clusterMetricsMap) {
         Inspector values = metric.field("values");
         ClusterInfo clusterInfo = getClusterInfoFromDimensions(metric.field("dimensions"));
-        DeploymentMetricsAggregator deploymentMetricsAggregator = clusterMetricsMap.computeIfAbsent(clusterInfo, c -> new DeploymentMetricsAggregator());
+        Supplier<DeploymentMetricsAggregator> aggregator = () -> clusterMetricsMap.computeIfAbsent(clusterInfo, c -> new DeploymentMetricsAggregator());
 
         switch (serviceName) {
             case VESPA_CONTAINER:
-                deploymentMetricsAggregator.addContainerLatency(
-                        values.field("query_latency.sum").asDouble(),
-                        values.field("query_latency.count").asDouble());
-                deploymentMetricsAggregator.addFeedLatency(
-                        values.field("feed.latency.sum").asDouble(),
-                        values.field("feed.latency.count").asDouble());
+                optionalDouble(values.field("query_latency.sum")).ifPresent(qlSum ->
+                        aggregator.get()
+                                .addContainerLatency(qlSum, values.field("query_latency.count").asDouble())
+                                .addFeedLatency(values.field("feed.latency.sum").asDouble(), values.field("feed.latency.count").asDouble()));
                 break;
             case VESPA_QRSERVER:
-                deploymentMetricsAggregator.addQrLatency(
-                        values.field("query_latency.sum").asDouble(),
-                        values.field("query_latency.count").asDouble());
+                optionalDouble(values.field("query_latency.sum")).ifPresent(qlSum ->
+                        aggregator.get()
+                                .addQrLatency(qlSum, values.field("query_latency.count").asDouble()));
                 break;
             case VESPA_DISTRIBUTOR:
-                deploymentMetricsAggregator.addDocumentCount(values.field("vds.distributor.docsstored.average").asDouble());
+                optionalDouble(values.field("vds.distributor.docsstored.average"))
+                        .ifPresent(docCount -> aggregator.get().addDocumentCount(docCount));
                 break;
             case VESPA_CONTAINER_CLUSTERCONTROLLER:
-                deploymentMetricsAggregator
-                        .addMemoryUsage(values.field("cluster-controller.resource_usage.max_memory_utilization.last").asDouble(),
-                                values.field("cluster-controller.resource_usage.memory_limit.last").asDouble())
-                        .addDiskUsage(values.field("cluster-controller.resource_usage.max_disk_utilization.last").asDouble(),
-                                values.field("cluster-controller.resource_usage.disk_limit.last").asDouble());
+                optionalDouble(values.field("cluster-controller.resource_usage.max_memory_utilization.last")).ifPresent(memoryUtil ->
+                        aggregator.get()
+                                .addMemoryUsage(memoryUtil,
+                                        values.field("cluster-controller.resource_usage.memory_limit.last").asDouble())
+                                .addDiskUsage(values.field("cluster-controller.resource_usage.max_disk_utilization.last").asDouble(),
+                                        values.field("cluster-controller.resource_usage.disk_limit.last").asDouble()));
                 break;
         }
     }
@@ -162,4 +164,8 @@ public class ClusterDeploymentMetricsRetriever {
             return new Slime();
         }
     }
+
+    private static OptionalDouble optionalDouble(Inspector field) {
+        return field.valid() ? OptionalDouble.of(field.asDouble()) : OptionalDouble.empty();
+    }
 }
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/ClusterDeploymentMetricsRetrieverTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/ClusterDeploymentMetricsRetrieverTest.java
index a5a46b67d13..7fdfbcdbf03 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/ClusterDeploymentMetricsRetrieverTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/ClusterDeploymentMetricsRetrieverTest.java
@@ -13,6 +13,7 @@ import java.nio.file.Path;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.Set;
 import java.util.function.BiConsumer;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -63,6 +64,7 @@ public class ClusterDeploymentMetricsRetrieverTest {
         ClusterInfo expectedContainerCluster = new ClusterInfo("container_cluster_id", "container");
 
         Map<ClusterInfo, DeploymentMetricsAggregator> aggregatorMap = new ClusterDeploymentMetricsRetriever().requestMetricsGroupedByCluster(hosts);
+        assertEquals(Set.of(expectedContainerCluster, expectedContentCluster), aggregatorMap.keySet());
 
         compareAggregators(
                 new DeploymentMetricsAggregator()
diff --git a/configserver/src/test/resources/metrics/clustercontroller_metrics.json b/configserver/src/test/resources/metrics/clustercontroller_metrics.json
index f487f95802c..9afcb34d77d 100644
--- a/configserver/src/test/resources/metrics/clustercontroller_metrics.json
+++ b/configserver/src/test/resources/metrics/clustercontroller_metrics.json
@@ -17,6 +17,15 @@
             "clustertype": "content",
             "clusterid": "content_cluster_id"
           }
+        },
+        {
+          "values": {
+            "some.other.metrics": 1
+          },
+          "dimensions": {
+            "clustertype": "admin",
+            "clusterid": "cluster-controllers"
+          }
         }
       ]
     }