diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2021-12-17 22:46:08 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-12-17 22:46:08 +0100 |
commit | 1de0224703dc0459a2def69e69ea8c3d25656d0d (patch) | |
tree | 164d3a5b1426e2b19ebbe826e7ce3549a90e693b /metrics-proxy | |
parent | 9b2c6af8709da342560154230c7873ccf992d921 (diff) | |
parent | 5dde463829fddd3d01789dcda8da4f146653e269 (diff) |
Merge pull request #20573 from vespa-engine/balder/preserve-number-type
Balder/preserve number type
Diffstat (limited to 'metrics-proxy')
14 files changed, 213 insertions, 181 deletions
diff --git a/metrics-proxy/pom.xml b/metrics-proxy/pom.xml index 1633d78668b..76f7e92ad43 100644 --- a/metrics-proxy/pom.xml +++ b/metrics-proxy/pom.xml @@ -147,11 +147,6 @@ <artifactId>jetty-http</artifactId> <scope>test</scope> </dependency> - <dependency> - <groupId>org.hamcrest</groupId> - <artifactId>hamcrest-core</artifactId> - <scope>test</scope> - </dependency> </dependencies> <build> <plugins> diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/MetricsPacket.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/MetricsPacket.java index 54ffcd4ae0f..35761ebcf18 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/MetricsPacket.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/MetricsPacket.java @@ -110,7 +110,7 @@ public class MetricsPacket { public Builder putMetrics(Collection<Metric> extraMetrics) { if (extraMetrics != null) - extraMetrics.forEach(metric -> metrics.put(metric.getName(), metric.getValue().doubleValue())); + extraMetrics.forEach(metric -> metrics.put(metric.getName(), metric.getValue())); return this; } diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/HttpMetricFetcher.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/HttpMetricFetcher.java index 65c9bb1ff76..87bed1c79e3 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/HttpMetricFetcher.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/HttpMetricFetcher.java @@ -77,15 +77,14 @@ public abstract class HttpMetricFetcher { } void handleException(Exception e, Object data, int timesFetched) { - logMessage("Unable to parse json '" + data + "' for service '" + service + "': " + - Exceptions.toMessageString(e), timesFetched); + logMessage("Unable to parse json '" + data + "' for service '" + service + "': ", e, timesFetched); } - private void logMessage(String message, int timesFetched) { + private void logMessage(String message, Exception e, int timesFetched) { if (service.isAlive() && timesFetched > 5) { - log.log(Level.INFO, message); + log.log(Level.INFO, message, e); } else { - log.log(Level.FINE, message); + log.log(Level.FINE, message, e); } } diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/MetricsParser.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/MetricsParser.java index f9443c46bad..701af8730d7 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/MetricsParser.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/MetricsParser.java @@ -14,6 +14,8 @@ import java.io.InputStream; import java.util.HashMap; import java.util.Iterator; import java.util.Map; +import java.util.logging.Level; +import java.util.logging.Logger; import static ai.vespa.metricsproxy.metric.model.DimensionId.toDimensionId; @@ -23,6 +25,7 @@ import static ai.vespa.metricsproxy.metric.model.DimensionId.toDimensionId; * @author Jo Kristian Bergum */ public class MetricsParser { + private final static Logger log = Logger.getLogger(MetricsParser.class.getName()); public interface Consumer { void consume(Metric metric); } @@ -128,15 +131,23 @@ public class MetricsParser { StringBuilder sb = new StringBuilder(); for (Iterator<?> it = dimensions.fieldNames(); it.hasNext(); ) { String k = (String) it.next(); - String v = dimensions.get(k).asText(); - sb.append(toDimensionId(k)).append(v); + String v = dimensions.get(k).textValue(); + if (v != null) { + sb.append(toDimensionId(k)).append(v); + } } if ( ! uniqueDimensions.containsKey(sb.toString())) { dim = new HashMap<>(); for (Iterator<?> it = dimensions.fieldNames(); it.hasNext(); ) { String k = (String) it.next(); String v = dimensions.get(k).textValue(); - dim.put(toDimensionId(k), v); + if (v != null) { + dim.put(toDimensionId(k), v); + } else { + // TODO This should never happen, but it has been seen. This should be flagged as warning, + // but will try to find root cause before flooding the log. + log.log(Level.FINE, "Metric '" + name + "': dimension '" + k + "' is null"); + } } uniqueDimensions.put(sb.toString(), Map.copyOf(dim)); } diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/RemoteMetricsFetcher.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/RemoteMetricsFetcher.java index f375fd1586f..5ca9e6fd950 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/RemoteMetricsFetcher.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/RemoteMetricsFetcher.java @@ -28,12 +28,8 @@ public class RemoteMetricsFetcher extends HttpMetricFetcher { } } - void createMetrics(String data, MetricsParser.Consumer consumer, int fetchCount) { - try { - MetricsParser.parse(data, consumer); - } catch (Exception e) { - handleException(e, data, fetchCount); - } + void createMetrics(String data, MetricsParser.Consumer consumer, int fetchCount) throws IOException { + MetricsParser.parse(data, consumer); } private void createMetrics(InputStream data, MetricsParser.Consumer consumer, int fetchCount) throws IOException { try { diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/core/MetricsManagerTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/core/MetricsManagerTest.java index c02e76320f2..9e5a24ccd3c 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/core/MetricsManagerTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/core/MetricsManagerTest.java @@ -34,11 +34,8 @@ import static ai.vespa.metricsproxy.metric.ExternalMetrics.ROLE_DIMENSION; import static ai.vespa.metricsproxy.metric.model.DimensionId.toDimensionId; import static ai.vespa.metricsproxy.metric.model.MetricId.toMetricId; import static ai.vespa.metricsproxy.metric.model.ServiceId.toServiceId; -import static org.hamcrest.CoreMatchers.containsString; -import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; /** @@ -72,40 +69,40 @@ public class MetricsManagerTest { getMetricsConsumers(),getApplicationDimensions(), getNodeDimensions()); List<MetricsPacket> packets = metricsManager.getMetrics(testServices, Instant.EPOCH); - assertThat(packets.size(), is(1)); + assertEquals(1, packets.size()); assertTrue(packets.get(0).metrics().isEmpty()); - assertThat(packets.get(0).dimensions().get(toDimensionId("instance")), is(DownService.NAME)); - assertThat(packets.get(0).dimensions().get(toDimensionId("global")), is("value")); + assertEquals(DownService.NAME, packets.get(0).dimensions().get(toDimensionId("instance"))); + assertEquals("value", packets.get(0).dimensions().get(toDimensionId("global"))); } @Test public void each_service_gets_separate_metrics_packets() { List<MetricsPacket> packets = metricsManager.getMetrics(testServices, Instant.EPOCH); - assertThat(packets.size(), is(2)); + assertEquals(2, packets.size()); - assertThat(packets.get(0).dimensions().get(toDimensionId("instance")), is("dummy0")); - assertThat(packets.get(0).metrics().get(toMetricId("c.test")), is(1.0)); - assertThat(packets.get(0).metrics().get(toMetricId("val")), is(1.05)); + assertEquals("dummy0", packets.get(0).dimensions().get(toDimensionId("instance"))); + assertEquals(1, packets.get(0).metrics().get(toMetricId("c.test"))); + assertEquals(1.05, packets.get(0).metrics().get(toMetricId("val"))); - assertThat(packets.get(1).dimensions().get(toDimensionId("instance")), is("dummy1")); - assertThat(packets.get(1).metrics().get(toMetricId("c.test")), is(6.0)); - assertThat(packets.get(1).metrics().get(toMetricId("val")), is(2.35)); + assertEquals("dummy1", packets.get(1).dimensions().get(toDimensionId("instance"))); + assertEquals(6, packets.get(1).metrics().get(toMetricId("c.test"))); + assertEquals(2.35, packets.get(1).metrics().get(toMetricId("val"))); } @Test public void verify_expected_output_from_getMetricsById() { String dummy0Metrics = metricsManager.getMetricsByConfigId(SERVICE_0_ID); - assertThat(dummy0Metrics, containsString("'dummy.id.0'.val=1.050")); - assertThat(dummy0Metrics, containsString("'dummy.id.0'.c_test=1")); + assertTrue(dummy0Metrics.contains("'dummy.id.0'.val=1.050")); + assertTrue(dummy0Metrics.contains("'dummy.id.0'.c_test=1")); String dummy1Metrics = metricsManager.getMetricsByConfigId(SERVICE_1_ID); - assertThat(dummy1Metrics, containsString("'dummy.id.1'.val=2.350")); - assertThat(dummy1Metrics, containsString("'dummy.id.1'.c_test=6")); + assertTrue(dummy1Metrics.contains("'dummy.id.1'.val=2.350")); + assertTrue(dummy1Metrics.contains("'dummy.id.1'.c_test=6")); } @Test public void getServices_returns_service_types() { - assertThat(metricsManager.getAllVespaServices(), is("dummy")); + assertEquals("dummy", metricsManager.getAllVespaServices()); } @Test @@ -134,8 +131,8 @@ public class MetricsManagerTest { assertEquals(3, packets.size()); MetricsPacket systemPacket = packets.get(0); // system metrics are added before other metrics - assertThat(systemPacket.metrics().get(toMetricId("cpu")), is(1.0)); - assertThat(systemPacket.dimensions().get(toDimensionId("metrictype")), is("system")); + assertEquals(1, systemPacket.metrics().get(toMetricId("cpu"))); + assertEquals("system", systemPacket.dimensions().get(toDimensionId("metrictype"))); service0.setSystemMetrics(oldSystemMetrics); } @@ -148,7 +145,7 @@ public class MetricsManagerTest { .putMetrics(ImmutableList.of(new Metric(WHITELISTED_METRIC_ID, 0))))); List<MetricsPacket> packets = metricsManager.getMetrics(testServices, Instant.EPOCH); - assertThat(packets.size(), is(3)); + assertEquals(3, packets.size()); } @Test @@ -158,7 +155,7 @@ public class MetricsManagerTest { .putMetrics(ImmutableList.of(new Metric(toMetricId("not-whitelisted"), 0))))); List<MetricsPacket> packets = metricsManager.getMetrics(testServices, Instant.EPOCH); - assertThat(packets.size(), is(2)); + assertEquals(2, packets.size()); } @Test @@ -185,7 +182,7 @@ public class MetricsManagerTest { List<MetricsPacket> packets = metricsManager.getMetrics(testServices, Instant.EPOCH); for (MetricsPacket packet : packets) { - assertThat(packet.dimensions().get(ROLE_DIMENSION), is("role from extraMetrics")); + assertEquals("role from extraMetrics", packet.dimensions().get(ROLE_DIMENSION)); } } @@ -197,9 +194,9 @@ public class MetricsManagerTest { .putDimension(METRIC_TYPE_DIMENSION_ID, "from extraMetrics"))); List<MetricsPacket> packets = metricsManager.getMetrics(testServices, Instant.EPOCH); - assertThat(packets.get(0).dimensions().get(METRIC_TYPE_DIMENSION_ID), is("standard")); - assertThat(packets.get(1).dimensions().get(METRIC_TYPE_DIMENSION_ID), is("standard")); - assertThat(packets.get(2).dimensions().get(METRIC_TYPE_DIMENSION_ID), is("from extraMetrics")); + assertEquals("standard", packets.get(0).dimensions().get(METRIC_TYPE_DIMENSION_ID)); + assertEquals("standard", packets.get(1).dimensions().get(METRIC_TYPE_DIMENSION_ID)); + assertEquals("from extraMetrics", packets.get(2).dimensions().get(METRIC_TYPE_DIMENSION_ID)); } @Test 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 1cd0281b7cd..46cc9b7b7fd 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 @@ -9,15 +9,15 @@ import org.junit.Test; import java.util.HashMap; import java.util.Map; import static ai.vespa.metricsproxy.metric.model.MetricId.toMetricId; - -import static org.hamcrest.CoreMatchers.is; -import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; /** * @author Unknowm */ public class MetricsTest { - + private static final double EPSILON = 0.00000000001; @Test public void testIterator() { Metrics m = new Metrics(); @@ -33,36 +33,36 @@ public class MetricsTest { for (Metric metric: m.list()) { String k = metric.getName().id; - assertThat(map.containsKey(k), is(false)); + assertFalse(map.containsKey(k)); map.put(k, metric.getValue()); } - assertThat(map.get("a").intValue(), is(1)); - assertThat(map.get("b").doubleValue(), is(2.5)); + assertEquals(1, map.get("a").intValue()); + assertEquals(2.5, map.get("b").doubleValue(), EPSILON); } @Test public void testBasicMetric() { Metrics m = new Metrics(); m.add(new Metric(toMetricId("count"), 1, System.currentTimeMillis() / 1000)); - assertThat(m.list().size(), is(1)); - assertThat(m.list().get(0).getName(), is(toMetricId("count"))); + assertEquals(1, m.list().size()); + assertEquals(toMetricId("count"), m.list().get(0).getName()); } @Test public void testHealthMetric() { HealthMetric m = HealthMetric.get(null, null); - assertThat(m.isOk(), is(false)); + assertFalse(m.isOk()); m = HealthMetric.get("up", "test message"); - assertThat(m.isOk(), is(true)); - assertThat(m.getMessage(), is("test message")); + assertTrue(m.isOk()); + assertEquals("test message", m.getMessage()); m = HealthMetric.get("ok", "test message"); - assertThat(m.isOk(), is(true)); - assertThat(m.getMessage(), is("test message")); + assertTrue(m.isOk()); + assertEquals("test message", m.getMessage()); m = HealthMetric.get("bad", "test message"); - assertThat(m.isOk(), is(false)); - assertThat(m.getStatus(), is(StatusCode.UNKNOWN)); + assertFalse(m.isOk()); + assertEquals(StatusCode.UNKNOWN, m.getStatus()); } @Test @@ -70,30 +70,30 @@ public class MetricsTest { MetricsFormatter formatter = new MetricsFormatter(false, false); VespaService service = new DummyService(0, "config.id"); String data = formatter.format(service, "key", 1); - assertThat(data, is("'config.id'.key=1")); + assertEquals("'config.id'.key=1", data); formatter = new MetricsFormatter(true, false); data = formatter.format(service, "key", 1); - assertThat(data, is("dummy.'config.id'.key=1")); + assertEquals("dummy.'config.id'.key=1", data); formatter = new MetricsFormatter(true, true); data = formatter.format(service, "key", 1); - assertThat(data, is("dummy.config.'id'.key=1")); + assertEquals("dummy.config.'id'.key=1", data); formatter = new MetricsFormatter(false, true); data = formatter.format(service, "key", 1); - assertThat(data, is("config.'id'.key=1")); + assertEquals("config.'id'.key=1", data); } @Test public void testTimeAdjustment() { - assertThat(Metric.adjustTime(0L, 0L), is(0L)); - assertThat(Metric.adjustTime(59L, 59L), is(59L)); - assertThat(Metric.adjustTime(60L, 60L), is(60L)); - assertThat(Metric.adjustTime(59L, 60L), is(60L)); - assertThat(Metric.adjustTime(60L, 59L), is(60L)); - assertThat(Metric.adjustTime(59L, 61L), is(59L)); + assertEquals(0L, Metric.adjustTime(0L, 0L)); + assertEquals(59L, Metric.adjustTime(59L, 59L)); + assertEquals(60L, Metric.adjustTime(60L, 60L)); + assertEquals(60L, Metric.adjustTime(59L, 60L)); + assertEquals(60L, Metric.adjustTime(60L, 59L)); + assertEquals(59L, Metric.adjustTime(59L, 61L)); } } diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/rpc/RpcHealthMetricsTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/rpc/RpcHealthMetricsTest.java index 268eab9f5d3..98fa337fa78 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/rpc/RpcHealthMetricsTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/rpc/RpcHealthMetricsTest.java @@ -16,9 +16,10 @@ import java.util.List; import static ai.vespa.metricsproxy.TestUtil.getFileContents; import static ai.vespa.metricsproxy.rpc.IntegrationTester.SERVICE_1_CONFIG_ID; -import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; /** * @author jobergum @@ -40,21 +41,21 @@ public class RpcHealthMetricsTest { mockHttpServer.setResponse(HEALTH_OK_RESPONSE); List<VespaService> services = tester.vespaServices().getInstancesById(SERVICE_1_CONFIG_ID); - assertThat(services.size(), is(1)); + assertEquals(1, services.size()); VespaService qrserver = services.get(0); HealthMetric h = qrserver.getHealth(); assertNotNull("Health metric should never be null", h); - assertThat("Status failed, reason = " + h.getMessage(), h.isOk(), is(true)); - assertThat(h.getMessage(), is("WORKING")); + assertTrue("Status failed, reason = " + h.getMessage(), h.isOk()); + assertEquals("WORKING", h.getMessage()); mockHttpServer.setResponse(HEALTH_FAILED_RESPONSE); h = qrserver.getHealth(); assertNotNull("Health metric should never be null", h); - assertThat("Status should be failed" + h.getMessage(), h.isOk(), is(false)); - assertThat(h.getMessage(), is("SOMETHING FAILED")); + assertFalse("Status should be failed" + h.getMessage(), h.isOk()); + assertEquals("SOMETHING FAILED", h.getMessage()); String jsonRPCMessage = getHealthMetrics(tester, qrserver.getMonitoringName().id); - assertThat(jsonRPCMessage, is(WANTED_RPC_RESPONSE)); + assertEquals(WANTED_RPC_RESPONSE, jsonRPCMessage); } } @@ -62,7 +63,7 @@ public class RpcHealthMetricsTest { public void non_existent_service_name_returns_an_error_message() { try (IntegrationTester tester = new IntegrationTester()) { String jsonRPCMessage = getHealthMetrics(tester, "non-existing service"); - assertThat(jsonRPCMessage, is("105: No service with name 'non-existing service'")); + assertEquals("105: No service with name 'non-existing service'", jsonRPCMessage); } } 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 8a489ac70c7..beb0d3fce60 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 @@ -28,12 +28,9 @@ import static ai.vespa.metricsproxy.rpc.IntegrationTester.MONITORING_SYSTEM; import static ai.vespa.metricsproxy.rpc.IntegrationTester.SERVICE_1_CONFIG_ID; import static ai.vespa.metricsproxy.rpc.IntegrationTester.SERVICE_2_CONFIG_ID; import static ai.vespa.metricsproxy.service.VespaServices.ALL_SERVICES; -import static org.hamcrest.CoreMatchers.is; -import static org.hamcrest.CoreMatchers.notNullValue; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; /** @@ -72,8 +69,8 @@ public class RpcMetricsTest { // Verify that application is used as serviceId, and that metric exists. JsonNode extraMetrics = findExtraMetricsObject(allServicesResponse); - assertThat(extraMetrics.get("metrics").get("foo.count").intValue(), is(3)); - assertThat(extraMetrics.get("dimensions").get("role").textValue(), is("extra-role")); + assertEquals(3, extraMetrics.get("metrics").get("foo.count").intValue()); + assertEquals("extra-role", extraMetrics.get("dimensions").get("role").textValue()); } } } @@ -101,13 +98,13 @@ public class RpcMetricsTest { tester.httpServer().setResponse(METRICS_RESPONSE); List<VespaService> services = tester.vespaServices().getInstancesById(SERVICE_1_CONFIG_ID); - assertThat("#Services should be 1 for config id " + SERVICE_1_CONFIG_ID, services.size(), is(1)); + assertEquals("#Services should be 1 for config id " + SERVICE_1_CONFIG_ID, 1, services.size()); VespaService qrserver = services.get(0); - assertThat(qrserver.getMonitoringName().id, is(MONITORING_SYSTEM + VespaService.SEPARATOR + "qrserver")); + assertEquals(MONITORING_SYSTEM + VespaService.SEPARATOR + "qrserver", qrserver.getMonitoringName().id); Metrics metrics = qrserver.getMetrics(); - assertThat("Fetched number of metrics is not correct", metrics.size(), is(2)); + assertEquals("Fetched number of metrics is not correct", 2, metrics.size()); Metric m = getMetric("foo.count", metrics); assertNotNull("Did not find expected metric with name 'foo.count'", m); Metric m2 = getMetric("bar.count", metrics); @@ -117,16 +114,16 @@ public class RpcMetricsTest { verifyMetricsFromRpcRequest(qrserver, rpcClient); services = tester.vespaServices().getInstancesById(SERVICE_2_CONFIG_ID); - assertThat("#Services should be 1 for config id " + SERVICE_2_CONFIG_ID, services.size(), is(1)); + assertEquals("#Services should be 1 for config id " + SERVICE_2_CONFIG_ID, 1, services.size()); VespaService storageService = services.get(0); verfiyMetricsFromServiceObject(storageService); String metricsById = getMetricsById(storageService.getConfigId(), rpcClient); - assertThat(metricsById, is("'storage.cluster.storage.storage.0'.foo_count=1 ")); + assertEquals("'storage.cluster.storage.storage.0'.foo_count=1 ", metricsById); String jsonResponse = getMetricsForYamas("non-existing", rpcClient).trim(); - assertThat(jsonResponse, is("105: No service with name 'non-existing'")); + assertEquals("105: No service with name 'non-existing'", jsonResponse); verifyMetricsFromRpcRequestForAllServices(rpcClient); @@ -145,21 +142,21 @@ public class RpcMetricsTest { 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"); - assertThat("Expected 3 metric messages", metrics.size(), is(3)); + assertEquals("Expected 3 metric messages", 3, metrics.size()); for (int i = 0; i < metrics.size() - 1; i++) { // The last "metric message" contains only status code/message JsonNode jsonObject = metrics.get(i); assertFalse(jsonObject.has("status_code")); assertFalse(jsonObject.has("status_msg")); assertEquals("bar", jsonObject.get("dimensions").get("foo").textValue()); - assertThat(jsonObject.get("dimensions").get("bar").textValue(), is("foo")); - assertThat(jsonObject.get("dimensions").get("serviceDim").textValue(), is("serviceDimValue")); - assertThat(jsonObject.get("routing").get("yamas").get("namespaces").size(), is(1)); + assertEquals("foo", jsonObject.get("dimensions").get("bar").textValue()); + assertEquals("serviceDimValue", jsonObject.get("dimensions").get("serviceDim").textValue()); + assertEquals(1, jsonObject.get("routing").get("yamas").get("namespaces").size()); if (jsonObject.get("metrics").has("foo_count")) { - assertThat(jsonObject.get("metrics").get("foo_count").intValue(), is(1)); - assertThat(jsonObject.get("routing").get("yamas").get("namespaces").get(0).textValue(), is(vespaMetricsConsumerId.id)); + assertEquals(1, jsonObject.get("metrics").get("foo_count").intValue()); + assertEquals(vespaMetricsConsumerId.id, jsonObject.get("routing").get("yamas").get("namespaces").get(0).textValue()); } else { - assertThat(jsonObject.get("metrics").get("foo.count").intValue(), is(1)); - assertThat(jsonObject.get("routing").get("yamas").get("namespaces").get(0).textValue(), is(CUSTOM_CONSUMER_ID.id)); + assertEquals(1, jsonObject.get("metrics").get("foo.count").intValue()); + assertEquals(CUSTOM_CONSUMER_ID.id, jsonObject.get("routing").get("yamas").get("namespaces").get(0).textValue()); } } @@ -168,21 +165,21 @@ public class RpcMetricsTest { private void verfiyMetricsFromServiceObject(VespaService service) { Metrics storageMetrics = service.getMetrics(); - assertThat(storageMetrics.size(), is(2)); + assertEquals(2, storageMetrics.size()); 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)); - assertThat("Metric foo did not contain correct dimension", foo.getDimensions().get(toDimensionId("foo")), is("bar")); - assertThat("Metric foo did not contain correct dimension", foo.getDimensions().containsKey(toDimensionId("bar")), is(true)); - assertThat("Metric foo did not contain correct dimension for key = bar", foo.getDimensions().get(toDimensionId("bar")), is("foo")); + assertEquals("Expected 2 dimensions for metric foo", 2, foo.getDimensions().size()); + assertTrue("Metric foo did not contain correct dimension mapping for key = foo.count", foo.getDimensions().containsKey(toDimensionId("foo"))); + assertEquals("Metric foo did not contain correct dimension", "bar", foo.getDimensions().get(toDimensionId("foo"))); + assertTrue("Metric foo did not contain correct dimension", foo.getDimensions().containsKey(toDimensionId("bar"))); + assertEquals("Metric foo did not contain correct dimension for key = bar", "foo", foo.getDimensions().get(toDimensionId("bar"))); } private void verifyMetricsFromRpcRequestForAllServices(RpcClient client) throws IOException { // Verify that metrics for all services can be retrieved in one request. String allServicesResponse = getMetricsForYamas(ALL_SERVICES, client).trim(); ArrayNode allServicesMetrics = (ArrayNode) jsonMapper.readTree(allServicesResponse).get("metrics"); - assertThat(allServicesMetrics.size(), is(5)); + assertEquals(5, allServicesMetrics.size()); } @Test @@ -192,9 +189,9 @@ public class RpcMetricsTest { tester.httpServer().setResponse(METRICS_RESPONSE); List<VespaService> services = tester.vespaServices().getInstancesById(SERVICE_1_CONFIG_ID); - assertThat(services.size(), is(1)); + assertEquals(1, services.size()); Metrics metrics = services.get(0).getMetrics(); - assertThat("Fetched number of metrics is not correct", metrics.size(), is(2)); + assertEquals("Fetched number of metrics is not correct", 2, metrics.size()); Metric m = getMetric("foo.count", metrics); assertNotNull("Did not find expected metric with name 'foo.count'", m); @@ -203,7 +200,7 @@ public class RpcMetricsTest { try (RpcClient rpcClient = new RpcClient(tester.rpcPort())) { String response = getAllMetricNamesForService(services.get(0).getMonitoringName().id, vespaMetricsConsumerId, rpcClient); - assertThat(response, is("foo.count=ON;output-name=foo_count,bar.count=OFF,")); + assertEquals("foo.count=ON;output-name=foo_count,bar.count=OFF,", response); } } } @@ -263,11 +260,11 @@ public class RpcMetricsTest { } private static void verifyStatusMessage(JsonNode jsonObject) { - assertThat(jsonObject.get("status_code").intValue(), is(0)); - assertThat(jsonObject.get("status_msg").textValue(), notNullValue()); - assertThat(jsonObject.get("application").textValue(), notNullValue()); - assertThat(jsonObject.get("routing"), notNullValue()); - assertThat(jsonObject.size(), is(4)); + assertEquals(0, jsonObject.get("status_code").intValue()); + assertNotNull(jsonObject.get("status_msg")); + assertNotNull(jsonObject.get("application")); + assertNotNull(jsonObject.get("routing")); + assertEquals(4, jsonObject.size()); } } diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/ConfigSentinelClientTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/ConfigSentinelClientTest.java index 6d009d2fd88..0ace697d545 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/ConfigSentinelClientTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/ConfigSentinelClientTest.java @@ -6,8 +6,10 @@ import org.junit.Test; import java.util.ArrayList; import java.util.List; -import static org.hamcrest.CoreMatchers.is; -import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertEquals; + /** * @author Unknown @@ -29,35 +31,35 @@ public class ConfigSentinelClientTest { try (MockConfigSentinelClient client = new MockConfigSentinelClient(configsentinel)) { client.updateServiceStatuses(services); - assertThat(qrserver.getPid(), is(6520)); - assertThat(qrserver.getState(), is("RUNNING")); - assertThat(qrserver.isAlive(), is(true)); - assertThat(searchnode4.getPid(), is(6534)); - assertThat(searchnode4.getState(), is("RUNNING")); - assertThat(searchnode4.isAlive(), is(true)); + assertEquals(6520, qrserver.getPid()); + assertEquals("RUNNING", qrserver.getState()); + assertTrue(qrserver.isAlive()); + assertEquals(6534, searchnode4.getPid()); + assertEquals("RUNNING", searchnode4.getState()); + assertTrue(searchnode4.isAlive()); - assertThat(docproc.getPid(), is(-1)); - assertThat(docproc.getState(), is("FINISHED")); - assertThat(docproc.isAlive(), is(false)); + assertEquals(-1, docproc.getPid()); + assertEquals("FINISHED", docproc.getState()); + assertFalse(docproc.isAlive()); configsentinel.reConfigure(); client.ping(docproc); - assertThat(docproc.getPid(), is(100)); - assertThat(docproc.getState(), is("RUNNING")); - assertThat(docproc.isAlive(), is(true)); + assertEquals(100, docproc.getPid()); + assertEquals("RUNNING", docproc.getState()); + assertTrue(docproc.isAlive()); //qrserver has yet not been checked - assertThat(qrserver.isAlive(), is(true)); + assertTrue(qrserver.isAlive()); client.updateServiceStatuses(services); - assertThat(docproc.getPid(), is(100)); - assertThat(docproc.getState(), is("RUNNING")); - assertThat(docproc.isAlive(), is(true)); + assertEquals(100, docproc.getPid()); + assertEquals("RUNNING", docproc.getState()); + assertTrue(docproc.isAlive()); //qrserver is no longer running on this node - so should be false - assertThat(qrserver.isAlive(), is(false)); + assertFalse(qrserver.isAlive()); } } @@ -90,13 +92,13 @@ public class ConfigSentinelClientTest { try (MockConfigSentinelClient client = new MockConfigSentinelClient(configsentinel)) { client.updateServiceStatuses(services); - assertThat(container.isAlive(),is(true)); - assertThat(container.getPid(),is(14338)); - assertThat(container.getState(),is("RUNNING")); + assertTrue(container.isAlive()); + assertEquals(14338, container.getPid()); + assertEquals("RUNNING", container.getState()); - assertThat(containerClusterController.isAlive(),is(true)); - assertThat(containerClusterController.getPid(),is(25020)); - assertThat(containerClusterController.getState(),is("RUNNING")); + assertTrue(containerClusterController.isAlive()); + assertEquals(25020, containerClusterController.getPid()); + assertEquals("RUNNING", containerClusterController.getState()); } } 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 44f0e9608ac..79dab3338e9 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 @@ -11,8 +11,8 @@ import static ai.vespa.metricsproxy.TestUtil.getFileContents; import static ai.vespa.metricsproxy.metric.model.DimensionId.toDimensionId; 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.assertThat; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; /** * @author Unknown @@ -45,15 +45,15 @@ public class ContainerServiceTest { count++; System.out.println("Name: " + m.getName() + " value: " + m.getValue()); if (m.getDimensions().get(toDimensionId("chain")).equals("asvBlendingResult")) { - assertThat((Double)m.getValue(), is(26.4)); + assertEquals(26.4, m.getValue()); } else if (m.getDimensions().get(toDimensionId("chain")).equals("blendingResult")) { - assertThat((Double)m.getValue(), is(0.36666666666666664)); + assertEquals(0.36666666666666664, m.getValue()); } else { - assertThat("Unknown unknown chain", false, is(true)); + assertTrue("Unknown unknown chain", false); } } } - assertThat(count, is(2)); + assertEquals(2, count); } @After 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 3af317ab36b..b30e871a543 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 @@ -4,27 +4,30 @@ package ai.vespa.metricsproxy.service; import ai.vespa.metricsproxy.TestUtil; import ai.vespa.metricsproxy.metric.Metric; import ai.vespa.metricsproxy.metric.Metrics; -import ai.vespa.metricsproxy.metric.model.MetricId; import org.junit.Test; +import java.io.IOException; + import static ai.vespa.metricsproxy.metric.model.MetricId.toMetricId; -import static org.hamcrest.CoreMatchers.is; -import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; /** */ public class MetricsFetcherTest { - private static int port = 9; //port number is not used in this test + private static final int port = 9; //port number is not used in this test - private class MetricsConsumer implements MetricsParser.Consumer { + private static class MetricsConsumer implements MetricsParser.Consumer { Metrics metrics = new Metrics(); @Override public void consume(Metric metric) { metrics.add(metric); } } - Metrics fetch(String data) { + Metrics fetch(String data) throws IOException { RemoteMetricsFetcher fetcher = new RemoteMetricsFetcher(new DummyService(0, "dummy/id/0"), port); MetricsConsumer consumer = new MetricsConsumer(); fetcher.createMetrics(data, consumer, 0); @@ -32,30 +35,59 @@ public class MetricsFetcherTest { } @Test - public void testStateFormatMetricsParse() { + public void testStateFormatMetricsParse() throws IOException { String jsonData = TestUtil.getFileContents("metrics-state.json"); Metrics metrics = fetch(jsonData); - assertThat(metrics.size(), is(10)); - 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)); + assertEquals(10, metrics.size()); + assertEquals(28, getMetric("query_hits.count", metrics).getValue()); + assertEquals(0.4667, getMetric("queries.rate", metrics).getValue()); + assertEquals(1334134700L, metrics.getTimeStamp()); } @Test - public void testEmptyJson() { + public void testEmptyJson() throws IOException { String jsonData = "{}"; Metrics metrics = fetch(jsonData); - assertThat("Wrong number of metrics", metrics.size(), is(0)); + assertEquals(0, metrics.size()); + } + + @Test + public void testSkippingNullDimensions() throws IOException { + String jsonData = + "{\"status\" : {\"code\" : \"up\",\"message\" : \"Everything ok here\"}," + + "\"metrics\" : {\"snapshot\" : {\"from\" : 1334134640.089,\"to\" : 1334134700.088" + " }," + + "\"values\" : [" + + "{" + + " \"name\" : \"some.bogus.metric\"," + + " \"values\" : {" + + " \"count\" : 12," + + " \"rate\" : 0.2" + + " }," + + " \"dimensions\" : {" + + " \"version\" : null" + + " }" + + " }" + + "]}}"; + + Metrics metrics = fetch(jsonData); + assertEquals(2, metrics.size()); + assertTrue(metrics.list().get(0).getDimensions().isEmpty()); + assertTrue(metrics.list().get(1).getDimensions().isEmpty()); } @Test - public void testErrors() { + public void testErrors() throws IOException { String jsonData; - Metrics metrics; + Metrics metrics = null; jsonData = ""; - metrics = fetch(jsonData); - assertThat("Wrong number of metrics", metrics.size(), is(0)); + try { + metrics = fetch(jsonData); + fail("Should have an IOException instead"); + } catch (IOException e) { + assertEquals("Expected start of object, got null", e.getMessage()); + } + assertNull(metrics); jsonData = "{\n" + "\"status\" : {\n" + @@ -64,7 +96,7 @@ public class MetricsFetcherTest { "}\n" + "}"; metrics = fetch(jsonData); - assertThat("Wrong number of metrics", metrics.size(), is(0)); + assertEquals(0, metrics.size()); jsonData = "{\n" + "\"status\" : {\n" + @@ -92,8 +124,14 @@ public class MetricsFetcherTest { "}\n" + "}"; - metrics = fetch(jsonData); - assertThat("Wrong number of metrics", metrics.size(), is(0)); + metrics = null; + try { + metrics = fetch(jsonData); + fail("Should have an IOException instead"); + } catch (IllegalArgumentException e) { + assertEquals("Value for aggregator 'count' is not a number", e.getMessage()); + } + assertNull(metrics); } public Metric getMetric(String metric, Metrics metrics) { 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 a54e050bac7..6c0e1865dcd 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 @@ -3,7 +3,6 @@ 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; import org.junit.Before; import org.junit.Test; @@ -11,9 +10,7 @@ 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; /** @@ -40,17 +37,17 @@ public class VespaServiceTest { @Test public void testService() { VespaService service = new VespaService("qrserver", "container/qrserver.0"); - assertThat(service.getServiceName(), is("qrserver")); - assertThat(service.getInstanceName(), is("qrserver")); - assertThat(service.getPid(), is(-1)); - assertThat(service.getConfigId(), is("container/qrserver.0")); + assertEquals("qrserver", service.getServiceName()); + assertEquals("qrserver", service.getInstanceName()); + assertEquals(-1, service.getPid()); + assertEquals("container/qrserver.0", service.getConfigId()); service = VespaService.create("qrserver2", "container/qrserver.0", -1); - assertThat(service.getServiceName(), is("qrserver")); - assertThat(service.getInstanceName(), is("qrserver2")); - assertThat(service.getPid(), is(-1)); - assertThat(service.getConfigId(), is("container/qrserver.0")); + assertEquals("qrserver", service.getServiceName()); + assertEquals("qrserver2", service.getInstanceName()); + assertEquals(-1, service.getPid()); + assertEquals("container/qrserver.0", service.getConfigId()); } @Test diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/VespaServicesTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/VespaServicesTest.java index 264406a6fc6..dda8af8559b 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/VespaServicesTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/VespaServicesTest.java @@ -7,8 +7,7 @@ import org.junit.Test; import java.util.List; import static ai.vespa.metricsproxy.service.VespaServices.ALL_SERVICES; -import static org.hamcrest.CoreMatchers.is; -import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertEquals; /** * TODO: add more tests @@ -24,7 +23,7 @@ public class VespaServicesTest { new DummyService(1, "dummy/id/1")); VespaServices services = new VespaServices(dummyServices); - assertThat(services.getMonitoringServices("vespa.dummy").size(), is(2)); + assertEquals(2, services.getMonitoringServices("vespa.dummy").size()); } @Test @@ -33,7 +32,7 @@ public class VespaServicesTest { new DummyService(0, "dummy/id/0")); VespaServices services = new VespaServices(dummyServices); - assertThat(services.getMonitoringServices(ALL_SERVICES).size(), is(1)); + assertEquals(1, services.getMonitoringServices(ALL_SERVICES).size()); } } |