Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add parameter to customize status codes mapped as 'NOT_FOUND' in OkHttpMetricsEventListener #4987

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -95,6 +95,8 @@ private static Method getMethod(Class<?>... parameterTypes) {

private final Function<Request, String> urlMapper;

private final Set<Integer> statusCodesMappedAsNotFound;

private final Iterable<Tag> extraTags;

private final Iterable<BiFunction<Request, Response, Tag>> contextSpecificTags;
Expand All @@ -107,17 +109,20 @@ private static Method getMethod(Class<?>... parameterTypes) {
final ConcurrentMap<Call, CallState> callState = new ConcurrentHashMap<>();

protected OkHttpMetricsEventListener(MeterRegistry registry, String requestsMetricName,
Function<Request, String> urlMapper, Iterable<Tag> extraTags,
Function<Request, String> urlMapper, Set<Integer> statusCodesMappedAsNotFound, Iterable<Tag> extraTags,
Iterable<BiFunction<Request, Response, Tag>> contextSpecificTags) {
this(registry, requestsMetricName, urlMapper, extraTags, contextSpecificTags, emptyList(), true);
this(registry, requestsMetricName, urlMapper, statusCodesMappedAsNotFound, extraTags, contextSpecificTags,
emptyList(), true);
}

OkHttpMetricsEventListener(MeterRegistry registry, String requestsMetricName, Function<Request, String> urlMapper,
Iterable<Tag> extraTags, Iterable<BiFunction<Request, Response, Tag>> contextSpecificTags,
Iterable<String> requestTagKeys, boolean includeHostTag) {
Set<Integer> statusCodesMappedAsNotFound, Iterable<Tag> extraTags,
Iterable<BiFunction<Request, Response, Tag>> contextSpecificTags, Iterable<String> requestTagKeys,
boolean includeHostTag) {
this.registry = registry;
this.requestsMetricName = requestsMetricName;
this.urlMapper = urlMapper;
this.statusCodesMappedAsNotFound = statusCodesMappedAsNotFound;
this.extraTags = extraTags;
this.contextSpecificTags = contextSpecificTags;
this.includeHostTag = includeHostTag;
Expand Down Expand Up @@ -200,7 +205,7 @@ private String getUriTag(CallState state, @Nullable Request request) {
if (request == null) {
return TAG_VALUE_UNKNOWN;
}
return state.response != null && (state.response.code() == 404 || state.response.code() == 301) ? "NOT_FOUND"
return state.response != null && statusCodesMappedAsNotFound.contains(state.response.code()) ? "NOT_FOUND"
: urlMapper.apply(request);
}

Expand Down Expand Up @@ -271,6 +276,8 @@ public static class Builder {
private Function<Request, String> uriMapper = (request) -> Optional.ofNullable(request.header(URI_PATTERN))
.orElse("none");

private Set<Integer> statusCodesMappedAsNotFound = new HashSet<>(Arrays.asList(301, 404));

private Tags tags = Tags.empty();

private Collection<BiFunction<Request, Response, Tag>> contextSpecificTags = new ArrayList<>();
Expand Down Expand Up @@ -316,6 +323,34 @@ public Builder uriMapper(Function<Request, String> uriMapper) {
return this;
}

/**
* Sets specific status codes to be mapped to a generic "NOT_FOUND" category for
* the 'uri' tag when recording metrics. This mapping helps in avoiding tag
* explosion and reducing metric dimensionality by treating these status codes
* identically.
* @param statusCodesMappedAsNotFound One or more status codes to be treated as
* "NOT_FOUND".
* @return this builder for chaining
*/
public Builder statusCodesMappedAsNotFound(Integer... statusCodesMappedAsNotFound) {
return statusCodesMappedAsNotFound(Arrays.asList(statusCodesMappedAsNotFound));
}

/**
* Sets specific status codes to be mapped to a generic "NOT_FOUND" category for
* the 'uri' tag when recording metrics. This mapping helps in avoiding tag
* explosion and reducing metric dimensionality by treating these status codes
* identically.
* @param statusCodesMappedAsNotFound One or more status codes to be treated as
* "NOT_FOUND".
* @return this builder for chaining
*/
public Builder statusCodesMappedAsNotFound(Iterable<Integer> statusCodesMappedAsNotFound) {
this.statusCodesMappedAsNotFound = new HashSet<>();
statusCodesMappedAsNotFound.forEach(this.statusCodesMappedAsNotFound::add);
return this;
}

/**
* Historically, OkHttp Metrics provided by {@link OkHttpMetricsEventListener}
* included a {@code host} tag for the target host being called. To align with
Expand Down Expand Up @@ -361,8 +396,8 @@ public Builder requestTagKeys(Iterable<String> requestTagKeys) {
}

public OkHttpMetricsEventListener build() {
return new OkHttpMetricsEventListener(registry, name, uriMapper, tags, contextSpecificTags, requestTagKeys,
includeHostTag);
return new OkHttpMetricsEventListener(registry, name, uriMapper, statusCodesMappedAsNotFound, tags,
contextSpecificTags, requestTagKeys, includeHostTag);
}

}
Expand Down
Expand Up @@ -162,6 +162,29 @@ void uriTagWorksWithUriMapper(@WiremockResolver.Wiremock WireMockServer server)
.count()).isEqualTo(1L);
}

@Test
void timeWithStatusCodesMappedAsNotFound(@WiremockResolver.Wiremock WireMockServer server) throws IOException {
server.stubFor(any(anyUrl()).willReturn(aResponse().withStatus(404)));
Request request = new Request.Builder().url(server.baseUrl()).build();

OkHttpClient client = new OkHttpClient.Builder()
.eventListener(OkHttpMetricsEventListener.builder(registry, "okhttp.requests")
.statusCodesMappedAsNotFound(404) // Specifying that 404 should be treated
// as 'NOT_FOUND'
.uriMapper(URI_MAPPER)
.tags(Tags.of("foo", "bar"))
.build())
.build();

client.newCall(request).execute().close();

assertThat(registry.get("okhttp.requests")
.tags("foo", "bar", "status", "404", "outcome", "CLIENT_ERROR", "uri", "NOT_FOUND", "target.host",
"localhost", "target.port", String.valueOf(server.port()), "target.scheme", "http")
.timer()
.count()).isEqualTo(1L);
}

@Test
void contextSpecificTags(@WiremockResolver.Wiremock WireMockServer server) throws IOException {
server.stubFor(any(anyUrl()));
Expand Down