Add outcome tag to HttpClient metrics (#3732)

Aligns the HTTP client instrumentations by consistently including an outcome tag. This can make querying these metrics easier as it groups a class of HTTP status codes into a single value that can be directly queried instead of using regex or ranges.
micrometer-metrics · Apr 4, 2023 · 63cfcbc · 63cfcbc
1 parent 9fbf990
commit 63cfcbc
Show file tree

Hide file tree

Showing 17 changed files with 131 additions and 33 deletions.
diff --git a/...meter/core/instrument/binder/httpcomponents/ApacheHttpClientObservationDocumentation.java b/...meter/core/instrument/binder/httpcomponents/ApacheHttpClientObservationDocumentation.java
@@ -52,6 +52,12 @@ public String asString() {
  return "status";
  }
  },
+ OUTCOME {
+ @Override
+ public String asString() {
+ return "outcome";
+ }
+ },
  METHOD {
  @Override
  public String asString() {

diff --git a/...r/core/instrument/binder/httpcomponents/DefaultApacheHttpClientObservationConvention.java b/...r/core/instrument/binder/httpcomponents/DefaultApacheHttpClientObservationConvention.java
@@ -17,6 +17,7 @@
 
 import io.micrometer.common.KeyValues;
 import io.micrometer.common.lang.Nullable;
+import io.micrometer.core.instrument.binder.http.Outcome;
 import org.apache.http.HttpException;
 import org.apache.http.HttpRequest;
 import org.apache.http.HttpResponse;
@@ -63,13 +64,19 @@ public KeyValues getLowCardinalityKeyValues(ApacheHttpClientContext context) {
  ApacheHttpClientObservationDocumentation.ApacheHttpClientKeyNames.URI
  .withValue(context.getUriMapper().apply(context.getCarrier())),
  ApacheHttpClientObservationDocumentation.ApacheHttpClientKeyNames.STATUS
- .withValue(getStatusValue(context.getResponse(), context.getError())));
+ .withValue(getStatusValue(context.getResponse(), context.getError())),
+ ApacheHttpClientObservationDocumentation.ApacheHttpClientKeyNames.OUTCOME
+ .withValue(getStatusOutcome(context.getResponse()).name()));
  if (context.shouldExportTagsForRoute()) {
  keyValues = keyValues.and(HttpContextUtils.generateTagStringsForRoute(context.getApacheHttpContext()));
  }
  return keyValues;
  }
 
