From f65f7fd6c52475359dece9b7cb2439ecc1bf9e1d Mon Sep 17 00:00:00 2001 From: HaloFour Date: Thu, 19 May 2022 13:00:46 -0400 Subject: [PATCH] Change SpanStatusExtractor to use a builder that can set status description (#6035) * Change SpanStatusExtractor to use a builder that can set status descripion * Refactor SpanStatusExtractor to only support builder approach to setting status * Revert setting the exception as the status description * PR feedback * Re-fix graghql instrumentation span extractor * Spotless --- .../http/HttpSpanStatusExtractor.java | 12 +- .../HttpClientSpanStatusExtractorTest.java | 60 ++++++--- .../http/HttpSpanStatusExtractorTest.java | 123 ++++++++++++------ .../DefaultSpanStatusExtractor.java | 11 +- .../api/instrumenter/Instrumenter.java | 7 +- .../api/instrumenter/SpanStatusBuilder.java | 46 +++++++ .../instrumenter/SpanStatusBuilderImpl.java | 23 ++++ .../api/instrumenter/SpanStatusExtractor.java | 13 +- .../DefaultSpanStatusExtractorTest.java | 30 +++-- .../apachecamel/CamelSingletons.java | 5 +- .../graphql/GraphQLTelemetry.java | 13 +- .../grpc/v1_6/GrpcSpanStatusExtractor.java | 15 ++- 12 files changed, 261 insertions(+), 97 deletions(-) create mode 100644 instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanStatusBuilder.java create mode 100644 instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanStatusBuilderImpl.java diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpSpanStatusExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpSpanStatusExtractor.java index 274cda6d9c70..373efa10fe07 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpSpanStatusExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpSpanStatusExtractor.java @@ -6,6 +6,7 @@ package io.opentelemetry.instrumentation.api.instrumenter.http; import io.opentelemetry.api.trace.StatusCode; +import io.opentelemetry.instrumentation.api.instrumenter.SpanStatusBuilder; import io.opentelemetry.instrumentation.api.instrumenter.SpanStatusExtractor; import javax.annotation.Nullable; @@ -49,16 +50,21 @@ private HttpSpanStatusExtractor( } @Override - public StatusCode extract(REQUEST request, @Nullable RESPONSE response, Throwable error) { + public void extract( + SpanStatusBuilder spanStatusBuilder, + REQUEST request, + @Nullable RESPONSE response, + Throwable error) { if (response != null) { Integer statusCode = getter.statusCode(request, response); if (statusCode != null) { StatusCode statusCodeObj = statusConverter.statusFromHttpStatus(statusCode); if (statusCodeObj == StatusCode.ERROR) { - return statusCodeObj; + spanStatusBuilder.setStatus(statusCodeObj); + return; } } } - return SpanStatusExtractor.getDefault().extract(request, response, error); + SpanStatusExtractor.getDefault().extract(spanStatusBuilder, request, response, error); } } diff --git a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientSpanStatusExtractorTest.java b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientSpanStatusExtractorTest.java index 584d8f2cea46..3fe7fa16c127 100644 --- a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientSpanStatusExtractorTest.java +++ b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientSpanStatusExtractorTest.java @@ -5,11 +5,13 @@ package io.opentelemetry.instrumentation.api.instrumenter.http; -import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.anyMap; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import io.opentelemetry.api.trace.StatusCode; +import io.opentelemetry.instrumentation.api.instrumenter.SpanStatusBuilder; import java.util.Collections; import java.util.Map; import org.junit.jupiter.api.Test; @@ -21,49 +23,69 @@ @ExtendWith(MockitoExtension.class) class HttpClientSpanStatusExtractorTest { + @Mock private HttpClientAttributesGetter, Map> getter; + @Mock private SpanStatusBuilder spanStatusBuilder; + @ParameterizedTest @ValueSource(ints = {1, 100, 101, 200, 201, 300, 301, 400, 401, 500, 501, 600, 601}) void hasStatus(int statusCode) { + StatusCode expectedStatusCode = HttpStatusConverter.CLIENT.statusFromHttpStatus(statusCode); when(getter.statusCode(anyMap(), anyMap())).thenReturn(statusCode); - assertThat( - HttpSpanStatusExtractor.create(getter) - .extract(Collections.emptyMap(), Collections.emptyMap(), null)) - .isEqualTo(HttpStatusConverter.CLIENT.statusFromHttpStatus(statusCode)); + + HttpSpanStatusExtractor.create(getter) + .extract(spanStatusBuilder, Collections.emptyMap(), Collections.emptyMap(), null); + + if (expectedStatusCode != StatusCode.UNSET) { + verify(spanStatusBuilder).setStatus(expectedStatusCode); + } else { + verifyNoInteractions(spanStatusBuilder); + } } @ParameterizedTest @ValueSource(ints = {1, 100, 101, 200, 201, 300, 301, 400, 401, 500, 501, 600, 601}) void hasStatusAndException(int statusCode) { + StatusCode expectedStatusCode = HttpStatusConverter.CLIENT.statusFromHttpStatus(statusCode); when(getter.statusCode(anyMap(), anyMap())).thenReturn(statusCode); // Presence of exception has no effect. - assertThat( - HttpSpanStatusExtractor.create(getter) - .extract( - Collections.emptyMap(), Collections.emptyMap(), new IllegalStateException())) - .isEqualTo(StatusCode.ERROR); + HttpSpanStatusExtractor.create(getter) + .extract( + spanStatusBuilder, + Collections.emptyMap(), + Collections.emptyMap(), + new IllegalStateException("test")); + + if (expectedStatusCode != StatusCode.UNSET) { + verify(spanStatusBuilder).setStatus(expectedStatusCode); + } else { + verify(spanStatusBuilder).setStatus(StatusCode.ERROR); + } } @Test void hasNoStatus_fallsBackToDefault_unset() { when(getter.statusCode(anyMap(), anyMap())).thenReturn(null); - assertThat( - HttpSpanStatusExtractor.create(getter) - .extract(Collections.emptyMap(), Collections.emptyMap(), null)) - .isEqualTo(StatusCode.UNSET); + HttpSpanStatusExtractor.create(getter) + .extract(spanStatusBuilder, Collections.emptyMap(), Collections.emptyMap(), null); + + verifyNoInteractions(spanStatusBuilder); } @Test void hasNoStatus_fallsBackToDefault_error() { when(getter.statusCode(anyMap(), anyMap())).thenReturn(null); - assertThat( - HttpSpanStatusExtractor.create(getter) - .extract( - Collections.emptyMap(), Collections.emptyMap(), new IllegalStateException())) - .isEqualTo(StatusCode.ERROR); + HttpSpanStatusExtractor.create(getter) + .extract( + spanStatusBuilder, + Collections.emptyMap(), + Collections.emptyMap(), + new IllegalStateException("test")); + + verify(spanStatusBuilder).setStatus(StatusCode.ERROR); } } diff --git a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpSpanStatusExtractorTest.java b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpSpanStatusExtractorTest.java index 4af7a46f85af..484a456eb68a 100644 --- a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpSpanStatusExtractorTest.java +++ b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpSpanStatusExtractorTest.java @@ -5,11 +5,13 @@ package io.opentelemetry.instrumentation.api.instrumenter.http; -import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.anyMap; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import io.opentelemetry.api.trace.StatusCode; +import io.opentelemetry.instrumentation.api.instrumenter.SpanStatusBuilder; import java.util.Collections; import java.util.Map; import org.junit.jupiter.api.Test; @@ -25,80 +27,125 @@ class HttpSpanStatusExtractorTest { @Mock private HttpClientAttributesGetter, Map> clientGetter; + @Mock private SpanStatusBuilder spanStatusBuilder; + @ParameterizedTest @ValueSource(ints = {1, 100, 101, 200, 201, 300, 301, 500, 501, 600, 601}) void hasServerStatus(int statusCode) { + StatusCode expectedStatusCode = HttpStatusConverter.SERVER.statusFromHttpStatus(statusCode); when(serverGetter.statusCode(anyMap(), anyMap())).thenReturn(statusCode); - assertThat( - HttpSpanStatusExtractor.create(serverGetter) - .extract(Collections.emptyMap(), Collections.emptyMap(), null)) - .isEqualTo(HttpStatusConverter.SERVER.statusFromHttpStatus(statusCode)); + + HttpSpanStatusExtractor.create(serverGetter) + .extract(spanStatusBuilder, Collections.emptyMap(), Collections.emptyMap(), null); + + if (expectedStatusCode != StatusCode.UNSET) { + verify(spanStatusBuilder).setStatus(expectedStatusCode); + } else { + verifyNoInteractions(spanStatusBuilder); + } } @ParameterizedTest @ValueSource(ints = {1, 100, 101, 200, 201, 300, 301, 400, 401, 500, 501, 600, 601}) void hasClientStatus(int statusCode) { + StatusCode expectedStatusCode = HttpStatusConverter.CLIENT.statusFromHttpStatus(statusCode); when(clientGetter.statusCode(anyMap(), anyMap())).thenReturn(statusCode); - assertThat( - HttpSpanStatusExtractor.create(clientGetter) - .extract(Collections.emptyMap(), Collections.emptyMap(), null)) - .isEqualTo(HttpStatusConverter.CLIENT.statusFromHttpStatus(statusCode)); + + HttpSpanStatusExtractor.create(clientGetter) + .extract(spanStatusBuilder, Collections.emptyMap(), Collections.emptyMap(), null); + + if (expectedStatusCode != StatusCode.UNSET) { + verify(spanStatusBuilder).setStatus(expectedStatusCode); + } else { + verifyNoInteractions(spanStatusBuilder); + } } @ParameterizedTest @ValueSource(ints = {1, 100, 101, 200, 201, 300, 301, 400, 401, 500, 501, 600, 601}) void hasServerStatusAndException(int statusCode) { + StatusCode expectedStatusCode = HttpStatusConverter.SERVER.statusFromHttpStatus(statusCode); when(serverGetter.statusCode(anyMap(), anyMap())).thenReturn(statusCode); // Presence of exception has no effect. - assertThat( - HttpSpanStatusExtractor.create(serverGetter) - .extract( - Collections.emptyMap(), Collections.emptyMap(), new IllegalStateException())) - .isEqualTo(StatusCode.ERROR); + HttpSpanStatusExtractor.create(serverGetter) + .extract( + spanStatusBuilder, + Collections.emptyMap(), + Collections.emptyMap(), + new IllegalStateException("test")); + + if (expectedStatusCode == StatusCode.ERROR) { + verify(spanStatusBuilder).setStatus(expectedStatusCode); + } else { + verify(spanStatusBuilder).setStatus(StatusCode.ERROR); + } } @ParameterizedTest @ValueSource(ints = {1, 100, 101, 200, 201, 300, 301, 400, 401, 500, 501, 600, 601}) void hasClientStatusAndException(int statusCode) { + StatusCode expectedStatusCode = HttpStatusConverter.CLIENT.statusFromHttpStatus(statusCode); when(clientGetter.statusCode(anyMap(), anyMap())).thenReturn(statusCode); // Presence of exception has no effect. - assertThat( - HttpSpanStatusExtractor.create(clientGetter) - .extract( - Collections.emptyMap(), Collections.emptyMap(), new IllegalStateException())) - .isEqualTo(StatusCode.ERROR); + HttpSpanStatusExtractor.create(clientGetter) + .extract( + spanStatusBuilder, + Collections.emptyMap(), + Collections.emptyMap(), + new IllegalStateException("test")); + + if (expectedStatusCode == StatusCode.ERROR) { + verify(spanStatusBuilder).setStatus(expectedStatusCode); + } else { + verify(spanStatusBuilder).setStatus(StatusCode.ERROR); + } } @Test - void hasNoStatus_fallsBackToDefault_unset() { - when(clientGetter.statusCode(anyMap(), anyMap())).thenReturn(null); + void hasNoServerStatus_fallsBackToDefault_unset() { when(serverGetter.statusCode(anyMap(), anyMap())).thenReturn(null); - assertThat( - HttpSpanStatusExtractor.create(serverGetter) - .extract(Collections.emptyMap(), Collections.emptyMap(), null)) - .isEqualTo(StatusCode.UNSET); - assertThat( - HttpSpanStatusExtractor.create(clientGetter) - .extract(Collections.emptyMap(), Collections.emptyMap(), null)) - .isEqualTo(StatusCode.UNSET); + HttpSpanStatusExtractor.create(serverGetter) + .extract(spanStatusBuilder, Collections.emptyMap(), Collections.emptyMap(), null); + + verifyNoInteractions(spanStatusBuilder); } @Test - void hasNoStatus_fallsBackToDefault_error() { + void hasNoClientStatus_fallsBackToDefault_unset() { when(clientGetter.statusCode(anyMap(), anyMap())).thenReturn(null); + + HttpSpanStatusExtractor.create(clientGetter) + .extract(spanStatusBuilder, Collections.emptyMap(), Collections.emptyMap(), null); + + verifyNoInteractions(spanStatusBuilder); + } + + @Test + void hasNoServerStatus_fallsBackToDefault_error() { when(serverGetter.statusCode(anyMap(), anyMap())).thenReturn(null); - StatusCode serverStatusCode = - HttpSpanStatusExtractor.create(serverGetter) - .extract(Collections.emptyMap(), Collections.emptyMap(), new IllegalStateException()); - assertThat(serverStatusCode).isEqualTo(StatusCode.ERROR); + HttpSpanStatusExtractor.create(serverGetter) + .extract( + spanStatusBuilder, + Collections.emptyMap(), + Collections.emptyMap(), + new IllegalStateException("test")); + verify(spanStatusBuilder).setStatus(StatusCode.ERROR); + } + + @Test + void hasNoClientStatus_fallsBackToDefault_error() { + when(clientGetter.statusCode(anyMap(), anyMap())).thenReturn(null); - StatusCode clientStatusCode = - HttpSpanStatusExtractor.create(clientGetter) - .extract(Collections.emptyMap(), Collections.emptyMap(), new IllegalStateException()); - assertThat(clientStatusCode).isEqualTo(StatusCode.ERROR); + HttpSpanStatusExtractor.create(clientGetter) + .extract( + spanStatusBuilder, + Collections.emptyMap(), + Collections.emptyMap(), + new IllegalStateException("test")); + verify(spanStatusBuilder).setStatus(StatusCode.ERROR); } } diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/DefaultSpanStatusExtractor.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/DefaultSpanStatusExtractor.java index 64c39d392801..ead9ce2b13d9 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/DefaultSpanStatusExtractor.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/DefaultSpanStatusExtractor.java @@ -14,11 +14,14 @@ final class DefaultSpanStatusExtractor static final SpanStatusExtractor INSTANCE = new DefaultSpanStatusExtractor<>(); @Override - public StatusCode extract( - REQUEST request, @Nullable RESPONSE response, @Nullable Throwable error) { + public void extract( + SpanStatusBuilder spanStatusBuilder, + REQUEST request, + @Nullable RESPONSE response, + @Nullable Throwable error) { + if (error != null) { - return StatusCode.ERROR; + spanStatusBuilder.setStatus(StatusCode.ERROR); } - return StatusCode.UNSET; } } diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java index 0a0497ffb685..8e45354aa6e3 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java @@ -9,7 +9,6 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanBuilder; import io.opentelemetry.api.trace.SpanKind; -import io.opentelemetry.api.trace.StatusCode; import io.opentelemetry.api.trace.Tracer; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.internal.SupportabilityMetrics; @@ -232,10 +231,8 @@ private void doEnd( } } - StatusCode statusCode = spanStatusExtractor.extract(request, response, error); - if (statusCode != StatusCode.UNSET) { - span.setStatus(statusCode); - } + SpanStatusBuilder spanStatusBuilder = new SpanStatusBuilderImpl(span); + spanStatusExtractor.extract(spanStatusBuilder, request, response, error); if (endTime != null) { span.end(endTime); diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanStatusBuilder.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanStatusBuilder.java new file mode 100644 index 000000000000..bc53432b487c --- /dev/null +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanStatusBuilder.java @@ -0,0 +1,46 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.api.instrumenter; + +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.StatusCode; + +/** A builder that exposes methods for setting the status of a span. */ +public interface SpanStatusBuilder { + + /** + * Sets the status to the {@code Span}. + * + *

If used, this will override the default {@code Span} status. Default status code is {@link + * StatusCode#UNSET}. + * + *

Only the value of the last call will be recorded, and implementations are free to ignore + * previous calls. + * + * @param statusCode the {@link StatusCode} to set. + * @return this. + * @see Span#setStatus(StatusCode) + */ + default SpanStatusBuilder setStatus(StatusCode statusCode) { + return setStatus(statusCode, ""); + } + + /** + * Sets the status to the {@code Span}. + * + *

If used, this will override the default {@code Span} status. Default status code is {@link + * StatusCode#UNSET}. + * + *

Only the value of the last call will be recorded, and implementations are free to ignore + * previous calls. + * + * @param statusCode the {@link StatusCode} to set. + * @param description the description of the {@code Status}. + * @return this. + * @see Span#setStatus(StatusCode, String) + */ + SpanStatusBuilder setStatus(StatusCode statusCode, String description); +} diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanStatusBuilderImpl.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanStatusBuilderImpl.java new file mode 100644 index 000000000000..565dd90be35a --- /dev/null +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanStatusBuilderImpl.java @@ -0,0 +1,23 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.api.instrumenter; + +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.StatusCode; + +final class SpanStatusBuilderImpl implements SpanStatusBuilder { + private final Span span; + + SpanStatusBuilderImpl(Span span) { + this.span = span; + } + + @Override + public SpanStatusBuilder setStatus(StatusCode statusCode, String description) { + span.setStatus(statusCode, description); + return this; + } +} diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanStatusExtractor.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanStatusExtractor.java index 38ebcad7dbb4..6c84b630d365 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanStatusExtractor.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanStatusExtractor.java @@ -9,14 +9,19 @@ import javax.annotation.Nullable; /** - * Extractor of {@link StatusCode}. The {@link #extract(Object, Object, Throwable)} method will be - * called after a request processing is completed to determine its final status. + * Extractor of {@link StatusCode}. The {@link #extract(SpanStatusBuilder, Object, Object, + * Throwable)} method will be called after a request processing is completed to determine its final + * status. */ @FunctionalInterface public interface SpanStatusExtractor { - /** Returns the {@link StatusCode}. */ - StatusCode extract(REQUEST request, @Nullable RESPONSE response, @Nullable Throwable error); + /** Extracts the status from the response and sets it to the {@code spanStatusBuilder}. */ + void extract( + SpanStatusBuilder spanStatusBuilder, + REQUEST request, + @Nullable RESPONSE response, + @Nullable Throwable error); /** * Returns the default {@link SpanStatusExtractor}, which returns {@link StatusCode#ERROR} if the diff --git a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/DefaultSpanStatusExtractorTest.java b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/DefaultSpanStatusExtractorTest.java index a097c30eae3a..53aa6c927159 100644 --- a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/DefaultSpanStatusExtractorTest.java +++ b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/DefaultSpanStatusExtractorTest.java @@ -5,30 +5,36 @@ package io.opentelemetry.instrumentation.api.instrumenter; -import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import io.opentelemetry.api.trace.StatusCode; import java.util.Collections; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +@ExtendWith(MockitoExtension.class) class DefaultSpanStatusExtractorTest { + @Mock SpanStatusBuilder spanStatusBuilder; + @Test void noException() { - assertThat( - SpanStatusExtractor.getDefault() - .extract(Collections.emptyMap(), Collections.emptyMap(), null)) - .isEqualTo(StatusCode.UNSET); + SpanStatusExtractor.getDefault() + .extract(spanStatusBuilder, Collections.emptyMap(), Collections.emptyMap(), null); + verifyNoInteractions(spanStatusBuilder); } @Test void exception() { - assertThat( - SpanStatusExtractor.getDefault() - .extract( - Collections.emptyMap(), - Collections.emptyMap(), - new IllegalStateException("test"))) - .isEqualTo(StatusCode.ERROR); + SpanStatusExtractor.getDefault() + .extract( + spanStatusBuilder, + Collections.emptyMap(), + Collections.emptyMap(), + new IllegalStateException("test")); + verify(spanStatusBuilder).setStatus(StatusCode.ERROR); } } diff --git a/instrumentation/apache-camel-2.20/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachecamel/CamelSingletons.java b/instrumentation/apache-camel-2.20/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachecamel/CamelSingletons.java index 84ffc6cf3fa3..b0b7b32673b2 100644 --- a/instrumentation/apache-camel-2.20/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachecamel/CamelSingletons.java +++ b/instrumentation/apache-camel-2.20/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachecamel/CamelSingletons.java @@ -63,11 +63,10 @@ public void onEnd( }; SpanStatusExtractor spanStatusExtractor = - (request, unused, error) -> { + (spanStatusBuilder, request, unused, error) -> { if (request.getExchange().isFailed()) { - return StatusCode.ERROR; + spanStatusBuilder.setStatus(StatusCode.ERROR); } - return null; }; InstrumenterBuilder builder = diff --git a/instrumentation/graphql-java-12.0/library/src/main/java/io/opentelemetry/instrumentation/graphql/GraphQLTelemetry.java b/instrumentation/graphql-java-12.0/library/src/main/java/io/opentelemetry/instrumentation/graphql/GraphQLTelemetry.java index 208f0ef1b363..926c1c3326a2 100644 --- a/instrumentation/graphql-java-12.0/library/src/main/java/io/opentelemetry/instrumentation/graphql/GraphQLTelemetry.java +++ b/instrumentation/graphql-java-12.0/library/src/main/java/io/opentelemetry/instrumentation/graphql/GraphQLTelemetry.java @@ -42,12 +42,17 @@ public static GraphQLTelemetryBuilder builder(OpenTelemetry openTelemetry) { Instrumenter.builder( openTelemetry, INSTRUMENTATION_NAME, ignored -> "GraphQL Query") .setSpanStatusExtractor( - (instrumentationExecutionParameters, executionResult, error) -> { + (spanStatusBuilder, instrumentationExecutionParameters, executionResult, error) -> { if (!executionResult.getErrors().isEmpty()) { - return StatusCode.ERROR; + spanStatusBuilder.setStatus(StatusCode.ERROR); + } else { + SpanStatusExtractor.getDefault() + .extract( + spanStatusBuilder, + instrumentationExecutionParameters, + executionResult, + error); } - return SpanStatusExtractor.getDefault() - .extract(instrumentationExecutionParameters, executionResult, error); }); if (captureExperimentalSpanAttributes) { builder.addAttributesExtractor(new ExperimentalAttributesExtractor()); diff --git a/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcSpanStatusExtractor.java b/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcSpanStatusExtractor.java index 85cf81e43b78..a79b1c3b5a08 100644 --- a/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcSpanStatusExtractor.java +++ b/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcSpanStatusExtractor.java @@ -9,12 +9,17 @@ import io.grpc.StatusException; import io.grpc.StatusRuntimeException; import io.opentelemetry.api.trace.StatusCode; +import io.opentelemetry.instrumentation.api.instrumenter.SpanStatusBuilder; import io.opentelemetry.instrumentation.api.instrumenter.SpanStatusExtractor; import javax.annotation.Nullable; final class GrpcSpanStatusExtractor implements SpanStatusExtractor { @Override - public StatusCode extract(GrpcRequest request, Status status, @Nullable Throwable error) { + public void extract( + SpanStatusBuilder spanStatusBuilder, + GrpcRequest request, + Status status, + @Nullable Throwable error) { if (status == null) { if (error instanceof StatusRuntimeException) { status = ((StatusRuntimeException) error).getStatus(); @@ -23,11 +28,11 @@ public StatusCode extract(GrpcRequest request, Status status, @Nullable Throwabl } } if (status != null) { - if (status.isOk()) { - return StatusCode.UNSET; + if (!status.isOk()) { + spanStatusBuilder.setStatus(StatusCode.ERROR); } - return StatusCode.ERROR; + } else { + SpanStatusExtractor.getDefault().extract(spanStatusBuilder, request, status, error); } - return SpanStatusExtractor.getDefault().extract(request, status, error); } }