Polish observability changes

Closes gh-29235
This commit is contained in:
Johnny Lim
2022-10-01 01:54:21 +09:00
committed by Brian Clozel
parent afac8dd8af
commit 380795e5b8
23 changed files with 106 additions and 87 deletions

View File

@@ -64,7 +64,7 @@ public enum ClientObservation implements DocumentedObservation {
},
/**
* URI template used for HTTP request, or {@code ""} if none was provided.
* URI template used for HTTP request, or {@code "none"} if none was provided.
*/
URI {
@Override

View File

@@ -22,6 +22,7 @@ import io.micrometer.common.KeyValue;
import io.micrometer.common.KeyValues;
import io.micrometer.observation.ObservationConvention;
import org.springframework.http.client.observation.ClientHttpObservation;
import org.springframework.http.observation.HttpOutcome;
import org.springframework.util.StringUtils;
@@ -40,8 +41,15 @@ public class DefaultClientObservationConvention implements ClientObservationConv
private static final KeyValue METHOD_NONE = KeyValue.of(ClientObservation.LowCardinalityKeyNames.METHOD, "none");
private static final KeyValue STATUS_IO_ERROR = KeyValue.of(ClientHttpObservation.LowCardinalityKeyNames.STATUS, "IO_ERROR");
private static final KeyValue STATUS_CLIENT_ERROR = KeyValue.of(ClientHttpObservation.LowCardinalityKeyNames.STATUS, "CLIENT_ERROR");
private static final KeyValue EXCEPTION_NONE = KeyValue.of(ClientObservation.LowCardinalityKeyNames.EXCEPTION, "none");
private static final KeyValue URI_EXPANDED_NONE = KeyValue.of(ClientHttpObservation.HighCardinalityKeyNames.URI_EXPANDED, "none");
private static final KeyValue CLIENT_NAME_NONE = KeyValue.of(ClientHttpObservation.HighCardinalityKeyNames.CLIENT_NAME, "none");
private final String name;
@@ -93,17 +101,17 @@ public class DefaultClientObservationConvention implements ClientObservationConv
}
protected KeyValue status(ClientObservationContext context) {
return KeyValue.of(ClientObservation.LowCardinalityKeyNames.STATUS, getStatusMessage(context));
}
private String getStatusMessage(ClientObservationContext context) {
if (context.getResponse() != null) {
return String.valueOf(context.getResponse().statusCode().value());
if (context.isAborted()) {
return STATUS_CLIENT_ERROR;
}
if (context.getError().isPresent()) {
return (context.getError().get() instanceof IOException) ? "IO_ERROR" : "CLIENT_ERROR";
ClientResponse response = context.getResponse();
if (response != null) {
return KeyValue.of(ClientObservation.LowCardinalityKeyNames.STATUS, String.valueOf(response.statusCode().value()));
}
return "CLIENT_ERROR";
if (context.getError().isPresent() && context.getError().get() instanceof IOException) {
return STATUS_IO_ERROR;
}
return STATUS_CLIENT_ERROR;
}
protected KeyValue exception(ClientObservationContext context) {
@@ -114,11 +122,11 @@ public class DefaultClientObservationConvention implements ClientObservationConv
}).orElse(EXCEPTION_NONE);
}
protected static KeyValue outcome(ClientObservationContext context) {
protected KeyValue outcome(ClientObservationContext context) {
if (context.isAborted()) {
return HttpOutcome.UNKNOWN.asKeyValue();
}
else if (context.getResponse() != null) {
if (context.getResponse() != null) {
HttpOutcome httpOutcome = HttpOutcome.forStatus(context.getResponse().statusCode());
return httpOutcome.asKeyValue();
}
@@ -134,15 +142,14 @@ public class DefaultClientObservationConvention implements ClientObservationConv
if (context.getCarrier() != null) {
return KeyValue.of(ClientObservation.HighCardinalityKeyNames.URI_EXPANDED, context.getCarrier().url().toASCIIString());
}
return KeyValue.of(ClientObservation.HighCardinalityKeyNames.URI_EXPANDED, "none");
return URI_EXPANDED_NONE;
}
protected KeyValue clientName(ClientObservationContext context) {
String host = "none";
if (context.getCarrier() != null && context.getCarrier().url().getHost() != null) {
host = context.getCarrier().url().getHost();
return KeyValue.of(ClientObservation.HighCardinalityKeyNames.CLIENT_NAME, context.getCarrier().url().getHost());
}
return KeyValue.of(ClientObservation.HighCardinalityKeyNames.CLIENT_NAME, host);
return CLIENT_NAME_NONE;
}
}

View File

@@ -26,7 +26,7 @@ import java.util.function.Function;
import java.util.function.IntPredicate;
import java.util.function.Predicate;
import io.micrometer.observation.Observation;
import io.micrometer.observation.ObservationConvention;
import io.micrometer.observation.ObservationRegistry;
import org.reactivestreams.Publisher;
import reactor.core.publisher.Flux;
@@ -346,7 +346,7 @@ public interface WebClient {
Builder observationRegistry(ObservationRegistry observationRegistry);
/**
* Provide a {@link Observation.ObservationConvention} to use for collecting
* Provide an {@link ObservationConvention} to use for collecting
* metadata for the current observation. Will use {@link DefaultClientObservationConvention}
* if none provided.
* @param observationConvention the observation convention to use

View File

@@ -34,7 +34,7 @@ import static org.assertj.core.api.Assertions.assertThat;
*/
class DefaultClientObservationConventionTests {
private DefaultClientObservationConvention observationConvention = new DefaultClientObservationConvention();
private final DefaultClientObservationConvention observationConvention = new DefaultClientObservationConvention();
@Test
void shouldHaveName() {

View File

@@ -39,19 +39,19 @@ import static org.mockito.Mockito.verifyNoMoreInteractions;
* Tests for the {@link WebClient} {@link io.micrometer.observation.Observation observations}.
* @author Brian Clozel
*/
public class DefaultClientObservationTests {
class WebClientObservationTests {
private final TestObservationRegistry observationRegistry = TestObservationRegistry.create();
private ExchangeFunction exchangeFunction = mock(ExchangeFunction.class);
private final ExchangeFunction exchangeFunction = mock(ExchangeFunction.class);
private ArgumentCaptor<ClientRequest> request = ArgumentCaptor.forClass(ClientRequest.class);
private final ArgumentCaptor<ClientRequest> request = ArgumentCaptor.forClass(ClientRequest.class);
private WebClient.Builder builder;
@BeforeEach
public void setup() {
void setup() {
ClientResponse mockResponse = mock(ClientResponse.class);
when(mockResponse.statusCode()).thenReturn(HttpStatus.OK);
when(mockResponse.bodyToMono(Void.class)).thenReturn(Mono.empty());