Remove special handling of 404/301 from JDK HTTP client instrumentation

See gh-5812 Signed-off-by: Johnny Lim <[email protected]>
micrometer-metrics · Jan 29, 2025 · 71d31e3 · 71d31e3
1 parent 099323d
commit 71d31e3
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 2 deletions.
diff --git a/...va/io/micrometer/java11/instrument/binder/jdk/DefaultHttpClientObservationConvention.java b/...va/io/micrometer/java11/instrument/binder/jdk/DefaultHttpClientObservationConvention.java
@@ -55,8 +55,7 @@ public KeyValues getLowCardinalityKeyValues(HttpClientContext context) {
 
     String getUri(HttpRequest request, @Nullable HttpResponse<?> httpResponse,
             Function<HttpRequest, String> uriMapper) {
-        return httpResponse != null && (httpResponse.statusCode() == 404 || httpResponse.statusCode() == 301)
-                ? "NOT_FOUND" : uriMapper.apply(request);
+        return uriMapper.apply(request);
     }
 
     String getStatus(@Nullable HttpResponse<?> response) {

diff --git a/...1/src/test/java/io/micrometer/java11/instrument/binder/jdk/MicrometerHttpClientTests.java b/...1/src/test/java/io/micrometer/java11/instrument/binder/jdk/MicrometerHttpClientTests.java
@@ -57,6 +57,7 @@ void setup() {
         stubFor(any(urlEqualTo("/metrics")).willReturn(ok().withBody("body")));
         stubFor(any(urlEqualTo("/test-fault"))
             .willReturn(new ResponseDefinitionBuilder().withFault(Fault.CONNECTION_RESET_BY_PEER)));
+        stubFor(any(urlEqualTo("/resources/1")).willReturn(notFound()));
     }
 
     @Test
@@ -98,6 +99,46 @@ void shouldInstrumentHttpClientWithTimer(WireMockRuntimeInfo wmInfo) throws IOEx
         thenMeterRegistryContainsHttpClientTags();
     }
 
+    @Test
+    void shouldInstrumentHttpClientWhenNotFound(WireMockRuntimeInfo wmInfo) throws IOException, InterruptedException {
+        HttpRequest request = HttpRequest.newBuilder()
+            .GET()
+            .uri(URI.create(wmInfo.getHttpBaseUrl() + "/resources/1"))
+            .build();
+
+        HttpClient observedClient = MicrometerHttpClient.instrumentationBuilder(httpClient, meterRegistry).build();
+        observedClient.send(request, HttpResponse.BodyHandlers.ofString());
+
+        then(meterRegistry.get("http.client.requests")
+            .tag("method", "GET")
+            .tag("status", "404")
+            .tag("outcome", "CLIENT_ERROR")
+            .tag("uri", "UNKNOWN")
+            .timer()).isNotNull();
+    }
+
+    @Test
+    void shouldInstrumentHttpClientWithUriPatternHeaderWhenNotFound(WireMockRuntimeInfo wmInfo)
+            throws IOException, InterruptedException {
+        String uriPattern = "/resources/{id}";
+
+        HttpRequest request = HttpRequest.newBuilder()
+            .header(MicrometerHttpClient.URI_PATTERN_HEADER, uriPattern)
+            .GET()
+            .uri(URI.create(wmInfo.getHttpBaseUrl() + "/resources/1"))
+            .build();
+
+        HttpClient observedClient = MicrometerHttpClient.instrumentationBuilder(httpClient, meterRegistry).build();
+        observedClient.send(request, HttpResponse.BodyHandlers.ofString());
+
+        then(meterRegistry.get("http.client.requests")
+            .tag("method", "GET")
+            .tag("status", "404")
+            .tag("outcome", "CLIENT_ERROR")
+            .tag("uri", uriPattern)
+            .timer()).isNotNull();
+    }
+
     @Test
     @Issue("#5136")
     void shouldThrowErrorFromSendAsync(WireMockRuntimeInfo wmInfo) {