From 3b3ed20f679674ca0c8b5172ae30cc56501dbf43 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Fri, 14 Jul 2023 12:08:33 +0200 Subject: [PATCH] Applied changes following the review --- ...ext.java => ClientObservationContext.java} | 19 +++- ...efaultHttpClientObservationConvention.java | 86 +++++++++----- .../HttpClientObservationDocumentation.java | 2 +- ...tionClient.java => ObservationClient.java} | 37 +++--- .../DefaultServiceObservationConvention.java | 105 +++++++++++------- ... HttpServiceObservationDocumentation.java} | 2 +- ...onService.java => ObservationService.java} | 41 ++++--- ...xt.java => ServiceObservationContext.java} | 18 ++- ... => ObservationClientIntegrationTest.java} | 10 +- ...ntTest.java => ObservationClientTest.java} | 12 +- .../observation/it/CustomObservationTest.java | 32 +++--- ...eTest.java => ObservationServiceTest.java} | 12 +- .../observation/SpanPropagationTest.java | 8 +- .../it/observation/BraveIntegrationTest.java | 8 +- 14 files changed, 241 insertions(+), 151 deletions(-) rename core/src/main/java/com/linecorp/armeria/client/observation/{HttpClientContext.java => ClientObservationContext.java} (77%) rename core/src/main/java/com/linecorp/armeria/client/observation/{MicrometerObservationClient.java => ObservationClient.java} (81%) rename core/src/main/java/com/linecorp/armeria/server/observation/{HttpServerObservationDocumentation.java => HttpServiceObservationDocumentation.java} (98%) rename core/src/main/java/com/linecorp/armeria/server/observation/{MicrometerObservationService.java => ObservationService.java} (78%) rename core/src/main/java/com/linecorp/armeria/server/observation/{HttpServerContext.java => ServiceObservationContext.java} (76%) rename core/src/test/java/com/linecorp/armeria/client/observation/{MicrometerObservationClientIntegrationTest.java => ObservationClientIntegrationTest.java} (95%) rename core/src/test/java/com/linecorp/armeria/client/observation/{MicrometerObservationClientTest.java => ObservationClientTest.java} (96%) rename core/src/test/java/com/linecorp/armeria/server/observation/{MicrometerObservationServiceTest.java => ObservationServiceTest.java} (96%) diff --git a/core/src/main/java/com/linecorp/armeria/client/observation/HttpClientContext.java b/core/src/main/java/com/linecorp/armeria/client/observation/ClientObservationContext.java similarity index 77% rename from core/src/main/java/com/linecorp/armeria/client/observation/HttpClientContext.java rename to core/src/main/java/com/linecorp/armeria/client/observation/ClientObservationContext.java index ec5c3c8c3a1e..d10f80ce92e9 100644 --- a/core/src/main/java/com/linecorp/armeria/client/observation/HttpClientContext.java +++ b/core/src/main/java/com/linecorp/armeria/client/observation/ClientObservationContext.java @@ -16,6 +16,8 @@ package com.linecorp.armeria.client.observation; +import com.google.common.base.MoreObjects; + import com.linecorp.armeria.client.ClientRequestContext; import com.linecorp.armeria.common.HttpRequest; import com.linecorp.armeria.common.RequestHeadersBuilder; @@ -28,7 +30,7 @@ import io.micrometer.observation.transport.RequestReplySenderContext; /** - * A {@link Context} which may be used in conjunction with {@link MicrometerObservationClient} + * A {@link Context} which may be used in conjunction with {@link ObservationClient} * to implement custom {@link ObservationConvention}s or {@link ObservationHandler}s. * 
{@code
  * ObservationConvention convention = ...
@@ -38,13 +40,14 @@
  * }
*/ @UnstableApi -public final class HttpClientContext extends RequestReplySenderContext { +public final class ClientObservationContext + extends RequestReplySenderContext { private final ClientRequestContext clientRequestContext; private final HttpRequest httpRequest; - HttpClientContext(ClientRequestContext clientRequestContext, RequestHeadersBuilder carrier, - HttpRequest httpRequest) { + ClientObservationContext(ClientRequestContext clientRequestContext, RequestHeadersBuilder carrier, + HttpRequest httpRequest) { super(RequestHeadersBuilder::add); this.clientRequestContext = clientRequestContext; this.httpRequest = httpRequest; @@ -64,4 +67,12 @@ public ClientRequestContext requestContext() { public HttpRequest httpRequest() { return httpRequest; } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this).omitNullValues() + .add("clientRequestContext", clientRequestContext) + .add("httpRequest", httpRequest) + .toString(); + } } diff --git a/core/src/main/java/com/linecorp/armeria/client/observation/DefaultHttpClientObservationConvention.java b/core/src/main/java/com/linecorp/armeria/client/observation/DefaultHttpClientObservationConvention.java index d228352112ac..0565bcc2a7bf 100644 --- a/core/src/main/java/com/linecorp/armeria/client/observation/DefaultHttpClientObservationConvention.java +++ b/core/src/main/java/com/linecorp/armeria/client/observation/DefaultHttpClientObservationConvention.java @@ -20,6 +20,8 @@ import java.net.InetSocketAddress; +import com.google.common.collect.ImmutableList; + import com.linecorp.armeria.client.ClientRequestContext; import com.linecorp.armeria.client.observation.HttpClientObservationDocumentation.HighCardinalityKeys; import com.linecorp.armeria.client.observation.HttpClientObservationDocumentation.LowCardinalityKeys; @@ -27,62 +29,89 @@ import com.linecorp.armeria.common.SessionProtocol; import com.linecorp.armeria.common.annotation.Nullable; import com.linecorp.armeria.common.logging.RequestLog; +import com.linecorp.armeria.common.logging.RequestLogAccess; +import com.linecorp.armeria.common.logging.RequestLogProperty; +import io.micrometer.common.KeyValue; import io.micrometer.common.KeyValues; import io.micrometer.observation.Observation.Context; import io.micrometer.observation.ObservationConvention; -class DefaultHttpClientObservationConvention implements ObservationConvention { +class DefaultHttpClientObservationConvention implements ObservationConvention { static final DefaultHttpClientObservationConvention INSTANCE = new DefaultHttpClientObservationConvention(); @Override - public KeyValues getLowCardinalityKeyValues(HttpClientContext context) { + public KeyValues getLowCardinalityKeyValues(ClientObservationContext context) { final ClientRequestContext ctx = context.requestContext(); - KeyValues keyValues = KeyValues.of( - LowCardinalityKeys.HTTP_METHOD.withValue(ctx.method().name())); + int expectedSize = 1; final RequestLog log = context.getResponse(); + KeyValue protocol = null; + KeyValue serializationFormat = null; + KeyValue statusCode = null; if (log != null) { - keyValues = keyValues.and(LowCardinalityKeys.HTTP_PROTOCOL.withValue(protocol(log))); + protocol = LowCardinalityKeys.HTTP_PROTOCOL.withValue(protocol(log)); + statusCode = LowCardinalityKeys.STATUS_CODE + .withValue(log.responseStatus().codeAsText()); + expectedSize = 3; final String serFmt = serializationFormat(log); if (serFmt != null) { - keyValues = keyValues.and(LowCardinalityKeys.HTTP_SERIALIZATION_FORMAT.withValue(serFmt)); + expectedSize = 4; + serializationFormat = LowCardinalityKeys.HTTP_SERIALIZATION_FORMAT.withValue(serFmt); } - keyValues = keyValues.and(LowCardinalityKeys.STATUS_CODE - .withValue(log.responseStatus().codeAsText())); } - return keyValues; + ImmutableList.Builder builder = ImmutableList.builderWithExpectedSize(expectedSize); + builder.add(LowCardinalityKeys.HTTP_METHOD.withValue(ctx.method().name())); + addIfNotNull(protocol, builder); + addIfNotNull(statusCode, builder); + addIfNotNull(serializationFormat, builder); + return KeyValues.of(builder.build()); + } + + private void addIfNotNull(@Nullable KeyValue keyValue, ImmutableList.Builder builder) { + if (keyValue != null) { + builder.add(keyValue); + } } @Override - public KeyValues getHighCardinalityKeyValues(HttpClientContext context) { + public KeyValues getHighCardinalityKeyValues(ClientObservationContext context) { final ClientRequestContext ctx = context.requestContext(); - KeyValues keyValues = KeyValues.of( - HighCardinalityKeys.HTTP_PATH.withValue(ctx.path()), - HighCardinalityKeys.HTTP_HOST.withValue(firstNonNull(ctx.authority(), "UNKNOWN")), - HighCardinalityKeys.HTTP_URL.withValue(ctx.uri().toString()) - ); + int expectedSize = 3; + KeyValue addressRemote = null; + KeyValue addressLocal = null; + KeyValue error = null; if (context.getResponse() != null) { final RequestLog log = ctx.log().ensureComplete(); final InetSocketAddress raddr = ctx.remoteAddress(); if (raddr != null) { - keyValues = keyValues.and(HighCardinalityKeys.ADDRESS_REMOTE.withValue(raddr.toString())); + expectedSize = expectedSize + 1; + addressRemote = HighCardinalityKeys.ADDRESS_REMOTE.withValue(raddr.toString()); } final InetSocketAddress laddr = ctx.localAddress(); if (laddr != null) { - keyValues = keyValues.and(HighCardinalityKeys.ADDRESS_LOCAL.withValue(laddr.toString())); + expectedSize = expectedSize + 1; + addressLocal = HighCardinalityKeys.ADDRESS_LOCAL.withValue(laddr.toString()); } final Throwable responseCause = log.responseCause(); if (responseCause != null) { - keyValues = keyValues.and(HighCardinalityKeys.ERROR.withValue(responseCause.toString())); + expectedSize = expectedSize + 1; + error = HighCardinalityKeys.ERROR.withValue(responseCause.toString()); } else if (log.responseStatus().isError()) { - keyValues = - keyValues.and(HighCardinalityKeys.ERROR.withValue(log.responseStatus().codeAsText())); + expectedSize = expectedSize + 1; + error = HighCardinalityKeys.ERROR.withValue(log.responseStatus().codeAsText()); } } - return keyValues; + ImmutableList.Builder builder = ImmutableList.builderWithExpectedSize(expectedSize); + builder.add(HighCardinalityKeys.HTTP_PATH.withValue(ctx.path()), + HighCardinalityKeys.HTTP_HOST.withValue(firstNonNull(ctx.authority(), "UNKNOWN")), + HighCardinalityKeys.HTTP_URL.withValue(ctx.uri().toString())); + addIfNotNull(addressRemote, builder); + addIfNotNull(addressLocal, builder); + addIfNotNull(error, builder); + return KeyValues.of(builder.build()); } /** @@ -107,15 +136,18 @@ public String getName() { } @Override - public String getContextualName(HttpClientContext context) { - final ClientRequestContext clientRequestContext = context.requestContext(); - final RequestLog log = clientRequestContext.log().ensureComplete(); - final String name = log.name(); - return firstNonNull(name, context.getName()); + public String getContextualName(ClientObservationContext context) { + final RequestLogAccess logAccess = context.requestContext().log(); + if (logAccess.isAvailable( + RequestLogProperty.NAME)) { + return logAccess.partial().fullName(); + } else { + return context.getName(); + } } @Override public boolean supportsContext(Context context) { - return context instanceof HttpClientContext; + return context instanceof ClientObservationContext; } } diff --git a/core/src/main/java/com/linecorp/armeria/client/observation/HttpClientObservationDocumentation.java b/core/src/main/java/com/linecorp/armeria/client/observation/HttpClientObservationDocumentation.java index 926467be8ad9..e52cfbe38927 100644 --- a/core/src/main/java/com/linecorp/armeria/client/observation/HttpClientObservationDocumentation.java +++ b/core/src/main/java/com/linecorp/armeria/client/observation/HttpClientObservationDocumentation.java @@ -29,7 +29,7 @@ enum HttpClientObservationDocumentation implements ObservationDocumentation { /** - * A span collected by {@link MicrometerObservationClient}. + * A span collected by {@link ObservationClient}. */ OBSERVATION { @Override diff --git a/core/src/main/java/com/linecorp/armeria/client/observation/MicrometerObservationClient.java b/core/src/main/java/com/linecorp/armeria/client/observation/ObservationClient.java similarity index 81% rename from core/src/main/java/com/linecorp/armeria/client/observation/MicrometerObservationClient.java rename to core/src/main/java/com/linecorp/armeria/client/observation/ObservationClient.java index bb1ebac0f5d7..1c14e8eb6a09 100644 --- a/core/src/main/java/com/linecorp/armeria/client/observation/MicrometerObservationClient.java +++ b/core/src/main/java/com/linecorp/armeria/client/observation/ObservationClient.java @@ -68,40 +68,39 @@ * } */ @UnstableApi -public final class MicrometerObservationClient extends SimpleDecoratingHttpClient { +public final class ObservationClient extends SimpleDecoratingHttpClient { /** * Creates a new micrometer observation integrated {@link HttpClient} decorator * using the specified {@link ObservationRegistry}. */ - public static Function newDecorator( + public static Function newDecorator( ObservationRegistry observationRegistry) { requireNonNull(observationRegistry, "observationRegistry"); - return delegate -> new MicrometerObservationClient(delegate, observationRegistry, null); + return delegate -> new ObservationClient(delegate, observationRegistry, null); } /** * Creates a new micrometer observation integrated {@link HttpClient} decorator * using the specified {@link ObservationRegistry} and {@link ObservationConvention}. */ - public static Function newDecorator( + public static Function newDecorator( ObservationRegistry observationRegistry, - ObservationConvention httpClientObservationConvention) { + @Nullable ObservationConvention observationConvention) { requireNonNull(observationRegistry, "observationRegistry"); - requireNonNull(observationConvention, "observationConvention"); - return delegate -> new MicrometerObservationClient(delegate, observationRegistry, - observationConvention); + return delegate -> new ObservationClient(delegate, observationRegistry, + observationConvention); } private final ObservationRegistry observationRegistry; @Nullable - private final ObservationConvention httpClientObservationConvention; + private final ObservationConvention httpClientObservationConvention; - private MicrometerObservationClient( + private ObservationClient( HttpClient delegate, ObservationRegistry observationRegistry, - @Nullable ObservationConvention httpClientObservationConvention) { + @Nullable ObservationConvention httpClientObservationConvention) { super(delegate); this.observationRegistry = requireNonNull(observationRegistry, "observationRegistry"); this.httpClientObservationConvention = httpClientObservationConvention; @@ -110,25 +109,29 @@ private MicrometerObservationClient( @Override public HttpResponse execute(ClientRequestContext ctx, HttpRequest req) throws Exception { final RequestHeadersBuilder newHeaders = req.headers().toBuilder(); - final HttpClientContext httpClientContext = new HttpClientContext(ctx, newHeaders, req); + final ClientObservationContext clientObservationContext = new ClientObservationContext(ctx, newHeaders, req); final Observation observation = HttpClientObservationDocumentation.OBSERVATION.observation( httpClientObservationConvention, DefaultHttpClientObservationConvention.INSTANCE, - () -> httpClientContext, observationRegistry).start(); + () -> clientObservationContext, observationRegistry).start(); final HttpRequest newReq = req.withHeaders(newHeaders); ctx.updateRequest(newReq); final RequestContextExtension ctxExtension = ctx.as(RequestContextExtension.class); - if (!observationRegistry.isNoop() && !observation.isNoop() && ctxExtension != null) { + if (observationRegistry.isNoop() || observation.isNoop()) { + return unwrap().execute(ctx, newReq); + } + + if (ctxExtension != null) { // Make the span the current span and run scope decorators when the ctx is pushed. ctxExtension.hook(observation::openScope); } - enrichObservation(ctx, httpClientContext, observation); + enrichObservation(ctx, clientObservationContext, observation); return observation.scopedChecked(() -> unwrap().execute(ctx, newReq)); } - private static void enrichObservation(ClientRequestContext ctx, HttpClientContext httpClientContext, + private static void enrichObservation(ClientRequestContext ctx, ClientObservationContext clientObservationContext, Observation observation) { if (observation.isNoop()) { // For no-op spans, we only need to inject into headers and don't set any other attributes. @@ -151,7 +154,7 @@ private static void enrichObservation(ClientRequestContext ctx, HttpClientContex .thenAccept(requestLog -> { // TODO: ClientConnectionTimings - there is no way to record events // with a specific timestamp for an observation - httpClientContext.setResponse(requestLog); + clientObservationContext.setResponse(requestLog); observation.stop(); }); } diff --git a/core/src/main/java/com/linecorp/armeria/server/observation/DefaultServiceObservationConvention.java b/core/src/main/java/com/linecorp/armeria/server/observation/DefaultServiceObservationConvention.java index ca75aa6adf29..94f77f7baec6 100644 --- a/core/src/main/java/com/linecorp/armeria/server/observation/DefaultServiceObservationConvention.java +++ b/core/src/main/java/com/linecorp/armeria/server/observation/DefaultServiceObservationConvention.java @@ -20,72 +20,98 @@ import java.net.InetSocketAddress; +import com.google.common.collect.ImmutableList; + import com.linecorp.armeria.common.SerializationFormat; import com.linecorp.armeria.common.SessionProtocol; import com.linecorp.armeria.common.annotation.Nullable; import com.linecorp.armeria.common.logging.RequestLog; +import com.linecorp.armeria.common.logging.RequestLogAccess; +import com.linecorp.armeria.common.logging.RequestLogProperty; import com.linecorp.armeria.server.ServiceRequestContext; -import com.linecorp.armeria.server.observation.HttpServerObservationDocumentation.HighCardinalityKeys; -import com.linecorp.armeria.server.observation.HttpServerObservationDocumentation.LowCardinalityKeys; +import com.linecorp.armeria.server.observation.HttpServiceObservationDocumentation.HighCardinalityKeys; +import com.linecorp.armeria.server.observation.HttpServiceObservationDocumentation.LowCardinalityKeys; +import io.micrometer.common.KeyValue; import io.micrometer.common.KeyValues; import io.micrometer.observation.Observation.Context; import io.micrometer.observation.ObservationConvention; -final class DefaultServiceObservationConvention implements ObservationConvention { +final class DefaultServiceObservationConvention implements ObservationConvention { static final DefaultServiceObservationConvention INSTANCE = new DefaultServiceObservationConvention(); @Override - public KeyValues getLowCardinalityKeyValues(HttpServerContext context) { + public KeyValues getLowCardinalityKeyValues(ServiceObservationContext context) { final ServiceRequestContext ctx = context.requestContext(); - KeyValues keyValues = KeyValues.of( - LowCardinalityKeys.HTTP_METHOD.withValue(ctx.method().name())); + int expectedSize = 1; + KeyValue protocol = null; + KeyValue serializationFormat = null; + KeyValue statusCode = null; if (context.getResponse() != null) { final RequestLog log = ctx.log().ensureComplete(); - keyValues = keyValues.and( - LowCardinalityKeys.HTTP_PROTOCOL.withValue(protocol(log))); + protocol = LowCardinalityKeys.HTTP_PROTOCOL.withValue(protocol(log)); + statusCode = LowCardinalityKeys.STATUS_CODE + .withValue(log.responseStatus().codeAsText()); + expectedSize = 3; final String serFmt = serializationFormat(log); if (serFmt != null) { - keyValues = keyValues.and( - LowCardinalityKeys.HTTP_SERIALIZATION_FORMAT.withValue(serFmt)); + expectedSize = 4; + serializationFormat = LowCardinalityKeys.HTTP_SERIALIZATION_FORMAT.withValue(serFmt); } - keyValues = keyValues.and( - LowCardinalityKeys.STATUS_CODE.withValue(log.responseStatus().codeAsText())); } - return keyValues; + ImmutableList.Builder builder = ImmutableList.builderWithExpectedSize(expectedSize); + builder.add(LowCardinalityKeys.HTTP_METHOD.withValue(ctx.method().name())); + addIfNotNull(protocol, builder); + addIfNotNull(statusCode, builder); + addIfNotNull(serializationFormat, builder); + return KeyValues.of(builder.build()); + } + + private void addIfNotNull(@Nullable KeyValue keyValue, ImmutableList.Builder builder) { + if (keyValue != null) { + builder.add(keyValue); + } } @Override - public KeyValues getHighCardinalityKeyValues(HttpServerContext context) { + public KeyValues getHighCardinalityKeyValues(ServiceObservationContext context) { final ServiceRequestContext ctx = context.requestContext(); - KeyValues keyValues = KeyValues.of( - HighCardinalityKeys.HTTP_PATH.withValue(ctx.path()), - HighCardinalityKeys.HTTP_HOST.withValue( - firstNonNull(context.httpRequest().authority(), "UNKNOWN")), - HighCardinalityKeys.HTTP_URL.withValue(ctx.uri().toString()) - ); - final RequestLog log = context.getResponse(); - if (log != null) { - + int expectedSize = 3; + KeyValue addressRemote = null; + KeyValue addressLocal = null; + KeyValue error = null; + if (context.getResponse() != null) { + final RequestLog log = ctx.log().ensureComplete(); final InetSocketAddress raddr = ctx.remoteAddress(); - keyValues = keyValues.and( - HighCardinalityKeys.ADDRESS_REMOTE.withValue(raddr.toString())); - + if (raddr != null) { + expectedSize = expectedSize + 1; + addressRemote = HighCardinalityKeys.ADDRESS_REMOTE.withValue(raddr.toString()); + } final InetSocketAddress laddr = ctx.localAddress(); - keyValues = keyValues.and( - HighCardinalityKeys.ADDRESS_LOCAL.withValue(laddr.toString())); + if (laddr != null) { + expectedSize = expectedSize + 1; + addressLocal = HighCardinalityKeys.ADDRESS_LOCAL.withValue(laddr.toString()); + } final Throwable responseCause = log.responseCause(); if (responseCause != null) { - keyValues = keyValues.and(HighCardinalityKeys.ERROR.withValue(responseCause.toString())); + expectedSize = expectedSize + 1; + error = HighCardinalityKeys.ERROR.withValue(responseCause.toString()); } else if (log.responseStatus().isError()) { - keyValues = keyValues.and( - HighCardinalityKeys.ERROR.withValue(log.responseStatus().codeAsText())); + expectedSize = expectedSize + 1; + error = HighCardinalityKeys.ERROR.withValue(log.responseStatus().codeAsText()); } } - return keyValues; + ImmutableList.Builder builder = ImmutableList.builderWithExpectedSize(expectedSize); + builder.add(HighCardinalityKeys.HTTP_PATH.withValue(ctx.path()), + HighCardinalityKeys.HTTP_HOST.withValue(firstNonNull(context.httpRequest().authority(), "UNKNOWN")), + HighCardinalityKeys.HTTP_URL.withValue(ctx.uri().toString())); + addIfNotNull(addressRemote, builder); + addIfNotNull(addressLocal, builder); + addIfNotNull(error, builder); + return KeyValues.of(builder.build()); } /** @@ -110,15 +136,18 @@ public String getName() { } @Override - public String getContextualName(HttpServerContext context) { - final ServiceRequestContext serviceRequestContext = context.requestContext(); - final RequestLog log = serviceRequestContext.log().ensureComplete(); - final String name = log.name(); - return firstNonNull(name, context.getName()); + public String getContextualName(ServiceObservationContext context) { + final RequestLogAccess logAccess = context.requestContext().log(); + if (logAccess.isAvailable( + RequestLogProperty.NAME)) { + return logAccess.partial().fullName(); + } else { + return context.getName(); + } } @Override public boolean supportsContext(Context context) { - return context instanceof HttpServerContext; + return context instanceof ServiceObservationContext; } } diff --git a/core/src/main/java/com/linecorp/armeria/server/observation/HttpServerObservationDocumentation.java b/core/src/main/java/com/linecorp/armeria/server/observation/HttpServiceObservationDocumentation.java similarity index 98% rename from core/src/main/java/com/linecorp/armeria/server/observation/HttpServerObservationDocumentation.java rename to core/src/main/java/com/linecorp/armeria/server/observation/HttpServiceObservationDocumentation.java index 223ae7f7f597..8604a447f8fe 100644 --- a/core/src/main/java/com/linecorp/armeria/server/observation/HttpServerObservationDocumentation.java +++ b/core/src/main/java/com/linecorp/armeria/server/observation/HttpServiceObservationDocumentation.java @@ -29,7 +29,7 @@ enum HttpServiceObservationDocumentation implements ObservationDocumentation { /** - * A span collected by {@link MicrometerObservationService}. + * A span collected by {@link ObservationService}. */ OBSERVATION { @Override diff --git a/core/src/main/java/com/linecorp/armeria/server/observation/MicrometerObservationService.java b/core/src/main/java/com/linecorp/armeria/server/observation/ObservationService.java similarity index 78% rename from core/src/main/java/com/linecorp/armeria/server/observation/MicrometerObservationService.java rename to core/src/main/java/com/linecorp/armeria/server/observation/ObservationService.java index 8699a7528b37..d917e0c60501 100644 --- a/core/src/main/java/com/linecorp/armeria/server/observation/MicrometerObservationService.java +++ b/core/src/main/java/com/linecorp/armeria/server/observation/ObservationService.java @@ -62,7 +62,7 @@ * * // add the decorator * Server.builder() - * .decorator(MicrometerObservationService.newDecorator(registry)) + * .decorator(ObservationService.newDecorator(registry)) * ... * } */ @@ -73,32 +73,32 @@ public final class ObservationService extends SimpleDecoratingHttpService { * Creates a new micrometer observation integrated {@link HttpService} decorator using the * specified {@link ObservationRegistry} instance. */ - public static Function + public static Function newDecorator(ObservationRegistry observationRegistry) { requireNonNull(observationRegistry, "observationRegistry"); - return service -> new MicrometerObservationService(service, observationRegistry, null); + return service -> new ObservationService(service, observationRegistry, null); } /** * Creates a new micrometer observation integrated {@link HttpService} decorator using the * specified {@link ObservationRegistry} and {@link ObservationConvention}. */ - public static Function + public static Function newDecorator(ObservationRegistry observationRegistry, - ObservationConvention observationConvention) { + ObservationConvention observationConvention) { requireNonNull(observationRegistry, "observationRegistry"); requireNonNull(observationConvention, "observationConvention"); - return service -> new MicrometerObservationService( + return service -> new ObservationService( service, observationRegistry, requireNonNull(observationConvention, "observationConvention")); } private final ObservationRegistry observationRegistry; @Nullable - private final ObservationConvention observationConvention; + private final ObservationConvention observationConvention; - private MicrometerObservationService( + private ObservationService( HttpService delegate, ObservationRegistry observationRegistry, - @Nullable ObservationConvention observationConvention) { + @Nullable ObservationConvention observationConvention) { super(delegate); this.observationRegistry = requireNonNull(observationRegistry, "observationRegistry"); this.observationConvention = observationConvention; @@ -112,23 +112,28 @@ public HttpResponse serve(ServiceRequestContext ctx, HttpRequest req) throws Exc return unwrap().serve(ctx, req); } - final HttpServerContext httpServerContext = new HttpServerContext(ctx, req); - final Observation observation = HttpServerObservationDocumentation.OBSERVATION.observation( + final ServiceObservationContext serviceObservationContext = new ServiceObservationContext(ctx, req); + final Observation observation = HttpServiceObservationDocumentation.OBSERVATION.observation( observationConvention, DefaultServiceObservationConvention.INSTANCE, - () -> httpServerContext, observationRegistry).start(); + () -> serviceObservationContext, observationRegistry).start(); final RequestContextExtension ctxExtension = ctx.as(RequestContextExtension.class); - if (!observationRegistry.isNoop() && !observation.isNoop() && ctxExtension != null) { + + if (observationRegistry.isNoop() || observation.isNoop()) { + return unwrap().serve(ctx, req); + } + + if (ctxExtension != null) { // Make the span the current span and run scope decorators when the ctx is pushed. ctxExtension.hook(observation::openScope); } - enrichObservation(ctx, httpServerContext, observation); + enrichObservation(ctx, serviceObservationContext, observation); return observation.scopedChecked(() -> unwrap().serve(ctx, req)); } - private void enrichObservation(ServiceRequestContext ctx, HttpServerContext httpServerContext, + private void enrichObservation(ServiceRequestContext ctx, ServiceObservationContext serviceObservationContext, Observation observation) { if (observation.isNoop()) { // For no-op spans, we only need to inject into headers and don't set any other attributes. @@ -137,19 +142,19 @@ private void enrichObservation(ServiceRequestContext ctx, HttpServerContext http ctx.log() .whenAvailable(RequestLogProperty.REQUEST_FIRST_BYTES_TRANSFERRED_TIME) - .thenAccept(requestLog -> observation.event(HttpServerObservationDocumentation.Events.WIRE_RECEIVE)); + .thenAccept(requestLog -> observation.event(HttpServiceObservationDocumentation.Events.WIRE_RECEIVE)); ctx.log() .whenAvailable(RequestLogProperty.RESPONSE_FIRST_BYTES_TRANSFERRED_TIME) .thenAccept(requestLog -> { if (requestLog.responseFirstBytesTransferredTimeNanos() != null) { - observation.event(HttpServerObservationDocumentation.Events.WIRE_SEND); + observation.event(HttpServiceObservationDocumentation.Events.WIRE_SEND); } }); ctx.log().whenComplete() .thenAccept(requestLog -> { - httpServerContext.setResponse(requestLog); + serviceObservationContext.setResponse(requestLog); observation.stop(); }); } diff --git a/core/src/main/java/com/linecorp/armeria/server/observation/HttpServerContext.java b/core/src/main/java/com/linecorp/armeria/server/observation/ServiceObservationContext.java similarity index 76% rename from core/src/main/java/com/linecorp/armeria/server/observation/HttpServerContext.java rename to core/src/main/java/com/linecorp/armeria/server/observation/ServiceObservationContext.java index 4b35844f39b7..2f5b89974216 100644 --- a/core/src/main/java/com/linecorp/armeria/server/observation/HttpServerContext.java +++ b/core/src/main/java/com/linecorp/armeria/server/observation/ServiceObservationContext.java @@ -16,6 +16,8 @@ package com.linecorp.armeria.server.observation; +import com.google.common.base.MoreObjects; + import com.linecorp.armeria.common.HttpRequest; import com.linecorp.armeria.common.annotation.UnstableApi; import com.linecorp.armeria.common.logging.RequestLog; @@ -27,22 +29,22 @@ import io.micrometer.observation.transport.RequestReplyReceiverContext; /** - * A {@link Context} which may be used in conjunction with {@link MicrometerObservationService} + * A {@link Context} which may be used in conjunction with {@link ObservationService} * to implement custom {@link ObservationConvention}s or {@link ObservationHandler}s. * 
{@code
  * ObservationConvention convention = ...
  * Server.builder()
- *       .decorator(MicrometerObservationService.newDecorator(registry, convention))
+ *       .decorator(ObservationService.newDecorator(registry, convention))
  * ...
  * }
*/ @UnstableApi -public final class HttpServerContext extends RequestReplyReceiverContext { +public final class ServiceObservationContext extends RequestReplyReceiverContext { private final ServiceRequestContext serviceRequestContext; private final HttpRequest httpRequest; - HttpServerContext(ServiceRequestContext serviceRequestContext, HttpRequest httpRequest) { + ServiceObservationContext(ServiceRequestContext serviceRequestContext, HttpRequest httpRequest) { super((c, key) -> c.headers().get(key)); this.serviceRequestContext = serviceRequestContext; this.httpRequest = httpRequest; @@ -62,4 +64,12 @@ public ServiceRequestContext requestContext() { public HttpRequest httpRequest() { return httpRequest; } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this).omitNullValues() + .add("serviceRequestContext", serviceRequestContext) + .add("httpRequest", httpRequest) + .toString(); + } } diff --git a/core/src/test/java/com/linecorp/armeria/client/observation/MicrometerObservationClientIntegrationTest.java b/core/src/test/java/com/linecorp/armeria/client/observation/ObservationClientIntegrationTest.java similarity index 95% rename from core/src/test/java/com/linecorp/armeria/client/observation/MicrometerObservationClientIntegrationTest.java rename to core/src/test/java/com/linecorp/armeria/client/observation/ObservationClientIntegrationTest.java index 166647d8b766..d0271ecc7be6 100644 --- a/core/src/test/java/com/linecorp/armeria/client/observation/MicrometerObservationClientIntegrationTest.java +++ b/core/src/test/java/com/linecorp/armeria/client/observation/ObservationClientIntegrationTest.java @@ -48,7 +48,7 @@ import okhttp3.Protocol; @RunWith(Parameterized.class) -public class MicrometerObservationClientIntegrationTest extends ITHttpAsyncClient { +public class ObservationClientIntegrationTest extends ITHttpAsyncClient { @Parameters public static List sessionProtocols() { @@ -73,7 +73,7 @@ public static void closeClientFactory() { private DefaultHttpClientObservationConvention clientObservationConvention; - public MicrometerObservationClientIntegrationTest(SessionProtocol sessionProtocol) { + public ObservationClientIntegrationTest(SessionProtocol sessionProtocol) { this.sessionProtocol = sessionProtocol; if (sessionProtocol == SessionProtocol.H2C) { @@ -99,7 +99,7 @@ protected CurrentTraceContext.Builder currentTraceContextBuilder() { protected WebClient newClient(int port) { return WebClient.builder(sessionProtocol.uriText() + "://127.0.0.1:" + port) .factory(clientFactoryWithoutUpgradeRequest) - .decorator(MicrometerObservationClient.newDecorator( + .decorator(ObservationClient.newDecorator( observationRegistry(), clientObservationConvention)) .build(); } @@ -145,7 +145,7 @@ public void supportsPortableCustomization() throws IOException { clientObservationConvention = new DefaultHttpClientObservationConvention() { @Override - public KeyValues getHighCardinalityKeyValues(HttpClientContext context) { + public KeyValues getHighCardinalityKeyValues(ClientObservationContext context) { context.setRemoteServiceName("remote-service"); // TODO: As a side effect KeyValues values = super.getHighCardinalityKeyValues( @@ -161,7 +161,7 @@ public KeyValues getHighCardinalityKeyValues(HttpClientContext context) { } @Override - public String getContextualName(HttpClientContext context) { + public String getContextualName(ClientObservationContext context) { return context.httpRequest().method() .toString().toLowerCase() + " " + context.httpRequest().path() diff --git a/core/src/test/java/com/linecorp/armeria/client/observation/MicrometerObservationClientTest.java b/core/src/test/java/com/linecorp/armeria/client/observation/ObservationClientTest.java similarity index 96% rename from core/src/test/java/com/linecorp/armeria/client/observation/MicrometerObservationClientTest.java rename to core/src/test/java/com/linecorp/armeria/client/observation/ObservationClientTest.java index b1701779453b..770e48ac856d 100644 --- a/core/src/test/java/com/linecorp/armeria/client/observation/MicrometerObservationClientTest.java +++ b/core/src/test/java/com/linecorp/armeria/client/observation/ObservationClientTest.java @@ -16,7 +16,7 @@ package com.linecorp.armeria.client.observation; -import static com.linecorp.armeria.client.observation.MicrometerObservationClient.newDecorator; +import static com.linecorp.armeria.client.observation.ObservationClient.newDecorator; import static com.linecorp.armeria.internal.common.observation.MicrometerObservationRegistryUtils.observationRegistry; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -66,11 +66,11 @@ import brave.sampler.Sampler; import io.micrometer.common.KeyValues; -class MicrometerObservationClientTest { +class ObservationClientTest { private static final String TEST_SERVICE = "test-service"; - private static final String TEST_SPAN = "hello"; + private static final String TEST_SPAN = "java.lang.Object/hello"; @AfterEach void tearDown() { @@ -84,7 +84,7 @@ void newDecorator_shouldWorkWhenRequestContextCurrentTraceContextNotConfigured() Tracing.newBuilder().build())), new DefaultHttpClientObservationConvention() { @Override - public KeyValues getHighCardinalityKeyValues(HttpClientContext context) { + public KeyValues getHighCardinalityKeyValues(ClientObservationContext context) { context.setRemoteServiceName("remote-service"); return super.getHighCardinalityKeyValues(context); } @@ -254,10 +254,10 @@ private static RequestLog testRemoteInvocation(Tracing tracing, @Nullable String final HttpClient delegate = mock(HttpClient.class); when(delegate.execute(any(), any())).thenReturn(res); - final MicrometerObservationClient stub = newDecorator(observationRegistry( + final ObservationClient stub = newDecorator(observationRegistry( HttpTracing.create(tracing)), new DefaultHttpClientObservationConvention() { @Override - public KeyValues getHighCardinalityKeyValues(HttpClientContext context) { + public KeyValues getHighCardinalityKeyValues(ClientObservationContext context) { context.setRemoteServiceName(remoteServiceName); return super.getHighCardinalityKeyValues(context); } diff --git a/core/src/test/java/com/linecorp/armeria/internal/common/observation/it/CustomObservationTest.java b/core/src/test/java/com/linecorp/armeria/internal/common/observation/it/CustomObservationTest.java index a65ce0b02b61..5be55c3cfa35 100644 --- a/core/src/test/java/com/linecorp/armeria/internal/common/observation/it/CustomObservationTest.java +++ b/core/src/test/java/com/linecorp/armeria/internal/common/observation/it/CustomObservationTest.java @@ -31,15 +31,15 @@ import com.linecorp.armeria.client.ClientRequestContextCaptor; import com.linecorp.armeria.client.Clients; import com.linecorp.armeria.client.HttpClient; -import com.linecorp.armeria.client.observation.HttpClientContext; -import com.linecorp.armeria.client.observation.MicrometerObservationClient; +import com.linecorp.armeria.client.observation.ClientObservationContext; +import com.linecorp.armeria.client.observation.ObservationClient; import com.linecorp.armeria.common.HttpResponse; import com.linecorp.armeria.internal.common.observation.MicrometerObservationRegistryUtils; import com.linecorp.armeria.internal.common.observation.SpanCollector; import com.linecorp.armeria.server.HttpService; import com.linecorp.armeria.server.ServerBuilder; -import com.linecorp.armeria.server.observation.HttpServerContext; -import com.linecorp.armeria.server.observation.MicrometerObservationService; +import com.linecorp.armeria.server.observation.ServiceObservationContext; +import com.linecorp.armeria.server.observation.ObservationService; import com.linecorp.armeria.testing.junit5.server.ServerExtension; import brave.Tracing; @@ -65,12 +65,12 @@ private static Tracing tracingBuilder(String name) { .build(); } - private static Function + private static Function customConventionServiceDecorator(String name) { - final ObservationConvention convention = - new ObservationConvention() { + final ObservationConvention convention = + new ObservationConvention() { @Override - public KeyValues getLowCardinalityKeyValues(HttpServerContext context) { + public KeyValues getLowCardinalityKeyValues(ServiceObservationContext context) { return KeyValues.of("ctx.id", context.requestContext().id().shortText()); } @@ -81,18 +81,18 @@ public String getName() { @Override public boolean supportsContext(Context context) { - return context instanceof HttpServerContext; + return context instanceof ServiceObservationContext; } }; - return MicrometerObservationService.newDecorator(newTracing(name), convention); + return ObservationService.newDecorator(newTracing(name), convention); } - private static Function + private static Function customConventionClientDecorator(String name) { - final ObservationConvention convention = - new ObservationConvention() { + final ObservationConvention convention = + new ObservationConvention() { @Override - public KeyValues getLowCardinalityKeyValues(HttpClientContext context) { + public KeyValues getLowCardinalityKeyValues(ClientObservationContext context) { return KeyValues.of("ctx.id", context.requestContext().id().shortText()); } @@ -103,10 +103,10 @@ public String getName() { @Override public boolean supportsContext(Context context) { - return context instanceof HttpClientContext; + return context instanceof ClientObservationContext; } }; - return MicrometerObservationClient.newDecorator(newTracing(name), convention); + return ObservationClient.newDecorator(newTracing(name), convention); } @RegisterExtension diff --git a/core/src/test/java/com/linecorp/armeria/server/observation/MicrometerObservationServiceTest.java b/core/src/test/java/com/linecorp/armeria/server/observation/ObservationServiceTest.java similarity index 96% rename from core/src/test/java/com/linecorp/armeria/server/observation/MicrometerObservationServiceTest.java rename to core/src/test/java/com/linecorp/armeria/server/observation/ObservationServiceTest.java index 94bdd0cd09ef..02c647d9823e 100644 --- a/core/src/test/java/com/linecorp/armeria/server/observation/MicrometerObservationServiceTest.java +++ b/core/src/test/java/com/linecorp/armeria/server/observation/ObservationServiceTest.java @@ -61,7 +61,7 @@ import brave.propagation.ThreadLocalCurrentTraceContext; import brave.sampler.Sampler; -class MicrometerObservationServiceTest { +class ObservationServiceTest { private static final String TEST_SERVICE = "test-service"; @@ -74,7 +74,7 @@ public void tearDown() { @Test void newDecorator_shouldWorkWhenRequestContextCurrentTraceContextConfigured() { - MicrometerObservationService.newDecorator( + ObservationService.newDecorator( MicrometerObservationRegistryUtils.observationRegistry(HttpTracing.create( Tracing.newBuilder() .currentTraceContext(ThreadLocalCurrentTraceContext.create()) @@ -179,8 +179,8 @@ public Set transientServiceOptions() { logBuilder.endRequest(); try (SafeCloseable ignored = ctx.push()) { - final MicrometerObservationService - service = MicrometerObservationService.newDecorator( + final ObservationService + service = ObservationService.newDecorator( MicrometerObservationRegistryUtils.observationRegistry(tracing)).apply(transientService); // do invoke @@ -220,8 +220,8 @@ private static RequestLog testServiceInvocation(SpanHandler spanHandler, try (SafeCloseable ignored = ctx.push()) { final HttpService delegate = mock(HttpService.class); - final MicrometerObservationService - service = MicrometerObservationService.newDecorator( + final ObservationService + service = ObservationService.newDecorator( MicrometerObservationRegistryUtils.observationRegistry(httpTracing)).apply(delegate); when(delegate.serve(ctx, req)).thenReturn(res); diff --git a/core/src/test/java/com/linecorp/armeria/server/observation/SpanPropagationTest.java b/core/src/test/java/com/linecorp/armeria/server/observation/SpanPropagationTest.java index 1be3d100e64a..3cf63cfaa845 100644 --- a/core/src/test/java/com/linecorp/armeria/server/observation/SpanPropagationTest.java +++ b/core/src/test/java/com/linecorp/armeria/server/observation/SpanPropagationTest.java @@ -33,7 +33,7 @@ import com.linecorp.armeria.client.Clients; import com.linecorp.armeria.client.WebClient; import com.linecorp.armeria.client.logging.LoggingClient; -import com.linecorp.armeria.client.observation.MicrometerObservationClient; +import com.linecorp.armeria.client.observation.ObservationClient; import com.linecorp.armeria.common.HttpResponse; import com.linecorp.armeria.internal.common.observation.MicrometerObservationRegistryUtils; import com.linecorp.armeria.server.ServerBuilder; @@ -73,7 +73,7 @@ protected void configure(ServerBuilder sb) { serviceTraceCtx.set(tracing.currentTraceContext().get()); }, ctx.eventLoop()); return HttpResponse.from( - server.webClient(cb -> cb.decorator(MicrometerObservationClient.newDecorator( + server.webClient(cb -> cb.decorator(ObservationClient.newDecorator( MicrometerObservationRegistryUtils.observationRegistry(tracing)))) .get("/bar").aggregate().thenApply(res -> { return HttpResponse.of("OK"); @@ -84,7 +84,7 @@ protected void configure(ServerBuilder sb) { return HttpResponse.of("OK"); }); sb.decorator(LoggingService.newDecorator()); - sb.decorator(MicrometerObservationService.newDecorator( + sb.decorator(ObservationService.newDecorator( MicrometerObservationRegistryUtils.observationRegistry(tracing))); } }; @@ -102,7 +102,7 @@ void mdcScopeDecorator() { clientTraceCtx.set(tracing.currentTraceContext().get()); return delegate.execute(ctx, req); })) - .decorator(MicrometerObservationClient.newDecorator( + .decorator(ObservationClient.newDecorator( MicrometerObservationRegistryUtils.observationRegistry( tracing))) .decorator(LoggingClient.newDecorator()) diff --git a/thrift/thrift0.13/src/test/java/com/linecorp/armeria/it/observation/BraveIntegrationTest.java b/thrift/thrift0.13/src/test/java/com/linecorp/armeria/it/observation/BraveIntegrationTest.java index 4181b9db7844..3e929b5b9984 100644 --- a/thrift/thrift0.13/src/test/java/com/linecorp/armeria/it/observation/BraveIntegrationTest.java +++ b/thrift/thrift0.13/src/test/java/com/linecorp/armeria/it/observation/BraveIntegrationTest.java @@ -19,7 +19,7 @@ import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.util.concurrent.Futures.allAsList; import static com.google.common.util.concurrent.Futures.transformAsync; -import static com.linecorp.armeria.client.observation.MicrometerObservationClient.newDecorator; +import static com.linecorp.armeria.client.observation.ObservationClient.newDecorator; import static com.linecorp.armeria.common.SessionProtocol.H1C; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -68,7 +68,7 @@ import com.linecorp.armeria.server.HttpService; import com.linecorp.armeria.server.ServerBuilder; import com.linecorp.armeria.server.ServiceRequestContext; -import com.linecorp.armeria.server.observation.MicrometerObservationService; +import com.linecorp.armeria.server.observation.ObservationService; import com.linecorp.armeria.server.thrift.THttpService; import com.linecorp.armeria.service.test.thrift.main.HelloService; import com.linecorp.armeria.service.test.thrift.main.HelloService.AsyncIface; @@ -240,8 +240,8 @@ void shouldHaveNoExtraSpans() { assertThat(spanHandler.spans).isEmpty(); } - private static MicrometerObservationService decorate(String name, HttpService service) { - return MicrometerObservationService.newDecorator(newTracing(name)).apply(service); + private static ObservationService decorate(String name, HttpService service) { + return ObservationService.newDecorator(newTracing(name)).apply(service); } private static HelloService.AsyncIface newClient(String path) {