Implement `error.type` in `spring-webflux` and `reactor-netty` instrumentations (#9967)

This commit is contained in:
Mateusz Rzeszutek 2023-12-05 13:27:09 +01:00 committed by GitHub
parent df8b334ccf
commit 1ac8c4d606
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 95 additions and 92 deletions

View File

@ -76,7 +76,7 @@ public interface HttpCommonAttributesGetter<REQUEST, RESPONSE> {
*/
@Nullable
default String getErrorType(
REQUEST request, @Nullable RESPONSE response, @Nullable Throwable throwable) {
REQUEST request, @Nullable RESPONSE response, @Nullable Throwable error) {
return null;
}
}

View File

@ -72,13 +72,15 @@ final class ReactorNettyHttpClientAttributesGetter
@Nullable
@Override
public String getServerAddress(HttpClientRequest request) {
return getHost(request);
String resourceUrl = request.resourceUrl();
return resourceUrl == null ? null : UrlParser.getHost(resourceUrl);
}
@Nullable
@Override
public Integer getServerPort(HttpClientRequest request) {
return getPort(request);
String resourceUrl = request.resourceUrl();
return resourceUrl == null ? null : UrlParser.getPort(resourceUrl);
}
@Nullable
@ -99,14 +101,14 @@ final class ReactorNettyHttpClientAttributesGetter
}
@Nullable
private static String getHost(HttpClientRequest request) {
String resourceUrl = request.resourceUrl();
return resourceUrl == null ? null : UrlParser.getHost(resourceUrl);
}
@Nullable
private static Integer getPort(HttpClientRequest request) {
String resourceUrl = request.resourceUrl();
return resourceUrl == null ? null : UrlParser.getPort(resourceUrl);
@Override
public String getErrorType(
HttpClientRequest request, @Nullable HttpClientResponse response, @Nullable Throwable error) {
// if both response and error are null it means the request has been cancelled -- see the
// ConnectionWrapper class
if (response == null && error == null) {
return "cancelled";
}
return null;
}
}

View File

@ -310,7 +310,7 @@ abstract class AbstractReactorNettyHttpClientTest
equalTo(SemanticAttributes.URL_FULL, uri.toString()),
equalTo(SemanticAttributes.SERVER_ADDRESS, "localhost"),
equalTo(SemanticAttributes.SERVER_PORT, uri.getPort()),
equalTo(HttpAttributes.ERROR_TYPE, "_OTHER")),
equalTo(HttpAttributes.ERROR_TYPE, "cancelled")),
span ->
span.hasName("test-http-server")
.hasKind(SpanKind.SERVER)

View File

