Skip to content

Commit fa620c6

Browse files
authored
Remove special handling of 404/301 from OkHttp instrumentation (#5814)
See gh-5812
1 parent 1a9c7fb commit fa620c6

File tree

3 files changed

+8
-11
lines changed

3 files changed

+8
-11
lines changed

Diff for: micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/DefaultOkHttpObservationConvention.java

+3-5
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ public KeyValues getLowCardinalityKeyValues(OkHttpContext context) {
8888
// TODO: Tags to key values and back - maybe we can improve this?
8989
KeyValues keyValues = KeyValues
9090
.of(METHOD.withValue(requestAvailable ? request.method() : TAG_VALUE_UNKNOWN),
91-
URI.withValue(getUriTag(urlMapper, state, request)),
91+
URI.withValue(getUriTag(urlMapper, request)),
9292
STATUS.withValue(getStatusMessage(state.response, state.exception)),
9393
OUTCOME.withValue(getStatusOutcome(state.response).name()))
9494
.and(extraTags)
@@ -105,13 +105,11 @@ public KeyValues getLowCardinalityKeyValues(OkHttpContext context) {
105105
return keyValues;
106106
}
107107

108-
private String getUriTag(Function<Request, String> urlMapper, OkHttpObservationInterceptor.CallState state,
109-
@Nullable Request request) {
108+
private String getUriTag(Function<Request, String> urlMapper, @Nullable Request request) {
110109
if (request == null) {
111110
return TAG_VALUE_UNKNOWN;
112111
}
113-
return state.response != null && (state.response.code() == 404 || state.response.code() == 301) ? "NOT_FOUND"
114-
: urlMapper.apply(request);
112+
return urlMapper.apply(request);
115113
}
116114

117115
private Outcome getStatusOutcome(@Nullable Response response) {

Diff for: micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpMetricsEventListener.java

+4-5
Original file line numberDiff line numberDiff line change
@@ -167,8 +167,8 @@ void time(CallState state) {
167167
boolean requestAvailable = request != null;
168168

169169
Iterable<Tag> tags = Tags
170-
.of("method", requestAvailable ? request.method() : TAG_VALUE_UNKNOWN, "uri", getUriTag(state, request),
171-
"status", getStatusMessage(state.response, state.exception))
170+
.of("method", requestAvailable ? request.method() : TAG_VALUE_UNKNOWN, "uri", getUriTag(request), "status",
171+
getStatusMessage(state.response, state.exception))
172172
.and(getStatusOutcome(state.response).asTag())
173173
.and(extraTags)
174174
.and(stream(contextSpecificTags.spliterator(), false)
@@ -196,12 +196,11 @@ private Tags generateTagsForRoute(@Nullable Request request) {
196196
TAG_TARGET_PORT, Integer.toString(request.url().port()));
197197
}
198198

199-
private String getUriTag(CallState state, @Nullable Request request) {
199+
private String getUriTag(@Nullable Request request) {
200200
if (request == null) {
201201
return TAG_VALUE_UNKNOWN;
202202
}
203-
return state.response != null && (state.response.code() == 404 || state.response.code() == 301) ? "NOT_FOUND"
204-
: urlMapper.apply(request);
203+
return urlMapper.apply(request);
205204
}
206205

207206
private Iterable<Tag> getRequestTags(@Nullable Request request) {

Diff for: micrometer-core/src/test/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpMetricsEventListenerTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ void timeNotFound(@WiremockResolver.Wiremock WireMockServer server) throws IOExc
8686
client.newCall(request).execute().close();
8787

8888
assertThat(registry.get("okhttp.requests")
89-
.tags("foo", "bar", "status", "404", "outcome", "CLIENT_ERROR", "uri", "NOT_FOUND", "target.host",
89+
.tags("foo", "bar", "status", "404", "outcome", "CLIENT_ERROR", "uri", URI_EXAMPLE_VALUE, "target.host",
9090
"localhost", "target.port", String.valueOf(server.port()), "target.scheme", "http")
9191
.timer()
9292
.count()).isEqualTo(1L);

0 commit comments

Comments
 (0)