From 2132ab285831f368f204210be8de409714573df3 Mon Sep 17 00:00:00 2001
From: Amund Bergland Kvalsvik <akvalsvik@verizonmedia.com>
Date: Fri, 26 Jun 2020 12:01:42 +0200
Subject: Updated ApplicationMetricsHandler. Removed PrometheusV1Handler

---
 .../metricsproxy/MetricsProxyContainerCluster.java |   2 +-
 .../container/ApplicationContainerCluster.java     |  15 +--
 .../handler/metrics/PrometheusV1Handler.java       |  90 ----------------
 .../handler/metrics/PrometheusV1HandlerTest.java   | 116 ---------------------
 .../application/ApplicationMetricsHandler.java     |  35 ++++++-
 .../application/ApplicationMetricsHandlerTest.java |   8 +-
 6 files changed, 37 insertions(+), 229 deletions(-)
 delete mode 100644 container-core/src/main/java/com/yahoo/container/handler/metrics/PrometheusV1Handler.java
 delete mode 100644 container-core/src/test/java/com/yahoo/container/handler/metrics/PrometheusV1HandlerTest.java

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java
index 20f2bfe6636..d698d42b2d9 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java
@@ -117,7 +117,7 @@ public class MetricsProxyContainerCluster extends ContainerCluster<MetricsProxyC
         addHttpHandler(PrometheusHandler.class, PrometheusHandler.V1_PATH);
         addHttpHandler(YamasHandler.class, YamasHandler.V1_PATH);
 
-        addHttpHandler(ApplicationMetricsHandler.class, ApplicationMetricsHandler.V1_PATH);
+        addHttpHandler(ApplicationMetricsHandler.class, ApplicationMetricsHandler.V1_METRICS);
         addMetricsProxyComponent(ApplicationMetricsRetriever.class);
 
         addTelegrafComponents();
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java
index b9f849405da..55ce199eddf 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java
@@ -13,7 +13,6 @@ import com.yahoo.container.bundle.BundleInstantiationSpecification;
 import com.yahoo.container.handler.ThreadpoolConfig;
 import com.yahoo.container.handler.metrics.MetricsProxyApiConfig;
 import com.yahoo.container.handler.metrics.MetricsV2Handler;
-import com.yahoo.container.handler.metrics.PrometheusV1Handler;
 import com.yahoo.container.jdisc.ContainerMbusConfig;
 import com.yahoo.container.jdisc.messagebus.MbusServerProvider;
 import com.yahoo.jdisc.http.ServletPathsConfig;
@@ -59,10 +58,6 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat
     public static final String METRICS_V2_HANDLER_BINDING_1 = "http://*" + MetricsV2Handler.V2_PATH;
     public static final String METRICS_V2_HANDLER_BINDING_2 = METRICS_V2_HANDLER_BINDING_1 + "/*";
 
-    public static final String PROMETHEUS_V1_HANDLER_CLASS = PrometheusV1Handler.class.getName();
-    public static final String PROMETHEUS_V1_HANDLER_BINDING_1 = "http://*" + PrometheusV1Handler.V1_PATH;
-    public static final String PROMETHEUS_V1_HANDLER_BINDING_2 = PROMETHEUS_V1_HANDLER_BINDING_1 + "/*";
-
     public static final int heapSizePercentageOfTotalNodeMemory = 60;
     public static final int heapSizePercentageOfTotalNodeMemoryWhenCombinedCluster = 17;
 
@@ -94,7 +89,6 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat
         addSimpleComponent("com.yahoo.container.jdisc.AthenzIdentityProviderProvider");
         addSimpleComponent("com.yahoo.container.jdisc.SystemInfoProvider");
         addMetricsV2Handler();
-        addPrometheusV1Handler();
         addTestrunnerComponentsIfTester(deployState);
     }
 
@@ -129,13 +123,6 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat
         addComponent(handler);
     }
 
