Skip to content

Commit

Permalink
Polish Apache HttpComponents instrumentation changes
Browse files Browse the repository at this point in the history 
See micrometer-metricsgh-3800
See micrometer-metricsgh-3941
  • Loading branch information
@izeye @shakuzen
izeye authored and shakuzen committed Feb 5, 2024
1 parent c38de20 commit d574120
Show file tree
Hide file tree
Showing 8 changed files with 183 additions and 130 deletions.
15 changes: 11 additions & 4 deletions ...java/io/micrometer/core/instrument/binder/httpcomponents/hc5/ApacheHttpClientContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@
*/
public class ApacheHttpClientContext extends RequestReplySenderContext<HttpRequest, HttpResponse> {

@SuppressWarnings("deprecation")
private static final DefaultUriMapper DEFAULT_URI_MAPPER = new DefaultUriMapper();

private final HttpClientContext clientContext;

private final Function<HttpRequest, String> uriMapper;
Expand All @@ -45,7 +48,7 @@ public class ApacheHttpClientContext extends RequestReplySenderContext<HttpReque
* @param apacheHttpContext the HTTP client context
* @param uriMapper the mapper that detects the URI template
* @param exportTagsForRoute whether route tags should be contributed
* @deprecated in favor of
* @deprecated as of 1.12.0 in favor of
* {@link #ApacheHttpClientContext(HttpRequest, HttpClientContext)}.
*/
@Deprecated
Expand All @@ -67,10 +70,10 @@ public ApacheHttpClientContext(HttpRequest request, HttpContext apacheHttpContex
* context} for the Apache HTTP Client 5 instrumentation.
* @param request the client request
* @param apacheHttpContext the HTTP client context
* @since 1.12.0
*/
@SuppressWarnings("deprecation")
public ApacheHttpClientContext(HttpRequest request, HttpClientContext apacheHttpContext) {
this(request, apacheHttpContext, new DefaultUriMapper(), true);
this(request, apacheHttpContext, DEFAULT_URI_MAPPER, true);
}

/**
Expand All @@ -84,6 +87,8 @@ public HttpContext getApacheHttpContext() {

/**
* Return the client context associated with the current HTTP request.
* @return HTTP client context
* @since 1.12.0
*/
public HttpClientContext getHttpClientContext() {
return this.clientContext;
Expand All @@ -92,7 +97,8 @@ public HttpClientContext getHttpClientContext() {
/**
* Return the function that extracts the URI template information from the current
* request.
* @deprecated as of 1.12.0 in favor of a {@link HttpClientContext} attribute.
* @return URI mapper
* @deprecated as of 1.12.0 in favor of an {@link HttpClientContext} attribute.
* @see ApacheHttpClientObservationConvention#URI_TEMPLATE_ATTRIBUTE
*/
@Deprecated
Expand All @@ -102,6 +108,7 @@ public Function<HttpRequest, String> getUriMapper() {

/**
* Whether the route information should be contributed as tags with metrics.
* @return whether the route information should be contributed as tags with metrics
* @deprecated as of 1.12.0 with no replacement.
*/
@Deprecated
Expand Down
2 changes: 1 addition & 1 deletion ...eter/core/instrument/binder/httpcomponents/hc5/ApacheHttpClientObservationConvention.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public interface ApacheHttpClientObservationConvention extends ObservationConven
* that should hold the String representation of the URI template used for creating
* the client URL.
* <p>
* This value can be contributed as a {@link io.micrometer.common.KeyValue} to the
* This value can be contributed as an {@link io.micrometer.common.KeyValue} to the
* recorded observations. <pre>
* String uriTemplate = "/users/{id}";
* HttpClientContext clientContext = ...
Expand Down
2 changes: 1 addition & 1 deletion ...r/core/instrument/binder/httpcomponents/hc5/ApacheHttpClientObservationDocumentation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public String asString() {
},
/**
* Key name for exception.
* @since 1.11.0
* @since 1.12.0
*/
EXCEPTION {
@Override
Expand Down
99 changes: 78 additions & 21 deletions ...re/instrument/binder/httpcomponents/hc5/DefaultApacheHttpClientObservationConvention.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,25 @@ public class DefaultApacheHttpClientObservationConvention implements ApacheHttpC
*/
public static final DefaultApacheHttpClientObservationConvention INSTANCE = new DefaultApacheHttpClientObservationConvention();

private static final KeyValue METHOD_UNKNOWN = KeyValue.of(ApacheHttpClientKeyNames.METHOD, "UNKNOWN");
private static final String CONTEXTUAL_NAME_UNKNOWN = "HTTP UNKNOWN";

private static final KeyValue URI_UNKNOWN = KeyValue.of(ApacheHttpClientKeyNames.URI, "UNKNOWN");
private static final KeyValue METHOD_UNKNOWN = ApacheHttpClientKeyNames.METHOD.withValue("UNKNOWN");

private static final KeyValue STATUS_IO_ERROR = KeyValue.of(ApacheHttpClientKeyNames.STATUS, "IO_ERROR");
private static final KeyValue URI_UNKNOWN = ApacheHttpClientKeyNames.URI.withValue("UNKNOWN");

private static final KeyValue STATUS_CLIENT_ERROR = KeyValue.of(ApacheHttpClientKeyNames.STATUS, "CLIENT_ERROR");
private static final KeyValue STATUS_IO_ERROR = ApacheHttpClientKeyNames.STATUS.withValue("IO_ERROR");

private static final KeyValue EXCEPTION_NONE = KeyValue.of(ApacheHttpClientKeyNames.EXCEPTION, KeyValue.NONE_VALUE);
private static final KeyValue STATUS_CLIENT_ERROR = ApacheHttpClientKeyNames.STATUS.withValue("CLIENT_ERROR");

private static final KeyValue EXCEPTION_NONE = ApacheHttpClientKeyNames.EXCEPTION.withValue(KeyValue.NONE_VALUE);

private static final KeyValue OUTCOME_UNKNOWN = ApacheHttpClientKeyNames.OUTCOME.withValue(Outcome.UNKNOWN.name());

private static final KeyValue TARGET_HOST_UNKNOWN = ApacheHttpClientKeyNames.TARGET_HOST.withValue("UNKNOWN");

private static final KeyValue TARGET_PORT_UNKNOWN = ApacheHttpClientKeyNames.TARGET_PORT.withValue("UNKNOWN");

private static final KeyValue TARGET_SCHEME_UNKNOWN = ApacheHttpClientKeyNames.TARGET_SCHEME.withValue("UNKNOWN");

// There is no need to instantiate this class multiple times, but it may be extended,
// hence protected visibility.
Expand All @@ -65,11 +75,10 @@ public String getName() {
@Override
public String getContextualName(ApacheHttpClientContext context) {
HttpRequest request = context.getCarrier();
String methodName = "UNKNOWN";
if (request != null && request.getMethod() != null) {
methodName = request.getMethod();
return "HTTP " + request.getMethod();
}
return "HTTP " + methodName;
return CONTEXTUAL_NAME_UNKNOWN;
}

@Override
Expand All @@ -78,14 +87,26 @@ public KeyValues getLowCardinalityKeyValues(ApacheHttpClientContext context) {
targetPort(context), targetScheme(context), uri(context));
}

/**
* Extract {@code exception} key value from context.
* @param context HTTP client context
* @return extracted {@code exception} key value
* @since 1.12.0
*/
protected KeyValue exception(ApacheHttpClientContext context) {
Throwable error = context.getError();
if (error != null) {
return KeyValue.of(ApacheHttpClientKeyNames.EXCEPTION, error.getClass().getSimpleName());
return ApacheHttpClientKeyNames.EXCEPTION.withValue(error.getClass().getSimpleName());
}
return EXCEPTION_NONE;
}

/**
* Extract {@code method} key value from context.
* @param context HTTP client context
* @return extracted {@code method} key value
* @since 1.12.0
*/
protected KeyValue method(ApacheHttpClientContext context) {
HttpRequest request = context.getCarrier();
if (request == null || request.getMethod() == null) {
Expand All @@ -94,52 +115,88 @@ protected KeyValue method(ApacheHttpClientContext context) {
return ApacheHttpClientKeyNames.METHOD.withValue(request.getMethod());
}

/**
* Extract {@code outcome} key value from context.
* @param context HTTP client context
* @return extracted {@code outcome} key value
* @since 1.12.0
*/
protected KeyValue outcome(ApacheHttpClientContext context) {
HttpResponse response = context.getResponse();
if (response == null) {
return KeyValue.of(ApacheHttpClientKeyNames.OUTCOME, Outcome.UNKNOWN.name());
return OUTCOME_UNKNOWN;
}
return KeyValue.of(ApacheHttpClientKeyNames.OUTCOME, Outcome.forStatus(response.getCode()).name());
return ApacheHttpClientKeyNames.OUTCOME.withValue(Outcome.forStatus(response.getCode()).name());
}

/**
* Extract {@code status} key value from context.
* @param context HTTP client context
* @return extracted {@code status} key value
* @since 1.12.0
*/
protected KeyValue status(ApacheHttpClientContext context) {
Throwable error = context.getError();
HttpResponse response = context.getResponse();
if (error instanceof IOException || error instanceof HttpException || error instanceof RuntimeException) {
return STATUS_IO_ERROR;
}
else if (response == null) {
HttpResponse response = context.getResponse();
if (response == null) {
return STATUS_CLIENT_ERROR;
}
return KeyValue.of(ApacheHttpClientKeyNames.STATUS, Integer.toString(response.getCode()));
return ApacheHttpClientKeyNames.STATUS.withValue(String.valueOf(response.getCode()));
}

/**
* Extract {@code target.host} key value from context.
* @param context HTTP client context
* @return extracted {@code target.host} key value
* @since 1.12.0
*/
protected KeyValue targetHost(ApacheHttpClientContext context) {
RouteInfo httpRoute = context.getHttpClientContext().getHttpRoute();
if (httpRoute != null) {
return KeyValue.of(ApacheHttpClientKeyNames.TARGET_HOST, httpRoute.getTargetHost().getHostName());
return ApacheHttpClientKeyNames.TARGET_HOST.withValue(httpRoute.getTargetHost().getHostName());
}
return KeyValue.of(ApacheHttpClientKeyNames.TARGET_HOST, "UNKNOWN");
return TARGET_HOST_UNKNOWN;
}

/**
* Extract {@code target.port} key value from context.
* @param context HTTP client context
* @return extracted {@code target.port} key value
* @since 1.12.0
*/
protected KeyValue targetPort(ApacheHttpClientContext context) {
Object routeAttribute = context.getHttpClientContext().getAttribute("http.route");
if (routeAttribute instanceof HttpRoute) {
int port = ((HttpRoute) routeAttribute).getTargetHost().getPort();
return KeyValue.of(ApacheHttpClientKeyNames.TARGET_PORT, String.valueOf(port));
return ApacheHttpClientKeyNames.TARGET_PORT.withValue(String.valueOf(port));
}
return KeyValue.of(ApacheHttpClientKeyNames.TARGET_PORT, "UNKNOWN");
return TARGET_PORT_UNKNOWN;
}

/**
* Extract {@code target.scheme} key value from context.
* @param context HTTP client context
* @return extracted {@code target.scheme} key value
* @since 1.12.0
*/
protected KeyValue targetScheme(ApacheHttpClientContext context) {
Object routeAttribute = context.getHttpClientContext().getAttribute("http.route");
if (routeAttribute instanceof HttpRoute) {
return KeyValue.of(ApacheHttpClientKeyNames.TARGET_SCHEME,
((HttpRoute) routeAttribute).getTargetHost().getSchemeName());
return ApacheHttpClientKeyNames.TARGET_SCHEME
.withValue(((HttpRoute) routeAttribute).getTargetHost().getSchemeName());
}
return KeyValue.of(ApacheHttpClientKeyNames.TARGET_SCHEME, "UNKNOWN");
return TARGET_SCHEME_UNKNOWN;
}

/**
* Extract {@code uri} key value from context.
* @param context HTTP client context
* @return extracted {@code uri} key value
* @since 1.12.0
*/
@SuppressWarnings("deprecation")
protected KeyValue uri(ApacheHttpClientContext context) {
HttpClientContext clientContext = context.getHttpClientContext();
Expand Down
21 changes: 10 additions & 11 deletions .../io/micrometer/core/instrument/binder/httpcomponents/hc5/ObservationExecChainHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@
import org.apache.hc.core5.http.*;
import org.apache.hc.core5.http.nio.AsyncDataConsumer;
import org.apache.hc.core5.http.nio.AsyncEntityProducer;
import org.apache.hc.core5.util.Args;

import java.io.IOException;
import java.util.Objects;
import java.util.concurrent.atomic.AtomicMarkableReference;

/**
Expand Down Expand Up @@ -84,7 +84,7 @@ public void execute(HttpRequest request, AsyncEntityProducer entityProducer, Asy
.observation(observationConvention, DefaultApacheHttpClientObservationConvention.INSTANCE,
() -> observationContext, observationRegistry)
.start();
ObervableCancellableDependency cancellable = new ObervableCancellableDependency(observation);
ObservableCancellableDependency cancellable = new ObservableCancellableDependency(observation);
chain.proceed(request, entityProducer, cancellable.wrapScope(scope), new AsyncExecCallback() {

@Override
Expand Down Expand Up @@ -123,10 +123,10 @@ public ClassicHttpResponse execute(ClassicHttpRequest request, ExecChain.Scope s
.observation(observationConvention, DefaultApacheHttpClientObservationConvention.INSTANCE,
() -> observationContext, observationRegistry)
.start();
ClassicHttpResponse response;
try {
response = chain.proceed(request, scope);
ClassicHttpResponse response = chain.proceed(request, scope);
observationContext.setResponse(response);
return response;
}
catch (Throwable exc) {
observation.error(exc);
Expand All @@ -135,16 +135,15 @@ public ClassicHttpResponse execute(ClassicHttpRequest request, ExecChain.Scope s
finally {
observation.stop();
}
return response;
}

private static class ObervableCancellableDependency implements CancellableDependency {
private static class ObservableCancellableDependency implements CancellableDependency {

private final Observation observation;

private final AtomicMarkableReference<Cancellable> dependencyRef;

public ObervableCancellableDependency(Observation observation) {
ObservableCancellableDependency(Observation observation) {
this.observation = observation;
this.dependencyRef = new AtomicMarkableReference<>(null, false);
}
Expand All @@ -155,9 +154,9 @@ public boolean isCancelled() {
}

@Override
public void setDependency(final Cancellable dependency) {
Args.notNull(dependency, "dependency");
final Cancellable actualDependency = dependencyRef.getReference();
public void setDependency(Cancellable dependency) {
Objects.requireNonNull(dependency, "dependency");
Cancellable actualDependency = dependencyRef.getReference();
if (!dependencyRef.compareAndSet(actualDependency, dependency, false, false)) {
dependency.cancel();
}
Expand All @@ -166,7 +165,7 @@ public void setDependency(final Cancellable dependency) {
@Override
public boolean cancel() {
while (!dependencyRef.isMarked()) {
final Cancellable actualDependency = dependencyRef.getReference();
Cancellable actualDependency = dependencyRef.getReference();
if (dependencyRef.compareAndSet(actualDependency, actualDependency, false, true)) {
if (actualDependency != null) {
actualDependency.cancel();
Expand Down

0 comments on commit d574120

Please sign in to comment.