Move MetricReceiverWrapper tester methods to test

4 files changed, 100 insertions, 89 deletions

diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/DimensionMetrics.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/DimensionMetrics.java
index 770ff5e2216..432474d7270 100644
--- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/DimensionMetrics.java
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/DimensionMetrics.java
@@ -4,32 +4,29 @@ package com.yahoo.vespa.hosted.dockerapi.metrics;
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 
-import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
 /**
  * @author freva
  */
 public class DimensionMetrics {
-    private final static ObjectMapper objectMapper = new ObjectMapper();
+    private static final ObjectMapper objectMapper = new ObjectMapper();
+    private static final Map<String, Object> routing = Map.of("yamas", Map.of("namespaces", List.of("Vespa")));
 
     private final String application;
     private final Dimensions dimensions;
     private final Map<String, Number> metrics;
 
     DimensionMetrics(String application, Dimensions dimensions, Map<String, Number> metrics) {
-        this.application = application;
-        this.dimensions = dimensions;
-        this.metrics = metrics;
+        this.application = Objects.requireNonNull(application);
+        this.dimensions = Objects.requireNonNull(dimensions);
+        this.metrics = Objects.requireNonNull(metrics);
     }
 
     Map<String, Object> getMetrics() {
-        final Map<String, Object> routing = new HashMap<>();
-        final Map<String, Object> routingMonitoring = new HashMap<>();
-        routing.put("yamas", routingMonitoring);
-        routingMonitoring.put("namespaces", Collections.singletonList("Vespa"));
-
         Map<String, Object> report = new HashMap<>();
         report.put("application", application);
         report.put("dimensions", dimensions.dimensionsMap);
@@ -45,6 +42,21 @@ public class DimensionMetrics {
         return objectMapper.writeValueAsString(report);
     }
 
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        DimensionMetrics that = (DimensionMetrics) o;
+        return application.equals(that.application) &&
+                dimensions.equals(that.dimensions) &&
+                metrics.equals(that.metrics);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(application, dimensions, metrics);
+    }
+
     public static class Builder {
         private final String application;
         private final Dimensions dimensions;
diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/Dimensions.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/Dimensions.java
index 586622100fb..e5e3306a31a 100644
--- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/Dimensions.java
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/Dimensions.java
@@ -1,7 +1,6 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.dockerapi.metrics;
 
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -11,10 +10,13 @@ import java.util.Map;
  * @author valerijf
  */
 public class Dimensions {
-    final Map<String, Object> dimensionsMap;
 
-    private Dimensions(Map<String, Object> dimensionsMap) {
-        this.dimensionsMap = dimensionsMap;
+    public static final Dimensions NONE = new Dimensions(Map.of());
+
+    final Map<String, String> dimensionsMap;
+
+    private Dimensions(Map<String, String> dimensionsMap) {
+        this.dimensionsMap = Map.copyOf(dimensionsMap);
     }
 
     @Override
@@ -45,7 +47,7 @@ public class Dimensions {
         }
 
         public Dimensions build() {
-            return new Dimensions(Collections.unmodifiableMap(new HashMap<>(dimensionsMap)));
+            return new Dimensions(dimensionsMap);
         }
     }
 }
diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/MetricReceiverWrapper.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/MetricReceiverWrapper.java
index 58126a59cbb..d263d138314 100644
--- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/MetricReceiverWrapper.java
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/MetricReceiverWrapper.java
@@ -6,14 +6,11 @@ import com.yahoo.metrics.simple.MetricReceiver;
 import com.yahoo.metrics.simple.Point;
 
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
-import java.util.Set;
 import java.util.stream.Collectors;
 
 /**
@@ -24,7 +21,6 @@ import java.util.stream.Collectors;
  */
 public class MetricReceiverWrapper {
     // Application names used
-    public static final String APPLICATION_DOCKER = "docker";
     public static final String APPLICATION_HOST = "vespa.host";
     public static final String APPLICATION_NODE = "vespa.node";
 
@@ -38,13 +34,23 @@ public class MetricReceiverWrapper {
     }
 
     /**
-     *  Declaring the same dimensions and name results in the same CounterWrapper instance (idempotent).
+     * Creates a counter metric under vespa.host application, with no dimensions and default dimension type
+     * See {@link #declareCounter(String, String, Dimensions, DimensionType)}
      */
-    public CounterWrapper declareCounter(String application, Dimensions dimensions, String name) {
-        return declareCounter(application, dimensions, name, DimensionType.DEFAULT);
+    public CounterWrapper declareCounter(String name) {
+        return declareCounter(name, Dimensions.NONE);
     }
 
-    public CounterWrapper declareCounter(String application, Dimensions dimensions, String name, DimensionType type) {
+    /**
+     * Creates a counter metric under vespa.host application, with the given dimensions and default dimension type
+     * See {@link #declareCounter(String, String, Dimensions, DimensionType)}
+     */
+    public CounterWrapper declareCounter(String name, Dimensions dimensions) {
+        return declareCounter(APPLICATION_HOST, name, dimensions, DimensionType.DEFAULT);
+    }
+
+    /** Creates a counter metric. This method is idempotent. */
+    public CounterWrapper declareCounter(String application, String name, Dimensions dimensions, DimensionType type) {
         synchronized (monitor) {
             Map<Dimensions, Map<String, MetricValue>> metricsByDimensions = getOrCreateApplicationMetrics(application, type);
             if (!metricsByDimensions.containsKey(dimensions)) metricsByDimensions.put(dimensions, new HashMap<>());
@@ -58,13 +64,23 @@ public class MetricReceiverWrapper {
     }
 
     /**
-     *  Declaring the same dimensions and name results in the same GaugeWrapper instance (idempotent).
+     * Creates a gauge metric under vespa.host application, with no dimensions and default dimension type
+     * See {@link #declareGauge(String, String, Dimensions, DimensionType)}
+     */
+    public GaugeWrapper declareGauge(String name) {
+        return declareGauge(name, Dimensions.NONE);
+    }
+
+    /**
+     * Creates a gauge metric under vespa.host application, with the given dimensions and default dimension type
+     * See {@link #declareGauge(String, String, Dimensions, DimensionType)}
      */
-    public GaugeWrapper declareGauge(String application, Dimensions dimensions, String name) {
-        return declareGauge(application, dimensions, name, DimensionType.DEFAULT);
+    public GaugeWrapper declareGauge(String name, Dimensions dimensions) {
+        return declareGauge(APPLICATION_HOST, name, dimensions, DimensionType.DEFAULT);
     }
 
-    public GaugeWrapper declareGauge(String application, Dimensions dimensions, String name, DimensionType type) {
+    /** Creates a gauge metric. This method is idempotent */
+    public GaugeWrapper declareGauge(String application, String name, Dimensions dimensions, DimensionType type) {
         synchronized (monitor) {
             Map<Dimensions, Map<String, MetricValue>> metricsByDimensions = getOrCreateApplicationMetrics(application, type);
             if (!metricsByDimensions.containsKey(dimensions))
@@ -82,24 +98,10 @@ public class MetricReceiverWrapper {
         return getMetricsByType(DimensionType.DEFAULT);
     }
 
-    // For testing, returns same as getDefaultMetrics(), but without "timestamp"
-    public Set<Map<String, Object>> getDefaultMetricsRaw() {
-        synchronized (monitor) {
-            Set<Map<String, Object>> dimensionMetrics = new HashSet<>();
-            metrics.getOrDefault(DimensionType.DEFAULT, new HashMap<>())
-                    .forEach((application, applicationMetrics) -> applicationMetrics.metricsByDimensions().entrySet().stream()
-                    .map(entry -> new DimensionMetrics(application, entry.getKey(),
-                            entry.getValue().entrySet().stream().collect(Collectors.toMap(Map.Entry::getKey, value -> value.getValue().getValue()))))
-                    .map(DimensionMetrics::getMetrics)
-                    .forEach(dimensionMetrics::add));
-            return dimensionMetrics;
-        }
-    }
-
     public List<DimensionMetrics> getMetricsByType(DimensionType type) {
         synchronized (monitor) {
             List<DimensionMetrics> dimensionMetrics = new ArrayList<>();
-            metrics.getOrDefault(type, new HashMap<>())
+            metrics.getOrDefault(type, Map.of())
                     .forEach((application, applicationMetrics) -> applicationMetrics.metricsByDimensions().entrySet().stream()
                     .map(entry -> new DimensionMetrics(application, entry.getKey(),
                             entry.getValue().entrySet().stream().collect(Collectors.toMap(Map.Entry::getKey, value -> value.getValue().getValue()))))
@@ -117,24 +119,10 @@ public class MetricReceiverWrapper {
         }
     }
 
-    // For testing
-    Map<String, Number> getMetricsForDimension(String application, Dimensions dimensions) {
-        synchronized (monitor) {
-            Map<Dimensions, Map<String, MetricValue>> metricsByDimensions = getOrCreateApplicationMetrics(application, DimensionType.DEFAULT);
-            return metricsByDimensions.getOrDefault(dimensions, Collections.emptyMap())
-                    .entrySet()
-                    .stream()
-                    .collect(Collectors.toMap(Map.Entry::getKey, entry -> entry.getValue().getValue()));
-        }
-    }
-
-    private Map<Dimensions, Map<String, MetricValue>> getOrCreateApplicationMetrics(String application, DimensionType type) {
-        Map<String, ApplicationMetrics> applicationMetrics = metrics.computeIfAbsent(type, m -> new HashMap<>());
-        if (! applicationMetrics.containsKey(application)) {
-            ApplicationMetrics metrics = new ApplicationMetrics();
-            applicationMetrics.put(application, metrics);
-        }
-        return applicationMetrics.get(application).metricsByDimensions();
+    Map<Dimensions, Map<String, MetricValue>> getOrCreateApplicationMetrics(String application, DimensionType type) {
+        return metrics.computeIfAbsent(type, m -> new HashMap<>())
+                .computeIfAbsent(application, app -> new ApplicationMetrics())
+                .metricsByDimensions();
     }
 
     // "Application" is the monitoring application, not Vespa application
@@ -147,5 +135,10 @@ public class MetricReceiverWrapper {
     }
 
     // Used to distinguish whether metrics have been populated with all tag vaules
-    public enum DimensionType {DEFAULT, PRETAGGED}
+    public enum DimensionType {
+        /** Default metrics get added default dimensions set in check config */
+        DEFAULT,
+
+        /** Pretagged metrics will only get the dimensions explicitly set when creating the counter/gauge */
+        PRETAGGED}
 }
diff --git a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/metrics/MetricReceiverWrapperTest.java b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/metrics/MetricReceiverWrapperTest.java
index c20e64d906e..f9cbff98cde 100644
--- a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/metrics/MetricReceiverWrapperTest.java
+++ b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/metrics/MetricReceiverWrapperTest.java
@@ -5,7 +5,10 @@ import com.yahoo.metrics.simple.MetricReceiver;
 import org.junit.Test;
 
 import java.util.Map;
+import java.util.stream.Collectors;
 
+import static com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper.APPLICATION_HOST;
+import static com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper.DimensionType.DEFAULT;
 import static org.junit.Assert.assertEquals;
 
 /**
@@ -13,84 +16,85 @@ import static org.junit.Assert.assertEquals;
  */
 public class MetricReceiverWrapperTest {
     private static final Dimensions hostDimension = new Dimensions.Builder().add("host", "abc.yahoo.com").build();
-    private static final String applicationDocker = MetricReceiverWrapper.APPLICATION_DOCKER;
+    private final MetricReceiverWrapper metricReceiver = new MetricReceiverWrapper(MetricReceiver.nullImplementation);
 
     @Test
     public void testDefaultValue() {
-        MetricReceiverWrapper metricReceiver = new MetricReceiverWrapper(MetricReceiver.nullImplementation);
+        metricReceiver.declareCounter("some.name", hostDimension);
 
-        metricReceiver.declareCounter(applicationDocker, hostDimension, "some.name");
-
-        assertEquals(metricReceiver.getMetricsForDimension(applicationDocker, hostDimension).get("some.name"), 0L);
+        assertEquals(getMetricsForDimension(hostDimension).get("some.name"), 0L);
     }
 
     @Test
     public void testSimpleIncrementMetric() {
-        MetricReceiverWrapper metricReceiver = new MetricReceiverWrapper(MetricReceiver.nullImplementation);
-        CounterWrapper counter = metricReceiver.declareCounter(applicationDocker, hostDimension, "a_counter.value");
+        CounterWrapper counter = metricReceiver.declareCounter("a_counter.value", hostDimension);
 
         counter.add(5);
         counter.add(8);
 
-        Map<String, Number> latestMetrics = metricReceiver.getMetricsForDimension(applicationDocker, hostDimension);
+        Map<String, Number> latestMetrics = getMetricsForDimension(hostDimension);
         assertEquals("Expected only 1 metric value to be set", 1, latestMetrics.size());
         assertEquals(latestMetrics.get("a_counter.value"), 13L); // 5 + 8
     }
 
     @Test
     public void testSimpleGauge() {
-        MetricReceiverWrapper metricReceiver = new MetricReceiverWrapper(MetricReceiver.nullImplementation);
-        GaugeWrapper gauge = metricReceiver.declareGauge(applicationDocker, hostDimension, "test.gauge");
+        GaugeWrapper gauge = metricReceiver.declareGauge("test.gauge", hostDimension);
 
         gauge.sample(42);
         gauge.sample(-342.23);
 
-        Map<String, Number> latestMetrics = metricReceiver.getMetricsForDimension(applicationDocker, hostDimension);
+        Map<String, Number> latestMetrics = getMetricsForDimension(hostDimension);
         assertEquals("Expected only 1 metric value to be set", 1, latestMetrics.size());
         assertEquals(latestMetrics.get("test.gauge"), -342.23);
     }
 
     @Test
     public void testRedeclaringSameGauge() {
-        MetricReceiverWrapper metricReceiver = new MetricReceiverWrapper(MetricReceiver.nullImplementation);
-        GaugeWrapper gauge = metricReceiver.declareGauge(applicationDocker, hostDimension, "test.gauge");
+        GaugeWrapper gauge = metricReceiver.declareGauge("test.gauge", hostDimension);
         gauge.sample(42);
 
         // Same as hostDimension, but new instance.
         Dimensions newDimension = new Dimensions.Builder().add("host", "abc.yahoo.com").build();
-        GaugeWrapper newGauge = metricReceiver.declareGauge(applicationDocker, newDimension, "test.gauge");
+        GaugeWrapper newGauge = metricReceiver.declareGauge("test.gauge", newDimension);
         newGauge.sample(56);
 
-        assertEquals(metricReceiver.getMetricsForDimension(applicationDocker, hostDimension).get("test.gauge"), 56.);
+        assertEquals(getMetricsForDimension(hostDimension).get("test.gauge"), 56.);
     }
 
     @Test
     public void testSameMetricNameButDifferentDimensions() {
-        MetricReceiverWrapper metricReceiver = new MetricReceiverWrapper(MetricReceiver.nullImplementation);
-        GaugeWrapper gauge = metricReceiver.declareGauge(applicationDocker, hostDimension, "test.gauge");
+        GaugeWrapper gauge = metricReceiver.declareGauge("test.gauge", hostDimension);
         gauge.sample(42);
 
         // Not the same as hostDimension.
         Dimensions newDimension = new Dimensions.Builder().add("host", "abcd.yahoo.com").build();
-        GaugeWrapper newGauge = metricReceiver.declareGauge(applicationDocker, newDimension, "test.gauge");
+        GaugeWrapper newGauge = metricReceiver.declareGauge("test.gauge", newDimension);
         newGauge.sample(56);
 
-        assertEquals(metricReceiver.getMetricsForDimension(applicationDocker, hostDimension).get("test.gauge"), 42.);
-        assertEquals(metricReceiver.getMetricsForDimension(applicationDocker, newDimension).get("test.gauge"), 56.);
+        assertEquals(getMetricsForDimension(hostDimension).get("test.gauge"), 42.);
+        assertEquals(getMetricsForDimension(newDimension).get("test.gauge"), 56.);
     }
 
     @Test
     public void testDeletingMetric() {
-        MetricReceiverWrapper metricReceiver = new MetricReceiverWrapper(MetricReceiver.nullImplementation);
-        metricReceiver.declareGauge(applicationDocker, hostDimension, "test.gauge");
+        metricReceiver.declareGauge("test.gauge", hostDimension);
 
         Dimensions differentDimension = new Dimensions.Builder().add("host", "abcd.yahoo.com").build();
-        metricReceiver.declareGauge(applicationDocker, differentDimension, "test.gauge");
+        metricReceiver.declareGauge("test.gauge", differentDimension);
+
+        assertEquals(2, metricReceiver.getMetricsByType(DEFAULT).size());
+        metricReceiver.deleteMetricByDimension(APPLICATION_HOST, differentDimension, DEFAULT);
+        assertEquals(1, metricReceiver.getMetricsByType(DEFAULT).size());
+        assertEquals(getMetricsForDimension(hostDimension).size(), 1);
+        assertEquals(getMetricsForDimension(differentDimension).size(), 0);
+    }
 
-        assertEquals(2, metricReceiver.getDefaultMetricsRaw().size());
-        metricReceiver.deleteMetricByDimension(applicationDocker, differentDimension, MetricReceiverWrapper.DimensionType.DEFAULT);
-        assertEquals(1, metricReceiver.getDefaultMetricsRaw().size());
-        assertEquals(metricReceiver.getMetricsForDimension(applicationDocker, hostDimension).size(), 1);
-        assertEquals(metricReceiver.getMetricsForDimension(applicationDocker, differentDimension).size(), 0);
+    private Map<String, Number> getMetricsForDimension(Dimensions dimensions) {
+        return metricReceiver.getOrCreateApplicationMetrics(APPLICATION_HOST, DEFAULT)
+                .getOrDefault(dimensions, Map.of())
+                .entrySet()
+                .stream()
+                .collect(Collectors.toMap(Map.Entry::getKey, entry -> entry.getValue().getValue()));
     }
 }