From fa620c68c884de70b3636485d27501af0bbeabbe Mon Sep 17 00:00:00 2001
From: Tommy Ludwig <8924140+shakuzen@users.noreply.github.com>
Date: Wed, 22 Jan 2025 04:58:17 +0900
Subject: [PATCH] Remove special handling of 404/301 from OkHttp
 instrumentation (#5814)

See gh-5812
---
 .../okhttp3/DefaultOkHttpObservationConvention.java      | 8 +++-----
 .../binder/okhttp3/OkHttpMetricsEventListener.java       | 9 ++++-----
 .../binder/okhttp3/OkHttpMetricsEventListenerTest.java   | 2 +-
 3 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/DefaultOkHttpObservationConvention.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/DefaultOkHttpObservationConvention.java
index a4756b4235..0ef6d36bfd 100644
--- a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/DefaultOkHttpObservationConvention.java
+++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/DefaultOkHttpObservationConvention.java
@@ -88,7 +88,7 @@ public KeyValues getLowCardinalityKeyValues(OkHttpContext context) {
         // TODO: Tags to key values and back - maybe we can improve this?
         KeyValues keyValues = KeyValues
             .of(METHOD.withValue(requestAvailable ? request.method() : TAG_VALUE_UNKNOWN),
-                    URI.withValue(getUriTag(urlMapper, state, request)),
+                    URI.withValue(getUriTag(urlMapper, request)),
                     STATUS.withValue(getStatusMessage(state.response, state.exception)),
                     OUTCOME.withValue(getStatusOutcome(state.response).name()))
             .and(extraTags)
@@ -105,13 +105,11 @@ public KeyValues getLowCardinalityKeyValues(OkHttpContext context) {
         return keyValues;
     }
 
-    private String getUriTag(Function<Request, String> urlMapper, OkHttpObservationInterceptor.CallState state,
-            @Nullable Request request) {
+    private String getUriTag(Function<Request, String> urlMapper, @Nullable Request request) {
         if (request == null) {
             return TAG_VALUE_UNKNOWN;
         }
-        return state.response != null && (state.response.code() == 404 || state.response.code() == 301) ? "NOT_FOUND"
-                : urlMapper.apply(request);
+        return urlMapper.apply(request);
     }
 
     private Outcome getStatusOutcome(@Nullable Response response) {
diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpMetricsEventListener.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpMetricsEventListener.java
index 8b1809303d..c19ff9d6df 100644
--- a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpMetricsEventListener.java
+++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpMetricsEventListener.java
@@ -167,8 +167,8 @@ void time(CallState state) {
         boolean requestAvailable = request != null;
 
         Iterable<Tag> tags = Tags
-            .of("method", requestAvailable ? request.method() : TAG_VALUE_UNKNOWN, "uri", getUriTag(state, request),
-                    "status", getStatusMessage(state.response, state.exception))
+            .of("method", requestAvailable ? request.method() : TAG_VALUE_UNKNOWN, "uri", getUriTag(request), "status",
+                    getStatusMessage(state.response, state.exception))
             .and(getStatusOutcome(state.response).asTag())
             .and(extraTags)
             .and(stream(contextSpecificTags.spliterator(), false)
@@ -196,12 +196,11 @@ private Tags generateTagsForRoute(@Nullable Request request) {
                 TAG_TARGET_PORT, Integer.toString(request.url().port()));
     }
 
-    private String getUriTag(CallState state, @Nullable Request request) {
+    private String getUriTag(@Nullable Request request) {
         if (request == null) {
             return TAG_VALUE_UNKNOWN;
         }
-        return state.response != null && (state.response.code() == 404 || state.response.code() == 301) ? "NOT_FOUND"
-                : urlMapper.apply(request);
+        return urlMapper.apply(request);
     }
 
     private Iterable<Tag> getRequestTags(@Nullable Request request) {
diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpMetricsEventListenerTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpMetricsEventListenerTest.java
index 4a14a91588..3e6809089a 100644
--- a/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpMetricsEventListenerTest.java
+++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpMetricsEventListenerTest.java
@@ -86,7 +86,7 @@ void timeNotFound(@WiremockResolver.Wiremock WireMockServer server) throws IOExc
         client.newCall(request).execute().close();
 
         assertThat(registry.get("okhttp.requests")
-            .tags("foo", "bar", "status", "404", "outcome", "CLIENT_ERROR", "uri", "NOT_FOUND", "target.host",
+            .tags("foo", "bar", "status", "404", "outcome", "CLIENT_ERROR", "uri", URI_EXAMPLE_VALUE, "target.host",
                     "localhost", "target.port", String.valueOf(server.port()), "target.scheme", "http")
             .timer()
             .count()).isEqualTo(1L);