Skip to content

Commit

Permalink
Apply micrometer-metricsgh-5553 to io.micrometer.core.instrument.bind…
Browse files Browse the repository at this point in the history 
…er.jdk
  • Loading branch information
@izeye
izeye committed Nov 23, 2024
1 parent 045ce20 commit 2969819
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 31 deletions.
35 changes: 21 additions & 14 deletions ...va11/io/micrometer/core/instrument/binder/jdk/DefaultHttpClientObservationConvention.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,29 +43,36 @@ public KeyValues getLowCardinalityKeyValues(HttpClientContext context) {
return KeyValues.empty();
}
HttpRequest httpRequest = context.getCarrier().build();
KeyValues keyValues = KeyValues.of(
return KeyValues.of(
HttpClientObservationDocumentation.LowCardinalityKeys.METHOD.withValue(httpRequest.method()),
HttpClientObservationDocumentation.LowCardinalityKeys.URI
.withValue(getUriTag(httpRequest, context.getResponse(), context.getUriMapper())));
if (context.getResponse() != null) {
keyValues = keyValues
.and(HttpClientObservationDocumentation.LowCardinalityKeys.STATUS
.withValue(String.valueOf(context.getResponse().statusCode())))
.and(HttpClientObservationDocumentation.LowCardinalityKeys.OUTCOME
.withValue(Outcome.forStatus(context.getResponse().statusCode()).name()));
}
return keyValues;
.withValue(getUriTag(httpRequest, context.getResponse(), context.getUriMapper())),
HttpClientObservationDocumentation.LowCardinalityKeys.STATUS
.withValue(getStatus(context.getResponse())),
HttpClientObservationDocumentation.LowCardinalityKeys.OUTCOME
.withValue(getOutcome(context.getResponse())));
}

