Polish MicrometerHttpClientInterceptor changes (#1921)

micrometer-metrics · Mar 20, 2020 · e6ca290 · e6ca290
1 parent 6adf2cc
commit e6ca290
Show file tree

Hide file tree

Showing 7 changed files with 71 additions and 8 deletions.
diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/Timer.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/Timer.java
@@ -282,7 +282,7 @@ public long stop(Timer timer) {
 
         /**
          * Records the duration of the operation. Using this method, any tags
-         * stored on the sample are NOT recorded with the timing.
+         * stored on the sample are recorded with the timing.
          *
          * @param registry     The registry to which the timer will be registered.
          * @param timerBuilder The timer to record the sample to.
@@ -297,6 +297,7 @@ public long stop(MeterRegistry registry, Timer.Builder timerBuilder) {
         /**
          * @param tags Must be an even number of arguments representing key/value pairs of tags.
          * @return This builder.
+         * @since 1.4.0
          */
         @Incubating(since = "1.4.0")
         public Sample tags(String... tags) {

diff --git a/...e/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/DefaultUriMapper.java b/...e/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/DefaultUriMapper.java
@@ -21,12 +21,21 @@
 import java.util.function.Function;
 
 /**
- * Extracts the pattern from the request header of the request if available.
+ * Extracts the URI pattern from the predefined request header, {@value URI_PATTERN_HEADER} if available.
+ *
+ * @author Benjamin Hubert
+ * @since 1.4.0
  */
-class DefaultUriMapper implements Function<HttpRequest, String> {
+public class DefaultUriMapper implements Function<HttpRequest, String> {
+
+    /**
+     * Header name for URI pattern.
+     */
+    public static final String URI_PATTERN_HEADER = "URI_PATTERN";
+
     @Override
     public String apply(HttpRequest httpRequest) {
-        Header uriPattern = httpRequest.getLastHeader(MicrometerHttpRequestExecutor.DEFAULT_URI_PATTERN_HEADER);
+        Header uriPattern = httpRequest.getLastHeader(URI_PATTERN_HEADER);
         if (uriPattern != null && uriPattern.getValue() != null) {
             return uriPattern.getValue();
         }

diff --git a/...e/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/HttpContextUtils.java b/...e/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/HttpContextUtils.java
@@ -21,7 +21,7 @@
 import org.apache.http.protocol.HttpContext;
 
 class HttpContextUtils {
-    public static Tags generateTagsForRoute(HttpContext context) {
+    static Tags generateTagsForRoute(HttpContext context) {
         String targetScheme = "UNKNOWN";
         String targetHost = "UNKNOWN";
         String targetPort = "UNKNOWN";

diff --git a/.../io/micrometer/core/instrument/binder/httpcomponents/MicrometerHttpClientInterceptor.java b/.../io/micrometer/core/instrument/binder/httpcomponents/MicrometerHttpClientInterceptor.java
@@ -44,6 +44,7 @@
  *             .build();
  * }</pre>
  *
+ * @author Jon Schneider
  * @since 1.4.0
  */
 @Incubating(since = "1.4.0")
@@ -55,6 +56,14 @@ public class MicrometerHttpClientInterceptor {
     private final HttpRequestInterceptor requestInterceptor;
     private final HttpResponseInterceptor responseInterceptor;
 
+    /**
+     * Create a {@code MicrometerHttpClientInterceptor} instance.
+     *
+     * @param meterRegistry meter registry to bind
+     * @param uriMapper URI mapper to create {@code uri} tag
+     * @param extraTags extra tags
+     * @param exportTagsForRoute whether to export tags for route
+     */
     public MicrometerHttpClientInterceptor(MeterRegistry meterRegistry,
                                            Function<HttpRequest, String> uriMapper,
                                            Iterable<Tag> extraTags,
@@ -70,6 +79,20 @@ public MicrometerHttpClientInterceptor(MeterRegistry meterRegistry,
         };
     }
 
+
+    /**
+     * Create a {@code MicrometerHttpClientInterceptor} instance with {@link DefaultUriMapper}.
+     *
+     * @param meterRegistry meter registry to bind
+     * @param extraTags extra tags
+     * @param exportTagsForRoute whether to export tags for route
+     */
+    public MicrometerHttpClientInterceptor(MeterRegistry meterRegistry,
+                                           Iterable<Tag> extraTags,
+                                           boolean exportTagsForRoute) {
+        this(meterRegistry, new DefaultUriMapper(), extraTags, exportTagsForRoute);
+    }
+
     public HttpRequestInterceptor getRequestInterceptor() {
         return requestInterceptor;
     }

diff --git a/...va/io/micrometer/core/instrument/binder/httpcomponents/MicrometerHttpRequestExecutor.java b/...va/io/micrometer/core/instrument/binder/httpcomponents/MicrometerHttpRequestExecutor.java
@@ -55,8 +55,10 @@ public class MicrometerHttpRequestExecutor extends HttpRequestExecutor {
 
     /**
      * Default header name for URI pattern.
+     * @deprecated use {@link DefaultUriMapper#URI_PATTERN_HEADER} since 1.4.0
      */
-    public static final String DEFAULT_URI_PATTERN_HEADER = "URI_PATTERN";
+    @Deprecated
+    public static final String DEFAULT_URI_PATTERN_HEADER = DefaultUriMapper.URI_PATTERN_HEADER;
 
     private static final String METER_NAME = "httpcomponents.httpclient.request";
 
@@ -161,7 +163,7 @@ public Builder tags(Iterable<Tag> tags) {
          * values, which could cause problems in your meter registry.
          *
          * By default, this feature is almost disabled. It only exposes values
-         * of the {@link #DEFAULT_URI_PATTERN_HEADER} HTTP header.
+         * of the {@value DefaultUriMapper#URI_PATTERN_HEADER} HTTP header.
          *
          * @param uriMapper A mapper that allows mapping and exposing request
          *                  paths.

diff --git a/...micrometer/core/instrument/binder/httpcomponents/MicrometerHttpClientInterceptorTest.java b/...micrometer/core/instrument/binder/httpcomponents/MicrometerHttpClientInterceptorTest.java
@@ -36,6 +36,12 @@
 import static com.github.tomakehurst.wiremock.client.WireMock.anyUrl;
 import static org.assertj.core.api.Assertions.assertThat;
 
+/**
+ * Tests for {@link MicrometerHttpClientInterceptor}.
+ *
+ * @author Jon Schneider
+ * @author Johnny Lim
+ */
 @ExtendWith(WiremockResolver.class)
 class MicrometerHttpClientInterceptorTest {
     private MeterRegistry registry;
@@ -61,12 +67,34 @@ void asyncRequest(@WiremockResolver.Wiremock WireMockServer server) throws Excep
         client.close();
     }
 
+    @Test
+    void uriIsReadFromHttpHeader(@WiremockResolver.Wiremock WireMockServer server) throws Exception {
+        server.stubFor(any(anyUrl()));
+        MicrometerHttpClientInterceptor interceptor = new MicrometerHttpClientInterceptor(registry, Tags.empty(), true);
+        CloseableHttpAsyncClient client = asyncClient(interceptor);
+        client.start();
+        HttpGet request = new HttpGet(server.baseUrl());
+        request.addHeader(DefaultUriMapper.URI_PATTERN_HEADER, "/some/pattern");
+
+        Future<HttpResponse> future = client.execute(request, null);
+        HttpResponse response = future.get();
+
+        assertThat(response.getStatusLine().getStatusCode()).isEqualTo(200);
+        assertThat(registry.get("httpcomponents.httpclient.request").tag("uri", "/some/pattern").timer().count())
+                .isEqualTo(1);
+
+        client.close();
+    }
+
     private CloseableHttpAsyncClient asyncClient() {
         MicrometerHttpClientInterceptor interceptor = new MicrometerHttpClientInterceptor(registry,
                 request -> request.getRequestLine().getUri(),
                 Tags.empty(),
                 true);
+        return asyncClient(interceptor);
+    }
 
+    private CloseableHttpAsyncClient asyncClient(MicrometerHttpClientInterceptor interceptor) {
         return HttpAsyncClients.custom()
                 .addInterceptorFirst(interceptor.getRequestInterceptor())
                 .addInterceptorLast(interceptor.getResponseInterceptor())

diff --git a/...o/micrometer/core/instrument/binder/httpcomponents/MicrometerHttpRequestExecutorTest.java b/...o/micrometer/core/instrument/binder/httpcomponents/MicrometerHttpRequestExecutorTest.java
@@ -121,7 +121,7 @@ void uriIsReadFromHttpHeader(@WiremockResolver.Wiremock WireMockServer server) t
         server.stubFor(any(anyUrl()));
         HttpClient client = client(executor(false));
         HttpGet getWithHeader = new HttpGet(server.baseUrl());
-        getWithHeader.addHeader(MicrometerHttpRequestExecutor.DEFAULT_URI_PATTERN_HEADER, "/some/pattern");
+        getWithHeader.addHeader(DefaultUriMapper.URI_PATTERN_HEADER, "/some/pattern");
         EntityUtils.consume(client.execute(getWithHeader).getEntity());
         assertThat(registry.get(EXPECTED_METER_NAME)
                 .tags("uri", "/some/pattern")