Merge branch 'gjoranv/prometheus-handler' of github.com:vespa-engine/vespa into metrics-proxy-gather-node-metrics

4 files changed, 111 insertions, 103 deletions

diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/HttpHandlerBase.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/HttpHandlerBase.java
new file mode 100644
index 00000000000..d582abdba57
--- /dev/null
+++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/HttpHandlerBase.java
@@ -0,0 +1,90 @@
+/*
+ * Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+ */
+
+package ai.vespa.metricsproxy.http;
+
+import ai.vespa.metricsproxy.core.MetricsConsumers;
+import ai.vespa.metricsproxy.core.MetricsManager;
+import ai.vespa.metricsproxy.service.VespaServices;
+import com.yahoo.container.jdisc.HttpRequest;
+import com.yahoo.container.jdisc.HttpResponse;
+import com.yahoo.container.jdisc.ThreadedHttpRequestHandler;
+import com.yahoo.restapi.Path;
+import org.json.JSONArray;
+import org.json.JSONException;
+import org.json.JSONObject;
+
+import java.net.URI;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.Executor;
+
+import static com.yahoo.jdisc.Response.Status.INTERNAL_SERVER_ERROR;
+import static com.yahoo.jdisc.Response.Status.METHOD_NOT_ALLOWED;
+import static com.yahoo.jdisc.Response.Status.NOT_FOUND;
+import static com.yahoo.jdisc.Response.Status.OK;
+import static com.yahoo.jdisc.http.HttpRequest.Method.GET;
+import static java.util.logging.Level.WARNING;
+
+/**
+ * @author gjoranv
+ */
+public abstract class HttpHandlerBase extends ThreadedHttpRequestHandler {
+
+    protected final ValuesFetcher valuesFetcher;
+
+    protected HttpHandlerBase(Executor executor,
+                              MetricsManager metricsManager,
+                              VespaServices vespaServices,
+                              MetricsConsumers metricsConsumers) {
+        super(executor);
+        valuesFetcher = new ValuesFetcher(metricsManager, vespaServices, metricsConsumers);
+    }
+
+    protected abstract Optional<HttpResponse> doHandle(URI requestUri, Path apiPath, String consumer);
+
+    @Override
+    public final HttpResponse handle(HttpRequest request) {
+        if (request.getMethod() != GET) return new JsonResponse(METHOD_NOT_ALLOWED, "Only GET is supported");
+
+        Path path = new Path(request.getUri());
+
+        return doHandle(request.getUri(), path, getConsumer(request))
+                .orElse(new ErrorResponse(NOT_FOUND, "No content at given path"));
+    }
+
+    private String getConsumer(HttpRequest request) {
+        return request.getProperty("consumer");
+    }
+
+    protected JsonResponse resourceListResponse(URI requestUri, List<String> resources) {
+        try {
+            return new JsonResponse(OK, resourceList(requestUri, resources));
+        } catch (JSONException e) {
+            log.log(WARNING, "Bad JSON construction in generated resource list for " + requestUri.getPath(), e);
+            return new ErrorResponse(INTERNAL_SERVER_ERROR,
+                                     "An error occurred when generating the list of api resources.");
+        }
+    }
+
+    // TODO: Use jackson with a "Resources" class instead of JSONObject
+    private static String resourceList(URI requestUri, List<String> resources) throws JSONException {
+        int port = requestUri.getPort();
+        String host = requestUri.getHost();
+        StringBuilder base = new StringBuilder("http://");
+        base.append(host);
+        if (port >= 0) {
+            base.append(":").append(port);
+        }
+        String uriBase = base.toString();
+        JSONArray linkList = new JSONArray();
+        for (String api : resources) {
+            JSONObject resource = new JSONObject();
+            resource.put("url", uriBase + api);
+            linkList.put(resource);
+        }
+        return new JSONObject().put("resources", linkList).toString(4);
+    }
+
+}
diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/MetricsHandler.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/MetricsHandler.java
index 1eb3429f6d4..1f163dc6b9a 100644
--- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/MetricsHandler.java
+++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/MetricsHandler.java
@@ -10,59 +10,46 @@ import ai.vespa.metricsproxy.metric.model.MetricsPacket;
 import ai.vespa.metricsproxy.metric.model.json.JsonRenderingException;
 import ai.vespa.metricsproxy.service.VespaServices;
 import com.google.inject.Inject;
-import com.yahoo.container.jdisc.HttpRequest;
 import com.yahoo.container.jdisc.HttpResponse;
