No functional changes

9 files changed, 48 insertions, 46 deletions

diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/VespaMetrics.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/VespaMetrics.java
index 3629e81582a..7a3f260cd34 100644
--- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/VespaMetrics.java
+++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/core/VespaMetrics.java
@@ -171,7 +171,7 @@ public class VespaMetrics {
         builder.putDimension(METRIC_TYPE_DIMENSION_ID, "system")
                 .putDimension(INSTANCE_DIMENSION_ID, service.getInstanceName())
                 .putDimensions(service.getDimensions())
-                .putMetrics(systemMetrics.getMetrics());
+                .putMetrics(systemMetrics.list());
 
         builder.addConsumers(metricsConsumers.getAllConsumers());
         return Optional.of(builder);
diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/Metrics.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/Metrics.java
index 571ffc030de..8450d9f6be7 100644
--- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/Metrics.java
+++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/Metrics.java
@@ -1,16 +1,12 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package ai.vespa.metricsproxy.metric;
 
-import ai.vespa.metricsproxy.metric.model.MetricId;
-
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 
 /**
  * Once a getter is called, the instance is frozen and no more metrics can be added.
- *
- * @author Unknown
  */
 // TODO: remove timestamp, only used as temporary storage.
 // TODO: instances of this class can probably be replaced by a simple freezable map.
@@ -52,37 +48,17 @@ public class Metrics {
         this.metrics.add(m);
     }
 
-    /**
-     * Get the size of the metrics covered. Note that this might also contain expired metrics
-     *
-     * @return size of metrics
-     */
+    /** Returns the size of the metrics covered. Note that this might also contain expired metrics. */
     public int size() {
         return this.metrics.size();
     }
 
-    /**
-     * TODO: Remove, might be multiple metrics with same name but different dimensions
-     *
-     * @param key metric name
-     * @return the metric, or null
-     */
-    public Metric getMetric(MetricId key) {
-        isFrozen = true;
-        for (Metric m: metrics) {
-            if (m.getName().equals(key)) {
-                return m;
-            }
-        }
-        return null;
-    }
-
-    public List<Metric> getMetrics() {
+    public List<Metric> list() {
         isFrozen = true;
         return Collections.unmodifiableList(metrics);
     }
 
-
+    @Override
     public String toString() {
         StringBuilder sb = new StringBuilder();
         for (Metric m : metrics) {
diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/node/NodeMetricGatherer.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/node/NodeMetricGatherer.java
index 5aa95e9efc0..9686bb1bf6b 100644
--- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/node/NodeMetricGatherer.java
+++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/node/NodeMetricGatherer.java
@@ -30,8 +30,6 @@ import static ai.vespa.metricsproxy.node.ServiceHealthGatherer.gatherServiceHeal
  *
  * @author olaa
  */
-
-
 public class NodeMetricGatherer {
 
     private final VespaServices vespaServices;
diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/MetricsTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/MetricsTest.java
index 809eb441c14..1cd0281b7cd 100644
--- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/MetricsTest.java
+++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/MetricsTest.java
@@ -30,7 +30,7 @@ public class MetricsTest {
 
         Map<String, Number> map = new HashMap<>();
 
-        for (Metric metric: m.getMetrics()) {
+        for (Metric metric: m.list()) {
             String k = metric.getName().id;
 
             assertThat(map.containsKey(k), is(false));
@@ -45,8 +45,8 @@ public class MetricsTest {
     public void testBasicMetric() {
         Metrics m = new Metrics();
         m.add(new Metric(toMetricId("count"), 1, System.currentTimeMillis() / 1000));
-        assertThat(m.getMetrics().size(), is(1));
-        assertThat(m.getMetrics().get(0).getName(), is(toMetricId("count")));
+        assertThat(m.list().size(), is(1));
+        assertThat(m.list().get(0).getName(), is(toMetricId("count")));
     }
 
     @Test
diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/rpc/RpcMetricsTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/rpc/RpcMetricsTest.java
index d06ca43f8f8..214df36d6a1 100644
--- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/rpc/RpcMetricsTest.java
+++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/rpc/RpcMetricsTest.java
@@ -108,9 +108,9 @@ public class RpcMetricsTest {
 
             Metrics metrics = qrserver.getMetrics();
             assertThat("Fetched number of metrics is not correct", metrics.size(), is(2));
-            Metric m = metrics.getMetric(toMetricId("foo.count"));
+            Metric m = getMetric("foo.count", metrics);
             assertNotNull("Did not find expected metric with name 'foo.count'", m);
-            Metric m2 = metrics.getMetric(toMetricId("bar.count"));
+            Metric m2 = getMetric("bar.count", metrics);
             assertNotNull("Did not find expected metric with name 'bar.count'", m2);
 
             try (RpcClient rpcClient = new RpcClient(tester.rpcPort())) {
@@ -134,6 +134,14 @@ public class RpcMetricsTest {
         }
     }
 
+    public Metric getMetric(String metric, Metrics metrics) {
+        for (Metric m: metrics.list()) {
+            if (m.getName().equals(toMetricId(metric)))
+                return m;
+        }
+        return null;
+    }
+
     private static void verifyMetricsFromRpcRequest(VespaService service, RpcClient client) throws IOException {
         String jsonResponse = getMetricsForYamas(service.getMonitoringName().id, client).trim();
         ArrayNode metrics = (ArrayNode) jsonMapper.readTree(jsonResponse).get("metrics");
@@ -161,7 +169,7 @@ public class RpcMetricsTest {
     private void verfiyMetricsFromServiceObject(VespaService service) {
         Metrics storageMetrics = service.getMetrics();
         assertThat(storageMetrics.size(), is(2));
-        Metric foo = storageMetrics.getMetric(toMetricId("foo.count"));
+        Metric foo = getMetric("foo.count", storageMetrics);
         assertNotNull("Did not find expected metric with name 'foo.count'", foo);
         assertThat("Expected 2 dimensions for metric foo", foo.getDimensions().size(), is(2));
         assertThat("Metric foo did not contain correct dimension mapping for key = foo.count", foo.getDimensions().containsKey(toDimensionId("foo")), is(true));
@@ -187,10 +195,10 @@ public class RpcMetricsTest {
             assertThat(services.size(), is(1));
             Metrics metrics = services.get(0).getMetrics();
             assertThat("Fetched number of metrics is not correct", metrics.size(), is(2));
-            Metric m = metrics.getMetric(toMetricId("foo.count"));
+            Metric m = getMetric("foo.count", metrics);
             assertNotNull("Did not find expected metric with name 'foo.count'", m);
 
-            Metric m2 = metrics.getMetric(toMetricId("bar.count"));
+            Metric m2 = getMetric("bar.count", metrics);
             assertNotNull("Did not find expected metric with name 'bar'", m2);
 
             try (RpcClient rpcClient = new RpcClient(tester.rpcPort())) {
diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/ContainerServiceTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/ContainerServiceTest.java
index 7b047cee0a7..44f0e9608ac 100644
--- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/ContainerServiceTest.java
+++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/ContainerServiceTest.java
@@ -40,7 +40,7 @@ public class ContainerServiceTest {
     public void testMultipleQueryDimensions() {
         int count = 0;
         VespaService service = VespaService.create("service1", "id", httpServer.port());
-        for (Metric m : service.getMetrics().getMetrics()) {
+        for (Metric m : service.getMetrics().list()) {
             if (m.getName().equals(toMetricId("queries.rate"))) {
                 count++;
                 System.out.println("Name: " + m.getName() + " value: " + m.getValue());
diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/MetricsFetcherTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/MetricsFetcherTest.java
index b571ddcf84b..3af317ab36b 100644
--- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/MetricsFetcherTest.java
+++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/MetricsFetcherTest.java
@@ -7,6 +7,7 @@ import ai.vespa.metricsproxy.metric.Metrics;
 import ai.vespa.metricsproxy.metric.model.MetricId;
 import org.junit.Test;
 
+import static ai.vespa.metricsproxy.metric.model.MetricId.toMetricId;
 import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertThat;
 
@@ -35,8 +36,8 @@ public class MetricsFetcherTest {
         String jsonData = TestUtil.getFileContents("metrics-state.json");
         Metrics metrics = fetch(jsonData);
         assertThat(metrics.size(), is(10));
-        assertThat(metrics.getMetric(MetricId.toMetricId("query_hits.count")).getValue().intValue(), is(28));
-        assertThat(metrics.getMetric(MetricId.toMetricId("queries.rate")).getValue().doubleValue(), is(0.4667));
+        assertThat(getMetric("query_hits.count", metrics).getValue().intValue(), is(28));
+        assertThat(getMetric("queries.rate", metrics).getValue().doubleValue(), is(0.4667));
         assertThat(metrics.getTimeStamp(), is(1334134700L));
     }
 
@@ -94,4 +95,13 @@ public class MetricsFetcherTest {
         metrics = fetch(jsonData);
         assertThat("Wrong number of metrics", metrics.size(), is(0));
     }
+
+    public Metric getMetric(String metric, Metrics metrics) {
+        for (Metric m: metrics.list()) {
+            if (m.getName().equals(toMetricId(metric)))
+                return m;
+        }
+        return null;
+    }
+
 }
diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/SystemPollerTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/SystemPollerTest.java
index 2db8d6b543b..f5ae0048275 100644
--- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/SystemPollerTest.java
+++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/SystemPollerTest.java
@@ -142,7 +142,7 @@ public class SystemPollerTest {
         SystemPoller.JiffiesAndCpus diff = next.diff(prev);
 
         Metrics m = s1.getSystemMetrics();
-        List<Metric> metricList = m.getMetrics();
+        List<Metric> metricList = m.list();
         assertEquals(4, metricList.size());
         assertEquals(new Metric(MetricId.toMetricId("memory_virt"), 0L, 1), metricList.get(0));
         assertEquals(new Metric(MetricId.toMetricId("memory_rss"), 0L, 1), metricList.get(1));
diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/VespaServiceTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/VespaServiceTest.java
index 54004e7872a..a54e050bac7 100644
--- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/VespaServiceTest.java
+++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/VespaServiceTest.java
@@ -1,6 +1,7 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package ai.vespa.metricsproxy.service;
 
+import ai.vespa.metricsproxy.metric.Metric;
 import ai.vespa.metricsproxy.metric.Metrics;
 import ai.vespa.metricsproxy.metric.model.MetricId;
 import org.junit.After;
@@ -8,9 +9,12 @@ import org.junit.Before;
 import org.junit.Test;
 
 import static ai.vespa.metricsproxy.TestUtil.getFileContents;
+import static ai.vespa.metricsproxy.metric.model.MetricId.toMetricId;
 import static ai.vespa.metricsproxy.service.RemoteMetricsFetcher.METRICS_PATH;
 import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
 
 /**
  * @author Unknown
@@ -53,15 +57,13 @@ public class VespaServiceTest {
     // TODO: Make it possible to test this without running a HTTP server to create the response
     public void testMetricsFetching() {
         VespaService service = VespaService.create("service1", "id", httpServer.port());
-        Metrics metrics = service.getMetrics();
-        assertThat(metrics.getMetric(MetricId.toMetricId("queries.count")).getValue().intValue(), is(28));
+        assertEquals(28, getMetric("queries.count", service.getMetrics()).getValue().intValue());
 
         // Shutdown server and check that no metrics are returned (should use empty metrics
         // when unable to fetch new metrics)
         shutdown();
 
-        metrics = service.getMetrics();
-        assertThat(metrics.size(), is(0));
+        assertTrue(service.getMetrics().list().isEmpty());
     }
 
     @After
@@ -69,4 +71,12 @@ public class VespaServiceTest {
         this.httpServer.close();
     }
 
+    public Metric getMetric(String metric, Metrics metrics) {
+        for (Metric m: metrics.list()) {
+            if (m.getName().equals(toMetricId(metric)))
+                return m;
+        }
+        return null;
+    }
+
 }