@ -14,7 +14,6 @@ import io.opentelemetry.instrumentation.spring.webflux.v5_3.internal.ClientInstr
import io.opentelemetry.instrumentation.spring.webflux.v5_3.internal.WebClientHttpAttributesGetter;
import io.opentelemetry.instrumentation.spring.webflux.v5_3.internal.WebClientTracingFilter;
import io.opentelemetry.javaagent.bootstrap.internal.CommonConfig;
import io.opentelemetry.javaagent.bootstrap.internal.InstrumentationConfig;
import java.util.List;
import org.springframework.web.reactive.function.client.ClientRequest;
import org.springframework.web.reactive.function.client.ClientResponse;
@ -35,9 +34,6 @@ public final class WebClientHelper {
HttpClientPeerServiceAttributesExtractor.create(
WebClientHttpAttributesGetter.INSTANCE,
CommonConfig.get().getPeerServiceResolver())),
InstrumentationConfig.get()
.getBoolean(
"otel.instrumentation.spring-webflux.experimental-span-attributes", false),
CommonConfig.get().shouldEmitExperimentalHttpClientTelemetry());
public static void addFilter(List<ExchangeFilterFunction> exchangeFilterFunctions) {

View File

@ -45,6 +45,11 @@ interface.
a `WebFilter` and using the OpenTelemetry Reactor instrumentation to ensure context is
passed around correctly.
### Web client instrumentation
The `WebClient` instrumentation will emit the `error.type` attribute with value `cancelled` whenever
an outgoing HTTP request is cancelled.
### Setup
Here is how to set up client and server instrumentation respectively:

View File

@ -53,9 +53,8 @@ public final class SpringWebfluxTelemetryBuilder {
clientExtractorConfigurer = builder -> {};
private Consumer<HttpSpanNameExtractorBuilder<ClientRequest>> clientSpanNameExtractorConfigurer =
builder -> {};
private boolean captureExperimentalSpanAttributes = false;
private boolean emitExperimentalHttpClientMetrics = false;
private boolean emitExperimentalHttpServerMetrics = false;
private boolean emitExperimentalHttpClientTelemetry = false;
private boolean emitExperimentalHttpServerTelemetry = false;
SpringWebfluxTelemetryBuilder(OpenTelemetry openTelemetry) {
this.openTelemetry = openTelemetry;
@ -100,18 +99,6 @@ public final class SpringWebfluxTelemetryBuilder {
return this;
}
/**
* Sets whether experimental attributes should be set to spans. These attributes may be changed or
* removed in the future, so only enable this if you know you do not require attributes filled by
* this instrumentation to be stable across versions.
*/
@CanIgnoreReturnValue
public SpringWebfluxTelemetryBuilder setCaptureExperimentalSpanAttributes(
boolean captureExperimentalSpanAttributes) {
this.captureExperimentalSpanAttributes = captureExperimentalSpanAttributes;
return this;
}
/**
* Adds an additional {@link AttributesExtractor} to invoke to set attributes to instrumented
* items.
@ -178,26 +165,26 @@ public final class SpringWebfluxTelemetryBuilder {
/**
* Configures the instrumentation to emit experimental HTTP client metrics.
*
* @param emitExperimentalHttpClientMetrics {@code true} if the experimental HTTP client metrics
* @param emitExperimentalHttpClientTelemetry {@code true} if the experimental HTTP client metrics
* are to be emitted.
*/
@CanIgnoreReturnValue
public SpringWebfluxTelemetryBuilder setEmitExperimentalHttpClientMetrics(
boolean emitExperimentalHttpClientMetrics) {
this.emitExperimentalHttpClientMetrics = emitExperimentalHttpClientMetrics;
public SpringWebfluxTelemetryBuilder setEmitExperimentalHttpClientTelemetry(
boolean emitExperimentalHttpClientTelemetry) {
this.emitExperimentalHttpClientTelemetry = emitExperimentalHttpClientTelemetry;
return this;
}
/**
* Configures the instrumentation to emit experimental HTTP server metrics.
*
* @param emitExperimentalHttpServerMetrics {@code true} if the experimental HTTP server metrics
* @param emitExperimentalHttpServerTelemetry {@code true} if the experimental HTTP server metrics
* are to be emitted.
*/
@CanIgnoreReturnValue
public SpringWebfluxTelemetryBuilder setEmitExperimentalHttpServerMetrics(
boolean emitExperimentalHttpServerMetrics) {
this.emitExperimentalHttpServerMetrics = emitExperimentalHttpServerMetrics;
public SpringWebfluxTelemetryBuilder setEmitExperimentalHttpServerTelemetry(
boolean emitExperimentalHttpServerTelemetry) {
this.emitExperimentalHttpServerTelemetry = emitExperimentalHttpServerTelemetry;
return this;
}
@ -213,8 +200,7 @@ public final class SpringWebfluxTelemetryBuilder {
clientExtractorConfigurer,
clientSpanNameExtractorConfigurer,
clientAdditionalExtractors,
captureExperimentalSpanAttributes,
emitExperimentalHttpClientMetrics);
emitExperimentalHttpClientTelemetry);
Instrumenter<ServerWebExchange, ServerWebExchange> serverInstrumenter =
buildServerInstrumenter();
@ -234,7 +220,7 @@ public final class SpringWebfluxTelemetryBuilder {
.addAttributesExtractors(serverAdditionalExtractors)
.addContextCustomizer(httpServerRouteBuilder.build())
.addOperationMetrics(HttpServerMetrics.get());
if (emitExperimentalHttpServerMetrics) {
if (emitExperimentalHttpServerTelemetry) {
builder
.addAttributesExtractor(HttpExperimentalAttributesExtractor.create(getter))
.addOperationMetrics(HttpServerExperimentalMetrics.get());

View File

@ -40,8 +40,7 @@ public final class ClientInstrumenterFactory {
extractorConfigurer,
Consumer<HttpSpanNameExtractorBuilder<ClientRequest>> spanNameExtractorConfigurer,
List<AttributesExtractor<ClientRequest, ClientResponse>> additionalExtractors,
boolean captureExperimentalSpanAttributes,
boolean emitExperimentalHttpClientMetrics) {
boolean emitExperimentalHttpClientTelemetry) {
WebClientHttpAttributesGetter httpAttributesGetter = WebClientHttpAttributesGetter.INSTANCE;
@ -61,10 +60,7 @@ public final class ClientInstrumenterFactory {
.addAttributesExtractors(additionalExtractors)
.addOperationMetrics(HttpClientMetrics.get());
if (captureExperimentalSpanAttributes) {
clientBuilder.addAttributesExtractor(new WebClientExperimentalAttributesExtractor());
}
if (emitExperimentalHttpClientMetrics) {
if (emitExperimentalHttpClientTelemetry) {
clientBuilder
.addAttributesExtractor(HttpExperimentalAttributesExtractor.create(httpAttributesGetter))
.addOperationMetrics(HttpClientExperimentalMetrics.get());

View File

@ -1,43 +0,0 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.instrumentation.spring.webflux.v5_3.internal;
import static io.opentelemetry.api.common.AttributeKey.stringKey;
import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.api.common.AttributesBuilder;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
import javax.annotation.Nullable;
import org.springframework.web.reactive.function.client.ClientRequest;
import org.springframework.web.reactive.function.client.ClientResponse;
final class WebClientExperimentalAttributesExtractor
implements AttributesExtractor<ClientRequest, ClientResponse> {
private static final AttributeKey<String> SPRING_WEBFLUX_EVENT =
stringKey("spring-webflux.event");
private static final AttributeKey<String> SPRING_WEBFLUX_MESSAGE =
stringKey("spring-webflux.message");
@Override
public void onStart(AttributesBuilder attributes, Context parentContext, ClientRequest request) {}
@Override
public void onEnd(
AttributesBuilder attributes,
Context context,
ClientRequest request,
@Nullable ClientResponse response,
@Nullable Throwable error) {
// no response and no error means that the request has been cancelled
if (response == null && error == null) {
attributes.put(SPRING_WEBFLUX_EVENT, "cancelled");
attributes.put(SPRING_WEBFLUX_MESSAGE, "The subscription was cancelled");
}
}
}

View File

@ -59,4 +59,16 @@ public enum WebClientHttpAttributesGetter
public Integer getServerPort(ClientRequest request) {
return request.url().getPort();
}
@Nullable
@Override
public String getErrorType(
ClientRequest request, @Nullable ClientResponse response, @Nullable Throwable error) {
// if both response and error are null it means the request has been cancelled -- see the
// WebClientTracingFilter class
if (response == null && error == null) {
return "cancelled";
}
return null;
}
}

View File

@ -5,20 +5,29 @@
package io.opentelemetry.instrumentation.spring.webflux.client;
import static io.opentelemetry.api.trace.SpanKind.CLIENT;
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;
import static java.util.Collections.emptyMap;
import static java.util.Objects.requireNonNull;
import static org.assertj.core.api.Assertions.catchThrowable;
import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.api.trace.SpanKind;
import io.opentelemetry.instrumentation.api.semconv.http.internal.HttpAttributes;
import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpClientTest;
import io.opentelemetry.instrumentation.testing.junit.http.HttpClientResult;
import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions;
import io.opentelemetry.sdk.trace.data.StatusData;
import io.opentelemetry.semconv.SemanticAttributes;
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.net.URI;
import java.time.Duration;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import org.junit.jupiter.api.Test;
import org.springframework.http.HttpMethod;
import org.springframework.web.reactive.function.client.ClientResponse;
import org.springframework.web.reactive.function.client.WebClient;
@ -142,4 +151,44 @@ public abstract class AbstractSpringWebfluxClientInstrumentationTest
throw new AssertionError(e);
}
}
@Test
void shouldEndSpanOnMonoTimeout() {
URI uri = resolveAddress("/read-timeout");
Throwable thrown =
catchThrowable(
() ->
testing.runWithSpan(
"parent",
() ->
buildRequest("GET", uri, emptyMap())
.exchange()
// apply Mono timeout that is way shorter than HTTP request timeout
.timeout(Duration.ofSeconds(1))
.block()));
testing.waitAndAssertTraces(
trace ->
trace.hasSpansSatisfyingExactly(
span ->
span.hasName("parent")
.hasKind(SpanKind.INTERNAL)
.hasNoParent()
.hasStatus(StatusData.error())
.hasException(thrown),
span ->
span.hasName("GET")
.hasKind(CLIENT)
.hasParent(trace.getSpan(0))
.hasAttributesSatisfyingExactly(
equalTo(SemanticAttributes.HTTP_REQUEST_METHOD, "GET"),
equalTo(SemanticAttributes.URL_FULL, uri.toString()),
equalTo(SemanticAttributes.SERVER_ADDRESS, "localhost"),
equalTo(SemanticAttributes.SERVER_PORT, uri.getPort()),
equalTo(HttpAttributes.ERROR_TYPE, "cancelled")),
span ->
span.hasName("test-http-server")
.hasKind(SpanKind.SERVER)
.hasParent(trace.getSpan(1))));
}
}