+ Outcome getStatusOutcome(@Nullable HttpResponse response) {
+ return response != null ? Outcome.forStatus(response.getStatusLine().getStatusCode()) : Outcome.UNKNOWN;
+ }
+
  String getStatusValue(@Nullable HttpResponse response, Throwable error) {
  if (error instanceof IOException || error instanceof HttpException || error instanceof RuntimeException) {
  return "IO_ERROR";

diff --git a/...va/io/micrometer/core/instrument/binder/httpcomponents/MicrometerHttpRequestExecutor.java b/...va/io/micrometer/core/instrument/binder/httpcomponents/MicrometerHttpRequestExecutor.java
@@ -21,6 +21,7 @@
 import io.micrometer.core.instrument.Tag;
 import io.micrometer.core.instrument.Tags;
 import io.micrometer.core.instrument.Timer;
+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;
@@ -118,11 +119,13 @@ public HttpResponse execute(HttpRequest request, HttpClientConnection conn, Http
  () -> new ApacheHttpClientContext(request, context, uriMapper, exportTagsForRoute), convention,
  DefaultApacheHttpClientObservationConvention.INSTANCE);
  String statusCodeOrError = "UNKNOWN";
+ Outcome statusOutcome = Outcome.UNKNOWN;
 
  try {
  HttpResponse response = super.execute(request, conn, context);
  sample.setResponse(response);
  statusCodeOrError = DefaultApacheHttpClientObservationConvention.INSTANCE.getStatusValue(response, null);
+ statusOutcome = DefaultApacheHttpClientObservationConvention.INSTANCE.getStatusOutcome(response);
  return response;
  }
  catch (IOException | HttpException | RuntimeException e) {
@@ -132,10 +135,11 @@ public HttpResponse execute(HttpRequest request, HttpClientConnection conn, Http
  }
  finally {
  String status = statusCodeOrError;
+ String outcome = statusOutcome.name();
  sample.stop(METER_NAME, "Duration of Apache HttpClient request execution",
  () -> Tags
  .of("method", DefaultApacheHttpClientObservationConvention.INSTANCE.getMethodString(request),
- "uri", uriMapper.apply(request), "status", status)
+ "uri", uriMapper.apply(request), "status", status, "outcome", outcome)
  .and(exportTagsForRoute ? HttpContextUtils.generateTagsForRoute(context) : Tags.empty())
  .and(extraTags));
  }

diff --git a/...r/core/instrument/binder/httpcomponents/hc5/ApacheHttpClientObservationDocumentation.java b/...r/core/instrument/binder/httpcomponents/hc5/ApacheHttpClientObservationDocumentation.java
@@ -48,6 +48,12 @@ public String asString() {
  return "status";
  }
  },
+ OUTCOME {
+ @Override
+ public String asString() {
+ return "outcome";
+ }
+ },
  METHOD {
  @Override
  public String asString() {

diff --git a/...re/instrument/binder/httpcomponents/hc5/DefaultApacheHttpClientObservationConvention.java b/...re/instrument/binder/httpcomponents/hc5/DefaultApacheHttpClientObservationConvention.java
@@ -17,6 +17,7 @@
 
 import io.micrometer.common.KeyValues;
 import io.micrometer.common.lang.Nullable;
+import io.micrometer.core.instrument.binder.http.Outcome;
 import org.apache.hc.core5.http.HttpException;
 import org.apache.hc.core5.http.HttpRequest;
 import org.apache.hc.core5.http.HttpResponse;
@@ -59,13 +60,19 @@ public KeyValues getLowCardinalityKeyValues(ApacheHttpClientContext context) {
  ApacheHttpClientObservationDocumentation.ApacheHttpClientKeyNames.URI
  .withValue(context.getUriMapper().apply(context.getCarrier())),
  ApacheHttpClientObservationDocumentation.ApacheHttpClientKeyNames.STATUS
- .withValue(getStatusValue(context.getResponse(), context.getError())));
+ .withValue(getStatusValue(context.getResponse(), context.getError())),
+ ApacheHttpClientObservationDocumentation.ApacheHttpClientKeyNames.OUTCOME
+ .withValue(getStatusOutcome(context.getResponse()).name()));
  if (context.shouldExportTagsForRoute()) {
  keyValues = keyValues.and(HttpContextUtils.generateTagStringsForRoute(context.getApacheHttpContext()));
  }
  return keyValues;
  }
 
+ Outcome getStatusOutcome(@Nullable HttpResponse response) {
+ return response != null ? Outcome.forStatus(response.getCode()) : Outcome.UNKNOWN;
+ }
+
  String getStatusValue(@Nullable HttpResponse response, Throwable error) {
  if (error instanceof IOException || error instanceof HttpException || error instanceof RuntimeException) {
  return "IO_ERROR";

diff --git a/...o/micrometer/core/instrument/binder/httpcomponents/hc5/MicrometerHttpRequestExecutor.java b/...o/micrometer/core/instrument/binder/httpcomponents/hc5/MicrometerHttpRequestExecutor.java
@@ -20,6 +20,7 @@
 import io.micrometer.core.instrument.Tag;
 import io.micrometer.core.instrument.Tags;
 import io.micrometer.core.instrument.Timer;
+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;
@@ -105,11 +106,13 @@ public ClassicHttpResponse execute(ClassicHttpRequest request, HttpClientConnect
  () -> new ApacheHttpClientContext(request, context, uriMapper, exportTagsForRoute), convention,
  DefaultApacheHttpClientObservationConvention.INSTANCE);
  String statusCodeOrError = "UNKNOWN";
+ Outcome statusOutcome = Outcome.UNKNOWN;
 
  try {
  ClassicHttpResponse response = super.execute(request, conn, context);
  sample.setResponse(response);
  statusCodeOrError = DefaultApacheHttpClientObservationConvention.INSTANCE.getStatusValue(response, null);
+ statusOutcome = DefaultApacheHttpClientObservationConvention.INSTANCE.getStatusOutcome(response);
  return response;
  }
  catch (IOException | HttpException | RuntimeException e) {
@@ -119,10 +122,11 @@ public ClassicHttpResponse execute(ClassicHttpRequest request, HttpClientConnect
  }
  finally {
  String status = statusCodeOrError;
+ String outcome = statusOutcome.name();
  sample.stop(METER_NAME, "Duration of Apache HttpClient request execution",
  () -> Tags
  .of("method", DefaultApacheHttpClientObservationConvention.INSTANCE.getMethodString(request),
- "uri", uriMapper.apply(request), "status", status)
+ "uri", uriMapper.apply(request), "status", status, "outcome", outcome)
  .and(exportTagsForRoute ? HttpContextUtils.generateTagsForRoute(context) : Tags.empty())
  .and(extraTags));
  }

diff --git a/...java/io/micrometer/core/instrument/binder/okhttp3/DefaultOkHttpObservationConvention.java b/...java/io/micrometer/core/instrument/binder/okhttp3/DefaultOkHttpObservationConvention.java
@@ -22,6 +22,7 @@
 import io.micrometer.common.lang.Nullable;
 import io.micrometer.core.instrument.Tag;
 import io.micrometer.core.instrument.Tags;
+import io.micrometer.core.instrument.binder.http.Outcome;
 import okhttp3.Request;
 import okhttp3.Response;
 
@@ -88,7 +89,8 @@ public KeyValues getLowCardinalityKeyValues(OkHttpContext context) {
  KeyValues keyValues = KeyValues
  .of(METHOD.withValue(requestAvailable ? request.method() : TAG_VALUE_UNKNOWN),
  URI.withValue(getUriTag(urlMapper, state, request)),
- STATUS.withValue(getStatusMessage(state.response, state.exception)))
+ STATUS.withValue(getStatusMessage(state.response, state.exception)),
+ OUTCOME.withValue(getStatusOutcome(state.response).name()))
  .and(extraTags)
  .and(stream(contextSpecificTags.spliterator(), false)
  .map(contextTag -> contextTag.apply(request, state.response))
@@ -112,6 +114,14 @@ private String getUriTag(Function<Request, String> urlMapper, OkHttpObservationI
  : urlMapper.apply(request);
  }
 
+ private Outcome getStatusOutcome(@Nullable Response response) {
+ if (response == null) {
+ return Outcome.UNKNOWN;
+ }
+
+ return Outcome.forStatus(response.code());
+ }
+
  private String getStatusMessage(@Nullable Response response, @Nullable IOException exception) {
  if (exception != null) {
  return "IO_ERROR";

diff --git a/...rc/main/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpMetricsEventListener.java b/...rc/main/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpMetricsEventListener.java
@@ -22,6 +22,7 @@
 import io.micrometer.core.instrument.Tag;
 import io.micrometer.core.instrument.Tags;
 import io.micrometer.core.instrument.Timer;
+import io.micrometer.core.instrument.binder.http.Outcome;
 import okhttp3.EventListener;
 import okhttp3.*;
 
@@ -168,6 +169,7 @@ void time(CallState state) {
  Iterable<Tag> tags = Tags
  .of("method", requestAvailable ? request.method() : TAG_VALUE_UNKNOWN, "uri", getUriTag(state, request),
  "status", getStatusMessage(state.response, state.exception))
+ .and(getStatusOutcome(state.response).asTag())
  .and(extraTags)
  .and(stream(contextSpecificTags.spliterator(), false)
  .map(contextTag -> contextTag.apply(request, state.response))
@@ -219,6 +221,14 @@ private Iterable<Tag> getRequestTags(@Nullable Request request) {
  return Tags.empty();
  }
 
+ private Outcome getStatusOutcome(@Nullable Response response) {
+ if (response == null) {
+ return Outcome.UNKNOWN;
+ }
+
+ return Outcome.forStatus(response.code());
+ }
+
  private String getStatusMessage(@Nullable Response response, @Nullable IOException exception) {
  if (exception != null) {
  return "IO_ERROR";

diff --git a/...ain/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpObservationDocumentation.java b/...ain/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpObservationDocumentation.java
@@ -95,6 +95,13 @@ public String asString() {
  public String asString() {
  return "status";
  }
+ },
+
+ OUTCOME {
+ @Override
+ public String asString() {
+ return "outcome";
+ }
  }
 
  }

diff --git a/...va11/io/micrometer/core/instrument/binder/jdk/DefaultHttpClientObservationConvention.java b/...va11/io/micrometer/core/instrument/binder/jdk/DefaultHttpClientObservationConvention.java
@@ -18,6 +18,7 @@
 import io.micrometer.common.KeyValues;
 import io.micrometer.common.lang.NonNull;
 import io.micrometer.common.lang.Nullable;
+import io.micrometer.core.instrument.binder.http.Outcome;
 
 import java.net.http.HttpRequest;
 import java.net.http.HttpResponse;
@@ -47,8 +48,11 @@ public KeyValues getLowCardinalityKeyValues(HttpClientContext context) {
  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())));
+ 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;
  }

diff --git a/...n/java11/io/micrometer/core/instrument/binder/jdk/HttpClientObservationDocumentation.java b/...n/java11/io/micrometer/core/instrument/binder/jdk/HttpClientObservationDocumentation.java
@@ -60,6 +60,13 @@ public String asString() {
  }
  },
 
+ OUTCOME {
+ @Override
+ public String asString() {
+ return "outcome";
+ }
+ },
+
  /**
  * HTTP URI.
  */

diff --git a/...r-core/src/main/java11/io/micrometer/core/instrument/binder/jdk/MicrometerHttpClient.java b/...r-core/src/main/java11/io/micrometer/core/instrument/binder/jdk/MicrometerHttpClient.java
@@ -19,6 +19,7 @@
 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;
@@ -238,8 +239,11 @@ private <T> void stopObservationOrTimer(
  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())));
+ 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;
  });

diff --git a/...o/micrometer/core/instrument/binder/httpcomponents/MicrometerHttpRequestExecutorTest.java b/...o/micrometer/core/instrument/binder/httpcomponents/MicrometerHttpRequestExecutorTest.java
@@ -96,12 +96,18 @@ void httpStatusCodeIsTagged(boolean configureObservationRegistry, @WiremockResol
  EntityUtils.consume(client.execute(new HttpGet(server.baseUrl() + "/ok")).getEntity());
  EntityUtils.consume(client.execute(new HttpGet(server.baseUrl() + "/notfound")).getEntity());
  EntityUtils.consume(client.execute(new HttpGet(server.baseUrl() + "/error")).getEntity());
- assertThat(registry.get(EXPECTED_METER_NAME).tags("method", "GET", "status", "200").timer().count())
- .isEqualTo(2L);
- assertThat(registry.get(EXPECTED_METER_NAME).tags("method", "GET", "status", "404").timer().count())
- .isEqualTo(1L);
- assertThat(registry.get(EXPECTED_METER_NAME).tags("method", "GET", "status", "500").timer().count())
- .isEqualTo(1L);
+ assertThat(registry.get(EXPECTED_METER_NAME)
+ .tags("method", "GET", "status", "200", "outcome", "SUCCESS")
+ .timer()
+ .count()).isEqualTo(2L);
+ assertThat(registry.get(EXPECTED_METER_NAME)
+ .tags("method", "GET", "status", "404", "outcome", "CLIENT_ERROR")
+ .timer()
+ .count()).isEqualTo(1L);
+ assertThat(registry.get(EXPECTED_METER_NAME)
+ .tags("method", "GET", "status", "500", "outcome", "SERVER_ERROR")
+ .timer()
+ .count()).isEqualTo(1L);
  }
 
  @ParameterizedTest
@@ -329,8 +335,10 @@ void httpStatusCodeIsTaggedWithIoError(boolean configureObservationRegistry,
  assertThatThrownBy(
  () -> EntityUtils.consume(client.execute(new HttpGet(server.baseUrl() + "/error")).getEntity()))
  .isInstanceOf(ClientProtocolException.class);
- assertThat(registry.get(EXPECTED_METER_NAME).tags("method", "GET", "status", "IO_ERROR").timer().count())
- .isEqualTo(1L);
+ assertThat(registry.get(EXPECTED_METER_NAME)
+ .tags("method", "GET", "status", "IO_ERROR", "outcome", "UNKNOWN")
+ .timer()
+ .count()).isEqualTo(1L);
  }
 
  static class CustomGlobalApacheHttpConvention extends DefaultApacheHttpClientObservationConvention

diff --git a/...crometer/core/instrument/binder/httpcomponents/hc5/MicrometerHttpRequestExecutorTest.java b/...crometer/core/instrument/binder/httpcomponents/hc5/MicrometerHttpRequestExecutorTest.java
@@ -107,12 +107,18 @@ void httpStatusCodeIsTagged(boolean configureObservationRegistry, @WiremockResol
  execute(client, new HttpGet(server.baseUrl() + "/ok"));
  execute(client, new HttpGet(server.baseUrl() + "/notfound"));
  execute(client, new HttpGet(server.baseUrl() + "/error"));
- assertThat(registry.get(EXPECTED_METER_NAME).tags("method", "GET", "status", "200").timer().count())
- .isEqualTo(2L);
- assertThat(registry.get(EXPECTED_METER_NAME).tags("method", "GET", "status", "404").timer().count())
- .isEqualTo(1L);
- assertThat(registry.get(EXPECTED_METER_NAME).tags("method", "GET", "status", "500").timer().count())
- .isEqualTo(1L);
+ assertThat(registry.get(EXPECTED_METER_NAME)
+ .tags("method", "GET", "status", "200", "outcome", "SUCCESS")
+ .timer()
+ .count()).isEqualTo(2L);
+ assertThat(registry.get(EXPECTED_METER_NAME)
+ .tags("method", "GET", "status", "404", "outcome", "CLIENT_ERROR")
+ .timer()
+ .count()).isEqualTo(1L);
+ assertThat(registry.get(EXPECTED_METER_NAME)
+ .tags("method", "GET", "status", "500", "outcome", "SERVER_ERROR")
+ .timer()
+ .count()).isEqualTo(1L);
  }
 
  @ParameterizedTest
@@ -323,8 +329,10 @@ void httpStatusCodeIsTaggedWithIoError(boolean configureObservationRegistry,
  CloseableHttpClient client = client(executor(false, configureObservationRegistry));
  assertThatThrownBy(() -> execute(client, new HttpGet(server.baseUrl() + "/error")))
  .isInstanceOf(ClientProtocolException.class);
- assertThat(registry.get(EXPECTED_METER_NAME).tags("method", "GET", "status", "IO_ERROR").timer().count())
- .isEqualTo(1L);
+ assertThat(registry.get(EXPECTED_METER_NAME)
+ .tags("method", "GET", "status", "IO_ERROR", "outcome", "UNKNOWN")
+ .timer()
+ .count()).isEqualTo(1L);
  }
 
  static class CustomGlobalApacheHttpConvention extends DefaultApacheHttpClientObservationConvention

diff --git a/...est/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpMetricsEventListenerTest.java b/...est/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpMetricsEventListenerTest.java
@@ -72,8 +72,8 @@ void timeSuccessful(@WiremockResolver.Wiremock WireMockServer server) throws IOE
  client.newCall(request).execute().close();
 
  assertThat(registry.get("okhttp.requests")
- .tags("foo", "bar", "status", "200", "uri", URI_EXAMPLE_VALUE, "target.host", "localhost", "target.port",
- String.valueOf(server.port()), "target.scheme", "http")
+ .tags("foo", "bar", "status", "200", "outcome", "SUCCESS", "uri", URI_EXAMPLE_VALUE, "target.host",
+ "localhost", "target.port", String.valueOf(server.port()), "target.scheme", "http")
  .timer()
  .count()).isEqualTo(1L);
  }
@@ -86,8 +86,8 @@ void timeNotFound(@WiremockResolver.Wiremock WireMockServer server) throws IOExc
  client.newCall(request).execute().close();
 
  assertThat(registry.get("okhttp.requests")
- .tags("foo", "bar", "status", "404", "uri", "NOT_FOUND", "target.host", "localhost", "target.port",
- String.valueOf(server.port()), "target.scheme", "http")
+ .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);
  }
@@ -114,7 +114,8 @@ void timeFailureDueToTimeout(@WiremockResolver.Wiremock WireMockServer server) {
  }
 
  assertThat(registry.get("okhttp.requests")
- .tags("foo", "bar", "uri", URI_EXAMPLE_VALUE, "status", "IO_ERROR", "target.host", "localhost")
+ .tags("foo", "bar", "uri", URI_EXAMPLE_VALUE, "status", "IO_ERROR", "outcome", "UNKNOWN", "target.host",
+ "localhost")
  .timer()
  .count()).isEqualTo(1L);
  }