Merge pull request #5380 from vespa-engine/mpolden/follow-dimension-naming-convention

Follow dimension naming convention

3 files changed, 24 insertions, 21 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporter.java
index 967db0dff99..2f1345eb49e 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporter.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporter.java
@@ -117,8 +117,10 @@ public class MetricsReporter extends Maintainer {
     private void reportDeploymentMetrics() {
         metric.set(deploymentFailMetric, deploymentFailRatio() * 100, metric.createContext(Collections.emptyMap()));
         for (Map.Entry<ApplicationId, Duration> entry : averageDeploymentDurations().entrySet()) {
-            metric.set(deploymentAverageDuration, entry.getValue().getSeconds(),
-                       metric.createContext(Collections.singletonMap("application", entry.getKey().toString())));
+            Map<String, String> dimensions = new HashMap<>();
+            dimensions.put("tenant", entry.getKey().tenant().value());
+            dimensions.put("app", entry.getKey().application().value() + "." + entry.getKey().instance().value());
+            metric.set(deploymentAverageDuration, entry.getValue().getSeconds(), metric.createContext(dimensions));
         }
     }
     
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/MetricsMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/MetricsMock.java
index 7f23a40435f..8b903f5c921 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/MetricsMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/MetricsMock.java
@@ -9,7 +9,7 @@ import java.util.HashMap;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
-import java.util.function.BiPredicate;
+import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
 /**
@@ -55,23 +55,20 @@ public class MetricsMock implements Metric {
     }
 
     /** Returns metric and context for any metric matching the given dimension predicate */
-    public Map<MapContext, Map<String, Number>> getMetrics(BiPredicate<String, String> dimension) {
+    public Map<MapContext, Map<String, Number>> getMetrics(Predicate<Map<String, String>> dimensionMatcher) {
         return metrics.entrySet()
                       .stream()
-                      .filter(context -> ((MapContext) context.getKey())
-                              .getDimensions().entrySet()
-                              .stream()
-                              .anyMatch(d -> dimension.test(d.getKey(), d.getValue())))
+                      .filter(context -> dimensionMatcher.test(((MapContext) context.getKey()).getDimensions()))
                       .collect(Collectors.toMap(entry -> (MapContext) entry.getKey(), Map.Entry::getValue));
     }
 
     /** Returns metric filtered by dimension and name */
-    public Optional<Number> getMetric(BiPredicate<String, String> dimension, String name) {
-        Map<String, Number> metrics = getMetrics(dimension).entrySet()
-                                                           .stream()
-                                                           .map(Map.Entry::getValue)
-                                                           .findFirst()
-                                                           .orElseGet(Collections::emptyMap);
+    public Optional<Number> getMetric(Predicate<Map<String, String>> dimensionMatcher, String name) {
+        Map<String, Number> metrics = getMetrics(dimensionMatcher).entrySet()
+                                                                  .stream()
+                                                                  .map(Map.Entry::getValue)
+                                                                  .findFirst()
+                                                                  .orElseGet(Collections::emptyMap);
         return Optional.ofNullable(metrics.get(name));
     }
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java
index 93fdf3618a7..bc9eabc19c1 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java
@@ -157,17 +157,13 @@ public class MetricsReporterTest {
     }
 
     private Duration getAverageDeploymentDuration(Application application) {
-        return metrics.getMetric((dimension, value) -> dimension.equals("application") &&
-                                                       value.equals(application.id().toString()),
+        return metrics.getMetric((dimensions) -> application.id().tenant().value().equals(dimensions.get("tenant")) &&
+                                                 appDimension(application).equals(dimensions.get("app")),
                                  MetricsReporter.deploymentAverageDuration)
                       .map(seconds -> Duration.ofSeconds(seconds.longValue()))
                       .orElseThrow(() -> new RuntimeException("Expected metric to exist for " + application.id()));
     }
 
-    private void assertDimension(MapContext metricContext, String dimensionName, String expectedValue) {
-        assertEquals(expectedValue, metricContext.getDimensions().get(dimensionName));
-    }
-
     private MetricsReporter createReporter(Controller controller, MetricsMock metricsMock, SystemName system) {
         return createReporter(controller.clock(), controller, metricsMock, system);
     }
@@ -189,7 +185,7 @@ public class MetricsReporterTest {
     }
 
     private Map<MapContext, Map<String, Number>> getMetricsByHost(String hostname) {
-        return metrics.getMetrics((dimension, value) -> dimension.equals("host") && value.equals(hostname));
+        return metrics.getMetrics((dimensions) -> hostname.equals(dimensions.get("host")));
     }
     
     private MapContext getMetricContextByHost(Controller controller, String hostname) {
@@ -204,5 +200,13 @@ public class MetricsReporterTest {
         return metricEntry.getKey();
     }
 
+    private static void assertDimension(MapContext metricContext, String dimensionName, String expectedValue) {
+        assertEquals(expectedValue, metricContext.getDimensions().get(dimensionName));
+    }
+
+    private static String appDimension(Application application) {
+        return application.id().application().value() + "." + application.id().instance().value();
+    }
+
 }