Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,15 @@ public String getNetworkProtocolVersion(
@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
Expand All @@ -99,14 +101,14 @@ public InetSocketAddress getNetworkPeerInetSocketAddress(
}

@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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ void shouldEndSpanOnMonoTimeout() {
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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
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;
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -100,18 +99,6 @@ public SpringWebfluxTelemetryBuilder setCapturedClientResponseHeaders(
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.
Expand Down Expand Up @@ -178,26 +165,26 @@ public SpringWebfluxTelemetryBuilder setKnownMethods(Set<String> knownMethods) {
/**
* 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;
}

Expand All @@ -213,8 +200,7 @@ public SpringWebfluxTelemetry build() {
clientExtractorConfigurer,
clientSpanNameExtractorConfigurer,
clientAdditionalExtractors,
captureExperimentalSpanAttributes,
emitExperimentalHttpClientMetrics);
emitExperimentalHttpClientTelemetry);

Instrumenter<ServerWebExchange, ServerWebExchange> serverInstrumenter =
buildServerInstrumenter();
Expand All @@ -234,7 +220,7 @@ private Instrumenter<ServerWebExchange, ServerWebExchange> buildServerInstrument
.addAttributesExtractors(serverAdditionalExtractors)
.addContextCustomizer(httpServerRouteBuilder.build())
.addOperationMetrics(HttpServerMetrics.get());
if (emitExperimentalHttpServerMetrics) {
if (emitExperimentalHttpServerTelemetry) {
builder
.addAttributesExtractor(HttpExperimentalAttributesExtractor.create(getter))
.addOperationMetrics(HttpServerExperimentalMetrics.get());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@ public static Instrumenter<ClientRequest, ClientResponse> create(
extractorConfigurer,
Consumer<HttpSpanNameExtractorBuilder<ClientRequest>> spanNameExtractorConfigurer,
List<AttributesExtractor<ClientRequest, ClientResponse>> additionalExtractors,
boolean captureExperimentalSpanAttributes,
boolean emitExperimentalHttpClientMetrics) {
boolean emitExperimentalHttpClientTelemetry) {

WebClientHttpAttributesGetter httpAttributesGetter = WebClientHttpAttributesGetter.INSTANCE;

Expand All @@ -61,10 +60,7 @@ public static Instrumenter<ClientRequest, ClientResponse> create(
.addAttributesExtractors(additionalExtractors)
.addOperationMetrics(HttpClientMetrics.get());

if (captureExperimentalSpanAttributes) {
clientBuilder.addAttributesExtractor(new WebClientExperimentalAttributesExtractor());
}
if (emitExperimentalHttpClientMetrics) {
if (emitExperimentalHttpClientTelemetry) {
clientBuilder
.addAttributesExtractor(HttpExperimentalAttributesExtractor.create(httpAttributesGetter))
.addOperationMetrics(HttpClientExperimentalMetrics.get());
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,16 @@ public String getServerAddress(ClientRequest request) {
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
mateuszrzeszutek marked this conversation as resolved.
Show resolved Hide resolved
if (response == null && error == null) {
return "cancelled";
}
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -142,4 +151,44 @@ private static int getStatusCode(ClientResponse response) {
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))));
}
}
Loading