diff options
5 files changed, 116 insertions, 53 deletions
diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/ExternalMetrics.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/ExternalMetrics.java index 64ede137e8e..017b2c57370 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/ExternalMetrics.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/ExternalMetrics.java @@ -35,12 +35,13 @@ import static java.util.stream.Collectors.toCollection; public class ExternalMetrics { private static final Logger log = Logger.getLogger(ExternalMetrics.class.getName()); + // NOTE: node service id must be kept in sync with the same constant _value_ used in docker-api:Metrics.java + public static final ServiceId VESPA_NODE_SERVICE_ID = toServiceId("vespa.node"); + public static final DimensionId ROLE_DIMENSION = toDimensionId("role"); public static final DimensionId STATE_DIMENSION = toDimensionId("state"); public static final DimensionId ORCHESTRATOR_STATE_DIMENSION = toDimensionId("orchestratorState"); - public static final ServiceId VESPA_NODE_SERVICE_ID = toServiceId("vespa.node"); - private volatile List<MetricsPacket.Builder> metrics = new ArrayList<>(); private final MetricsConsumers consumers; @@ -58,7 +59,6 @@ public class ExternalMetrics { log.log(DEBUG, () -> "Setting new external metrics with " + externalPackets.size() + " metrics packets."); externalPackets.forEach(packet -> { packet.addConsumers(consumers.getAllConsumers()) - .service(VESPA_NODE_SERVICE_ID) .retainMetrics(metricsToRetain()) .applyOutputNames(outputNamesById()); }); 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 e441c353292..bc83712ac70 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 @@ -15,6 +15,7 @@ import ai.vespa.metricsproxy.metric.dimensions.NodeDimensions; import ai.vespa.metricsproxy.metric.dimensions.NodeDimensionsConfig; import ai.vespa.metricsproxy.metric.model.DimensionId; import ai.vespa.metricsproxy.metric.model.MetricsPacket; +import ai.vespa.metricsproxy.metric.model.ServiceId; import ai.vespa.metricsproxy.service.DownService; import ai.vespa.metricsproxy.service.DummyService; import ai.vespa.metricsproxy.service.VespaService; @@ -38,6 +39,7 @@ 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; @@ -162,6 +164,21 @@ public class MetricsManagerTest { } @Test + public void application_from_extra_metrics_packets_is_used_as_service_in_result_packets() { + final ServiceId serviceId = toServiceId("custom-service"); + metricsManager.setExtraMetrics(ImmutableList.of( + new MetricsPacket.Builder(serviceId) + .putMetrics(ImmutableList.of(new Metric(WHITELISTED_METRIC_ID, 0))))); + + List<MetricsPacket> packets = metricsManager.getMetrics(testServices, Instant.EPOCH); + MetricsPacket extraPacket = null; + for (MetricsPacket packet : packets) { + if (packet.service.equals(serviceId)) extraPacket = packet; + } + assertNotNull(extraPacket); + } + + @Test public void extra_dimensions_are_added_to_metrics_packets_that_do_not_have_those_dimensions() { metricsManager.setExtraMetrics(ImmutableList.of( new MetricsPacket.Builder(toServiceId("foo")) diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/GenericMetricsHandlerTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/GenericMetricsHandlerTest.java index 29ab8c66694..dc89e5bb9f2 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/GenericMetricsHandlerTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/GenericMetricsHandlerTest.java @@ -37,6 +37,7 @@ import java.util.concurrent.Executors; import static ai.vespa.metricsproxy.core.VespaMetrics.INSTANCE_DIMENSION_ID; import static ai.vespa.metricsproxy.http.GenericMetricsHandler.DEFAULT_PUBLIC_CONSUMER_ID; +import static ai.vespa.metricsproxy.metric.ExternalMetrics.VESPA_NODE_SERVICE_ID; import static ai.vespa.metricsproxy.metric.model.ServiceId.toServiceId; import static ai.vespa.metricsproxy.metric.model.StatusCode.DOWN; import static ai.vespa.metricsproxy.metric.model.json.JacksonUtil.createObjectMapper; @@ -74,7 +75,7 @@ public class GenericMetricsHandlerTest { public static void setup() { MetricsManager metricsManager = TestUtil.createMetricsManager(vespaServices, getMetricsConsumers(), getApplicationDimensions(), getNodeDimensions()); metricsManager.setExtraMetrics(ImmutableList.of( - new MetricsPacket.Builder(toServiceId("foo")) + new MetricsPacket.Builder(VESPA_NODE_SERVICE_ID) .timestamp(Instant.now().getEpochSecond()) .putMetrics(ImmutableList.of(new Metric(CPU_METRIC, 12.345))))); GenericMetricsHandler handler = new GenericMetricsHandler(Executors.newSingleThreadExecutor(), metricsManager, vespaServices, getMetricsConsumers()); diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/ExternalMetricsTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/ExternalMetricsTest.java index 11c271d46e4..2cce2f66039 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/ExternalMetricsTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/ExternalMetricsTest.java @@ -8,6 +8,7 @@ import ai.vespa.metricsproxy.core.ConsumersConfig; import ai.vespa.metricsproxy.core.MetricsConsumers; import ai.vespa.metricsproxy.metric.model.ConsumerId; import ai.vespa.metricsproxy.metric.model.MetricsPacket; +import ai.vespa.metricsproxy.metric.model.ServiceId; import com.google.common.collect.ImmutableList; import org.junit.Test; @@ -38,15 +39,17 @@ public class ExternalMetricsTest { } @Test - public void service_id_is_set_to_vespa_node_id() { + public void service_id_from_extra_packets_is_not_replaced() { + final ServiceId SERVICE_ID = toServiceId("do-not-replace"); + MetricsConsumers noConsumers = new MetricsConsumers(new ConsumersConfig.Builder().build()); ExternalMetrics externalMetrics = new ExternalMetrics(noConsumers); externalMetrics.setExtraMetrics(ImmutableList.of( - new MetricsPacket.Builder(toServiceId("replace_with_vespa_node_id")))); + new MetricsPacket.Builder(SERVICE_ID))); List<MetricsPacket.Builder> packets = externalMetrics.getMetrics(); assertEquals(1, packets.size()); - assertEquals(VESPA_NODE_SERVICE_ID, packets.get(0).build().service); + assertEquals(SERVICE_ID, packets.get(0).build().service); } @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 d4777618546..d6084e3e03a 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 @@ -17,7 +17,9 @@ import com.yahoo.jrt.Transport; import org.json.JSONArray; import org.json.JSONException; import org.json.JSONObject; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import java.util.List; @@ -34,6 +36,8 @@ import static org.hamcrest.CoreMatchers.notNullValue; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; /** * @author jobergum @@ -41,13 +45,60 @@ import static org.junit.Assert.assertThat; */ public class RpcMetricsTest { - private static final String METRICS_RESPONSE_CCL = - getFileContents("metrics-storage-simple.json").trim(); + private static final String METRICS_RESPONSE = getFileContents("metrics-storage-simple.json").trim(); + private static final String EXTRA_APP = "extra"; + + private static class RpcClient implements AutoCloseable { + private final Supervisor supervisor; + private final Target target; + + RpcClient(int port) { + supervisor = new Supervisor(new Transport()); + target = supervisor.connect(new Spec("localhost", port)); + } + + @Override + public void close() { + target.close(); + supervisor.transport().shutdown().join(); + } + } + + @Test + public void extra_metrics_are_added_to_output() throws Exception { + String extraMetricsPayload = "{\"timestamp\":1557754772,\"application\":\"" + EXTRA_APP + + "\",\"metrics\":{\"foo.count\":3},\"dimensions\":{\"role\":\"extra-role\"}}"; + + try (IntegrationTester tester = new IntegrationTester()) { + try (RpcClient rpcClient = new RpcClient(tester.rpcPort())) { + Request req = new Request("setExtraMetrics"); + req.parameters().add(new StringValue(extraMetricsPayload)); + invoke(req, rpcClient, false); + String allServicesResponse = getMetricsForYamas(ALL_SERVICES, rpcClient).trim(); + + // Verify that application is used as serviceId, and that metric exists. + JSONObject extraMetrics = findExtraMetricsObject(allServicesResponse); + assertThat(extraMetrics.getJSONObject("metrics").getInt("foo.count"), is(3)); + assertThat(extraMetrics.getJSONObject("dimensions").getString("role"), is("extra-role")); + } + } + } + + private JSONObject findExtraMetricsObject(String jsonResponse) throws JSONException { + JSONArray metrics = new JSONObject(jsonResponse).getJSONArray("metrics"); + for (int i = 0; i < metrics.length(); i++) { + JSONObject jsonObject = metrics.getJSONObject(i); + assertTrue(jsonObject.has("application")); + if (jsonObject.getString("application").equals(EXTRA_APP)) return jsonObject; + } + fail("Metrics from setExtraMetrics was missing."); + throw new RuntimeException(); + } @Test public void testGetMetrics() throws Exception { try (IntegrationTester tester = new IntegrationTester()) { - tester.httpServer().setResponse(METRICS_RESPONSE_CCL); + 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)); @@ -62,34 +113,29 @@ public class RpcMetricsTest { Metric m2 = metrics.getMetric("bar.count"); assertNotNull("Did not find expected metric with name 'bar.count'", m2); - // Setup RPC client - Supervisor supervisor = new Supervisor(new Transport()); - Target target = supervisor.connect(new Spec("localhost", tester.rpcPort())); + try (RpcClient rpcClient = new RpcClient(tester.rpcPort())) { + verifyMetricsFromRpcRequest(qrserver, rpcClient); - verifyMetricsFromRpcRequest(qrserver, target); + services = tester.vespaServices().getInstancesById(SERVICE_2_CONFIG_ID); + assertThat("#Services should be 1 for config id " + SERVICE_2_CONFIG_ID, services.size(), is(1)); - services = tester.vespaServices().getInstancesById(SERVICE_2_CONFIG_ID); - assertThat("#Services should be 1 for config id " + SERVICE_2_CONFIG_ID, services.size(), is(1)); + VespaService storageService = services.get(0); + verfiyMetricsFromServiceObject(storageService); - VespaService storageService = services.get(0); - verfiyMetricsFromServiceObject(storageService); + String metricsById = getMetricsById(storageService.getConfigId(), rpcClient); + assertThat(metricsById, is("'storage.cluster.storage.storage.0'.foo_count=1 ")); - String metricsById = getMetricsById(storageService.getConfigId(), target); - assertThat(metricsById, is("'storage.cluster.storage.storage.0'.foo_count=1 ")); + String jsonResponse = getMetricsForYamas("non-existing", rpcClient).trim(); + assertThat(jsonResponse, is("105: No service with name 'non-existing'")); - String jsonResponse = getMetricsForYamas("non-existing", target).trim(); - assertThat(jsonResponse, is("105: No service with name 'non-existing'")); + verifyMetricsFromRpcRequestForAllServices(rpcClient); - verifyMetricsFromRpcRequestForAllServices(target); - - // Shutdown RPC - target.close(); - supervisor.transport().shutdown().join(); + } } } - private static void verifyMetricsFromRpcRequest(VespaService service, Target target) throws JSONException { - String jsonResponse = getMetricsForYamas(service.getMonitoringName(), target).trim(); + private static void verifyMetricsFromRpcRequest(VespaService service, RpcClient client) throws JSONException { + String jsonResponse = getMetricsForYamas(service.getMonitoringName(), client).trim(); JSONArray metrics = new JSONObject(jsonResponse).getJSONArray("metrics"); assertThat("Expected 3 metric messages", metrics.length(), is(3)); for (int i = 0; i < metrics.length() - 1; i++) { // The last "metric message" contains only status code/message @@ -124,18 +170,18 @@ public class RpcMetricsTest { assertThat("Metric foo did not contain correct dimension for key = bar", foo.getDimensions().get(toDimensionId("bar")), is("foo")); } - private void verifyMetricsFromRpcRequestForAllServices(Target target) throws JSONException { + private void verifyMetricsFromRpcRequestForAllServices(RpcClient client) throws JSONException { // Verify that metrics for all services can be retrieved in one request. - String allServicesResponse = getMetricsForYamas(ALL_SERVICES, target).trim(); + String allServicesResponse = getMetricsForYamas(ALL_SERVICES, client).trim(); JSONArray allServicesMetrics = new JSONObject(allServicesResponse).getJSONArray("metrics"); assertThat(allServicesMetrics.length(), is(5)); } @Test - public void testGetAllMetricNames() { + public void testGetAllMetricNames() throws Exception { try (IntegrationTester tester = new IntegrationTester()) { - tester.httpServer().setResponse(METRICS_RESPONSE_CCL); + tester.httpServer().setResponse(METRICS_RESPONSE); List<VespaService> services = tester.vespaServices().getInstancesById(SERVICE_1_CONFIG_ID); assertThat(services.size(), is(1)); @@ -144,52 +190,48 @@ public class RpcMetricsTest { Metric m = metrics.getMetric("foo.count"); assertNotNull("Did not find expected metric with name 'foo.count'", m); - Metric m2 = metrics.getMetric("bar.count"); assertNotNull("Did not find expected metric with name 'bar'", m2); - // Setup RPC - Supervisor supervisor = new Supervisor(new Transport()); - Target target = supervisor.connect(new Spec("localhost", tester.rpcPort())); - - String response = getAllMetricNamesForService(services.get(0).getMonitoringName(), VESPA_CONSUMER_ID, target); - assertThat(response, is("foo.count=ON;output-name=foo_count,bar.count=OFF,")); - - // Shutdown RPC - target.close(); - supervisor.transport().shutdown().join(); + try (RpcClient rpcClient = new RpcClient(tester.rpcPort())) { + String response = getAllMetricNamesForService(services.get(0).getMonitoringName(), VESPA_CONSUMER_ID, rpcClient); + assertThat(response, is("foo.count=ON;output-name=foo_count,bar.count=OFF,")); + } } } - private static String getMetricsForYamas(String service, Target target) { + private static String getMetricsForYamas(String service, RpcClient client) { Request req = new Request("getMetricsForYamas"); req.parameters().add(new StringValue(service)); - return invoke(req, target); + return invoke(req, client, true); } - private String getMetricsById(String service, Target target) { + private String getMetricsById(String service, RpcClient client) { Request req = new Request("getMetricsById"); req.parameters().add(new StringValue(service)); - return invoke(req, target); + return invoke(req, client, true); } - private String getAllMetricNamesForService(String service, ConsumerId consumer, Target target) { + private String getAllMetricNamesForService(String service, ConsumerId consumer, RpcClient client) { Request req = new Request("getAllMetricNamesForService"); req.parameters().add(new StringValue(service)); req.parameters().add(new StringValue(consumer.id)); - return invoke(req, target); + return invoke(req, client, true); } - private static String invoke(Request req, Target target) { + private static String invoke(Request req, RpcClient client, boolean expectReturnValue) { String returnValue; - target.invokeSync(req, 20.0); + client.target.invokeSync(req, 20.0); if (req.checkReturnTypes("s")) { returnValue = req.returnValues().get(0).asString(); - } else { + } else if (expectReturnValue) { System.out.println(req.methodName() + " from rpcserver - Invocation failed " + req.errorCode() + ": " + req.errorMessage()); returnValue = req.errorCode() + ": " + req.errorMessage(); } + else { + return ""; + } return returnValue; } |