From 57702988ee62dcf551b018631b59317b8ea97756 Mon Sep 17 00:00:00 2001 From: Ruth Linehan Date: Fri, 30 Sep 2016 14:48:58 -0700 Subject: [PATCH 1/4] (maint) Bump dependencies to newer versions --- project.clj | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/project.clj b/project.clj index 5369698..21d5187 100644 --- a/project.clj +++ b/project.clj @@ -1,5 +1,5 @@ -(def ks-version "1.2.0") -(def tk-version "1.1.1") +(def ks-version "1.3.0") +(def tk-version "1.5.1") (def tk-jetty-version "1.5.0") (defproject puppetlabs/http-client "0.5.1-SNAPSHOT" @@ -17,7 +17,7 @@ [org.apache.httpcomponents/httpasyncclient "4.1.2"] [org.apache.commons/commons-lang3 "3.4"] [prismatic/schema "1.0.4"] - [org.slf4j/slf4j-api "1.7.13"] + [org.slf4j/slf4j-api "1.7.20"] [commons-io "2.4"] [io.dropwizard.metrics/metrics-core "3.1.2"] From 4f92c1ba55923b225ad08bd71580e86fdb2f3cc0 Mon Sep 17 00:00:00 2001 From: Ruth Linehan Date: Fri, 30 Sep 2016 15:35:07 -0700 Subject: [PATCH 2/4] (TK-402) Allow metric namespace to be configurable Add two new client options - `server-id` and `metric-prefix` that allow the metric namespace to be configured rather than always `puppetlabs.http-client.experimental`. If `server-id` is set, the metric namespace becomes `puppetlabs..http-client.experimental`. If `metric-prefix` is set, the metric namespace becomes `.http-client.experimental`. If both are set, `metric-prefix` wins out and a warning message is logged. Also add a `get-client-metric-namespace`/`getMetricNamespace` method on the client (clojure and java) to get back the metric namespace. --- src/clj/puppetlabs/http/client/async.clj | 27 ++-- src/clj/puppetlabs/http/client/common.clj | 7 +- src/clj/puppetlabs/http/client/metrics.clj | 6 +- src/clj/puppetlabs/http/client/sync.clj | 19 +-- .../com/puppetlabs/http/client/Async.java | 8 +- .../http/client/AsyncHttpClient.java | 5 + .../puppetlabs/http/client/ClientOptions.java | 20 +++ src/java/com/puppetlabs/http/client/Sync.java | 5 +- .../http/client/SyncHttpClient.java | 5 + .../http/client/impl/JavaClient.java | 12 +- .../impl/PersistentAsyncHttpClient.java | 11 +- .../client/impl/PersistentSyncHttpClient.java | 11 +- .../http/client/impl/metrics/TimerUtils.java | 20 +-- .../http/client/metrics/Metrics.java | 26 +++- .../http/client/impl/metrics_unit_test.clj | 24 ++-- test/puppetlabs/http/client/metrics_test.clj | 124 ++++++++++++++++++ 16 files changed, 279 insertions(+), 51 deletions(-) diff --git a/src/clj/puppetlabs/http/client/async.clj b/src/clj/puppetlabs/http/client/async.clj index 08cfbcd..e33d2c4 100644 --- a/src/clj/puppetlabs/http/client/async.clj +++ b/src/clj/puppetlabs/http/client/async.clj @@ -16,7 +16,8 @@ (org.apache.http.nio.client HttpAsyncClient) (com.codahale.metrics MetricRegistry)) (:require [puppetlabs.http.client.common :as common] - [schema.core :as schema]) + [schema.core :as schema] + [puppetlabs.http.client.metrics :as metrics]) (:refer-clojure :exclude (get))) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -25,7 +26,8 @@ (schema/defn ^:always-validate create-default-client :- HttpAsyncClient [{:keys [ssl-context ssl-ca-cert ssl-cert ssl-key ssl-protocols cipher-suites follow-redirects force-redirects connect-timeout-milliseconds - socket-timeout-milliseconds metric-registry]}:- common/ClientOptions] + socket-timeout-milliseconds metric-registry server-id + metric-prefix]}:- common/ClientOptions] (let [client-options (ClientOptions.)] (cond-> client-options (some? ssl-context) (.setSslContext ssl-context) @@ -40,7 +42,9 @@ (.setConnectTimeoutMilliseconds connect-timeout-milliseconds) (some? socket-timeout-milliseconds) (.setSocketTimeoutMilliseconds socket-timeout-milliseconds) - (some? metric-registry) (.setMetricRegistry metric-registry)) + (some? metric-registry) (.setMetricRegistry metric-registry) + (some? server-id) (.setServerId server-id) + (some? metric-prefix) (.setMetricPrefix metric-prefix)) (JavaClient/createClient client-options))) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -161,11 +165,12 @@ ([opts :- common/RawUserRequestOptions callback :- common/ResponseCallbackFn client :- HttpAsyncClient] - (request-with-client opts callback client nil)) + (request-with-client opts callback client nil nil)) ([opts :- common/RawUserRequestOptions callback :- common/ResponseCallbackFn client :- HttpAsyncClient - metric-registry :- (schema/maybe MetricRegistry)] + metric-registry :- (schema/maybe MetricRegistry) + metric-namespace :- (schema/maybe schema/Str)] (let [result (promise) defaults {:headers {} :body nil @@ -176,7 +181,8 @@ java-method (clojure-method->java opts) response-delivery-delegate (get-response-delivery-delegate opts result)] (JavaClient/requestWithClient java-request-options java-method callback - client response-delivery-delegate metric-registry) + client response-delivery-delegate metric-registry + metric-namespace) result))) (schema/defn create-client :- (schema/protocol common/HTTPClient) @@ -223,7 +229,8 @@ * :ssl-ca-cert - path to a PEM file containing the CA cert" [opts :- common/ClientOptions] (let [client (create-default-client opts) - metric-registry (:metric-registry opts)] + metric-registry (:metric-registry opts) + metric-namespace (metrics/build-metric-namespace (:metric-prefix opts) (:server-id opts))] (reify common/HTTPClient (get [this url] (common/get this url {})) (get [this url opts] (common/make-request this url :get opts)) @@ -244,6 +251,8 @@ (make-request [this url method] (common/make-request this url method {})) (make-request [_ url method opts] (request-with-client (assoc opts :method method :url url) - nil client metric-registry)) + nil client metric-registry + metric-namespace)) (close [_] (.close client)) - (get-client-metric-registry [_] metric-registry)))) + (get-client-metric-registry [_] metric-registry) + (get-client-metric-namespace [_] metric-namespace)))) diff --git a/src/clj/puppetlabs/http/client/common.clj b/src/clj/puppetlabs/http/client/common.clj index 078672d..8e66837 100644 --- a/src/clj/puppetlabs/http/client/common.clj +++ b/src/clj/puppetlabs/http/client/common.clj @@ -23,7 +23,8 @@ (patch [this url] [this url opts]) (make-request [this url method] [this url method opts]) (close [this]) - (get-client-metric-registry [this])) + (get-client-metric-registry [this]) + (get-client-metric-namespace [this])) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;; Schemas @@ -113,7 +114,9 @@ (ok :follow-redirects) schema/Bool (ok :connect-timeout-milliseconds) schema/Int (ok :socket-timeout-milliseconds) schema/Int - (ok :metric-registry) MetricRegistry}) + (ok :metric-registry) MetricRegistry + (ok :server-id) schema/Str + (ok :metric-prefix) schema/Str}) (def UserRequestOptions "A cleaned-up version of RawUserRequestClientOptions, which is formed after diff --git a/src/clj/puppetlabs/http/client/metrics.clj b/src/clj/puppetlabs/http/client/metrics.clj index 8c89270..af7bf96 100644 --- a/src/clj/puppetlabs/http/client/metrics.clj +++ b/src/clj/puppetlabs/http/client/metrics.clj @@ -2,7 +2,7 @@ (:require [schema.core :as schema] [puppetlabs.http.client.common :as common]) (:import (com.puppetlabs.http.client.metrics Metrics$MetricType Metrics - ClientMetricData) + ClientMetricData) (com.codahale.metrics MetricRegistry))) (schema/defn get-base-metric-data :- common/BaseMetricData @@ -33,6 +33,10 @@ [method] (clojure.string/upper-case (name method))) +(defn build-metric-namespace + [metric-prefix server-id] + (Metrics/buildMetricNamespace metric-prefix server-id)) + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;; Public diff --git a/src/clj/puppetlabs/http/client/sync.clj b/src/clj/puppetlabs/http/client/sync.clj index 0095c49..3c88c1e 100644 --- a/src/clj/puppetlabs/http/client/sync.clj +++ b/src/clj/puppetlabs/http/client/sync.clj @@ -4,7 +4,8 @@ (ns puppetlabs.http.client.sync (:require [puppetlabs.http.client.async :as async] [schema.core :as schema] - [puppetlabs.http.client.common :as common]) + [puppetlabs.http.client.common :as common] + [puppetlabs.http.client.metrics :as metrics]) (:refer-clojure :exclude (get))) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -24,10 +25,10 @@ (defn request-with-client ([req client] - (request-with-client req client nil)) - ([req client metric-registry] + (request-with-client req client nil nil)) + ([req client metric-registry metric-namespace] (let [{:keys [error] :as resp} @(async/request-with-client - req nil client metric-registry)] + req nil client metric-registry metric-namespace)] (if error (throw error) resp)))) @@ -37,12 +38,13 @@ (defn request [req] (with-open [client (async/create-default-client (extract-client-opts req))] - (request-with-client (extract-request-opts req) client nil))) + (request-with-client (extract-request-opts req) client))) (schema/defn create-client :- (schema/protocol common/HTTPClient) [opts :- common/ClientOptions] (let [client (async/create-default-client opts) - metric-registry (:metric-registry opts)] + metric-registry (:metric-registry opts) + metric-namespace (metrics/build-metric-namespace (:metric-prefix opts) (:server-id opts))] (reify common/HTTPClient (get [this url] (common/get this url {})) (get [this url opts] (common/make-request this url :get opts)) @@ -63,9 +65,10 @@ (make-request [this url method] (common/make-request this url method {})) (make-request [_ url method opts] (request-with-client (assoc opts :method method :url url) - client metric-registry)) + client metric-registry metric-namespace)) (close [_] (.close client)) - (get-client-metric-registry [_] metric-registry)))) + (get-client-metric-registry [_] metric-registry) + (get-client-metric-namespace [_] metric-namespace)))) (defn get "Issue a synchronous HTTP GET request. This will raise an exception if an diff --git a/src/java/com/puppetlabs/http/client/Async.java b/src/java/com/puppetlabs/http/client/Async.java index f79eb17..69720a5 100644 --- a/src/java/com/puppetlabs/http/client/Async.java +++ b/src/java/com/puppetlabs/http/client/Async.java @@ -1,10 +1,8 @@ package com.puppetlabs.http.client; -import com.puppetlabs.http.client.impl.SslUtils; import com.puppetlabs.http.client.impl.JavaClient; import com.puppetlabs.http.client.impl.PersistentAsyncHttpClient; -import com.puppetlabs.http.client.impl.CoercedClientOptions; -import com.codahale.metrics.MetricRegistry; +import com.puppetlabs.http.client.metrics.Metrics; /** * This class allows you to create an AsyncHttpClient for use in making @@ -21,7 +19,9 @@ public class Async { * @return an AsyncHttpClient that can be used to make requests */ public static AsyncHttpClient createClient(ClientOptions clientOptions) { + final String metricNamespace = Metrics.buildMetricNamespace(clientOptions.getMetricPrefix(), + clientOptions.getServerId()); return new PersistentAsyncHttpClient(JavaClient.createClient(clientOptions), - clientOptions.getMetricRegistry()); + clientOptions.getMetricRegistry(),metricNamespace); } } diff --git a/src/java/com/puppetlabs/http/client/AsyncHttpClient.java b/src/java/com/puppetlabs/http/client/AsyncHttpClient.java index 62bdb07..bbb45dc 100644 --- a/src/java/com/puppetlabs/http/client/AsyncHttpClient.java +++ b/src/java/com/puppetlabs/http/client/AsyncHttpClient.java @@ -19,6 +19,11 @@ public interface AsyncHttpClient extends Closeable{ */ public MetricRegistry getMetricRegistry(); + /** + * @return the String metricNamespace for this Client + */ + public String getMetricNamespace(); + /** * Performs a GET request * @param url the URL against which to make the GET request diff --git a/src/java/com/puppetlabs/http/client/ClientOptions.java b/src/java/com/puppetlabs/http/client/ClientOptions.java index fcd6a86..823c787 100644 --- a/src/java/com/puppetlabs/http/client/ClientOptions.java +++ b/src/java/com/puppetlabs/http/client/ClientOptions.java @@ -27,6 +27,8 @@ public class ClientOptions { private int connectTimeoutMilliseconds = -1; private int socketTimeoutMilliseconds = -1; private MetricRegistry metricRegistry; + private String metricPrefix; + private String serverId; /** * Constructor for the ClientOptions class. When this constructor is called, @@ -183,4 +185,22 @@ public class ClientOptions { this.metricRegistry = metricRegistry; return this; } + + public String getMetricPrefix() { + return metricPrefix; + } + + public ClientOptions setMetricPrefix(String metricPrefix) { + this.metricPrefix = metricPrefix; + return this; + } + + public String getServerId() { + return serverId; + } + + public ClientOptions setServerId(String serverId) { + this.serverId = serverId; + return this; + } } diff --git a/src/java/com/puppetlabs/http/client/Sync.java b/src/java/com/puppetlabs/http/client/Sync.java index ed28908..9c4cc3e 100644 --- a/src/java/com/puppetlabs/http/client/Sync.java +++ b/src/java/com/puppetlabs/http/client/Sync.java @@ -2,6 +2,7 @@ package com.puppetlabs.http.client; import com.puppetlabs.http.client.impl.JavaClient; import com.puppetlabs.http.client.impl.PersistentSyncHttpClient; +import com.puppetlabs.http.client.metrics.Metrics; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -84,8 +85,10 @@ public class Sync { * @return A persistent synchronous HTTP client */ public static SyncHttpClient createClient(ClientOptions clientOptions) { + final String metricNamespace = Metrics.buildMetricNamespace(clientOptions.getMetricPrefix(), + clientOptions.getServerId()); return new PersistentSyncHttpClient(JavaClient.createClient(clientOptions), - clientOptions.getMetricRegistry()); + clientOptions.getMetricRegistry(), metricNamespace); } /** diff --git a/src/java/com/puppetlabs/http/client/SyncHttpClient.java b/src/java/com/puppetlabs/http/client/SyncHttpClient.java index c6ac7c8..da94f89 100644 --- a/src/java/com/puppetlabs/http/client/SyncHttpClient.java +++ b/src/java/com/puppetlabs/http/client/SyncHttpClient.java @@ -18,6 +18,11 @@ public interface SyncHttpClient extends Closeable { */ public MetricRegistry getMetricRegistry(); + /** + * @return the String metricNamespace for this Client + */ + public String getMetricNamespace(); + /** * Makes a configurable HTTP request * @param requestOptions the options to configure the request diff --git a/src/java/com/puppetlabs/http/client/impl/JavaClient.java b/src/java/com/puppetlabs/http/client/impl/JavaClient.java index d18c74e..7bae5b5 100644 --- a/src/java/com/puppetlabs/http/client/impl/JavaClient.java +++ b/src/java/com/puppetlabs/http/client/impl/JavaClient.java @@ -263,7 +263,8 @@ public class JavaClient { final FutureCallback futureCallback, final HttpRequestBase request, final MetricRegistry metricRegistry, - final String[] metricId) { + final String[] metricId, + final String metricNamespace) { /* * Create an Apache AsyncResponseConsumer that will return the response to us as soon as it is available, @@ -315,7 +316,7 @@ public class JavaClient { TimedFutureCallback timedStreamingCompleteCallback = new TimedFutureCallback<>(streamingCompleteCallback, - TimerUtils.startFullResponseTimers(metricRegistry, request, metricId)); + TimerUtils.startFullResponseTimers(metricRegistry, request, metricId, metricNamespace)); client.execute(HttpAsyncMethods.create(request), consumer, timedStreamingCompleteCallback); } @@ -324,7 +325,8 @@ public class JavaClient { final IResponseCallback callback, final CloseableHttpAsyncClient client, final ResponseDeliveryDelegate responseDeliveryDelegate, - final MetricRegistry registry) { + final MetricRegistry registry, + final String metricNamespace) { final CoercedRequestOptions coercedRequestOptions = coerceRequestOptions(requestOptions, method); @@ -355,11 +357,11 @@ public class JavaClient { final String[] metricId = requestOptions.getMetricId(); if (requestOptions.getAs() == ResponseBodyType.UNBUFFERED_STREAM) { - executeWithConsumer(client, futureCallback, request, registry, metricId); + executeWithConsumer(client, futureCallback, request, registry, metricId, metricNamespace); } else { TimedFutureCallback timedFutureCallback = new TimedFutureCallback<>(futureCallback, - TimerUtils.startFullResponseTimers(registry, request, metricId)); + TimerUtils.startFullResponseTimers(registry, request, metricId, metricNamespace)); client.execute(request, timedFutureCallback); } } diff --git a/src/java/com/puppetlabs/http/client/impl/PersistentAsyncHttpClient.java b/src/java/com/puppetlabs/http/client/impl/PersistentAsyncHttpClient.java index ecc125c..cf36070 100644 --- a/src/java/com/puppetlabs/http/client/impl/PersistentAsyncHttpClient.java +++ b/src/java/com/puppetlabs/http/client/impl/PersistentAsyncHttpClient.java @@ -14,11 +14,14 @@ import java.net.URISyntaxException; public class PersistentAsyncHttpClient implements AsyncHttpClient { private CloseableHttpAsyncClient client; private MetricRegistry metricRegistry; + private String metricNamespace; public PersistentAsyncHttpClient(CloseableHttpAsyncClient client, - MetricRegistry metricRegistry) { + MetricRegistry metricRegistry, + String metricNamespace) { this.client = client; this.metricRegistry = metricRegistry; + this.metricNamespace = metricNamespace; } public void close() throws IOException { @@ -29,11 +32,15 @@ public class PersistentAsyncHttpClient implements AsyncHttpClient { return metricRegistry; } + public String getMetricNamespace() { + return metricNamespace; + } + private Promise request(RequestOptions requestOptions, HttpMethod method) { final Promise promise = new Promise<>(); final JavaResponseDeliveryDelegate responseDelivery = new JavaResponseDeliveryDelegate(promise); JavaClient.requestWithClient(requestOptions, method, null, - client, responseDelivery, metricRegistry); + client, responseDelivery, metricRegistry, metricNamespace); return promise; } diff --git a/src/java/com/puppetlabs/http/client/impl/PersistentSyncHttpClient.java b/src/java/com/puppetlabs/http/client/impl/PersistentSyncHttpClient.java index bc2ff99..b272a05 100644 --- a/src/java/com/puppetlabs/http/client/impl/PersistentSyncHttpClient.java +++ b/src/java/com/puppetlabs/http/client/impl/PersistentSyncHttpClient.java @@ -17,12 +17,15 @@ import java.net.URISyntaxException; public class PersistentSyncHttpClient implements SyncHttpClient { private CloseableHttpAsyncClient client; private MetricRegistry metricRegistry; + private String metricNamespace; private static final Logger LOGGER = LoggerFactory.getLogger(PersistentSyncHttpClient.class); public PersistentSyncHttpClient(CloseableHttpAsyncClient client, - MetricRegistry metricRegistry) { + MetricRegistry metricRegistry, + String metricNamespace) { this.client = client; this.metricRegistry = metricRegistry; + this.metricNamespace = metricNamespace; } private static void logAndRethrow(String msg, Throwable t) { @@ -34,11 +37,15 @@ public class PersistentSyncHttpClient implements SyncHttpClient { return metricRegistry; } + public String getMetricNamespace() { + return metricNamespace; + } + public Response request(RequestOptions requestOptions, HttpMethod method) { final Promise promise = new Promise<>(); final JavaResponseDeliveryDelegate responseDelivery = new JavaResponseDeliveryDelegate(promise); JavaClient.requestWithClient(requestOptions, method, null, client, - responseDelivery, metricRegistry); + responseDelivery, metricRegistry, metricNamespace); Response response = null; try { response = promise.deref(); diff --git a/src/java/com/puppetlabs/http/client/impl/metrics/TimerUtils.java b/src/java/com/puppetlabs/http/client/impl/metrics/TimerUtils.java index 2d12b84..e782baa 100644 --- a/src/java/com/puppetlabs/http/client/impl/metrics/TimerUtils.java +++ b/src/java/com/puppetlabs/http/client/impl/metrics/TimerUtils.java @@ -41,7 +41,8 @@ public class TimerUtils { } private static ArrayList startFullResponseMetricIdTimers(MetricRegistry registry, - String[] metricId) { + String[] metricId, + String metricPrefix) { ArrayList timerContexts = new ArrayList<>(); for (int i = 0; i < metricId.length; i++) { ArrayList currentId = new ArrayList<>(); @@ -52,7 +53,7 @@ public class TimerUtils { currentIdWithNamespace.add(Metrics.NAMESPACE_METRIC_ID); currentIdWithNamespace.addAll(currentId); currentIdWithNamespace.add(Metrics.NAMESPACE_FULL_RESPONSE); - String metric_name = MetricRegistry.name(Metrics.NAMESPACE_PREFIX, + String metric_name = MetricRegistry.name(metricPrefix, currentIdWithNamespace.toArray(new String[currentIdWithNamespace.size()])); ClientTimer timer = new MetricIdClientTimer(metric_name, currentId, Metrics.MetricType.FULL_RESPONSE); @@ -62,16 +63,17 @@ public class TimerUtils { } private static ArrayList startFullResponseUrlTimers(MetricRegistry registry, - HttpRequest request) { + HttpRequest request, + String metricPrefix) { ArrayList timerContexts = new ArrayList<>(); try { final RequestLine requestLine = request.getRequestLine(); final String strippedUrl = Metrics.urlToMetricUrl(requestLine.getUri()); final String method = requestLine.getMethod(); - final String urlName = MetricRegistry.name(Metrics.NAMESPACE_PREFIX, Metrics.NAMESPACE_URL, + final String urlName = MetricRegistry.name(metricPrefix, Metrics.NAMESPACE_URL, strippedUrl, Metrics.NAMESPACE_FULL_RESPONSE); - final String urlAndMethodName = MetricRegistry.name(Metrics.NAMESPACE_PREFIX, Metrics.NAMESPACE_URL_AND_METHOD, + final String urlAndMethodName = MetricRegistry.name(metricPrefix, Metrics.NAMESPACE_URL_AND_METHOD, strippedUrl, method, Metrics.NAMESPACE_FULL_RESPONSE); ClientTimer urlTimer = new UrlClientTimer(urlName, strippedUrl, Metrics.MetricType.FULL_RESPONSE); @@ -91,13 +93,14 @@ public class TimerUtils { public static ArrayList startFullResponseTimers(MetricRegistry clientRegistry, HttpRequest request, - String[] metricId) { + String[] metricId, + String metricNamespace) { if (clientRegistry != null) { - ArrayList urlTimerContexts = startFullResponseUrlTimers(clientRegistry,request); + ArrayList urlTimerContexts = startFullResponseUrlTimers(clientRegistry, request, metricNamespace); ArrayList allTimerContexts = new ArrayList<>(urlTimerContexts); if (metricId != null) { ArrayList metricIdTimers = - startFullResponseMetricIdTimers(clientRegistry, metricId); + startFullResponseMetricIdTimers(clientRegistry, metricId, metricNamespace); allTimerContexts.addAll(metricIdTimers); } return allTimerContexts; @@ -106,4 +109,5 @@ public class TimerUtils { return null; } } + } diff --git a/src/java/com/puppetlabs/http/client/metrics/Metrics.java b/src/java/com/puppetlabs/http/client/metrics/Metrics.java index 6c16de8..5edd401 100644 --- a/src/java/com/puppetlabs/http/client/metrics/Metrics.java +++ b/src/java/com/puppetlabs/http/client/metrics/Metrics.java @@ -8,6 +8,8 @@ import com.puppetlabs.http.client.impl.metrics.MetricIdClientTimerFilter; import com.puppetlabs.http.client.impl.metrics.TimerMetricData; import com.puppetlabs.http.client.impl.metrics.UrlAndMethodClientTimerFilter; import com.puppetlabs.http.client.impl.metrics.UrlClientTimerFilter; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.net.URI; import java.net.URISyntaxException; @@ -17,14 +19,36 @@ import java.util.List; import java.util.Map; public class Metrics { - public static final String NAMESPACE_PREFIX = "puppetlabs.http-client.experimental"; + public static final String PUPPETLABS_NAMESPACE_PREFIX = "puppetlabs"; + public static final String HTTP_CLIENT_NAMESPACE_PREFIX = "http-client.experimental"; + public static final String DEFAULT_NAMESPACE_PREFIX = PUPPETLABS_NAMESPACE_PREFIX + + "." + HTTP_CLIENT_NAMESPACE_PREFIX; public static final String NAMESPACE_URL = "with-url"; public static final String NAMESPACE_URL_AND_METHOD = "with-url-and-method"; public static final String NAMESPACE_METRIC_ID = "with-metric-id"; public static final String NAMESPACE_FULL_RESPONSE = "full-response"; + private static final Logger LOGGER = LoggerFactory.getLogger(Metrics.class); + + public static String buildMetricNamespace(String metricPrefix, String serverId) { + if (metricPrefix != null) { + if (serverId != null) { + Metrics.LOGGER.warn("Metric prefix and server id both set. Using metric prefix '" + + metricPrefix + "' for metric namespace."); + } + return metricPrefix + "." + HTTP_CLIENT_NAMESPACE_PREFIX; + } else if (serverId != null) { + return PUPPETLABS_NAMESPACE_PREFIX + "." + serverId + "." + + HTTP_CLIENT_NAMESPACE_PREFIX; + } else { + return DEFAULT_NAMESPACE_PREFIX; + } + } + public enum MetricType { FULL_RESPONSE } public enum MetricCategory { URL, URL_AND_METHOD, METRIC_ID } + + public static String urlToMetricUrl(String uriString) throws URISyntaxException { final URI uri = new URI(uriString); final URI convertedUri = new URI(uri.getScheme(), null, uri.getHost(), diff --git a/test/com/puppetlabs/http/client/impl/metrics_unit_test.clj b/test/com/puppetlabs/http/client/impl/metrics_unit_test.clj index 6db5869..3f56a91 100644 --- a/test/com/puppetlabs/http/client/impl/metrics_unit_test.clj +++ b/test/com/puppetlabs/http/client/impl/metrics_unit_test.clj @@ -22,19 +22,22 @@ (let [metric-registry (MetricRegistry.)] (TimerUtils/startFullResponseTimers metric-registry (BasicHttpRequest. "GET" "http://localhost/foo") - nil) + nil + Metrics/DEFAULT_NAMESPACE_PREFIX) (is (= (set (list url-id url-method-id)) (set (keys (.getTimers metric-registry))))))) (testing "metric id timers are not created for a request with an empty metric id" (let [metric-registry (MetricRegistry.)] (TimerUtils/startFullResponseTimers metric-registry (BasicHttpRequest. "GET" "http://localhost/foo") - (into-array String [])) + (into-array String []) + Metrics/DEFAULT_NAMESPACE_PREFIX) (is (= (set (list url-id url-method-id)) (set (keys (.getTimers metric-registry))))))) (testing "metric id timers are created correctly for a request with a metric id" (let [metric-registry (MetricRegistry.)] (TimerUtils/startFullResponseTimers metric-registry (BasicHttpRequest. "GET" "http://localhost/foo") - (into-array ["foo" "bar" "baz"])) + (into-array ["foo" "bar" "baz"]) + Metrics/DEFAULT_NAMESPACE_PREFIX) (is (= (set (list url-id url-method-id (add-metric-ns "with-metric-id.foo.full-response") (add-metric-ns "with-metric-id.foo.bar.full-response") @@ -45,21 +48,25 @@ (TimerUtils/startFullResponseTimers metric-registry (BasicHttpRequest. "GET" "http://user:pwd@localhost:1234/foo%2cbar/baz?te%2cst=one") - nil) + nil + Metrics/DEFAULT_NAMESPACE_PREFIX) (TimerUtils/startFullResponseTimers metric-registry (BasicHttpRequest. "GET" "http://user:pwd@localhost:1234/foo%2cbar/baz#x%2cyz") - nil) + nil + Metrics/DEFAULT_NAMESPACE_PREFIX) (TimerUtils/startFullResponseTimers metric-registry (BasicHttpRequest. "GET" "http://user:pwd@localhost:1234/foo%2cbar/baz?te%2cst=one#x%2cyz") - nil) + nil + Metrics/DEFAULT_NAMESPACE_PREFIX) (TimerUtils/startFullResponseTimers metric-registry (BasicHttpRequest. "GET" "http://user:pwd@localhost:1234/foo%2cbar/baz?#x%2cyz") - nil) + nil + Metrics/DEFAULT_NAMESPACE_PREFIX) (is (= (set (list (add-metric-ns "with-url.http://localhost:1234/foo,bar/baz.full-response") @@ -81,7 +88,8 @@ (doseq [timer (TimerUtils/startFullResponseTimers registry req - id)] + id + Metrics/DEFAULT_NAMESPACE_PREFIX)] (.stop timer))) (deftest get-client-metrics-data-test diff --git a/test/puppetlabs/http/client/metrics_test.clj b/test/puppetlabs/http/client/metrics_test.clj index f3ce46d..27a8d6b 100644 --- a/test/puppetlabs/http/client/metrics_test.clj +++ b/test/puppetlabs/http/client/metrics_test.clj @@ -613,3 +613,127 @@ (catch TimeoutException e ;; Expected whenever a server-side failure is generated )))))) + +(deftest metric-namespace-test + (let [metric-prefix "my-metric-prefix" + server-id "my-server" + metric-name-with-prefix + (format "%s.http-client.experimental.with-url.%s.full-response" metric-prefix hello-url) + metric-name-with-server-id + (format "puppetlabs.%s.http-client.experimental.with-url.%s.full-response" + server-id hello-url) + get-metric-name (fn [metric-registry] + (.getMetricName (first (Metrics/getClientMetricsDataByUrl + metric-registry hello-url))))] + (testlogging/with-test-logging + (testutils/with-app-with-config + app + [jetty9/jetty9-service test-metric-web-service] + {:webserver {:port 10000}} + (testing "custom metric namespace works for java async client" + (testing "metric prefix works" + (let [metric-registry (MetricRegistry.)] + (with-open [client-with-metric-prefix (Async/createClient + (doto (ClientOptions.) + (.setMetricRegistry metric-registry) + (.setMetricPrefix metric-prefix)))] + (is (= (format "%s.http-client.experimental" metric-prefix) + (.getMetricNamespace client-with-metric-prefix))) + (-> client-with-metric-prefix (.get (RequestOptions. hello-url))) + (is (= metric-name-with-prefix (get-metric-name metric-registry)))))) + (testing "server id works" + (let [metric-registry (MetricRegistry.)] + (with-open [client-with-server-id (Async/createClient + (doto (ClientOptions.) + (.setMetricRegistry metric-registry) + (.setServerId server-id)))] + (-> client-with-server-id (.get (RequestOptions. hello-url))) + (is (= metric-name-with-server-id (get-metric-name metric-registry)))))) + (testing "metric prefix overrides server id if both are set" + (let [metric-registry (MetricRegistry.)] + (with-open [client-with-server-id (Async/createClient + (doto (ClientOptions.) + (.setMetricRegistry metric-registry) + (.setMetricPrefix metric-prefix) + (.setServerId server-id)))] + (-> client-with-server-id (.get (RequestOptions. hello-url))) + (is (= metric-name-with-prefix (get-metric-name metric-registry))) + (is (logged? #"Metric prefix and server id both set.*" :warn)))))) + (testing "custom metric namespace works for clojure async client" + (testing "metric prefix works" + (let [metric-registry (MetricRegistry.)] + (with-open [client-with-metric-prefix (async/create-client + {:metric-registry metric-registry + :metric-prefix metric-prefix})] + (is (= (format "%s.http-client.experimental" metric-prefix) + (common/get-client-metric-namespace client-with-metric-prefix))) + (-> client-with-metric-prefix (common/get hello-url)) + (is (= metric-name-with-prefix (get-metric-name metric-registry)))))) + (testing "server id works" + (let [metric-registry (MetricRegistry.)] + (with-open [client-with-server-id (async/create-client + {:metric-registry metric-registry + :server-id server-id})] + (-> client-with-server-id (common/get hello-url)) + (is (= metric-name-with-server-id (get-metric-name metric-registry)))))) + (testing "metric prefix overrides server id if both are set" + (let [metric-registry (MetricRegistry.)] + (with-open [client-with-server-id (async/create-client + {:metric-registry metric-registry + :metric-prefix metric-prefix + :server-id server-id})] + (-> client-with-server-id (common/get hello-url)) + (is (= metric-name-with-prefix (get-metric-name metric-registry))))))) + (testing "custom metric namespace works for Java sync client" + (testing "metric prefix works" + (let [metric-registry (MetricRegistry.)] + (with-open [client-with-metric-prefix (Sync/createClient + (doto (ClientOptions.) + (.setMetricRegistry metric-registry) + (.setMetricPrefix metric-prefix)))] + (is (= (format "%s.http-client.experimental" metric-prefix) + (.getMetricNamespace client-with-metric-prefix))) + (-> client-with-metric-prefix (.get (RequestOptions. hello-url))) + (is (= metric-name-with-prefix (get-metric-name metric-registry)))))) + (testing "server id works" + (let [metric-registry (MetricRegistry.)] + (with-open [client-with-server-id (Sync/createClient + (doto (ClientOptions.) + (.setMetricRegistry metric-registry) + (.setServerId server-id)))] + (-> client-with-server-id (.get (RequestOptions. hello-url))) + (is (= metric-name-with-server-id (get-metric-name metric-registry)))))) + (testing "metric prefix overrides server id if both are set" + (let [metric-registry (MetricRegistry.)] + (with-open [client-with-server-id (Sync/createClient + (doto (ClientOptions.) + (.setMetricRegistry metric-registry) + (.setMetricPrefix metric-prefix) + (.setServerId server-id)))] + (-> client-with-server-id (.get (RequestOptions. hello-url))) + (is (= metric-name-with-prefix (get-metric-name metric-registry))))))) + (testing "custom metric namespace works for clojure sync client" + (testing "metric prefix works" + (let [metric-registry (MetricRegistry.)] + (with-open [client-with-metric-prefix (sync/create-client + {:metric-registry metric-registry + :metric-prefix metric-prefix})] + (is (= (format "%s.http-client.experimental" metric-prefix) + (common/get-client-metric-namespace client-with-metric-prefix))) + (-> client-with-metric-prefix (common/get hello-url)) + (is (= metric-name-with-prefix (get-metric-name metric-registry)))))) + (testing "server id works" + (let [metric-registry (MetricRegistry.)] + (with-open [client-with-server-id (sync/create-client + {:metric-registry metric-registry + :server-id server-id})] + (-> client-with-server-id (common/get hello-url)) + (is (= metric-name-with-server-id (get-metric-name metric-registry)))))) + (testing "metric prefix overrides server id if both are set" + (let [metric-registry (MetricRegistry.)] + (with-open [client-with-server-id (sync/create-client + {:metric-registry metric-registry + :metric-prefix metric-prefix + :server-id server-id})] + (-> client-with-server-id (common/get hello-url)) + (is (= metric-name-with-prefix (get-metric-name metric-registry))))))))))) From 5237287c2be5d5392ab83dbb822da9fd0d038e11 Mon Sep 17 00:00:00 2001 From: Ruth Linehan Date: Mon, 3 Oct 2016 12:00:03 -0700 Subject: [PATCH 3/4] (TK-402) Update docs to mention configurable metric namespace --- doc/clojure-client.md | 5 +++++ doc/metrics.md | 23 ++++++++++++++++++++--- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/doc/clojure-client.md b/doc/clojure-client.md index 311385c..cb2984e 100644 --- a/doc/clojure-client.md +++ b/doc/clojure-client.md @@ -38,6 +38,11 @@ The following are the base set of options supported by the `create-client` funct to. If provided, metrics will automatically be registered for all requests made by the client. See the [metrics documentation](./metrics.md) for more info. +* `:server-id`: a string for the name of the server the request is being made + from. If specified, used in the namespace for metrics: + `puppetlabs..http-client.experimental`. +* `:metric-prefix`: a string for the prefix for metrics. If specified, metric + namespace is `.http-client.experimental`. ### SSL Options diff --git a/doc/metrics.md b/doc/metrics.md index d23177f..adfd8f7 100644 --- a/doc/metrics.md +++ b/doc/metrics.md @@ -10,7 +10,7 @@ change. For using metrics with either the Java client or the Clojure client you must already have created a Dropwizard `MetricRegistry`. -- [Metrics prefix](#metrics-prefix) +- [Metric namespace](#metric-namespace) - [Types of metrics](#types-of-metrics) - [Getting back metrics](#getting-back-metrics) - [Clojure API](#clojure-api) @@ -29,9 +29,26 @@ already have created a Dropwizard `MetricRegistry`. - [Filtering by metric-id](#filtering-by-metric-id-1) -## Metrics prefix +## Metric namespace -All http metrics are prefixed with `puppetlabs.http-client.experimental`. +By default, http metrics are prefixed with the namespace +`puppetlabs.http-client.experimental`. This namespace can be customized with two +client options, `server-id` and `metric-prefix`. + +When `server-id` is set, the metric namespace becomes +`puppetlabs..http-client.experimental`. + +When `metric-prefix` is set, the metric namespace becomes +`.http-client.experimental`. + +If both `server-id` and `metric-prefix` are set, `metric-prefix` wins out and +a warning message is logged. + +For a Clojure client, the `get-client-metric-namespace` protocol method can +be used to get back the metric namespace set for the client. + +For a Java client, the `getMetricNamespace` method can be used to get back the +configured metric namespace. ## Types of metrics From af9f144e1de6617d27e91e7a226fb626cd3e0ae9 Mon Sep 17 00:00:00 2001 From: Ruth Linehan Date: Tue, 4 Oct 2016 11:42:27 -0700 Subject: [PATCH 4/4] (TK-402) Add note about precedence of metric-prefix and server-id options Add note to clojure client docs about precendence of `metric-prefix` option if both it and `server-id` are specified as client options. Also remove some empty lines. --- doc/clojure-client.md | 4 +++- .../com/puppetlabs/http/client/impl/metrics/TimerUtils.java | 1 - src/java/com/puppetlabs/http/client/metrics/Metrics.java | 2 -- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/doc/clojure-client.md b/doc/clojure-client.md index cb2984e..6186f00 100644 --- a/doc/clojure-client.md +++ b/doc/clojure-client.md @@ -42,7 +42,9 @@ The following are the base set of options supported by the `create-client` funct from. If specified, used in the namespace for metrics: `puppetlabs..http-client.experimental`. * `:metric-prefix`: a string for the prefix for metrics. If specified, metric - namespace is `.http-client.experimental`. + namespace is `.http-client.experimental`. If both + `metric-prefix` and `server-id` are specified, `metric-prefix` takes + precendence. ### SSL Options diff --git a/src/java/com/puppetlabs/http/client/impl/metrics/TimerUtils.java b/src/java/com/puppetlabs/http/client/impl/metrics/TimerUtils.java index e782baa..e7e449a 100644 --- a/src/java/com/puppetlabs/http/client/impl/metrics/TimerUtils.java +++ b/src/java/com/puppetlabs/http/client/impl/metrics/TimerUtils.java @@ -109,5 +109,4 @@ public class TimerUtils { return null; } } - } diff --git a/src/java/com/puppetlabs/http/client/metrics/Metrics.java b/src/java/com/puppetlabs/http/client/metrics/Metrics.java index 5edd401..965e1fc 100644 --- a/src/java/com/puppetlabs/http/client/metrics/Metrics.java +++ b/src/java/com/puppetlabs/http/client/metrics/Metrics.java @@ -47,8 +47,6 @@ public class Metrics { public enum MetricType { FULL_RESPONSE } public enum MetricCategory { URL, URL_AND_METHOD, METRIC_ID } - - public static String urlToMetricUrl(String uriString) throws URISyntaxException { final URI uri = new URI(uriString); final URI convertedUri = new URI(uri.getScheme(), null, uri.getHost(),