From d0dd094291b43a69fdb1ad4131dad91cca40e9b0 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 15 Dec 2021 13:00:38 +0100 Subject: No functional changes --- .../ai/vespa/metricsproxy/core/VespaMetrics.java | 2 +- .../java/ai/vespa/metricsproxy/metric/Metrics.java | 30 +++------------------- .../metricsproxy/node/NodeMetricGatherer.java | 2 -- .../ai/vespa/metricsproxy/metric/MetricsTest.java | 6 ++--- .../ai/vespa/metricsproxy/rpc/RpcMetricsTest.java | 18 +++++++++---- .../metricsproxy/service/ContainerServiceTest.java | 2 +- .../metricsproxy/service/MetricsFetcherTest.java | 14 ++++++++-- .../metricsproxy/service/SystemPollerTest.java | 2 +- .../metricsproxy/service/VespaServiceTest.java | 18 ++++++++++--- 9 files changed, 48 insertions(+), 46 deletions(-) (limited to 'metrics-proxy') 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 getMetrics() { + public List 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 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 metricList = m.getMetrics(); + List 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; + } + } -- cgit v1.2.3