Polish "Apache HTTP client instrumentation with Observation"

See micrometer-metricsgh-3312
izeye · Sep 18, 2022 · 511ec47 · 511ec47
1 parent 05a1d93
commit 511ec47
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 23 deletions.
diff --git a/...crometer/core/instrument/binder/httpcomponents/ApacheHttpClientDocumentedObservation.java b/...crometer/core/instrument/binder/httpcomponents/ApacheHttpClientDocumentedObservation.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2017 VMware, Inc.
+ * Copyright 2022 VMware, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -35,11 +35,11 @@ public Class<? extends ObservationConvention<? extends Observation.Context>> get
 
         @Override
         public KeyName[] getLowCardinalityKeyNames() {
-            return ApacheHttpClientTags.values();
+            return ApacheHttpClientKeyNames.values();
         }
     };
 
-    enum ApacheHttpClientTags implements KeyName {
+    enum ApacheHttpClientKeyNames implements KeyName {
 
         STATUS {
             @Override

diff --git a/...r/core/instrument/binder/httpcomponents/DefaultApacheHttpClientObservationConvention.java b/...r/core/instrument/binder/httpcomponents/DefaultApacheHttpClientObservationConvention.java
@@ -51,11 +51,11 @@ public String getContextualName(ApacheHttpClientContext context) {
     @Override
     public KeyValues getLowCardinalityKeyValues(ApacheHttpClientContext context) {
         KeyValues keyValues = KeyValues.of(
-                ApacheHttpClientDocumentedObservation.ApacheHttpClientTags.METHOD
+                ApacheHttpClientDocumentedObservation.ApacheHttpClientKeyNames.METHOD
                         .withValue(getMethodString(context.getCarrier())),
-                ApacheHttpClientDocumentedObservation.ApacheHttpClientTags.URI
+                ApacheHttpClientDocumentedObservation.ApacheHttpClientKeyNames.URI
                         .withValue(context.getUriMapper().apply(context.getCarrier())),
-                ApacheHttpClientDocumentedObservation.ApacheHttpClientTags.STATUS
+                ApacheHttpClientDocumentedObservation.ApacheHttpClientKeyNames.STATUS
                         .withValue(getStatusValue(context.getResponse())));
         if (context.shouldExportTagsForRoute()) {
             keyValues = keyValues.and(HttpContextUtils.generateTagStringsForRoute(context.getApacheHttpContext()));

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
@@ -37,9 +37,9 @@ static String[] generateTagStringsForRoute(HttpContext context) {
             targetHost = host.getHostName();
             targetPort = String.valueOf(host.getPort());
         }
-        return new String[] { ApacheHttpClientDocumentedObservation.ApacheHttpClientTags.TARGET_SCHEME.asString(),
-                targetScheme, ApacheHttpClientDocumentedObservation.ApacheHttpClientTags.TARGET_HOST.asString(),
-                targetHost, ApacheHttpClientDocumentedObservation.ApacheHttpClientTags.TARGET_PORT.asString(),
+        return new String[] { ApacheHttpClientDocumentedObservation.ApacheHttpClientKeyNames.TARGET_SCHEME.asString(),
+                targetScheme, ApacheHttpClientDocumentedObservation.ApacheHttpClientKeyNames.TARGET_HOST.asString(),
+                targetHost, ApacheHttpClientDocumentedObservation.ApacheHttpClientKeyNames.TARGET_PORT.asString(),
                 targetPort };
     }
 

diff --git a/...va/io/micrometer/core/instrument/binder/httpcomponents/MicrometerHttpRequestExecutor.java b/...va/io/micrometer/core/instrument/binder/httpcomponents/MicrometerHttpRequestExecutor.java
@@ -218,6 +218,7 @@ public Builder exportTagsForRoute(boolean exportTagsForRoute) {
          * API instead of directly with a {@link Timer}.
          * @param observationRegistry registry with which to instrument
          * @return This builder instance.
+         * @since 1.10.0
          */
         public Builder observationRegistry(ObservationRegistry observationRegistry) {
             this.observationRegistry = observationRegistry;
@@ -226,11 +227,12 @@ public Builder observationRegistry(ObservationRegistry observationRegistry) {
 
         /**
          * Provide a custom convention to override the default convention used when
-         * instrumenting with the {@link Observation} API. This only takes effect when a
+         * instrumenting with the {@link Observation} API. This only takes effect when an
          * {@link #observationRegistry(ObservationRegistry)} is configured.
          * @param convention semantic convention to use
          * @return This builder instance.
          * @see #observationRegistry(ObservationRegistry)
+         * @since 1.10.0
          */
         public Builder observationConvention(ApacheHttpClientObservationConvention convention) {
             this.observationConvention = convention;

diff --git a/...o/micrometer/core/instrument/binder/httpcomponents/MicrometerHttpRequestExecutorTest.java b/...o/micrometer/core/instrument/binder/httpcomponents/MicrometerHttpRequestExecutorTest.java
@@ -265,14 +265,18 @@ void contextualNameContainsRequestMethod(String method, @WiremockResolver.Wiremo
         MicrometerHttpRequestExecutor micrometerHttpRequestExecutor = MicrometerHttpRequestExecutor.builder(registry)
                 .observationRegistry(observationRegistry).build();
         HttpClient client = client(micrometerHttpRequestExecutor);
-        if ("get".contentEquals(method)) {
-            EntityUtils.consume(client.execute(new HttpGet(server.baseUrl())).getEntity());
-        }
-        else if ("post".contentEquals(method)) {
-            EntityUtils.consume(client.execute(new HttpPost(server.baseUrl())).getEntity());
-        }
-        else {
-            EntityUtils.consume(client.execute(new HttpCustomMethod(method, server.baseUrl())).getEntity());
+        switch (method) {
+            case "get":
+                EntityUtils.consume(client.execute(new HttpGet(server.baseUrl())).getEntity());
+                break;
+
+            case "post":
+                EntityUtils.consume(client.execute(new HttpPost(server.baseUrl())).getEntity());
+                break;
+
+            default:
+                EntityUtils.consume(client.execute(new HttpCustomMethod(method, server.baseUrl())).getEntity());
+                break;
         }
         TestObservationRegistryAssert.assertThat(observationRegistry).hasSingleObservationThat()
                 .hasContextualNameEqualToIgnoringCase("http " + method);
@@ -283,7 +287,6 @@ private static class HttpCustomMethod extends HttpEntityEnclosingRequestBase {
         private final String method;
 
         private HttpCustomMethod(String method, String uri) {
-            super();
             setURI(URI.create(uri));
             this.method = method;
         }
@@ -311,10 +314,6 @@ private HttpClient client(HttpRequestExecutor executor) {
         return HttpClientBuilder.create().setRequestExecutor(executor).build();
     }
 
-    private HttpRequestExecutor executor(boolean exportRoutes) {
-        return executor(exportRoutes, false);
-    }
-
     private HttpRequestExecutor executor(boolean exportRoutes, boolean configureObservationRegistry) {
         MicrometerHttpRequestExecutor.Builder builder = MicrometerHttpRequestExecutor.builder(registry);
         if (configureObservationRegistry) {