open-telemetry
/
opentelemetry-java-instrumentation
mirror of https://github.com/open-telemetry/opentelemetry-java-instrumentation.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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
This commit is contained in:
HaloFour 2022-05-19 13:00:46 -04:00 committed by GitHub
parent 512a040025
commit b7fc80c98e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 261 additions and 97 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpSpanStatusExtractor.java
View File

@ -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 @@ public final class HttpSpanStatusExtractor<REQUEST, RESPONSE>
}
@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);
}
}

60
instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientSpanStatusExtractorTest.java
View File

@ -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 @@ import org.mockito.junit.jupiter.MockitoExtension;
@ExtendWith(MockitoExtension.class)
class HttpClientSpanStatusExtractorTest {
@Mock private HttpClientAttributesGetter<Map<String, String>, Map<String, String>> 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);
}
}

123
instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpSpanStatusExtractorTest.java
View File

@ -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<String, String>, Map<String, String>> 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);
}
StatusCode clientStatusCode =
HttpSpanStatusExtractor.create(clientGetter)
.extract(Collections.emptyMap(), Collections.emptyMap(), new IllegalStateException());
assertThat(clientStatusCode).isEqualTo(StatusCode.ERROR);
@Test
void hasNoClientStatus_fallsBackToDefault_error() {
when(clientGetter.statusCode(anyMap(), anyMap())).thenReturn(null);
HttpSpanStatusExtractor.create(clientGetter)
.extract(
spanStatusBuilder,
Collections.emptyMap(),
Collections.emptyMap(),
new IllegalStateException("test"));
verify(spanStatusBuilder).setStatus(StatusCode.ERROR);
}
}

11
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/DefaultSpanStatusExtractor.java
View File

@ -14,11 +14,14 @@ final class DefaultSpanStatusExtractor<REQUEST, RESPONSE>
static final SpanStatusExtractor<Object, Object> 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;
}
}

7
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java
View File

@ -9,7 +9,6 @@ import io.opentelemetry.api.OpenTelemetry;
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 @@ public class Instrumenter<REQUEST, RESPONSE> {
}
}
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);

46
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanStatusBuilder.java Normal file
View File

@ -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}.
*
* <p>If used, this will override the default {@code Span} status. Default status code is {@link
* StatusCode#UNSET}.
*
* <p>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}.
*
* <p>If used, this will override the default {@code Span} status. Default status code is {@link
* StatusCode#UNSET}.
*
* <p>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);
}

23
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanStatusBuilderImpl.java Normal file
View File

@ -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;
}
}

13
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanStatusExtractor.java
View File

@ -9,14 +9,19 @@ import io.opentelemetry.api.trace.StatusCode;
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<REQUEST, RESPONSE> {
/** 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

30
instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/DefaultSpanStatusExtractorTest.java
View File

@ -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);
}
}

5
instrumentation/apache-camel-2.20/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachecamel/CamelSingletons.java
View File

@ -63,11 +63,10 @@ public final class CamelSingletons {
};
SpanStatusExtractor<CamelRequest, Void> spanStatusExtractor =
(request, unused, error) -> {
(spanStatusBuilder, request, unused, error) -> {
if (request.getExchange().isFailed()) {
return StatusCode.ERROR;
spanStatusBuilder.setStatus(StatusCode.ERROR);
}
return null;
};
InstrumenterBuilder<CamelRequest, Void> builder =

13
instrumentation/graphql-java-12.0/library/src/main/java/io/opentelemetry/instrumentation/graphql/GraphQLTelemetry.java
View File

@ -42,12 +42,17 @@ public final class GraphQLTelemetry {
Instrumenter.<InstrumentationExecutionParameters, ExecutionResult>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());

15
instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcSpanStatusExtractor.java
View File

@ -9,12 +9,17 @@ import io.grpc.Status;
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<GrpcRequest, Status> {
@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 @@ final class GrpcSpanStatusExtractor implements SpanStatusExtractor<GrpcRequest,
}
}
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);
}
}