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 a79b1c3b5a..5e9d84dd29 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 @@ -5,15 +5,29 @@ package io.opentelemetry.instrumentation.grpc.v1_6; +import com.google.errorprone.annotations.Immutable; 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 java.util.Arrays; +import java.util.HashSet; +import java.util.Set; +import java.util.function.Predicate; import javax.annotation.Nullable; -final class GrpcSpanStatusExtractor implements SpanStatusExtractor { +enum GrpcSpanStatusExtractor implements SpanStatusExtractor { + CLIENT(GrpcSpanStatusExtractor::isClientError), + SERVER(GrpcSpanStatusExtractor::isServerError); + + private final ErrorStatusPredicate isError; + + GrpcSpanStatusExtractor(ErrorStatusPredicate isError) { + this.isError = isError; + } + @Override public void extract( SpanStatusBuilder spanStatusBuilder, @@ -28,11 +42,35 @@ final class GrpcSpanStatusExtractor implements SpanStatusExtractor serverErrorStatuses = new HashSet<>(); + + static { + serverErrorStatuses.addAll( + Arrays.asList( + Status.Code.UNKNOWN, + Status.Code.DEADLINE_EXCEEDED, + Status.Code.UNIMPLEMENTED, + Status.Code.INTERNAL, + Status.Code.UNAVAILABLE, + Status.Code.DATA_LOSS)); + } + + private static boolean isServerError(Status status) { + return serverErrorStatuses.contains(status.getCode()); + } + + private static boolean isClientError(Status status) { + return !status.isOk(); + } + + @Immutable + private interface ErrorStatusPredicate extends Predicate {} } diff --git a/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcTelemetryBuilder.java b/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcTelemetryBuilder.java index 8d19f65a20..1a15d94f81 100644 --- a/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcTelemetryBuilder.java +++ b/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcTelemetryBuilder.java @@ -25,7 +25,6 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.function.Function; -import java.util.stream.Stream; import javax.annotation.Nullable; /** A builder of {@link GrpcTelemetry}. */ @@ -167,13 +166,6 @@ public final class GrpcTelemetryBuilder { InstrumenterBuilder serverInstrumenterBuilder = Instrumenter.builder(openTelemetry, INSTRUMENTATION_NAME, serverSpanNameExtractor); - Stream.of(clientInstrumenterBuilder, serverInstrumenterBuilder) - .forEach( - instrumenter -> - instrumenter - .setSpanStatusExtractor(new GrpcSpanStatusExtractor()) - .addAttributesExtractors(additionalExtractors)); - GrpcClientNetworkAttributesGetter netClientAttributesGetter = new GrpcClientNetworkAttributesGetter(); GrpcNetworkServerAttributesGetter netServerAttributesGetter = @@ -181,6 +173,8 @@ public final class GrpcTelemetryBuilder { GrpcRpcAttributesGetter rpcAttributesGetter = GrpcRpcAttributesGetter.INSTANCE; clientInstrumenterBuilder + .setSpanStatusExtractor(GrpcSpanStatusExtractor.CLIENT) + .addAttributesExtractors(additionalExtractors) .addAttributesExtractor(RpcClientAttributesExtractor.create(rpcAttributesGetter)) .addAttributesExtractor(ServerAttributesExtractor.create(netClientAttributesGetter)) .addAttributesExtractors(additionalClientExtractors) @@ -189,6 +183,8 @@ public final class GrpcTelemetryBuilder { GrpcRpcAttributesGetter.INSTANCE, capturedClientRequestMetadata)) .addOperationMetrics(RpcClientMetrics.get()); serverInstrumenterBuilder + .setSpanStatusExtractor(GrpcSpanStatusExtractor.SERVER) + .addAttributesExtractors(additionalExtractors) .addAttributesExtractor(RpcServerAttributesExtractor.create(rpcAttributesGetter)) .addAttributesExtractor( ServerAttributesExtractor.createForServerSide(netServerAttributesGetter)) diff --git a/instrumentation/grpc-1.6/testing/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/AbstractGrpcTest.java b/instrumentation/grpc-1.6/testing/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/AbstractGrpcTest.java index 06a71be7b1..4d1e168106 100644 --- a/instrumentation/grpc-1.6/testing/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/AbstractGrpcTest.java +++ b/instrumentation/grpc-1.6/testing/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/AbstractGrpcTest.java @@ -604,6 +604,7 @@ public abstract class AbstractGrpcTest { assertThat(t.getStatus().getDescription()).isEqualTo(status.getDescription()); }); + boolean isServerError = status.getCode() != Status.Code.NOT_FOUND; testing() .waitAndAssertTraces( trace -> @@ -635,7 +636,7 @@ public abstract class AbstractGrpcTest { span.hasName("example.Greeter/SayHello") .hasKind(SpanKind.SERVER) .hasParent(trace.getSpan(0)) - .hasStatus(StatusData.error()) + .hasStatus(isServerError ? StatusData.error() : StatusData.unset()) .hasAttributesSatisfyingExactly( equalTo(SemanticAttributes.RPC_SYSTEM, "grpc"), equalTo(SemanticAttributes.RPC_SERVICE, "example.Greeter"), @@ -863,11 +864,19 @@ public abstract class AbstractGrpcTest { public Stream provideArguments(ExtensionContext context) { return Stream.of( arguments(Status.UNKNOWN.withCause(new RuntimeException("some error"))), - arguments(Status.PERMISSION_DENIED.withCause(new RuntimeException("some error"))), + arguments(Status.DEADLINE_EXCEEDED.withCause(new RuntimeException("some error"))), arguments(Status.UNIMPLEMENTED.withCause(new RuntimeException("some error"))), + arguments(Status.INTERNAL.withCause(new RuntimeException("some error"))), + arguments(Status.UNAVAILABLE.withCause(new RuntimeException("some error"))), + arguments(Status.DATA_LOSS.withCause(new RuntimeException("some error"))), + arguments(Status.NOT_FOUND.withCause(new RuntimeException("some error"))), arguments(Status.UNKNOWN.withDescription("some description")), - arguments(Status.PERMISSION_DENIED.withDescription("some description")), - arguments(Status.UNIMPLEMENTED.withDescription("some description"))); + arguments(Status.DEADLINE_EXCEEDED.withDescription("some description")), + arguments(Status.UNIMPLEMENTED.withDescription("some description")), + arguments(Status.INTERNAL.withDescription("some description")), + arguments(Status.UNAVAILABLE.withDescription("some description")), + arguments(Status.DATA_LOSS.withDescription("some description")), + arguments(Status.NOT_FOUND.withDescription("some description"))); } }