-    public void addPrometheusV1Handler() {
-        Handler<AbstractConfigProducer<?>> handler = new Handler<>(
-                new ComponentModel(PROMETHEUS_V1_HANDLER_CLASS, null, null, null));
-        handler.addServerBindings(PROMETHEUS_V1_HANDLER_BINDING_1, PROMETHEUS_V1_HANDLER_BINDING_2);
-        addComponent(handler);
-    }
-
     private void addTestrunnerComponentsIfTester(DeployState deployState) {
         if (deployState.isHosted() && deployState.getProperties().applicationId().instance().isTester())
             addPlatformBundle(Paths.get(Defaults.getDefaults().underVespaHome("lib/jars/vespa-testrunner-components-jar-with-dependencies.jar")));
@@ -227,7 +214,7 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat
     @Override
     public void getConfig(MetricsProxyApiConfig.Builder builder) {
         builder.metricsPort(MetricsProxyContainer.BASEPORT)
-                .metricsApiPath(ApplicationMetricsHandler.VALUES_PATH);
+                .metricsApiPath(ApplicationMetricsHandler.METRICS_VALUES_PATH);
     }
 
     @Override
diff --git a/container-core/src/main/java/com/yahoo/container/handler/metrics/PrometheusV1Handler.java b/container-core/src/main/java/com/yahoo/container/handler/metrics/PrometheusV1Handler.java
deleted file mode 100644
index 46bdb388cd1..00000000000
--- a/container-core/src/main/java/com/yahoo/container/handler/metrics/PrometheusV1Handler.java
+++ /dev/null
@@ -1,90 +0,0 @@
-package com.yahoo.container.handler.metrics;
-
-import ai.vespa.metricsproxy.http.TextResponse;
-import ai.vespa.metricsproxy.metric.model.MetricsPacket;
-import ai.vespa.metricsproxy.metric.model.json.GenericJsonUtil;
-import ai.vespa.metricsproxy.metric.model.prometheus.PrometheusUtil;
-import ai.vespa.util.http.VespaHttpClientBuilder;
-import com.google.inject.Inject;
-import com.yahoo.container.jdisc.HttpResponse;
-import com.yahoo.restapi.Path;
-import com.yahoo.restapi.StringResponse;
-import com.yahoo.vespa.jdk8compat.List;
-import com.yahoo.yolean.Exceptions;
-import java.io.IOException;
-import java.net.URI;
-import java.util.Optional;
-import java.util.concurrent.Executor;
-import java.util.stream.Collectors;
-import java.util.stream.Stream;
-import org.apache.http.client.HttpClient;
-import org.apache.http.client.config.RequestConfig;
-import org.apache.http.client.methods.HttpGet;
-import org.apache.http.impl.client.BasicResponseHandler;
-import org.apache.http.impl.client.CloseableHttpClient;
-import org.json.JSONException;
-import org.json.JSONObject;
-
-import static com.yahoo.jdisc.Response.Status.INTERNAL_SERVER_ERROR;
-
-public class PrometheusV1Handler extends HttpHandlerBase {
-
-    public static final String V1_PATH = "/prometheus/v1";
-    static final String VALUES_PATH = V1_PATH + "/values";
-
-    private static final int HTTP_CONNECT_TIMEOUT = 5000;
-    private static final int HTTP_SOCKET_TIMEOUT = 30000;
-
-    private final String metricsProxyUri;
-    private final HttpClient httpClient = createHttpClient();
-
-
-    @Inject
-    protected PrometheusV1Handler(Executor executor, MetricsProxyApiConfig config) {
-        super(executor);
-        metricsProxyUri = "http://localhost:+" + config.metricsPort() + config.metricsApiPath();
-    }
-
-    private static CloseableHttpClient createHttpClient() {
-        return VespaHttpClientBuilder.create()
-                .setUserAgent("application-prometheus-retriever")
-                .setDefaultRequestConfig(RequestConfig.custom()
-                        .setConnectTimeout(HTTP_CONNECT_TIMEOUT)
-                        .setSocketTimeout(HTTP_SOCKET_TIMEOUT)
-                        .build())
-                .build();
-    }
-
-    static String consumerQuery(String consumer) {
-        return (consumer == null || consumer.isEmpty()) ? "" : "?consumer=" + consumer;
-    }
-
-    @Override
-    protected Optional<HttpResponse> doHandle(URI requestUri, Path apiPath, String consumer) {
-        if (apiPath.matches(V1_PATH)) return Optional.of(resourceListResponse(requestUri, List.of(VALUES_PATH)));
-        if (apiPath.matches(VALUES_PATH)) return Optional.of(valuesResponse(consumer));
-        return Optional.empty();
-    }
-
-    private HttpResponse valuesResponse(String consumer) {
-        try {
-            String uri = metricsProxyUri + consumerQuery(consumer);
-            String metricsJson = httpClient.execute(new HttpGet(uri), new BasicResponseHandler());
-            return new TextResponse(200, convertToPrometheus(metricsJson));
-        } catch (IOException e) {
-            log.warning(String.format("Unable to retrieve prometheus metrics from %s: %s", metricsProxyUri, Exceptions.toMessageString(e)));
-            return new ErrorResponse(INTERNAL_SERVER_ERROR, e.getMessage());
-        }
-    }
-
-    private String convertToPrometheus(String metricsJson) {
-        return PrometheusUtil.toPrometheusModel(
-                GenericJsonUtil
-                        .toMetricsPackets(metricsJson)
-                        .stream()
-                        .map(MetricsPacket.Builder::build)
-                        .collect(Collectors.toList()))
-                .serialize();
-    }
-
-}
diff --git a/container-core/src/test/java/com/yahoo/container/handler/metrics/PrometheusV1HandlerTest.java b/container-core/src/test/java/com/yahoo/container/handler/metrics/PrometheusV1HandlerTest.java
deleted file mode 100644
index ece2e37085c..00000000000
--- a/container-core/src/test/java/com/yahoo/container/handler/metrics/PrometheusV1HandlerTest.java
+++ /dev/null
@@ -1,116 +0,0 @@
-package com.yahoo.container.handler.metrics;
-
-import com.github.tomakehurst.wiremock.junit.WireMockRule;
-import com.yahoo.container.jdisc.RequestHandlerTestDriver;
-import java.io.BufferedReader;
-import java.io.InputStream;
-import java.io.InputStreamReader;
-import java.util.concurrent.Executors;
-import java.util.stream.Collectors;
-import org.json.JSONArray;
-import org.json.JSONException;
-import org.json.JSONObject;
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
-
-import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
-import static com.github.tomakehurst.wiremock.client.WireMock.equalTo;
-import static com.github.tomakehurst.wiremock.client.WireMock.get;
-import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo;
-import static com.yahoo.container.handler.metrics.PrometheusV1Handler.consumerQuery;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
-
-public class PrometheusV1HandlerTest {
-
-    private static final String URI_BASE = "http://localhost";
-
-    private static final String V1_URI = URI_BASE + PrometheusV1Handler.V1_PATH;
-    private static final String VALUES_URI = URI_BASE + PrometheusV1Handler.VALUES_PATH;
-
-    // Mock applicationmetrics api
-    private static final String MOCK_METRICS_PATH = "/node0";
-
-    private static final String TEST_FILE = "application-metrics.json";
-    private static final String RESPONSE = getFileContents(TEST_FILE);
-    private static final String CPU_METRIC = "cpu";
-    private static final String REPLACED_CPU_METRIC = "cpu.util";
-    private static final String CUSTOM_CONSUMER = "custom-consumer";
-
-    private static RequestHandlerTestDriver testDriver;
-
-    @Rule
-    public WireMockRule wireMockRule = new WireMockRule(80);
-
-    private static String getFileContents(String filename) {
-        InputStream in = PrometheusV1HandlerTest.class.getClassLoader().getResourceAsStream(filename);
-        if (in == null) {
-            throw new RuntimeException("File not found: " + filename);
-        }
-        return new BufferedReader(new InputStreamReader(in)).lines().collect(Collectors.joining("\n"));
-    }
-
-    @Before
-    public void setup() {
-        setupWireMock();
-        var handler = new PrometheusV1Handler(Executors.newSingleThreadExecutor(),
-                new MetricsProxyApiConfig.Builder()
-                        .metricsPort(wireMockRule.port())
-                        .metricsApiPath(MOCK_METRICS_PATH)
-                        .build());
-        testDriver = new RequestHandlerTestDriver(handler);
-    }
-
-    private void setupWireMock() {
-        wireMockRule.stubFor(get(urlPathEqualTo(MOCK_METRICS_PATH))
-                .willReturn(aResponse().withBody(RESPONSE)));
-
-        // Add a slightly different response for a custom consumer.
-        String myConsumerResponse = RESPONSE.replace(CPU_METRIC, REPLACED_CPU_METRIC);
-        wireMockRule.stubFor(get(urlPathEqualTo(MOCK_METRICS_PATH))
-                .withQueryParam("consumer", equalTo(CUSTOM_CONSUMER))
-                .willReturn(aResponse().withBody(myConsumerResponse)));
-
-    }
-
-    @Test
-    public void v1_response_contains_values_uri() throws Exception {
-        String response = testDriver.sendRequest(V1_URI).readAll();
-        JSONObject root = new JSONObject(response);
-        assertTrue(root.has("resources"));
-
-        JSONArray resources = root.getJSONArray("resources");
-        assertEquals(1, resources.length());
-
-        JSONObject valuesUri = resources.getJSONObject(0);
-        assertEquals(VALUES_URI, valuesUri.getString("url"));
-    }
-
-    @Test
-    public void invalid_path_yields_error_response() throws Exception {
-        String response = testDriver.sendRequest(V1_URI + "/invalid").readAll();
-        JSONObject root = new JSONObject(response);
-        assertTrue(root.has("error"));
-        assertTrue(root.getString("error").startsWith("No content"));
-    }
-
-    @Test
-    public void values_response_is_equal_to_test_file() {
-        String response = testDriver.sendRequest(VALUES_URI).readAll();
-        assertEquals(RESPONSE, response);
-    }
-
-    @Test
-    public void consumer_is_propagated_to_metrics_proxy_api() {
-        String response = getResponseAsString(CUSTOM_CONSUMER);
-
-        assertTrue(response.contains(REPLACED_CPU_METRIC));
-
-
-    }
-
-    private String getResponseAsString(String consumer) {
-        return testDriver.sendRequest(VALUES_URI + consumerQuery(consumer)).readAll();
-    }
-}
diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsHandler.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsHandler.java
index c3231daab5f..46f3a1b18c4 100644
--- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsHandler.java
+++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsHandler.java
@@ -3,7 +3,12 @@
 package ai.vespa.metricsproxy.http.application;
 
 import ai.vespa.metricsproxy.core.MetricsConsumers;
+import ai.vespa.metricsproxy.http.TextResponse;
 import ai.vespa.metricsproxy.metric.model.ConsumerId;
+import ai.vespa.metricsproxy.metric.model.MetricsPacket;
+import ai.vespa.metricsproxy.metric.model.json.GenericJsonModel;
+import ai.vespa.metricsproxy.metric.model.json.GenericJsonUtil;
+import ai.vespa.metricsproxy.metric.model.prometheus.PrometheusUtil;
 import com.google.inject.Inject;
 import com.yahoo.container.handler.metrics.ErrorResponse;
 import com.yahoo.container.handler.metrics.HttpHandlerBase;
@@ -16,9 +21,12 @@ import java.util.List;
 import java.util.Optional;
 import java.util.concurrent.Executor;
 import java.util.logging.Level;
+import java.util.stream.Collectors;
 
 import static ai.vespa.metricsproxy.http.ValuesFetcher.getConsumerOrDefault;
 import static ai.vespa.metricsproxy.metric.model.json.GenericJsonUtil.toGenericApplicationModel;
+import static ai.vespa.metricsproxy.metric.model.json.GenericJsonUtil.toGenericJsonModel;
+import static ai.vespa.metricsproxy.metric.model.json.GenericJsonUtil.toMetricsPackets;
 import static com.yahoo.jdisc.Response.Status.INTERNAL_SERVER_ERROR;
 import static com.yahoo.jdisc.Response.Status.OK;
 
@@ -29,8 +37,11 @@ import static com.yahoo.jdisc.Response.Status.OK;
  */
 public class ApplicationMetricsHandler extends HttpHandlerBase {
 
-    public static final String V1_PATH = "/applicationmetrics/v1";
-    public static final String VALUES_PATH = V1_PATH + "/values";
+    public static final String V1_METRICS = "/applicationmetrics/v1";
+    public static final String METRICS_VALUES_PATH = V1_METRICS + "/values";
+
+    public static final String V1_PROMETHEUS = "/applicationprometheus/v1";
+    public static final String PROMETHEUS_VALUES_PATH = V1_PROMETHEUS + "/values";
 
     private final ApplicationMetricsRetriever metricsRetriever;
     private final MetricsConsumers metricsConsumers;
@@ -46,8 +57,12 @@ public class ApplicationMetricsHandler extends HttpHandlerBase {
 
     @Override
     public Optional<HttpResponse> doHandle(URI requestUri, Path apiPath, String consumer) {
-        if (apiPath.matches(V1_PATH)) return Optional.of(resourceListResponse(requestUri, List.of(VALUES_PATH)));
-        if (apiPath.matches(VALUES_PATH)) return Optional.of(applicationMetricsResponse(consumer));
+        if (apiPath.matches(V1_METRICS)) return Optional.of(resourceListResponse(requestUri, List.of(METRICS_VALUES_PATH)));
+        if (apiPath.matches(METRICS_VALUES_PATH)) return Optional.of(applicationMetricsResponse(consumer));
+
+        if (apiPath.matches(V1_PROMETHEUS)) return Optional.of(resourceListResponse(requestUri, List.of(PROMETHEUS_VALUES_PATH)));
+        if (apiPath.matches(PROMETHEUS_VALUES_PATH)) return Optional.of(applicationPrometheusResponse(consumer));
+
         return Optional.empty();
     }
 
@@ -64,4 +79,16 @@ public class ApplicationMetricsHandler extends HttpHandlerBase {
         }
     }
 
+    private TextResponse applicationPrometheusResponse(String requestedConsumer) {
+        ConsumerId consumer = getConsumerOrDefault(requestedConsumer, metricsConsumers);
+        var metricsByNode = metricsRetriever.getMetrics(consumer);
+
+
+        return new TextResponse(200, PrometheusUtil.toPrometheusModel(toGenericApplicationModel(metricsByNode).nodes.stream()
+                .flatMap(element -> GenericJsonUtil.toMetricsPackets(element).stream()
+                        .map(MetricsPacket.Builder::build))
+                .collect(Collectors.toList()))
+        .serialize());
+    }
+
 }
diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsHandlerTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsHandlerTest.java
index 155bbf094a1..cb0178b1aef 100644
--- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsHandlerTest.java
+++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsHandlerTest.java
@@ -25,8 +25,8 @@ import java.util.concurrent.Executors;
 
 import static ai.vespa.metricsproxy.TestUtil.getFileContents;
 import static ai.vespa.metricsproxy.http.ValuesFetcher.DEFAULT_PUBLIC_CONSUMER_ID;
-import static ai.vespa.metricsproxy.http.application.ApplicationMetricsHandler.V1_PATH;
-import static ai.vespa.metricsproxy.http.application.ApplicationMetricsHandler.VALUES_PATH;
+import static ai.vespa.metricsproxy.http.application.ApplicationMetricsHandler.V1_METRICS;
+import static ai.vespa.metricsproxy.http.application.ApplicationMetricsHandler.METRICS_VALUES_PATH;
 import static ai.vespa.metricsproxy.metric.model.json.JacksonUtil.createObjectMapper;
 import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
 import static com.github.tomakehurst.wiremock.client.WireMock.equalTo;
@@ -48,8 +48,8 @@ public class ApplicationMetricsHandlerTest {
 
     private static final String HOST = "localhost";
     private static final String URI_BASE = "http://" + HOST;
-    private static final String APP_METRICS_V1_URI = URI_BASE + V1_PATH;
-    private static final String APP_METRICS_VALUES_URI = URI_BASE + VALUES_PATH;
+    private static final String APP_METRICS_V1_URI = URI_BASE + V1_METRICS;
+    private static final String APP_METRICS_VALUES_URI = URI_BASE + METRICS_VALUES_PATH;
 
     private static final String TEST_FILE = "generic-sample.json";
     private static final String RESPONSE = getFileContents(TEST_FILE);
-- 
cgit v1.2.3