String getUriTag(@Nullable HttpRequest request, @Nullable HttpResponse<?> httpResponse,
String getUriTag(HttpRequest request, @Nullable HttpResponse<?> httpResponse,
Function<HttpRequest, String> uriMapper) {
if (request == null) {
return null;
}
return httpResponse != null && (httpResponse.statusCode() == 404 || httpResponse.statusCode() == 301)
? "NOT_FOUND" : uriMapper.apply(request);
}

String getStatus(@Nullable HttpResponse<?> response) {
if (response == null) {
return "UNKNOWN";
}
return String.valueOf(response.statusCode());
}

String getOutcome(@Nullable HttpResponse<?> response) {
if (response == null) {
return Outcome.UNKNOWN.name();
}
return Outcome.forStatus(response.statusCode()).name();
}

@Override
@NonNull
public String getName() {
Expand Down
28 changes: 11 additions & 17 deletions ...r-core/src/main/java11/io/micrometer/core/instrument/binder/jdk/MicrometerHttpClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,7 @@

import io.micrometer.common.lang.Nullable;
import io.micrometer.core.instrument.MeterRegistry;
import io.micrometer.core.instrument.Tag;
import io.micrometer.core.instrument.Tags;
import io.micrometer.core.instrument.binder.http.Outcome;
import io.micrometer.core.instrument.observation.ObservationOrTimerCompatibleInstrumentation;
import io.micrometer.observation.Observation;
import io.micrometer.observation.ObservationRegistry;
Expand All @@ -36,6 +34,7 @@
import java.time.Duration;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionException;
import java.util.concurrent.Executor;
import java.util.function.Function;

Expand Down Expand Up @@ -234,19 +233,13 @@ private <T> void stopObservationOrTimer(
ObservationOrTimerCompatibleInstrumentation<HttpClientContext> instrumentation, HttpRequest request,
@Nullable HttpResponse<T> res) {
instrumentation.stop(DefaultHttpClientObservationConvention.INSTANCE.getName(), "Timer for JDK's HttpClient",
() -> {
Tags tags = Tags.of(HttpClientObservationDocumentation.LowCardinalityKeys.METHOD.asString(),
request.method(), HttpClientObservationDocumentation.LowCardinalityKeys.URI.asString(),
DefaultHttpClientObservationConvention.INSTANCE.getUriTag(request, res, uriMapper));
if (res != null) {
tags = tags
.and(Tag.of(HttpClientObservationDocumentation.LowCardinalityKeys.STATUS.asString(),
String.valueOf(res.statusCode())))
.and(Tag.of(HttpClientObservationDocumentation.LowCardinalityKeys.OUTCOME.asString(),
Outcome.forStatus(res.statusCode()).name()));
}
return tags;
});
() -> Tags.of(HttpClientObservationDocumentation.LowCardinalityKeys.METHOD.asString(), request.method(),
HttpClientObservationDocumentation.LowCardinalityKeys.URI.asString(),
DefaultHttpClientObservationConvention.INSTANCE.getUriTag(request, res, uriMapper),
HttpClientObservationDocumentation.LowCardinalityKeys.STATUS.asString(),
DefaultHttpClientObservationConvention.INSTANCE.getStatus(res),
HttpClientObservationDocumentation.LowCardinalityKeys.OUTCOME.asString(),
DefaultHttpClientObservationConvention.INSTANCE.getOutcome(res)));
}

private ObservationOrTimerCompatibleInstrumentation<HttpClientContext> observationOrTimer(
Expand All @@ -272,11 +265,12 @@ public <T> CompletableFuture<HttpResponse<T>> sendAsync(HttpRequest httpRequest,
httpRequestBuilder);
HttpRequest request = httpRequestBuilder.build();
return client.sendAsync(request, bodyHandler, pushPromiseHandler).handle((response, throwable) -> {
instrumentation.setResponse(response);
stopObservationOrTimer(instrumentation, request, response);
if (throwable != null) {
instrumentation.setThrowable(throwable);
throw new CompletionException(throwable);
}
instrumentation.setResponse(response);
stopObservationOrTimer(instrumentation, request, response);
return response;
});
}
Expand Down
22 changes: 22 additions & 0 deletions ...e/src/test/java11/io/micrometer/core/instrument/binder/jdk/MicrometerHttpClientTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
*/
package io.micrometer.core.instrument.binder.jdk;

import com.github.tomakehurst.wiremock.client.ResponseDefinitionBuilder;
import com.github.tomakehurst.wiremock.http.Fault;
import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo;
import com.github.tomakehurst.wiremock.junit5.WireMockTest;
import io.micrometer.core.instrument.MeterRegistry;
Expand All @@ -33,8 +35,11 @@
import java.net.http.HttpRequest;
import java.net.http.HttpResponse;
import java.time.Duration;
import java.util.concurrent.CompletionException;

import static com.github.tomakehurst.wiremock.client.WireMock.*;
import static org.assertj.core.api.Assertions.assertThatNoException;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.assertj.core.api.BDDAssertions.then;

@WireMockTest
Expand All @@ -47,6 +52,8 @@ class MicrometerHttpClientTests {
@BeforeEach
void setup() {
stubFor(any(urlEqualTo("/metrics")).willReturn(ok().withBody("body")));
stubFor(any(urlEqualTo("/test-fault"))
.willReturn(new ResponseDefinitionBuilder().withFault(Fault.CONNECTION_RESET_BY_PEER)));
}

@Test
Expand Down Expand Up @@ -84,6 +91,21 @@ void shouldInstrumentHttpClientWithTimer(WireMockRuntimeInfo wmInfo) throws IOEx
thenMeterRegistryContainsHttpClientTags();
}

@Test
void shouldThrowErrorFromSendAsync(WireMockRuntimeInfo wmInfo) {
var client = MicrometerHttpClient.instrumentationBuilder(httpClient, meterRegistry).build();
var request = HttpRequest.newBuilder(URI.create(wmInfo.getHttpBaseUrl() + "/test-fault")).GET().build();
var response = client.sendAsync(request, HttpResponse.BodyHandlers.ofString());
assertThatThrownBy(response::join).isInstanceOf(CompletionException.class);
assertThatNoException().isThrownBy(() -> meterRegistry.get("http.client.requests")
.tag("method", "GET")
// TODO fix status/uri in exceptional cases where response may be null
.tag("uri", "UNKNOWN")
.tag("status", "UNKNOWN")
.tag("outcome", "UNKNOWN")
.timer());
}

private void thenMeterRegistryContainsHttpClientTags() {
then(meterRegistry.find("http.client.requests")
.tag("method", "GET")
Expand Down

0 comments on commit 2969819

Please sign in to comment.