diff options
author | gjoranv <gjoranv@gmail.com> | 2019-06-25 12:30:47 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-06-25 12:30:47 +0200 |
commit | 30e8e135a96534a193d14af242639ad6e009170b (patch) | |
tree | d7cba0dc4c6baa635a9a600fd0d5d1df42175d73 /metrics-proxy | |
parent | bfc319f49b8cd2e100316c00654448cf00ad9390 (diff) |
Revert "Revert "Gjoranv/add default public metrics consumer" MERGEOK"
Diffstat (limited to 'metrics-proxy')
5 files changed, 138 insertions, 36 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 2ca24dad1e2..ab9f4f6c9c4 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 @@ -117,7 +117,8 @@ public class VespaMetrics { .statusCode(health.getStatus().ordinal()) // TODO: MetricsPacket should use StatusCode instead of int .statusMessage(health.getMessage()) .putDimensions(service.getDimensions()) - .putDimension(INSTANCE_DIMENSION_ID, service.getInstanceName()); + .putDimension(INSTANCE_DIMENSION_ID, service.getInstanceName()) + .addConsumers(metricsConsumers.getAllConsumers()); } /** diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/GenericMetricsHandler.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/GenericMetricsHandler.java index cadfc053b94..f61a96917a9 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/GenericMetricsHandler.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/GenericMetricsHandler.java @@ -4,7 +4,9 @@ package ai.vespa.metricsproxy.http; +import ai.vespa.metricsproxy.core.MetricsConsumers; import ai.vespa.metricsproxy.core.MetricsManager; +import ai.vespa.metricsproxy.metric.model.ConsumerId; import ai.vespa.metricsproxy.metric.model.MetricsPacket; import ai.vespa.metricsproxy.metric.model.json.JsonRenderingException; import ai.vespa.metricsproxy.service.VespaServices; @@ -19,22 +21,33 @@ import java.nio.charset.Charset; import java.time.Instant; import java.util.List; import java.util.concurrent.Executor; +import java.util.logging.Logger; +import java.util.stream.Collectors; +import static ai.vespa.metricsproxy.metric.model.ConsumerId.toConsumerId; import static ai.vespa.metricsproxy.metric.model.json.GenericJsonUtil.toGenericJsonModel; /** - * Handler exposing the generic metrics format via http. + * Http handler that exposes the generic metrics format. * * @author gjoranv */ public class GenericMetricsHandler extends ThreadedHttpRequestHandler { + private static final Logger log = Logger.getLogger(GenericMetricsHandler.class.getName()); + public static final ConsumerId DEFAULT_PUBLIC_CONSUMER_ID = toConsumerId("default"); + + private final MetricsConsumers metricsConsumers; private final MetricsManager metricsManager; private final VespaServices vespaServices; @Inject - public GenericMetricsHandler(Executor executor, MetricsManager metricsManager, VespaServices vespaServices) { + public GenericMetricsHandler(Executor executor, + MetricsManager metricsManager, + VespaServices vespaServices, + MetricsConsumers metricsConsumers) { super(executor); + this.metricsConsumers = metricsConsumers; this.metricsManager = metricsManager; this.vespaServices = vespaServices; } @@ -42,13 +55,29 @@ public class GenericMetricsHandler extends ThreadedHttpRequestHandler { @Override public HttpResponse handle(HttpRequest request) { try { - List<MetricsPacket> metrics = metricsManager.getMetrics(vespaServices.getVespaServices(), Instant.now()); + ConsumerId consumer = getConsumerOrDefault(request.getProperty("consumer")); + + List<MetricsPacket> metrics = metricsManager.getMetrics(vespaServices.getVespaServices(), Instant.now()) + .stream() + .filter(metricsPacket -> metricsPacket.consumers().contains(consumer)) + .collect(Collectors.toList()); return new Response(200, toGenericJsonModel(metrics).serialize()); } catch (JsonRenderingException e) { return new Response(500, e.getMessageAsJson()); } } + private ConsumerId getConsumerOrDefault(String consumer) { + if (consumer == null) return DEFAULT_PUBLIC_CONSUMER_ID; + + ConsumerId consumerId = toConsumerId(consumer); + if (! metricsConsumers.getAllConsumers().contains(consumerId)) { + log.info("No consumer with id '" + consumer + "' - using the default consumer instead."); + return DEFAULT_PUBLIC_CONSUMER_ID; + } + return consumerId; + } + private static class Response extends HttpResponse { private final byte[] data; 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 26ae177d767..64ede137e8e 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 @@ -53,6 +53,8 @@ public class ExternalMetrics { } public void setExtraMetrics(List<MetricsPacket.Builder> externalPackets) { + // TODO: Metrics filtering per consumer is not yet implemented. + // Split each packet per metric, and re-aggregate based on the metrics each consumer wants. Then filter out all packages with no consumers. log.log(DEBUG, () -> "Setting new external metrics with " + externalPackets.size() + " metrics packets."); externalPackets.forEach(packet -> { packet.addConsumers(consumers.getAllConsumers()) 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 eb620fd37be..e441c353292 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 @@ -140,6 +140,7 @@ public class MetricsManagerTest { service0.setSystemMetrics(oldSystemMetrics); } + // TODO: test that non-whitelisted metrics are filtered out, but this is currently not the case, see ExternalMetrics.setExtraMetrics @Test public void extra_metrics_packets_containing_whitelisted_metrics_are_added() { metricsManager.setExtraMetrics(ImmutableList.of( 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 301dbf56c3f..29ab8c66694 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 @@ -6,6 +6,7 @@ package ai.vespa.metricsproxy.http; import ai.vespa.metricsproxy.TestUtil; import ai.vespa.metricsproxy.core.ConsumersConfig; +import ai.vespa.metricsproxy.core.ConsumersConfig.Consumer; import ai.vespa.metricsproxy.core.MetricsConsumers; import ai.vespa.metricsproxy.core.MetricsManager; import ai.vespa.metricsproxy.metric.HealthMetric; @@ -29,12 +30,13 @@ import org.junit.BeforeClass; import org.junit.Ignore; import org.junit.Test; +import java.io.IOException; import java.time.Instant; import java.util.List; import java.util.concurrent.Executors; import static ai.vespa.metricsproxy.core.VespaMetrics.INSTANCE_DIMENSION_ID; -import static ai.vespa.metricsproxy.core.VespaMetrics.VESPA_CONSUMER_ID; +import static ai.vespa.metricsproxy.http.GenericMetricsHandler.DEFAULT_PUBLIC_CONSUMER_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; @@ -43,6 +45,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.fail; /** * @author gjoranv @@ -55,38 +58,69 @@ public class GenericMetricsHandlerTest { new DummyService(1, ""), new DownService(HealthMetric.getDown("No response"))); + private static final VespaServices vespaServices = new VespaServices(testServices); + + private static final String DEFAULT_CONSUMER = "default"; + private static final String CUSTOM_CONSUMER = "custom-consumer"; + private static final String CPU_METRIC = "cpu"; private static final String URI = "http://localhost/metrics/v1/values"; - private static final VespaServices vespaServices = new VespaServices(testServices); private static RequestHandlerTestDriver testDriver; @BeforeClass - public static void setupMetricsManager() { + public static void setup() { MetricsManager metricsManager = TestUtil.createMetricsManager(vespaServices, getMetricsConsumers(), getApplicationDimensions(), getNodeDimensions()); metricsManager.setExtraMetrics(ImmutableList.of( new MetricsPacket.Builder(toServiceId("foo")) .timestamp(Instant.now().getEpochSecond()) .putMetrics(ImmutableList.of(new Metric(CPU_METRIC, 12.345))))); - GenericMetricsHandler handler = new GenericMetricsHandler(Executors.newSingleThreadExecutor(), metricsManager, vespaServices); + GenericMetricsHandler handler = new GenericMetricsHandler(Executors.newSingleThreadExecutor(), metricsManager, vespaServices, getMetricsConsumers()); testDriver = new RequestHandlerTestDriver(handler); } + private GenericJsonModel getResponseAsJsonModel(String consumer) { + String response = testDriver.sendRequest(URI + "?consumer=" + consumer).readAll(); + try { + return createObjectMapper().readValue(response, GenericJsonModel.class); + } catch (IOException e) { + fail("Failed to create json model: " + e.getMessage()); + throw new RuntimeException(e); + } + } + @Ignore @Test public void visually_inspect_response() throws Exception{ - String response = testDriver.sendRequest(URI).readAll(); + String response = testDriver.sendRequest(URI+ "?consumer=default").readAll(); ObjectMapper mapper = createObjectMapper(); var jsonModel = mapper.readValue(response, GenericJsonModel.class); System.out.println(mapper.writerWithDefaultPrettyPrinter().writeValueAsString(jsonModel)); } @Test - public void response_contains_node_metrics() throws Exception { + public void no_explicit_consumer_gives_the_default_consumer() { + String responseDefaultConsumer = testDriver.sendRequest(URI + "?consumer=default").readAll(); + String responseNoConsumer = testDriver.sendRequest(URI).readAll(); + assertEqualsExceptTimestamps(responseDefaultConsumer, responseNoConsumer); + } + + @Test + public void unknown_consumer_gives_the_default_consumer() { String response = testDriver.sendRequest(URI).readAll(); - var jsonModel = createObjectMapper().readValue(response, GenericJsonModel.class); + String responseUnknownConsumer = testDriver.sendRequest(URI + "?consumer=not_defined").readAll(); + assertEqualsExceptTimestamps(response, responseUnknownConsumer); + } + + private void assertEqualsExceptTimestamps(String s1, String s2) { + assertEquals(replaceTimestamps(s1), replaceTimestamps(s2)); + } + + @Test + public void response_contains_node_metrics() { + GenericJsonModel jsonModel = getResponseAsJsonModel(DEFAULT_CONSUMER); assertNotNull(jsonModel.node); assertEquals(1, jsonModel.node.metrics.size()); @@ -94,9 +128,8 @@ public class GenericMetricsHandlerTest { } @Test - public void response_contains_service_metrics() throws Exception { - String response = testDriver.sendRequest(URI).readAll(); - var jsonModel = createObjectMapper().readValue(response, GenericJsonModel.class); + public void response_contains_service_metrics() { + GenericJsonModel jsonModel = getResponseAsJsonModel(DEFAULT_CONSUMER); assertEquals(2, jsonModel.services.size()); GenericService dummyService = jsonModel.services.get(0); @@ -104,17 +137,50 @@ public class GenericMetricsHandlerTest { GenericMetrics dummy0Metrics = getMetricsForInstance("dummy0", dummyService); assertEquals(1L, dummy0Metrics.values.get(METRIC_1).longValue()); - assertEquals("metric-dim", dummy0Metrics.dimensions.get("dim0")); + assertEquals("default-val", dummy0Metrics.dimensions.get("consumer-dim")); GenericMetrics dummy1Metrics = getMetricsForInstance("dummy1", dummyService); assertEquals(6L, dummy1Metrics.values.get(METRIC_1).longValue()); - assertEquals("metric-dim", dummy1Metrics.dimensions.get("dim0")); + assertEquals("default-val", dummy1Metrics.dimensions.get("consumer-dim")); } @Test - public void response_contains_health_from_service_that_is_down() throws Exception { - String response = testDriver.sendRequest(URI).readAll(); - var jsonModel = createObjectMapper().readValue(response, GenericJsonModel.class); + public void all_consumers_get_health_from_service_that_is_down() { + assertDownServiceHealth(DEFAULT_CONSUMER); + assertDownServiceHealth(CUSTOM_CONSUMER); + } + + @Test + public void all_timestamps_are_equal_and_non_zero() { + GenericJsonModel jsonModel = getResponseAsJsonModel(DEFAULT_CONSUMER); + + Long nodeTimestamp = jsonModel.node.timestamp; + assertNotEquals(0L, (long) nodeTimestamp); + for (var service : jsonModel.services) + assertEquals(nodeTimestamp, service.timestamp); + } + + @Test + public void custom_consumer_gets_only_its_whitelisted_metrics() { + GenericJsonModel jsonModel = getResponseAsJsonModel(CUSTOM_CONSUMER); + + assertNotNull(jsonModel.node); + // TODO: see comment in ExternalMetrics.setExtraMetrics + // assertEquals(0, jsonModel.node.metrics.size()); + + assertEquals(2, jsonModel.services.size()); + GenericService dummyService = jsonModel.services.get(0); + assertEquals(2, dummyService.metrics.size()); + + GenericMetrics dummy0Metrics = getMetricsForInstance("dummy0", dummyService); + assertEquals("custom-val", dummy0Metrics.dimensions.get("consumer-dim")); + + GenericMetrics dummy1Metrics = getMetricsForInstance("dummy1", dummyService); + assertEquals("custom-val", dummy1Metrics.dimensions.get("consumer-dim")); + } + + private void assertDownServiceHealth(String consumer) { + GenericJsonModel jsonModel = getResponseAsJsonModel(consumer); GenericService downService = jsonModel.services.get(1); assertEquals(DOWN.status, downService.status.code); @@ -127,15 +193,8 @@ public class GenericMetricsHandlerTest { assertEquals(DownService.NAME, downService.metrics.get(0).dimensions.get(INSTANCE_DIMENSION_ID.id)); } - @Test - public void all_timestamps_are_equal_and_non_zero() throws Exception { - String response = testDriver.sendRequest(URI).readAll(); - var jsonModel = createObjectMapper().readValue(response, GenericJsonModel.class); - - Long nodeTimestamp = jsonModel.node.timestamp; - assertNotEquals(0L, (long) nodeTimestamp); - for (var service : jsonModel.services) - assertEquals(nodeTimestamp, service.timestamp); + private String replaceTimestamps(String s) { + return s.replaceAll("timestamp\":\\d+,", "timestamp\":1,"); } private static GenericMetrics getMetricsForInstance(String instance, GenericService service) { @@ -143,23 +202,33 @@ public class GenericMetricsHandlerTest { if (metrics.dimensions.get(INSTANCE_DIMENSION_ID.id).equals(instance)) return metrics; } - throw new RuntimeException("Could not find metrics for service instance " + instance); + fail("Could not find metrics for service instance " + instance); + throw new RuntimeException(); } private static MetricsConsumers getMetricsConsumers() { - ConsumersConfig.Consumer.Metric.Dimension.Builder metricDimension = new ConsumersConfig.Consumer.Metric.Dimension.Builder() - .key("dim0").value("metric-dim"); + var defaultConsumerDimension = new Consumer.Metric.Dimension.Builder() + .key("consumer-dim").value("default-val"); + + var customConsumerDimension = new Consumer.Metric.Dimension.Builder() + .key("consumer-dim").value("custom-val"); return new MetricsConsumers(new ConsumersConfig.Builder() - .consumer(new ConsumersConfig.Consumer.Builder() - .name(VESPA_CONSUMER_ID.id) - .metric(new ConsumersConfig.Consumer.Metric.Builder() + .consumer(new Consumer.Builder() + .name(DEFAULT_PUBLIC_CONSUMER_ID.id) + .metric(new Consumer.Metric.Builder() .name(CPU_METRIC) .outputname(CPU_METRIC)) - .metric(new ConsumersConfig.Consumer.Metric.Builder() + .metric(new Consumer.Metric.Builder() + .name(METRIC_1) + .outputname(METRIC_1) + .dimension(defaultConsumerDimension))) + .consumer(new Consumer.Builder() + .name(CUSTOM_CONSUMER) + .metric(new Consumer.Metric.Builder() .name(METRIC_1) .outputname(METRIC_1) - .dimension(metricDimension))) + .dimension(customConsumerDimension))) .build()); } |