-import com.yahoo.container.jdisc.ThreadedHttpRequestHandler;
 import com.yahoo.restapi.Path;
 
+import java.net.URI;
 import java.util.List;
+import java.util.Optional;
 import java.util.concurrent.Executor;
 
-import static ai.vespa.metricsproxy.http.RestApiUtil.resourceListResponse;
 import static ai.vespa.metricsproxy.metric.model.json.GenericJsonUtil.toGenericJsonModel;
 import static com.yahoo.jdisc.Response.Status.INTERNAL_SERVER_ERROR;
-import static com.yahoo.jdisc.Response.Status.METHOD_NOT_ALLOWED;
-import static com.yahoo.jdisc.Response.Status.NOT_FOUND;
 import static com.yahoo.jdisc.Response.Status.OK;
-import static com.yahoo.jdisc.http.HttpRequest.Method.GET;
 
 /**
  * Http handler for the metrics/v1 rest api.
  *
  * @author gjoranv
  */
-public class MetricsHandler extends ThreadedHttpRequestHandler {
+public class MetricsHandler extends HttpHandlerBase {
 
     public static final String V1_PATH = "/metrics/v1";
     static final String VALUES_PATH = V1_PATH + "/values";
 
-    private final ValuesFetcher valuesFetcher;
-
     @Inject
     public MetricsHandler(Executor executor,
                           MetricsManager metricsManager,
                           VespaServices vespaServices,
                           MetricsConsumers metricsConsumers) {
-        super(executor);
-
-        valuesFetcher = new ValuesFetcher(metricsManager, vespaServices, metricsConsumers);
+        super(executor, metricsManager, vespaServices, metricsConsumers);
     }
 
     @Override
-    public HttpResponse handle(HttpRequest request) {
-        if (request.getMethod() != GET) return new JsonResponse(METHOD_NOT_ALLOWED, "Only GET is supported");
-
-        Path path = new Path(request.getUri());
-
-        if (path.matches(V1_PATH)) return resourceListResponse(request.getUri(), List.of(VALUES_PATH));
-        if (path.matches(VALUES_PATH)) return valuesResponse(request);
-
-        return new ErrorResponse(NOT_FOUND, "No content at given path");
+    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(valuesResponse(consumer));
+        return Optional.empty();
     }
 
-    private JsonResponse valuesResponse(HttpRequest request) {
+    private JsonResponse valuesResponse(String consumer) {
         try {
-            List<MetricsPacket> metrics =  valuesFetcher.fetch(request.getProperty("consumer"));
+            List<MetricsPacket> metrics =  valuesFetcher.fetch(consumer);
             return new JsonResponse(OK, toGenericJsonModel(metrics).serialize());
         } catch (JsonRenderingException e) {
             return new ErrorResponse(INTERNAL_SERVER_ERROR, e.getMessage());
diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/RestApiUtil.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/RestApiUtil.java
deleted file mode 100644
index 9d9256e17e8..00000000000
--- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/RestApiUtil.java
+++ /dev/null
@@ -1,55 +0,0 @@
-/*
- * Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
- */
-
-package ai.vespa.metricsproxy.http;
-
-import org.json.JSONArray;
-import org.json.JSONException;
-import org.json.JSONObject;
-
-import java.net.URI;
-import java.util.List;
-import java.util.logging.Logger;
-
-import static com.yahoo.jdisc.Response.Status.INTERNAL_SERVER_ERROR;
-import static com.yahoo.jdisc.Response.Status.OK;
-import static java.util.logging.Level.WARNING;
-
-/**
- * @author gjoranv
- */
-public class RestApiUtil {
-    private static Logger log = Logger.getLogger(RestApiUtil.class.getName());
-
-
-    public static JsonResponse resourceListResponse(URI requestUri, List<String> resources) {
-        try {
-            return new JsonResponse(OK, RestApiUtil.resourceList(requestUri, resources));
-        } catch (JSONException e) {
-            log.log(WARNING, "Bad JSON construction in generated resource list for " + requestUri.getPath(), e);
-            return new ErrorResponse(INTERNAL_SERVER_ERROR,
-                                     "An error occurred when generating the list of api resources.");
-        }
-    }
-
-    // TODO: Use jackson with a "Resources" class instead of JSONObject
-    private static String resourceList(URI requestUri, List<String> resources) throws JSONException {
-        int port = requestUri.getPort();
-        String host = requestUri.getHost();
-        StringBuilder base = new StringBuilder("http://");
-        base.append(host);
-        if (port >= 0) {
-            base.append(":").append(port);
-        }
-        String uriBase = base.toString();
-        JSONArray linkList = new JSONArray();
-        for (String api : resources) {
-            JSONObject resource = new JSONObject();
-            resource.put("url", uriBase + api);
-            linkList.put(resource);
-        }
-        return new JSONObject().put("resources", linkList).toString(4);
-    }
-
-}
diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/prometheus/PrometheusHandler.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/prometheus/PrometheusHandler.java
index f2009280128..02f11c7e900 100644
--- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/prometheus/PrometheusHandler.java
+++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/prometheus/PrometheusHandler.java
@@ -6,64 +6,50 @@ package ai.vespa.metricsproxy.http.prometheus;
 
 import ai.vespa.metricsproxy.core.MetricsConsumers;
 import ai.vespa.metricsproxy.core.MetricsManager;
-import ai.vespa.metricsproxy.http.ErrorResponse;
-import ai.vespa.metricsproxy.http.JsonResponse;
+import ai.vespa.metricsproxy.http.HttpHandlerBase;
 import ai.vespa.metricsproxy.http.TextResponse;
-import ai.vespa.metricsproxy.http.ValuesFetcher;
 import ai.vespa.metricsproxy.metric.model.MetricsPacket;
 import ai.vespa.metricsproxy.metric.model.json.JsonRenderingException;
 import ai.vespa.metricsproxy.service.VespaServices;
 import com.google.inject.Inject;
-import com.yahoo.container.jdisc.HttpRequest;
 import com.yahoo.container.jdisc.HttpResponse;
-import com.yahoo.container.jdisc.ThreadedHttpRequestHandler;
 import com.yahoo.restapi.Path;
 
+import java.net.URI;
 import java.util.List;
+import java.util.Optional;
 import java.util.concurrent.Executor;
 
-import static ai.vespa.metricsproxy.http.RestApiUtil.resourceListResponse;
 import static ai.vespa.metricsproxy.metric.model.prometheus.PrometheusUtil.toPrometheusModel;
 import static com.yahoo.jdisc.Response.Status.INTERNAL_SERVER_ERROR;
-import static com.yahoo.jdisc.Response.Status.METHOD_NOT_ALLOWED;
-import static com.yahoo.jdisc.Response.Status.NOT_FOUND;
 import static com.yahoo.jdisc.Response.Status.OK;
-import static com.yahoo.jdisc.http.HttpRequest.Method.GET;
 
 /**
  * @author gjoranv
  */
-public class PrometheusHandler extends ThreadedHttpRequestHandler {
+public class PrometheusHandler extends HttpHandlerBase {
 
     public static final String V1_PATH = "/prometheus/v1";
     static final String VALUES_PATH = V1_PATH + "/values";
 
-    private final ValuesFetcher valuesFetcher;
-
     @Inject
     public PrometheusHandler(Executor executor,
                              MetricsManager metricsManager,
                              VespaServices vespaServices,
                              MetricsConsumers metricsConsumers) {
-        super(executor);
-        valuesFetcher = new ValuesFetcher(metricsManager, vespaServices, metricsConsumers);
+        super(executor, metricsManager, vespaServices, metricsConsumers);
     }
 
     @Override
-    public HttpResponse handle(HttpRequest request) {
-        if (request.getMethod() != GET) return new JsonResponse(METHOD_NOT_ALLOWED, "Only GET is supported");
-
-        Path path = new Path(request.getUri());
-
-        if (path.matches(V1_PATH)) return resourceListResponse(request.getUri(), List.of(VALUES_PATH));
-        if (path.matches(VALUES_PATH)) return valuesResponse(request);
-
-        return new ErrorResponse(NOT_FOUND, "No content at given path");
+    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(valuesResponse(consumer));
+        return Optional.empty();
     }
 
-    private TextResponse valuesResponse(HttpRequest request) {
+    private TextResponse valuesResponse(String consumer) {
         try {
-            List<MetricsPacket> metrics =  valuesFetcher.fetch(request.getProperty("consumer"));
+            List<MetricsPacket> metrics =  valuesFetcher.fetch(consumer);
             return new TextResponse(OK, toPrometheusModel(metrics).serialize());
         } catch (JsonRenderingException e) {
             return new TextResponse(INTERNAL_SERVER_ERROR, e.getMessage());