From fdbdef43be88347aeab310f15809391c917287a9 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Mon, 18 Sep 2023 12:20:14 +0200 Subject: [PATCH 1/5] Implement error.type attribute in HTTP semconv --- .../http/HttpClientAttributesExtractor.java | 1 + .../http/HttpCommonAttributesExtractor.java | 24 +++- .../http/HttpCommonAttributesGetter.java | 21 ++++ .../instrumenter/http/HttpMetricsAdvice.java | 5 + .../http/HttpServerAttributesExtractor.java | 1 + .../http/HttpSpanStatusExtractor.java | 12 +- .../http/HttpStatusCodeConverter.java | 108 ++++++++++++++++++ .../http/HttpStatusConverter.java | 34 ------ .../http/internal/HttpAttributes.java | 24 ++++ .../http/HttpSpanStatusExtractorTest.java | 4 +- ...=> HttpStatusCodeConverterClientTest.java} | 90 ++++++++++++++- ...=> HttpStatusCodeConverterServerTest.java} | 68 ++++++++++- ...tAttributesExtractorStableSemconvTest.java | 68 +++++++++++ ...tExperimentalMetricsStableSemconvTest.java | 4 + .../HttpClientMetricsStableSemconvTest.java | 3 + ...rAttributesExtractorStableSemconvTest.java | 68 +++++++++++ ...rExperimentalMetricsStableSemconvTest.java | 6 + .../HttpServerMetricsStableSemconvTest.java | 4 + .../AbstractGoogleHttpClientTest.java | 36 ++++-- .../NettyClientInstrumenterFactory.java | 33 ++++-- .../junit/http/AbstractHttpClientTest.java | 9 ++ .../junit/http/AbstractHttpServerTest.java | 5 + .../junit/http/HttpStatusCodeUtil.java | 84 ++++++++++++++ 23 files changed, 639 insertions(+), 73 deletions(-) create mode 100644 instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpStatusCodeConverter.java delete mode 100644 instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpStatusConverter.java create mode 100644 instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/internal/HttpAttributes.java rename instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/{HttpClientStatusConverterTest.java => HttpStatusCodeConverterClientTest.java} (50%) rename instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/{HttpServerStatusConverterTest.java => HttpStatusCodeConverterServerTest.java} (59%) create mode 100644 testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpStatusCodeUtil.java diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java index 8b03b1c0e6fa..2571a337e843 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java @@ -90,6 +90,7 @@ public static HttpClientAttributesExtractorBuilder builder) { super( builder.httpAttributesGetter, + HttpStatusCodeConverter.CLIENT, builder.capturedRequestHeaders, builder.capturedResponseHeaders, builder.knownMethods); diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java index 4afd5a8d60b7..f4e0a9450cdd 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java @@ -14,6 +14,7 @@ import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; +import io.opentelemetry.instrumentation.api.instrumenter.http.internal.HttpAttributes; import io.opentelemetry.instrumentation.api.internal.SemconvStability; import io.opentelemetry.semconv.SemanticAttributes; import java.util.HashSet; @@ -31,16 +32,19 @@ abstract class HttpCommonAttributesExtractor< implements AttributesExtractor { final GETTER getter; + private final HttpStatusCodeConverter statusCodeConverter; private final List capturedRequestHeaders; private final List capturedResponseHeaders; private final Set knownMethods; HttpCommonAttributesExtractor( GETTER getter, + HttpStatusCodeConverter statusCodeConverter, List capturedRequestHeaders, List capturedResponseHeaders, Set knownMethods) { this.getter = getter; + this.statusCodeConverter = statusCodeConverter; this.capturedRequestHeaders = lowercase(capturedRequestHeaders); this.capturedResponseHeaders = lowercase(capturedResponseHeaders); this.knownMethods = new HashSet<>(knownMethods); @@ -88,8 +92,9 @@ public void onEnd( internalSet(attributes, SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH, requestBodySize); } + Integer statusCode = null; if (response != null) { - Integer statusCode = getter.getHttpResponseStatusCode(request, response, error); + statusCode = getter.getHttpResponseStatusCode(request, response, error); if (statusCode != null && statusCode > 0) { if (SemconvStability.emitStableHttpSemconv()) { internalSet(attributes, SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, (long) statusCode); @@ -114,6 +119,23 @@ public void onEnd( } } } + + if (SemconvStability.emitStableHttpSemconv()) { + String errorType; + if (statusCode != null) { + errorType = statusCodeConverter.getErrorType(statusCode); + } else { + errorType = getter.getErrorType(request, response, error); + // fall back to exception class name & _OTHER + if (errorType == null && error != null) { + errorType = error.getClass().getName(); + } + if (errorType == null) { + errorType = _OTHER; + } + } + internalSet(attributes, HttpAttributes.ERROR_TYPE, errorType); + } } @Nullable diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesGetter.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesGetter.java index 320f1cfde5fc..ebcb4a82a26d 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesGetter.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesGetter.java @@ -53,4 +53,25 @@ public interface HttpCommonAttributesGetter { * returned instead. */ List getHttpResponseHeader(REQUEST request, RESPONSE response, String name); + + /** + * Returns a description of a class of error the operation ended with. + * + *

This method is only called if the request failed before response status code was sent or + * received. + * + *

If this method is not implemented, or if it returns {@code null}, the exception class name + * (if any was caught) or the value {@code _OTHER} will be used as error type. + * + *

The cardinality of the error type should be low. The instrumentations implementing this + * method are recommended to document the custom values they support. + * + *

Examples: {@code Bad Request}, {@code java.net.UnknownHostException}, {@code request + * cancelled}, {@code _OTHER}. + */ + @Nullable + default String getErrorType( + REQUEST request, @Nullable RESPONSE response, @Nullable Throwable throwable) { + return null; + } } diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpMetricsAdvice.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpMetricsAdvice.java index 88aa3783fd2a..ba6315b3b9a1 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpMetricsAdvice.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpMetricsAdvice.java @@ -13,6 +13,7 @@ import io.opentelemetry.extension.incubator.metrics.ExtendedDoubleHistogramBuilder; import io.opentelemetry.extension.incubator.metrics.ExtendedLongHistogramBuilder; import io.opentelemetry.extension.incubator.metrics.ExtendedLongUpDownCounterBuilder; +import io.opentelemetry.instrumentation.api.instrumenter.http.internal.HttpAttributes; import io.opentelemetry.semconv.SemanticAttributes; final class HttpMetricsAdvice { @@ -26,6 +27,7 @@ static void applyStableClientDurationAdvice(DoubleHistogramBuilder builder) { asList( SemanticAttributes.HTTP_REQUEST_METHOD, SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, + HttpAttributes.ERROR_TYPE, SemanticAttributes.NETWORK_PROTOCOL_NAME, SemanticAttributes.NETWORK_PROTOCOL_VERSION, SemanticAttributes.SERVER_ADDRESS, @@ -60,6 +62,7 @@ static void applyClientRequestSizeAdvice(LongHistogramBuilder builder) { // stable attributes SemanticAttributes.HTTP_REQUEST_METHOD, SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, + HttpAttributes.ERROR_TYPE, SemanticAttributes.NETWORK_PROTOCOL_NAME, SemanticAttributes.NETWORK_PROTOCOL_VERSION, SemanticAttributes.SERVER_ADDRESS, @@ -84,6 +87,7 @@ static void applyStableServerDurationAdvice(DoubleHistogramBuilder builder) { SemanticAttributes.HTTP_ROUTE, SemanticAttributes.HTTP_REQUEST_METHOD, SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, + HttpAttributes.ERROR_TYPE, SemanticAttributes.NETWORK_PROTOCOL_NAME, SemanticAttributes.NETWORK_PROTOCOL_VERSION, SemanticAttributes.URL_SCHEME)); @@ -119,6 +123,7 @@ static void applyServerRequestSizeAdvice(LongHistogramBuilder builder) { SemanticAttributes.HTTP_ROUTE, SemanticAttributes.HTTP_REQUEST_METHOD, SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, + HttpAttributes.ERROR_TYPE, SemanticAttributes.NETWORK_PROTOCOL_NAME, SemanticAttributes.NETWORK_PROTOCOL_VERSION, SemanticAttributes.URL_SCHEME, diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java index 844c8d5d0a43..eb851e56c04c 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java @@ -93,6 +93,7 @@ public static HttpServerAttributesExtractorBuilder builder) { super( builder.httpAttributesGetter, + HttpStatusCodeConverter.SERVER, builder.capturedRequestHeaders, builder.capturedResponseHeaders, builder.knownMethods); 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 d2878dd03d02..4bdbf8ddc3eb 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 @@ -26,7 +26,7 @@ public final class HttpSpanStatusExtractor */ public static SpanStatusExtractor create( HttpClientAttributesGetter getter) { - return new HttpSpanStatusExtractor<>(getter, HttpStatusConverter.CLIENT); + return new HttpSpanStatusExtractor<>(getter, HttpStatusCodeConverter.CLIENT); } /** @@ -36,17 +36,17 @@ public static SpanStatusExtractor create( */ public static SpanStatusExtractor create( HttpServerAttributesGetter getter) { - return new HttpSpanStatusExtractor<>(getter, HttpStatusConverter.SERVER); + return new HttpSpanStatusExtractor<>(getter, HttpStatusCodeConverter.SERVER); } private final HttpCommonAttributesGetter getter; - private final HttpStatusConverter statusConverter; + private final HttpStatusCodeConverter statusCodeConverter; private HttpSpanStatusExtractor( HttpCommonAttributesGetter getter, - HttpStatusConverter statusConverter) { + HttpStatusCodeConverter statusCodeConverter) { this.getter = getter; - this.statusConverter = statusConverter; + this.statusCodeConverter = statusCodeConverter; } @Override @@ -59,7 +59,7 @@ public void extract( if (response != null) { Integer statusCode = getter.getHttpResponseStatusCode(request, response, error); if (statusCode != null) { - StatusCode statusCodeObj = statusConverter.statusFromHttpStatus(statusCode); + StatusCode statusCodeObj = statusCodeConverter.getSpanStatus(statusCode); if (statusCodeObj == StatusCode.ERROR) { spanStatusBuilder.setStatus(statusCodeObj); return; diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpStatusCodeConverter.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpStatusCodeConverter.java new file mode 100644 index 000000000000..ce0d2ec9b335 --- /dev/null +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpStatusCodeConverter.java @@ -0,0 +1,108 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.api.instrumenter.http; + +import static java.util.Collections.unmodifiableMap; + +import io.opentelemetry.api.trace.StatusCode; +import java.util.HashMap; +import java.util.Map; +import javax.annotation.Nullable; + +// https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-spans.md#status +enum HttpStatusCodeConverter { + SERVER { + + @Override + StatusCode getSpanStatus(int responseStatusCode) { + if (responseStatusCode >= 100 && responseStatusCode < 500) { + return StatusCode.UNSET; + } + + return StatusCode.ERROR; + } + + @Nullable + @Override + String getErrorType(int responseStatusCode) { + return serverErrorTypes.get(responseStatusCode); + } + }, + CLIENT { + @Override + StatusCode getSpanStatus(int responseStatusCode) { + if (responseStatusCode >= 100 && responseStatusCode < 400) { + return StatusCode.UNSET; + } + + return StatusCode.ERROR; + } + + @Nullable + @Override + String getErrorType(int responseStatusCode) { + return clientErrorTypes.get(responseStatusCode); + } + }; + + abstract StatusCode getSpanStatus(int responseStatusCode); + + @Nullable + abstract String getErrorType(int responseStatusCode); + + private static final Map serverErrorTypes; + private static final Map clientErrorTypes; + + // https://en.wikipedia.org/wiki/List_of_HTTP_status_codes + static { + Map serverPhrases = new HashMap<>(); + serverPhrases.put(500, "Internal Server Error"); + serverPhrases.put(501, "Not Implemented"); + serverPhrases.put(502, "Bad Gateway"); + serverPhrases.put(503, "Service Unavailable"); + serverPhrases.put(504, "Gateway Timeout"); + serverPhrases.put(505, "HTTP Version Not Supported"); + serverPhrases.put(506, "Variant Also Negotiates"); + serverPhrases.put(507, "Insufficient Storage"); + serverPhrases.put(508, "Loop Detected"); + serverPhrases.put(510, "Not Extended"); + serverPhrases.put(511, "Network Authentication Required"); + serverErrorTypes = unmodifiableMap(serverPhrases); + + // include all server error types + Map clientPhrases = new HashMap<>(serverPhrases); + clientPhrases.put(400, "Bad Request"); + clientPhrases.put(401, "Unauthorized"); + clientPhrases.put(402, "Payment Required"); + clientPhrases.put(403, "Forbidden"); + clientPhrases.put(404, "Not Found"); + clientPhrases.put(405, "Method Not Allowed"); + clientPhrases.put(406, "Not Acceptable"); + clientPhrases.put(407, "Proxy Authentication Required"); + clientPhrases.put(408, "Request Timeout"); + clientPhrases.put(409, "Conflict"); + clientPhrases.put(410, "Gone"); + clientPhrases.put(411, "Length Required"); + clientPhrases.put(412, "Precondition Failed"); + clientPhrases.put(413, "Content Too Large"); + clientPhrases.put(414, "URI Too Long"); + clientPhrases.put(415, "Unsupported Media Type"); + clientPhrases.put(416, "Range Not Satisfiable"); + clientPhrases.put(417, "Expectation Failed"); + clientPhrases.put(418, "I'm a teapot"); + clientPhrases.put(421, "Misdirected Request"); + clientPhrases.put(422, "Unprocessable Content"); + clientPhrases.put(423, "Locked"); + clientPhrases.put(424, "Failed Dependency"); + clientPhrases.put(425, "Too Early"); + clientPhrases.put(426, "Upgrade Required"); + clientPhrases.put(428, "Precondition Required"); + clientPhrases.put(429, "Too Many Requests"); + clientPhrases.put(431, "Request Header Fields Too Large"); + clientPhrases.put(451, "Unavailable For Legal Reasons"); + clientErrorTypes = unmodifiableMap(clientPhrases); + } +} diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpStatusConverter.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpStatusConverter.java deleted file mode 100644 index d7e3becb91ea..000000000000 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpStatusConverter.java +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.instrumentation.api.instrumenter.http; - -import io.opentelemetry.api.trace.StatusCode; - -// https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/http.md#status -enum HttpStatusConverter { - SERVER { - @Override - StatusCode statusFromHttpStatus(int httpStatus) { - if (httpStatus >= 100 && httpStatus < 500) { - return StatusCode.UNSET; - } - - return StatusCode.ERROR; - } - }, - CLIENT { - @Override - StatusCode statusFromHttpStatus(int httpStatus) { - if (httpStatus >= 100 && httpStatus < 400) { - return StatusCode.UNSET; - } - - return StatusCode.ERROR; - } - }; - - abstract StatusCode statusFromHttpStatus(int httpStatus); -} diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/internal/HttpAttributes.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/internal/HttpAttributes.java new file mode 100644 index 000000000000..ea24f4242ef7 --- /dev/null +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/internal/HttpAttributes.java @@ -0,0 +1,24 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.api.instrumenter.http.internal; + +import static io.opentelemetry.api.common.AttributeKey.stringKey; + +import io.opentelemetry.api.common.AttributeKey; + +/** + * This class is internal and is hence not for public use. Its APIs are unstable and can change at + * any time. + */ +public final class HttpAttributes { + + // FIXME: remove this class and replace its usages with SemanticAttributes once schema 1.22 is + // released + + public static final AttributeKey ERROR_TYPE = stringKey("error.type"); + + private HttpAttributes() {} +} 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 e7ef16725111..24ff40b5d946 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 @@ -34,7 +34,7 @@ class HttpSpanStatusExtractorTest { @ParameterizedTest @ValueSource(ints = {1, 100, 101, 200, 201, 300, 301, 500, 501, 600, 601}) void hasServerStatus(int statusCode) { - StatusCode expectedStatusCode = HttpStatusConverter.SERVER.statusFromHttpStatus(statusCode); + StatusCode expectedStatusCode = HttpStatusCodeConverter.SERVER.getSpanStatus(statusCode); when(serverGetter.getHttpResponseStatusCode(anyMap(), anyMap(), isNull())) .thenReturn(statusCode); @@ -51,7 +51,7 @@ void hasServerStatus(int statusCode) { @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); + StatusCode expectedStatusCode = HttpStatusCodeConverter.CLIENT.getSpanStatus(statusCode); when(clientGetter.getHttpResponseStatusCode(anyMap(), anyMap(), isNull())) .thenReturn(statusCode); diff --git a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientStatusConverterTest.java b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpStatusCodeConverterClientTest.java similarity index 50% rename from instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientStatusConverterTest.java rename to instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpStatusCodeConverterClientTest.java index 7840ad60ed1e..d3285f45aa33 100644 --- a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientStatusConverterTest.java +++ b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpStatusCodeConverterClientTest.java @@ -5,24 +5,32 @@ package io.opentelemetry.instrumentation.api.instrumenter.http; -import static io.opentelemetry.instrumentation.api.instrumenter.http.HttpStatusConverter.CLIENT; +import static io.opentelemetry.instrumentation.api.instrumenter.http.HttpStatusCodeConverter.CLIENT; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.params.provider.Arguments.arguments; import io.opentelemetry.api.trace.StatusCode; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.IntStream; import java.util.stream.Stream; +import org.junit.jupiter.api.extension.ExtensionContext; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.ArgumentsProvider; +import org.junit.jupiter.params.provider.ArgumentsSource; import org.junit.jupiter.params.provider.MethodSource; -public class HttpClientStatusConverterTest { +class HttpStatusCodeConverterClientTest { @ParameterizedTest - @MethodSource("generateParams") + @MethodSource("spanStatusCodes") void httpStatusCodeToOtelStatus(int numeric, StatusCode code) { - assertEquals(code, CLIENT.statusFromHttpStatus(numeric)); + assertEquals(code, CLIENT.getSpanStatus(numeric)); } - static Stream generateParams() { + static Stream spanStatusCodes() { return Stream.of( Arguments.of(100, StatusCode.UNSET), Arguments.of(101, StatusCode.UNSET), @@ -91,4 +99,76 @@ static Stream generateParams() { Arguments.of(99, StatusCode.ERROR), Arguments.of(600, StatusCode.ERROR)); } + + // using the provider from the server test is intentional + @ArgumentsSource(ClientErrorStatusCodes.class) + @ArgumentsSource(HttpStatusCodeConverterServerTest.ServerErrorStatusCodes.class) + @ParameterizedTest + void shouldReturnErrorType(int httpStatusCode, String errorType) { + assertThat(CLIENT.getErrorType(httpStatusCode)).isEqualTo(errorType); + } + + static class ClientErrorStatusCodes implements ArgumentsProvider { + + @Override + public Stream provideArguments(ExtensionContext extensionContext) { + return Stream.of( + arguments(400, "Bad Request"), + arguments(401, "Unauthorized"), + arguments(402, "Payment Required"), + arguments(403, "Forbidden"), + arguments(404, "Not Found"), + arguments(405, "Method Not Allowed"), + arguments(406, "Not Acceptable"), + arguments(407, "Proxy Authentication Required"), + arguments(408, "Request Timeout"), + arguments(409, "Conflict"), + arguments(410, "Gone"), + arguments(411, "Length Required"), + arguments(412, "Precondition Failed"), + arguments(413, "Content Too Large"), + arguments(414, "URI Too Long"), + arguments(415, "Unsupported Media Type"), + arguments(416, "Range Not Satisfiable"), + arguments(417, "Expectation Failed"), + arguments(418, "I'm a teapot"), + arguments(421, "Misdirected Request"), + arguments(422, "Unprocessable Content"), + arguments(423, "Locked"), + arguments(424, "Failed Dependency"), + arguments(425, "Too Early"), + arguments(426, "Upgrade Required"), + arguments(428, "Precondition Required"), + arguments(429, "Too Many Requests"), + arguments(431, "Request Header Fields Too Large"), + arguments(451, "Unavailable For Legal Reasons")); + } + } + + @ArgumentsSource(OtherStatusCodes.class) + @ParameterizedTest + void noErrorTypeForOtherStatusCodes(int httpStatusCode) { + assertThat(CLIENT.getErrorType(httpStatusCode)).isNull(); + } + + static class OtherStatusCodes implements ArgumentsProvider { + + @Override + public Stream provideArguments(ExtensionContext extensionContext) + throws Exception { + + Set errorCodes = + Stream.concat( + new HttpStatusCodeConverterServerTest.ServerErrorStatusCodes() + .provideArguments(extensionContext), + new ClientErrorStatusCodes().provideArguments(extensionContext)) + .map(args -> args.get()[0]) + .map(Integer.class::cast) + .collect(Collectors.toSet()); + + return IntStream.range(100, 600) + .filter(statusCode -> !errorCodes.contains(statusCode)) + .mapToObj(Arguments::of); + } + } } diff --git a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerStatusConverterTest.java b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpStatusCodeConverterServerTest.java similarity index 59% rename from instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerStatusConverterTest.java rename to instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpStatusCodeConverterServerTest.java index 532c5338095e..adf552194a98 100644 --- a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerStatusConverterTest.java +++ b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpStatusCodeConverterServerTest.java @@ -5,24 +5,32 @@ package io.opentelemetry.instrumentation.api.instrumenter.http; -import static io.opentelemetry.instrumentation.api.instrumenter.http.HttpStatusConverter.SERVER; +import static io.opentelemetry.instrumentation.api.instrumenter.http.HttpStatusCodeConverter.SERVER; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.params.provider.Arguments.arguments; import io.opentelemetry.api.trace.StatusCode; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.IntStream; import java.util.stream.Stream; +import org.junit.jupiter.api.extension.ExtensionContext; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.ArgumentsProvider; +import org.junit.jupiter.params.provider.ArgumentsSource; import org.junit.jupiter.params.provider.MethodSource; -public class HttpServerStatusConverterTest { +class HttpStatusCodeConverterServerTest { @ParameterizedTest - @MethodSource("generateParams") + @MethodSource("spanStatusCodes") void httpStatusCodeToOtelStatus(int numeric, StatusCode code) { - assertEquals(code, SERVER.statusFromHttpStatus(numeric)); + assertEquals(code, SERVER.getSpanStatus(numeric)); } - static Stream generateParams() { + static Stream spanStatusCodes() { return Stream.of( Arguments.of(100, StatusCode.UNSET), Arguments.of(101, StatusCode.UNSET), @@ -91,4 +99,54 @@ static Stream generateParams() { Arguments.of(99, StatusCode.ERROR), Arguments.of(600, StatusCode.ERROR)); } + + @ArgumentsSource(ServerErrorStatusCodes.class) + @ParameterizedTest + void shouldReturnErrorTypeForServerErrors(int httpStatusCode, String errorType) { + assertThat(SERVER.getErrorType(httpStatusCode)).isEqualTo(errorType); + } + + static class ServerErrorStatusCodes implements ArgumentsProvider { + + @Override + public Stream provideArguments(ExtensionContext extensionContext) { + return Stream.of( + arguments(500, "Internal Server Error"), + arguments(501, "Not Implemented"), + arguments(502, "Bad Gateway"), + arguments(503, "Service Unavailable"), + arguments(504, "Gateway Timeout"), + arguments(505, "HTTP Version Not Supported"), + arguments(506, "Variant Also Negotiates"), + arguments(507, "Insufficient Storage"), + arguments(508, "Loop Detected"), + arguments(510, "Not Extended"), + arguments(511, "Network Authentication Required")); + } + } + + @ArgumentsSource(OtherStatusCodes.class) + @ParameterizedTest + void noErrorTypeForOtherStatusCodes(int httpStatusCode) { + assertThat(SERVER.getErrorType(httpStatusCode)).isNull(); + } + + static class OtherStatusCodes implements ArgumentsProvider { + + @Override + public Stream provideArguments(ExtensionContext extensionContext) + throws Exception { + + Set serverErrorCodes = + new ServerErrorStatusCodes() + .provideArguments(extensionContext) + .map(args -> args.get()[0]) + .map(Integer.class::cast) + .collect(Collectors.toSet()); + + return IntStream.range(100, 600) + .filter(statusCode -> !serverErrorCodes.contains(statusCode)) + .mapToObj(Arguments::of); + } + } } diff --git a/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorStableSemconvTest.java b/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorStableSemconvTest.java index 6c6702dfb843..5d9f44e426b5 100644 --- a/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorStableSemconvTest.java +++ b/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorStableSemconvTest.java @@ -18,8 +18,10 @@ import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; +import io.opentelemetry.instrumentation.api.instrumenter.http.internal.HttpAttributes; import io.opentelemetry.instrumentation.api.internal.HttpConstants; import io.opentelemetry.semconv.SemanticAttributes; +import java.net.ConnectException; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -110,6 +112,15 @@ public Integer getServerPort(Map request) { String value = request.get("serverPort"); return value == null ? null : Integer.parseInt(value); } + + @Nullable + @Override + public String getErrorType( + Map request, + @Nullable Map respobse, + @Nullable Throwable error) { + return request.get("errorType"); + } } @Test @@ -307,4 +318,61 @@ void shouldUseOtherForUnknownMethods_override(String requestMethod) { .containsEntry(SemanticAttributes.HTTP_REQUEST_METHOD, HttpConstants._OTHER) .containsEntry(SemanticAttributes.HTTP_REQUEST_METHOD_ORIGINAL, requestMethod); } + + @Test + void shouldExtractErrorType_httpStatusCode() { + Map response = new HashMap<>(); + response.put("statusCode", "400"); + + AttributesExtractor, Map> extractor = + HttpClientAttributesExtractor.create(new TestHttpClientAttributesGetter()); + + AttributesBuilder attributes = Attributes.builder(); + extractor.onStart(attributes, Context.root(), emptyMap()); + extractor.onEnd(attributes, Context.root(), emptyMap(), response, null); + + assertThat(attributes.build()) + .containsEntry(SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 400) + .containsEntry(HttpAttributes.ERROR_TYPE, "Bad Request"); + } + + @Test + void shouldExtractErrorType_getter() { + Map request = new HashMap<>(); + request.put("errorType", "custom error type"); + + AttributesExtractor, Map> extractor = + HttpClientAttributesExtractor.create(new TestHttpClientAttributesGetter()); + + AttributesBuilder attributes = Attributes.builder(); + extractor.onStart(attributes, Context.root(), emptyMap()); + extractor.onEnd(attributes, Context.root(), request, emptyMap(), null); + + assertThat(attributes.build()).containsEntry(HttpAttributes.ERROR_TYPE, "custom error type"); + } + + @Test + void shouldExtractErrorType_exceptionClassName() { + AttributesExtractor, Map> extractor = + HttpClientAttributesExtractor.create(new TestHttpClientAttributesGetter()); + + AttributesBuilder attributes = Attributes.builder(); + extractor.onStart(attributes, Context.root(), emptyMap()); + extractor.onEnd(attributes, Context.root(), emptyMap(), emptyMap(), new ConnectException()); + + assertThat(attributes.build()) + .containsEntry(HttpAttributes.ERROR_TYPE, "java.net.ConnectException"); + } + + @Test + void shouldExtractErrorType_other() { + AttributesExtractor, Map> extractor = + HttpClientAttributesExtractor.create(new TestHttpClientAttributesGetter()); + + AttributesBuilder attributes = Attributes.builder(); + extractor.onStart(attributes, Context.root(), emptyMap()); + extractor.onEnd(attributes, Context.root(), emptyMap(), emptyMap(), null); + + assertThat(attributes.build()).containsEntry(HttpAttributes.ERROR_TYPE, HttpConstants._OTHER); + } } diff --git a/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientExperimentalMetricsStableSemconvTest.java b/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientExperimentalMetricsStableSemconvTest.java index d35d02d67955..8762719bbf99 100644 --- a/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientExperimentalMetricsStableSemconvTest.java +++ b/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientExperimentalMetricsStableSemconvTest.java @@ -15,6 +15,7 @@ import io.opentelemetry.api.trace.TraceState; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.instrumenter.OperationListener; +import io.opentelemetry.instrumentation.api.instrumenter.http.internal.HttpAttributes; import io.opentelemetry.sdk.metrics.SdkMeterProvider; import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; import io.opentelemetry.semconv.SemanticAttributes; @@ -48,6 +49,7 @@ void collectsMetrics() { .put(SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 200) .put(SemanticAttributes.HTTP_REQUEST_BODY_SIZE, 100) .put(SemanticAttributes.HTTP_RESPONSE_BODY_SIZE, 200) + .put(HttpAttributes.ERROR_TYPE, "Bad Request") .put(SemanticAttributes.NETWORK_PROTOCOL_NAME, "http") .put(SemanticAttributes.NETWORK_PROTOCOL_VERSION, "2.0") .put(SemanticAttributes.SERVER_SOCKET_ADDRESS, "1.2.3.4") @@ -92,6 +94,7 @@ void collectsMetrics() { equalTo(SemanticAttributes.HTTP_REQUEST_METHOD, "GET"), equalTo( SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 200), + equalTo(HttpAttributes.ERROR_TYPE, "Bad Request"), equalTo( SemanticAttributes.NETWORK_PROTOCOL_NAME, "http"), equalTo( @@ -118,6 +121,7 @@ void collectsMetrics() { equalTo(SemanticAttributes.HTTP_REQUEST_METHOD, "GET"), equalTo( SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 200), + equalTo(HttpAttributes.ERROR_TYPE, "Bad Request"), equalTo( SemanticAttributes.NETWORK_PROTOCOL_NAME, "http"), equalTo( diff --git a/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientMetricsStableSemconvTest.java b/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientMetricsStableSemconvTest.java index 7360965693ce..0d6f6c91a5fc 100644 --- a/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientMetricsStableSemconvTest.java +++ b/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientMetricsStableSemconvTest.java @@ -15,6 +15,7 @@ import io.opentelemetry.api.trace.TraceState; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.instrumenter.OperationListener; +import io.opentelemetry.instrumentation.api.instrumenter.http.internal.HttpAttributes; import io.opentelemetry.sdk.metrics.SdkMeterProvider; import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; import io.opentelemetry.semconv.SemanticAttributes; @@ -50,6 +51,7 @@ void collectsMetrics() { .put(SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 200) .put(SemanticAttributes.HTTP_REQUEST_BODY_SIZE, 100) .put(SemanticAttributes.HTTP_RESPONSE_BODY_SIZE, 200) + .put(HttpAttributes.ERROR_TYPE, "Bad Request") .put(SemanticAttributes.NETWORK_PROTOCOL_NAME, "http") .put(SemanticAttributes.NETWORK_PROTOCOL_VERSION, "2.0") .put(SemanticAttributes.SERVER_SOCKET_ADDRESS, "1.2.3.4") @@ -94,6 +96,7 @@ void collectsMetrics() { equalTo(SemanticAttributes.HTTP_REQUEST_METHOD, "GET"), equalTo( SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 200), + equalTo(HttpAttributes.ERROR_TYPE, "Bad Request"), equalTo( SemanticAttributes.NETWORK_PROTOCOL_NAME, "http"), equalTo( diff --git a/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorStableSemconvTest.java b/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorStableSemconvTest.java index 6722784a6917..92234e03738e 100644 --- a/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorStableSemconvTest.java +++ b/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorStableSemconvTest.java @@ -18,8 +18,10 @@ import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; +import io.opentelemetry.instrumentation.api.instrumenter.http.internal.HttpAttributes; import io.opentelemetry.instrumentation.api.internal.HttpConstants; import io.opentelemetry.semconv.SemanticAttributes; +import java.net.ConnectException; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -142,6 +144,15 @@ public Integer getServerSocketPort( String value = request.get("serverSocketPort"); return value == null ? null : Integer.parseInt(value); } + + @Nullable + @Override + public String getErrorType( + Map request, + @Nullable Map respobse, + @Nullable Throwable error) { + return request.get("errorType"); + } } @Test @@ -348,4 +359,61 @@ void shouldUseOtherForUnknownMethods_override(String requestMethod) { .containsEntry(SemanticAttributes.HTTP_REQUEST_METHOD, HttpConstants._OTHER) .containsEntry(SemanticAttributes.HTTP_REQUEST_METHOD_ORIGINAL, requestMethod); } + + @Test + void shouldExtractErrorType_httpStatusCode() { + Map response = new HashMap<>(); + response.put("statusCode", "500"); + + AttributesExtractor, Map> extractor = + HttpServerAttributesExtractor.create(new TestHttpServerAttributesGetter()); + + AttributesBuilder attributes = Attributes.builder(); + extractor.onStart(attributes, Context.root(), emptyMap()); + extractor.onEnd(attributes, Context.root(), emptyMap(), response, null); + + assertThat(attributes.build()) + .containsEntry(SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 500) + .containsEntry(HttpAttributes.ERROR_TYPE, "Internal Server Error"); + } + + @Test + void shouldExtractErrorType_getter() { + Map request = new HashMap<>(); + request.put("errorType", "custom error type"); + + AttributesExtractor, Map> extractor = + HttpServerAttributesExtractor.create(new TestHttpServerAttributesGetter()); + + AttributesBuilder attributes = Attributes.builder(); + extractor.onStart(attributes, Context.root(), emptyMap()); + extractor.onEnd(attributes, Context.root(), request, emptyMap(), null); + + assertThat(attributes.build()).containsEntry(HttpAttributes.ERROR_TYPE, "custom error type"); + } + + @Test + void shouldExtractErrorType_exceptionClassName() { + AttributesExtractor, Map> extractor = + HttpServerAttributesExtractor.create(new TestHttpServerAttributesGetter()); + + AttributesBuilder attributes = Attributes.builder(); + extractor.onStart(attributes, Context.root(), emptyMap()); + extractor.onEnd(attributes, Context.root(), emptyMap(), emptyMap(), new ConnectException()); + + assertThat(attributes.build()) + .containsEntry(HttpAttributes.ERROR_TYPE, "java.net.ConnectException"); + } + + @Test + void shouldExtractErrorType_other() { + AttributesExtractor, Map> extractor = + HttpServerAttributesExtractor.create(new TestHttpServerAttributesGetter()); + + AttributesBuilder attributes = Attributes.builder(); + extractor.onStart(attributes, Context.root(), emptyMap()); + extractor.onEnd(attributes, Context.root(), emptyMap(), emptyMap(), null); + + assertThat(attributes.build()).containsEntry(HttpAttributes.ERROR_TYPE, HttpConstants._OTHER); + } } diff --git a/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerExperimentalMetricsStableSemconvTest.java b/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerExperimentalMetricsStableSemconvTest.java index 1a96811b8f54..2e2d6fe72d65 100644 --- a/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerExperimentalMetricsStableSemconvTest.java +++ b/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerExperimentalMetricsStableSemconvTest.java @@ -15,6 +15,7 @@ import io.opentelemetry.api.trace.TraceState; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.instrumenter.OperationListener; +import io.opentelemetry.instrumentation.api.instrumenter.http.internal.HttpAttributes; import io.opentelemetry.sdk.metrics.SdkMeterProvider; import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; import io.opentelemetry.semconv.SemanticAttributes; @@ -51,6 +52,7 @@ void collectsMetrics() { .put(SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 200) .put(SemanticAttributes.HTTP_REQUEST_BODY_SIZE, 100) .put(SemanticAttributes.HTTP_RESPONSE_BODY_SIZE, 200) + .put(HttpAttributes.ERROR_TYPE, "Internal Server Error") .put(SemanticAttributes.CLIENT_SOCKET_ADDRESS, "1.2.3.4") .put(SemanticAttributes.CLIENT_SOCKET_PORT, 8080) .put(SemanticAttributes.SERVER_SOCKET_ADDRESS, "4.3.2.1") @@ -154,6 +156,8 @@ void collectsMetrics() { equalTo(SemanticAttributes.HTTP_REQUEST_METHOD, "GET"), equalTo( SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 200), + equalTo( + HttpAttributes.ERROR_TYPE, "Internal Server Error"), equalTo( SemanticAttributes.NETWORK_PROTOCOL_NAME, "http"), equalTo( @@ -179,6 +183,8 @@ void collectsMetrics() { equalTo(SemanticAttributes.HTTP_REQUEST_METHOD, "GET"), equalTo( SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 200), + equalTo( + HttpAttributes.ERROR_TYPE, "Internal Server Error"), equalTo( SemanticAttributes.NETWORK_PROTOCOL_NAME, "http"), equalTo( diff --git a/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetricsStableSemconvTest.java b/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetricsStableSemconvTest.java index 8b5d47b04016..7eab64ff5ccf 100644 --- a/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetricsStableSemconvTest.java +++ b/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetricsStableSemconvTest.java @@ -15,6 +15,7 @@ import io.opentelemetry.api.trace.TraceState; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.instrumenter.OperationListener; +import io.opentelemetry.instrumentation.api.instrumenter.http.internal.HttpAttributes; import io.opentelemetry.sdk.metrics.SdkMeterProvider; import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; import io.opentelemetry.semconv.SemanticAttributes; @@ -53,6 +54,7 @@ void collectsMetrics() { .put(SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 200) .put(SemanticAttributes.HTTP_REQUEST_BODY_SIZE, 100) .put(SemanticAttributes.HTTP_RESPONSE_BODY_SIZE, 200) + .put(HttpAttributes.ERROR_TYPE, "Internal Server Error") .put(SemanticAttributes.CLIENT_SOCKET_ADDRESS, "1.2.3.4") .put(SemanticAttributes.CLIENT_SOCKET_PORT, 8080) .put(SemanticAttributes.SERVER_SOCKET_ADDRESS, "4.3.2.1") @@ -97,6 +99,8 @@ void collectsMetrics() { equalTo(SemanticAttributes.HTTP_REQUEST_METHOD, "GET"), equalTo( SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 200), + equalTo( + HttpAttributes.ERROR_TYPE, "Internal Server Error"), equalTo( SemanticAttributes.NETWORK_PROTOCOL_NAME, "http"), equalTo( diff --git a/instrumentation/google-http-client-1.19/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/googlehttpclient/AbstractGoogleHttpClientTest.java b/instrumentation/google-http-client-1.19/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/googlehttpclient/AbstractGoogleHttpClientTest.java index 7d062b2d2e0c..d1e20a40d9aa 100644 --- a/instrumentation/google-http-client-1.19/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/googlehttpclient/AbstractGoogleHttpClientTest.java +++ b/instrumentation/google-http-client-1.19/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/googlehttpclient/AbstractGoogleHttpClientTest.java @@ -17,19 +17,25 @@ import com.google.api.client.util.ClassInfo; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.trace.SpanKind; +import io.opentelemetry.instrumentation.api.instrumenter.http.internal.HttpAttributes; import io.opentelemetry.instrumentation.api.internal.SemconvStability; import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpClientTest; import io.opentelemetry.instrumentation.testing.junit.http.HttpClientInstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions; +import io.opentelemetry.sdk.testing.assertj.AttributeAssertion; import io.opentelemetry.sdk.trace.data.StatusData; import io.opentelemetry.semconv.SemanticAttributes; import java.net.URI; +import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.HashSet; +import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Set; +import org.assertj.core.api.AbstractLongAssert; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -90,23 +96,31 @@ void errorTracesWhenExceptionIsNotThrown() throws Exception { int responseCode = sendRequest(request, "GET", uri, Collections.emptyMap()); assertThat(responseCode).isEqualTo(500); + + List attributes = + new ArrayList<>( + Arrays.asList( + equalTo(getAttributeKey(SemanticAttributes.NET_PEER_NAME), "localhost"), + satisfies( + getAttributeKey(SemanticAttributes.NET_PEER_PORT), + AbstractLongAssert::isPositive), + equalTo(getAttributeKey(SemanticAttributes.HTTP_URL), uri.toString()), + equalTo(getAttributeKey(SemanticAttributes.HTTP_METHOD), "GET"), + equalTo(getAttributeKey(SemanticAttributes.HTTP_STATUS_CODE), 500), + satisfies( + getAttributeKey(SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH), + AbstractLongAssert::isPositive))); + if (SemconvStability.emitStableHttpSemconv()) { + attributes.add(equalTo(HttpAttributes.ERROR_TYPE, "Internal Server Error")); + } + testing.waitAndAssertTraces( trace -> trace.hasSpansSatisfyingExactly( span -> span.hasKind(SpanKind.CLIENT) .hasStatus(StatusData.error()) - .hasAttributesSatisfyingExactly( - equalTo(getAttributeKey(SemanticAttributes.NET_PEER_NAME), "localhost"), - satisfies( - getAttributeKey(SemanticAttributes.NET_PEER_PORT), - port -> port.isPositive()), - equalTo(getAttributeKey(SemanticAttributes.HTTP_URL), uri.toString()), - equalTo(getAttributeKey(SemanticAttributes.HTTP_METHOD), "GET"), - equalTo(getAttributeKey(SemanticAttributes.HTTP_STATUS_CODE), 500), - satisfies( - getAttributeKey(SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH), - length -> length.isPositive())), + .hasAttributesSatisfyingExactly(attributes), span -> span.hasKind(SpanKind.SERVER).hasParent(trace.getSpan(0)))); } diff --git a/instrumentation/netty/netty-4-common/library/src/main/java/io/opentelemetry/instrumentation/netty/v4/common/internal/client/NettyClientInstrumenterFactory.java b/instrumentation/netty/netty-4-common/library/src/main/java/io/opentelemetry/instrumentation/netty/v4/common/internal/client/NettyClientInstrumenterFactory.java index d2035722f651..00c6287941b7 100644 --- a/instrumentation/netty/netty-4-common/library/src/main/java/io/opentelemetry/instrumentation/netty/v4/common/internal/client/NettyClientInstrumenterFactory.java +++ b/instrumentation/netty/netty-4-common/library/src/main/java/io/opentelemetry/instrumentation/netty/v4/common/internal/client/NettyClientInstrumenterFactory.java @@ -20,6 +20,8 @@ import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanNameExtractorBuilder; import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanStatusExtractor; import io.opentelemetry.instrumentation.api.instrumenter.net.PeerServiceAttributesExtractor; +import io.opentelemetry.instrumentation.api.instrumenter.network.NetworkAttributesExtractor; +import io.opentelemetry.instrumentation.api.instrumenter.network.ServerAttributesExtractor; import io.opentelemetry.instrumentation.netty.common.internal.NettyConnectionRequest; import io.opentelemetry.instrumentation.netty.v4.common.HttpRequestAndChannel; import java.util.List; @@ -92,19 +94,32 @@ public NettyConnectionInstrumenter createConnectionInstrumenter() { boolean connectionTelemetryFullyEnabled = connectionTelemetryState == NettyConnectionInstrumentationFlag.ENABLED; + NettyConnectHttpAttributesGetter getter = NettyConnectHttpAttributesGetter.INSTANCE; - Instrumenter instrumenter = + InstrumenterBuilder builder = Instrumenter.builder( openTelemetry, instrumentationName, NettyConnectionRequest::spanName) .addAttributesExtractor( - PeerServiceAttributesExtractor.create( - NettyConnectHttpAttributesGetter.INSTANCE, peerServiceMapping)) - .addAttributesExtractor( - HttpClientAttributesExtractor.create(NettyConnectHttpAttributesGetter.INSTANCE)) - .buildInstrumenter( - connectionTelemetryFullyEnabled - ? SpanKindExtractor.alwaysInternal() - : SpanKindExtractor.alwaysClient()); + PeerServiceAttributesExtractor.create(getter, peerServiceMapping)); + + if (connectionTelemetryFullyEnabled) { + // when the connection telemetry is fully enabled, CONNECT spans are created for every + // request; and semantically they're not HTTP spans, they must not use the HTTP client + // extractor + builder + .addAttributesExtractor(NetworkAttributesExtractor.create(getter)) + .addAttributesExtractor(ServerAttributesExtractor.create(getter)); + } else { + // in case the connection telemetry is emitted only on errors, the CONNECT span is a stand-in + // for the HTTP client span + builder.addAttributesExtractor(HttpClientAttributesExtractor.create(getter)); + } + + Instrumenter instrumenter = + builder.buildInstrumenter( + connectionTelemetryFullyEnabled + ? SpanKindExtractor.alwaysInternal() + : SpanKindExtractor.alwaysClient()); return connectionTelemetryFullyEnabled ? new NettyConnectionInstrumenterImpl(instrumenter) diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java index 8802434b7b5f..ffc09e95851d 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java @@ -5,6 +5,8 @@ package io.opentelemetry.instrumentation.testing.junit.http; +import static io.opentelemetry.api.common.AttributeKey.stringKey; +import static io.opentelemetry.instrumentation.testing.junit.http.HttpStatusCodeUtil.assertErrorTypeForStatusCode; import static io.opentelemetry.instrumentation.testing.util.TelemetryDataUtil.comparingRootSpanAttribute; import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat; import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo; @@ -1107,8 +1109,15 @@ SpanDataAssert assertClientSpan( getAttributeKey(SemanticAttributes.HTTP_STATUS_CODE); if (responseCode != null) { assertThat(attrs).containsEntry(httpResponseStatusKey, (long) responseCode); + if (responseCode >= 400) { + assertErrorTypeForStatusCode(attrs, responseCode); + } } else { assertThat(attrs).doesNotContainKey(httpResponseStatusKey); + if (SemconvStability.emitStableHttpSemconv()) { + // TODO: add more detailed assertions, per url + assertThat(attrs).containsKey(stringKey("error.type")); + } } if (resendCount != null) { diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java index 40b6f9fa0ae1..016a1c2d6e39 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java @@ -5,6 +5,7 @@ package io.opentelemetry.instrumentation.testing.junit.http; +import static io.opentelemetry.instrumentation.testing.junit.http.HttpStatusCodeUtil.assertErrorTypeForStatusCode; import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.CAPTURE_HEADERS; import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.CAPTURE_PARAMETERS; import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.ERROR; @@ -796,8 +797,12 @@ protected SpanDataAssert assertServerSpan( SemanticAttributes.CLIENT_PORT, port -> assertThat(port).isGreaterThan(0)); } assertThat(attrs).containsEntry(getAttributeKey(SemanticAttributes.HTTP_METHOD), method); + assertThat(attrs) .containsEntry(getAttributeKey(SemanticAttributes.HTTP_STATUS_CODE), statusCode); + if (statusCode >= 500) { + assertErrorTypeForStatusCode(attrs, statusCode); + } AttributeKey netProtocolKey = getAttributeKey(SemanticAttributes.NET_PROTOCOL_NAME); diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpStatusCodeUtil.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpStatusCodeUtil.java new file mode 100644 index 000000000000..35f4af5d28a2 --- /dev/null +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpStatusCodeUtil.java @@ -0,0 +1,84 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.testing.junit.http; + +import static io.opentelemetry.api.common.AttributeKey.stringKey; +import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat; +import static java.util.Collections.unmodifiableMap; + +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.instrumentation.api.internal.SemconvStability; +import java.util.HashMap; +import java.util.Map; +import javax.annotation.Nullable; + +public final class HttpStatusCodeUtil { + + private static final Map errorTypes; + + static { + Map phrases = new HashMap<>(); + phrases.put(500, "Internal Server Error"); + phrases.put(501, "Not Implemented"); + phrases.put(502, "Bad Gateway"); + phrases.put(503, "Service Unavailable"); + phrases.put(504, "Gateway Timeout"); + phrases.put(505, "HTTP Version Not Supported"); + phrases.put(506, "Variant Also Negotiates"); + phrases.put(507, "Insufficient Storage"); + phrases.put(508, "Loop Detected"); + phrases.put(510, "Not Extended"); + phrases.put(511, "Network Authentication Required"); + phrases.put(400, "Bad Request"); + phrases.put(401, "Unauthorized"); + phrases.put(402, "Payment Required"); + phrases.put(403, "Forbidden"); + phrases.put(404, "Not Found"); + phrases.put(405, "Method Not Allowed"); + phrases.put(406, "Not Acceptable"); + phrases.put(407, "Proxy Authentication Required"); + phrases.put(408, "Request Timeout"); + phrases.put(409, "Conflict"); + phrases.put(410, "Gone"); + phrases.put(411, "Length Required"); + phrases.put(412, "Precondition Failed"); + phrases.put(413, "Content Too Large"); + phrases.put(414, "URI Too Long"); + phrases.put(415, "Unsupported Media Type"); + phrases.put(416, "Range Not Satisfiable"); + phrases.put(417, "Expectation Failed"); + phrases.put(418, "I'm a teapot"); + phrases.put(421, "Misdirected Request"); + phrases.put(422, "Unprocessable Content"); + phrases.put(423, "Locked"); + phrases.put(424, "Failed Dependency"); + phrases.put(425, "Too Early"); + phrases.put(426, "Upgrade Required"); + phrases.put(428, "Precondition Required"); + phrases.put(429, "Too Many Requests"); + phrases.put(431, "Request Header Fields Too Large"); + phrases.put(451, "Unavailable For Legal Reasons"); + errorTypes = unmodifiableMap(phrases); + } + + @Nullable + public static String getErrorType(Integer statusCode) { + return errorTypes.get(statusCode); + } + + public static void assertErrorTypeForStatusCode(Attributes attributes, int statusCode) { + if (SemconvStability.emitStableHttpSemconv()) { + String errorType = errorTypes.get(statusCode); + if (errorType == null) { + assertThat(attributes).doesNotContainKey(stringKey("error.type")); + } else { + assertThat(attributes).containsEntry(stringKey("error.type"), errorType); + } + } + } + + private HttpStatusCodeUtil() {} +} From 080b8d9d3cc45761e19c1440b7e20176f107a4e5 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Mon, 18 Sep 2023 12:29:07 +0200 Subject: [PATCH 2/5] reorder --- .../junit/http/HttpStatusCodeUtil.java | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpStatusCodeUtil.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpStatusCodeUtil.java index 35f4af5d28a2..f56554b7a6c7 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpStatusCodeUtil.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpStatusCodeUtil.java @@ -21,17 +21,6 @@ public final class HttpStatusCodeUtil { static { Map phrases = new HashMap<>(); - phrases.put(500, "Internal Server Error"); - phrases.put(501, "Not Implemented"); - phrases.put(502, "Bad Gateway"); - phrases.put(503, "Service Unavailable"); - phrases.put(504, "Gateway Timeout"); - phrases.put(505, "HTTP Version Not Supported"); - phrases.put(506, "Variant Also Negotiates"); - phrases.put(507, "Insufficient Storage"); - phrases.put(508, "Loop Detected"); - phrases.put(510, "Not Extended"); - phrases.put(511, "Network Authentication Required"); phrases.put(400, "Bad Request"); phrases.put(401, "Unauthorized"); phrases.put(402, "Payment Required"); @@ -61,6 +50,17 @@ public final class HttpStatusCodeUtil { phrases.put(429, "Too Many Requests"); phrases.put(431, "Request Header Fields Too Large"); phrases.put(451, "Unavailable For Legal Reasons"); + phrases.put(500, "Internal Server Error"); + phrases.put(501, "Not Implemented"); + phrases.put(502, "Bad Gateway"); + phrases.put(503, "Service Unavailable"); + phrases.put(504, "Gateway Timeout"); + phrases.put(505, "HTTP Version Not Supported"); + phrases.put(506, "Variant Also Negotiates"); + phrases.put(507, "Insufficient Storage"); + phrases.put(508, "Loop Detected"); + phrases.put(510, "Not Extended"); + phrases.put(511, "Network Authentication Required"); errorTypes = unmodifiableMap(phrases); } From f1915b5b02431e2a22fd4bac18db934195b7c711 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Tue, 19 Sep 2023 12:32:28 +0200 Subject: [PATCH 3/5] fixes --- .../http/HttpCommonAttributesExtractor.java | 2 +- ...tAttributesExtractorStableSemconvTest.java | 1 + ...rAttributesExtractorStableSemconvTest.java | 1 + .../HttpUrlConnectionTest.java | 29 ++++++++++++------- .../NettyClientInstrumenterFactory.java | 9 +++--- 5 files changed, 26 insertions(+), 16 deletions(-) diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java index f4e0a9450cdd..e81c5c3223b9 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java @@ -122,7 +122,7 @@ public void onEnd( if (SemconvStability.emitStableHttpSemconv()) { String errorType; - if (statusCode != null) { + if (statusCode != null && statusCode > 0) { errorType = statusCodeConverter.getErrorType(statusCode); } else { errorType = getter.getErrorType(request, response, error); diff --git a/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorStableSemconvTest.java b/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorStableSemconvTest.java index 5d9f44e426b5..f0bdaad10a9d 100644 --- a/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorStableSemconvTest.java +++ b/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorStableSemconvTest.java @@ -339,6 +339,7 @@ void shouldExtractErrorType_httpStatusCode() { @Test void shouldExtractErrorType_getter() { Map request = new HashMap<>(); + request.put("statusCode", "0"); request.put("errorType", "custom error type"); AttributesExtractor, Map> extractor = diff --git a/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorStableSemconvTest.java b/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorStableSemconvTest.java index 92234e03738e..57fe41881dc6 100644 --- a/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorStableSemconvTest.java +++ b/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorStableSemconvTest.java @@ -380,6 +380,7 @@ void shouldExtractErrorType_httpStatusCode() { @Test void shouldExtractErrorType_getter() { Map request = new HashMap<>(); + request.put("statusCode", "0"); request.put("errorType", "custom error type"); AttributesExtractor, Map> extractor = diff --git a/instrumentation/http-url-connection/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/httpurlconnection/HttpUrlConnectionTest.java b/instrumentation/http-url-connection/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/httpurlconnection/HttpUrlConnectionTest.java index 1a58d6562e4d..82ae0e9331c5 100644 --- a/instrumentation/http-url-connection/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/httpurlconnection/HttpUrlConnectionTest.java +++ b/instrumentation/http-url-connection/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/httpurlconnection/HttpUrlConnectionTest.java @@ -15,11 +15,14 @@ import static org.assertj.core.api.Assertions.catchThrowable; import io.opentelemetry.api.trace.Span; +import io.opentelemetry.instrumentation.api.instrumenter.http.internal.HttpAttributes; +import io.opentelemetry.instrumentation.api.internal.SemconvStability; import io.opentelemetry.instrumentation.test.utils.PortUtils; import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpClientTest; import io.opentelemetry.instrumentation.testing.junit.http.HttpClientInstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions; +import io.opentelemetry.sdk.testing.assertj.AttributeAssertion; import io.opentelemetry.sdk.trace.data.StatusData; import io.opentelemetry.semconv.SemanticAttributes; import java.io.DataOutputStream; @@ -29,6 +32,8 @@ import java.net.URI; import java.net.URL; import java.net.URLConnection; +import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Map; @@ -337,6 +342,19 @@ public void traceRequestWithConnectionFailure(String scheme) { connection.getInputStream(); })); + List attributes = + new ArrayList<>( + Arrays.asList( + equalTo(getAttributeKey(SemanticAttributes.NET_PROTOCOL_NAME), "http"), + equalTo(getAttributeKey(SemanticAttributes.NET_PROTOCOL_VERSION), "1.1"), + equalTo(getAttributeKey(SemanticAttributes.NET_PEER_NAME), "localhost"), + equalTo(getAttributeKey(SemanticAttributes.NET_PEER_PORT), PortUtils.UNUSABLE_PORT), + equalTo(getAttributeKey(SemanticAttributes.HTTP_URL), uri), + equalTo(getAttributeKey(SemanticAttributes.HTTP_METHOD), "GET"))); + if (SemconvStability.emitStableHttpSemconv()) { + attributes.add(equalTo(HttpAttributes.ERROR_TYPE, "java.net.ConnectException")); + } + testing.waitAndAssertTraces( trace -> trace.hasSpansSatisfyingExactly( @@ -352,15 +370,6 @@ public void traceRequestWithConnectionFailure(String scheme) { .hasParent(trace.getSpan(0)) .hasStatus(StatusData.error()) .hasException(thrown) - .hasAttributesSatisfyingExactly( - equalTo(getAttributeKey(SemanticAttributes.NET_PROTOCOL_NAME), "http"), - equalTo( - getAttributeKey(SemanticAttributes.NET_PROTOCOL_VERSION), "1.1"), - equalTo(getAttributeKey(SemanticAttributes.NET_PEER_NAME), "localhost"), - equalTo( - getAttributeKey(SemanticAttributes.NET_PEER_PORT), - PortUtils.UNUSABLE_PORT), - equalTo(getAttributeKey(SemanticAttributes.HTTP_URL), uri), - equalTo(getAttributeKey(SemanticAttributes.HTTP_METHOD), "GET")))); + .hasAttributesSatisfyingExactly(attributes))); } } diff --git a/instrumentation/netty/netty-4-common/library/src/main/java/io/opentelemetry/instrumentation/netty/v4/common/internal/client/NettyClientInstrumenterFactory.java b/instrumentation/netty/netty-4-common/library/src/main/java/io/opentelemetry/instrumentation/netty/v4/common/internal/client/NettyClientInstrumenterFactory.java index 00c6287941b7..e404540bc106 100644 --- a/instrumentation/netty/netty-4-common/library/src/main/java/io/opentelemetry/instrumentation/netty/v4/common/internal/client/NettyClientInstrumenterFactory.java +++ b/instrumentation/netty/netty-4-common/library/src/main/java/io/opentelemetry/instrumentation/netty/v4/common/internal/client/NettyClientInstrumenterFactory.java @@ -20,8 +20,6 @@ import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanNameExtractorBuilder; import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanStatusExtractor; import io.opentelemetry.instrumentation.api.instrumenter.net.PeerServiceAttributesExtractor; -import io.opentelemetry.instrumentation.api.instrumenter.network.NetworkAttributesExtractor; -import io.opentelemetry.instrumentation.api.instrumenter.network.ServerAttributesExtractor; import io.opentelemetry.instrumentation.netty.common.internal.NettyConnectionRequest; import io.opentelemetry.instrumentation.netty.v4.common.HttpRequestAndChannel; import java.util.List; @@ -87,6 +85,7 @@ public Instrumenter createHttpInstrumenter( return builder.buildClientInstrumenter(HttpRequestHeadersSetter.INSTANCE); } + @SuppressWarnings("deprecation") // have to use the deprecated Net*AttributesExtractor for now public NettyConnectionInstrumenter createConnectionInstrumenter() { if (connectionTelemetryState == NettyConnectionInstrumentationFlag.DISABLED) { return NoopConnectionInstrumenter.INSTANCE; @@ -106,9 +105,9 @@ public NettyConnectionInstrumenter createConnectionInstrumenter() { // when the connection telemetry is fully enabled, CONNECT spans are created for every // request; and semantically they're not HTTP spans, they must not use the HTTP client // extractor - builder - .addAttributesExtractor(NetworkAttributesExtractor.create(getter)) - .addAttributesExtractor(ServerAttributesExtractor.create(getter)); + builder.addAttributesExtractor( + io.opentelemetry.instrumentation.api.instrumenter.net.NetClientAttributesExtractor.create( + getter)); } else { // in case the connection telemetry is emitted only on errors, the CONNECT span is a stand-in // for the HTTP client span From 9b1f3e8599a83a78125bdacc971741573b7b24e7 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Wed, 20 Sep 2023 13:59:32 +0200 Subject: [PATCH 4/5] Use string value of the status code number --- .../http/HttpCommonAttributesExtractor.java | 4 +- .../http/HttpSpanStatusExtractor.java | 5 +- .../http/HttpStatusCodeConverter.java | 100 +------- .../http/HttpSpanStatusExtractorTest.java | 12 +- .../HttpStatusCodeConverterClientTest.java | 213 ++++++------------ .../HttpStatusCodeConverterServerTest.java | 191 ++++++---------- ...tAttributesExtractorStableSemconvTest.java | 2 +- ...tExperimentalMetricsStableSemconvTest.java | 6 +- .../HttpClientMetricsStableSemconvTest.java | 4 +- ...rAttributesExtractorStableSemconvTest.java | 2 +- ...rExperimentalMetricsStableSemconvTest.java | 8 +- .../HttpServerMetricsStableSemconvTest.java | 5 +- .../AbstractGoogleHttpClientTest.java | 2 +- .../junit/http/AbstractHttpClientTest.java | 6 +- .../junit/http/AbstractHttpServerTest.java | 5 +- .../junit/http/HttpStatusCodeUtil.java | 84 ------- 16 files changed, 171 insertions(+), 478 deletions(-) delete mode 100644 testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpStatusCodeUtil.java diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java index e81c5c3223b9..142b203569c6 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java @@ -122,8 +122,8 @@ public void onEnd( if (SemconvStability.emitStableHttpSemconv()) { String errorType; - if (statusCode != null && statusCode > 0) { - errorType = statusCodeConverter.getErrorType(statusCode); + if (statusCode != null && statusCodeConverter.isError(statusCode)) { + errorType = statusCode.toString(); } else { errorType = getter.getErrorType(request, response, error); // fall back to exception class name & _OTHER 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 4bdbf8ddc3eb..289b0a9dfafc 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 @@ -59,9 +59,8 @@ public void extract( if (response != null) { Integer statusCode = getter.getHttpResponseStatusCode(request, response, error); if (statusCode != null) { - StatusCode statusCodeObj = statusCodeConverter.getSpanStatus(statusCode); - if (statusCodeObj == StatusCode.ERROR) { - spanStatusBuilder.setStatus(statusCodeObj); + if (statusCodeConverter.isError(statusCode)) { + spanStatusBuilder.setStatus(StatusCode.ERROR); return; } } diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpStatusCodeConverter.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpStatusCodeConverter.java index ce0d2ec9b335..b99ee3102f83 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpStatusCodeConverter.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpStatusCodeConverter.java @@ -5,104 +5,26 @@ package io.opentelemetry.instrumentation.api.instrumenter.http; -import static java.util.Collections.unmodifiableMap; - -import io.opentelemetry.api.trace.StatusCode; -import java.util.HashMap; -import java.util.Map; -import javax.annotation.Nullable; - // https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-spans.md#status enum HttpStatusCodeConverter { SERVER { - - @Override - StatusCode getSpanStatus(int responseStatusCode) { - if (responseStatusCode >= 100 && responseStatusCode < 500) { - return StatusCode.UNSET; - } - - return StatusCode.ERROR; - } - - @Nullable @Override - String getErrorType(int responseStatusCode) { - return serverErrorTypes.get(responseStatusCode); + boolean isError(int responseStatusCode) { + return responseStatusCode >= 500 + || + // invalid status code, does not exists + responseStatusCode < 100; } }, CLIENT { @Override - StatusCode getSpanStatus(int responseStatusCode) { - if (responseStatusCode >= 100 && responseStatusCode < 400) { - return StatusCode.UNSET; - } - - return StatusCode.ERROR; - } - - @Nullable - @Override - String getErrorType(int responseStatusCode) { - return clientErrorTypes.get(responseStatusCode); + boolean isError(int responseStatusCode) { + return responseStatusCode >= 400 + || + // invalid status code, does not exists + responseStatusCode < 100; } }; - abstract StatusCode getSpanStatus(int responseStatusCode); - - @Nullable - abstract String getErrorType(int responseStatusCode); - - private static final Map serverErrorTypes; - private static final Map clientErrorTypes; - - // https://en.wikipedia.org/wiki/List_of_HTTP_status_codes - static { - Map serverPhrases = new HashMap<>(); - serverPhrases.put(500, "Internal Server Error"); - serverPhrases.put(501, "Not Implemented"); - serverPhrases.put(502, "Bad Gateway"); - serverPhrases.put(503, "Service Unavailable"); - serverPhrases.put(504, "Gateway Timeout"); - serverPhrases.put(505, "HTTP Version Not Supported"); - serverPhrases.put(506, "Variant Also Negotiates"); - serverPhrases.put(507, "Insufficient Storage"); - serverPhrases.put(508, "Loop Detected"); - serverPhrases.put(510, "Not Extended"); - serverPhrases.put(511, "Network Authentication Required"); - serverErrorTypes = unmodifiableMap(serverPhrases); - - // include all server error types - Map clientPhrases = new HashMap<>(serverPhrases); - clientPhrases.put(400, "Bad Request"); - clientPhrases.put(401, "Unauthorized"); - clientPhrases.put(402, "Payment Required"); - clientPhrases.put(403, "Forbidden"); - clientPhrases.put(404, "Not Found"); - clientPhrases.put(405, "Method Not Allowed"); - clientPhrases.put(406, "Not Acceptable"); - clientPhrases.put(407, "Proxy Authentication Required"); - clientPhrases.put(408, "Request Timeout"); - clientPhrases.put(409, "Conflict"); - clientPhrases.put(410, "Gone"); - clientPhrases.put(411, "Length Required"); - clientPhrases.put(412, "Precondition Failed"); - clientPhrases.put(413, "Content Too Large"); - clientPhrases.put(414, "URI Too Long"); - clientPhrases.put(415, "Unsupported Media Type"); - clientPhrases.put(416, "Range Not Satisfiable"); - clientPhrases.put(417, "Expectation Failed"); - clientPhrases.put(418, "I'm a teapot"); - clientPhrases.put(421, "Misdirected Request"); - clientPhrases.put(422, "Unprocessable Content"); - clientPhrases.put(423, "Locked"); - clientPhrases.put(424, "Failed Dependency"); - clientPhrases.put(425, "Too Early"); - clientPhrases.put(426, "Upgrade Required"); - clientPhrases.put(428, "Precondition Required"); - clientPhrases.put(429, "Too Many Requests"); - clientPhrases.put(431, "Request Header Fields Too Large"); - clientPhrases.put(451, "Unavailable For Legal Reasons"); - clientErrorTypes = unmodifiableMap(clientPhrases); - } + abstract boolean isError(int responseStatusCode); } 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 24ff40b5d946..560af0c37047 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 @@ -34,15 +34,15 @@ class HttpSpanStatusExtractorTest { @ParameterizedTest @ValueSource(ints = {1, 100, 101, 200, 201, 300, 301, 500, 501, 600, 601}) void hasServerStatus(int statusCode) { - StatusCode expectedStatusCode = HttpStatusCodeConverter.SERVER.getSpanStatus(statusCode); + boolean isError = HttpStatusCodeConverter.SERVER.isError(statusCode); when(serverGetter.getHttpResponseStatusCode(anyMap(), anyMap(), isNull())) .thenReturn(statusCode); HttpSpanStatusExtractor.create(serverGetter) .extract(spanStatusBuilder, Collections.emptyMap(), Collections.emptyMap(), null); - if (expectedStatusCode != StatusCode.UNSET) { - verify(spanStatusBuilder).setStatus(expectedStatusCode); + if (isError) { + verify(spanStatusBuilder).setStatus(StatusCode.ERROR); } else { verifyNoInteractions(spanStatusBuilder); } @@ -51,15 +51,15 @@ void hasServerStatus(int statusCode) { @ParameterizedTest @ValueSource(ints = {1, 100, 101, 200, 201, 300, 301, 400, 401, 500, 501, 600, 601}) void hasClientStatus(int statusCode) { - StatusCode expectedStatusCode = HttpStatusCodeConverter.CLIENT.getSpanStatus(statusCode); + boolean isError = HttpStatusCodeConverter.CLIENT.isError(statusCode); when(clientGetter.getHttpResponseStatusCode(anyMap(), anyMap(), isNull())) .thenReturn(statusCode); HttpSpanStatusExtractor.create(clientGetter) .extract(spanStatusBuilder, Collections.emptyMap(), Collections.emptyMap(), null); - if (expectedStatusCode != StatusCode.UNSET) { - verify(spanStatusBuilder).setStatus(expectedStatusCode); + if (isError) { + verify(spanStatusBuilder).setStatus(StatusCode.ERROR); } else { verifyNoInteractions(spanStatusBuilder); } diff --git a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpStatusCodeConverterClientTest.java b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpStatusCodeConverterClientTest.java index d3285f45aa33..275c5dadfcfe 100644 --- a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpStatusCodeConverterClientTest.java +++ b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpStatusCodeConverterClientTest.java @@ -6,169 +6,88 @@ package io.opentelemetry.instrumentation.api.instrumenter.http; import static io.opentelemetry.instrumentation.api.instrumenter.http.HttpStatusCodeConverter.CLIENT; -import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.params.provider.Arguments.arguments; -import io.opentelemetry.api.trace.StatusCode; -import java.util.Set; -import java.util.stream.Collectors; -import java.util.stream.IntStream; import java.util.stream.Stream; -import org.junit.jupiter.api.extension.ExtensionContext; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.ArgumentsProvider; -import org.junit.jupiter.params.provider.ArgumentsSource; import org.junit.jupiter.params.provider.MethodSource; class HttpStatusCodeConverterClientTest { @ParameterizedTest @MethodSource("spanStatusCodes") - void httpStatusCodeToOtelStatus(int numeric, StatusCode code) { - assertEquals(code, CLIENT.getSpanStatus(numeric)); + void httpStatusCodeToOtelStatus(int numeric, boolean isError) { + assertEquals(isError, CLIENT.isError(numeric)); } static Stream spanStatusCodes() { return Stream.of( - Arguments.of(100, StatusCode.UNSET), - Arguments.of(101, StatusCode.UNSET), - Arguments.of(102, StatusCode.UNSET), - Arguments.of(103, StatusCode.UNSET), - Arguments.of(200, StatusCode.UNSET), - Arguments.of(201, StatusCode.UNSET), - Arguments.of(202, StatusCode.UNSET), - Arguments.of(203, StatusCode.UNSET), - Arguments.of(204, StatusCode.UNSET), - Arguments.of(205, StatusCode.UNSET), - Arguments.of(206, StatusCode.UNSET), - Arguments.of(207, StatusCode.UNSET), - Arguments.of(208, StatusCode.UNSET), - Arguments.of(226, StatusCode.UNSET), - Arguments.of(300, StatusCode.UNSET), - Arguments.of(301, StatusCode.UNSET), - Arguments.of(302, StatusCode.UNSET), - Arguments.of(303, StatusCode.UNSET), - Arguments.of(304, StatusCode.UNSET), - Arguments.of(305, StatusCode.UNSET), - Arguments.of(306, StatusCode.UNSET), - Arguments.of(307, StatusCode.UNSET), - Arguments.of(308, StatusCode.UNSET), - Arguments.of(400, StatusCode.ERROR), - Arguments.of(401, StatusCode.ERROR), - Arguments.of(403, StatusCode.ERROR), - Arguments.of(404, StatusCode.ERROR), - Arguments.of(405, StatusCode.ERROR), - Arguments.of(406, StatusCode.ERROR), - Arguments.of(407, StatusCode.ERROR), - Arguments.of(408, StatusCode.ERROR), - Arguments.of(409, StatusCode.ERROR), - Arguments.of(410, StatusCode.ERROR), - Arguments.of(411, StatusCode.ERROR), - Arguments.of(412, StatusCode.ERROR), - Arguments.of(413, StatusCode.ERROR), - Arguments.of(414, StatusCode.ERROR), - Arguments.of(415, StatusCode.ERROR), - Arguments.of(416, StatusCode.ERROR), - Arguments.of(417, StatusCode.ERROR), - Arguments.of(418, StatusCode.ERROR), - Arguments.of(421, StatusCode.ERROR), - Arguments.of(422, StatusCode.ERROR), - Arguments.of(423, StatusCode.ERROR), - Arguments.of(424, StatusCode.ERROR), - Arguments.of(425, StatusCode.ERROR), - Arguments.of(426, StatusCode.ERROR), - Arguments.of(428, StatusCode.ERROR), - Arguments.of(429, StatusCode.ERROR), - Arguments.of(431, StatusCode.ERROR), - Arguments.of(451, StatusCode.ERROR), - Arguments.of(500, StatusCode.ERROR), - Arguments.of(501, StatusCode.ERROR), - Arguments.of(502, StatusCode.ERROR), - Arguments.of(503, StatusCode.ERROR), - Arguments.of(504, StatusCode.ERROR), - Arguments.of(505, StatusCode.ERROR), - Arguments.of(506, StatusCode.ERROR), - Arguments.of(507, StatusCode.ERROR), - Arguments.of(508, StatusCode.ERROR), - Arguments.of(510, StatusCode.ERROR), - Arguments.of(511, StatusCode.ERROR), + Arguments.of(100, false), + Arguments.of(101, false), + Arguments.of(102, false), + Arguments.of(103, false), + Arguments.of(200, false), + Arguments.of(201, false), + Arguments.of(202, false), + Arguments.of(203, false), + Arguments.of(204, false), + Arguments.of(205, false), + Arguments.of(206, false), + Arguments.of(207, false), + Arguments.of(208, false), + Arguments.of(226, false), + Arguments.of(300, false), + Arguments.of(301, false), + Arguments.of(302, false), + Arguments.of(303, false), + Arguments.of(304, false), + Arguments.of(305, false), + Arguments.of(306, false), + Arguments.of(307, false), + Arguments.of(308, false), + Arguments.of(400, true), + Arguments.of(401, true), + Arguments.of(403, true), + Arguments.of(404, true), + Arguments.of(405, true), + Arguments.of(406, true), + Arguments.of(407, true), + Arguments.of(408, true), + Arguments.of(409, true), + Arguments.of(410, true), + Arguments.of(411, true), + Arguments.of(412, true), + Arguments.of(413, true), + Arguments.of(414, true), + Arguments.of(415, true), + Arguments.of(416, true), + Arguments.of(417, true), + Arguments.of(418, true), + Arguments.of(421, true), + Arguments.of(422, true), + Arguments.of(423, true), + Arguments.of(424, true), + Arguments.of(425, true), + Arguments.of(426, true), + Arguments.of(428, true), + Arguments.of(429, true), + Arguments.of(431, true), + Arguments.of(451, true), + Arguments.of(500, true), + Arguments.of(501, true), + Arguments.of(502, true), + Arguments.of(503, true), + Arguments.of(504, true), + Arguments.of(505, true), + Arguments.of(506, true), + Arguments.of(507, true), + Arguments.of(508, true), + Arguments.of(510, true), + Arguments.of(511, true), // Don't exist - Arguments.of(99, StatusCode.ERROR), - Arguments.of(600, StatusCode.ERROR)); - } - - // using the provider from the server test is intentional - @ArgumentsSource(ClientErrorStatusCodes.class) - @ArgumentsSource(HttpStatusCodeConverterServerTest.ServerErrorStatusCodes.class) - @ParameterizedTest - void shouldReturnErrorType(int httpStatusCode, String errorType) { - assertThat(CLIENT.getErrorType(httpStatusCode)).isEqualTo(errorType); - } - - static class ClientErrorStatusCodes implements ArgumentsProvider { - - @Override - public Stream provideArguments(ExtensionContext extensionContext) { - return Stream.of( - arguments(400, "Bad Request"), - arguments(401, "Unauthorized"), - arguments(402, "Payment Required"), - arguments(403, "Forbidden"), - arguments(404, "Not Found"), - arguments(405, "Method Not Allowed"), - arguments(406, "Not Acceptable"), - arguments(407, "Proxy Authentication Required"), - arguments(408, "Request Timeout"), - arguments(409, "Conflict"), - arguments(410, "Gone"), - arguments(411, "Length Required"), - arguments(412, "Precondition Failed"), - arguments(413, "Content Too Large"), - arguments(414, "URI Too Long"), - arguments(415, "Unsupported Media Type"), - arguments(416, "Range Not Satisfiable"), - arguments(417, "Expectation Failed"), - arguments(418, "I'm a teapot"), - arguments(421, "Misdirected Request"), - arguments(422, "Unprocessable Content"), - arguments(423, "Locked"), - arguments(424, "Failed Dependency"), - arguments(425, "Too Early"), - arguments(426, "Upgrade Required"), - arguments(428, "Precondition Required"), - arguments(429, "Too Many Requests"), - arguments(431, "Request Header Fields Too Large"), - arguments(451, "Unavailable For Legal Reasons")); - } - } - - @ArgumentsSource(OtherStatusCodes.class) - @ParameterizedTest - void noErrorTypeForOtherStatusCodes(int httpStatusCode) { - assertThat(CLIENT.getErrorType(httpStatusCode)).isNull(); - } - - static class OtherStatusCodes implements ArgumentsProvider { - - @Override - public Stream provideArguments(ExtensionContext extensionContext) - throws Exception { - - Set errorCodes = - Stream.concat( - new HttpStatusCodeConverterServerTest.ServerErrorStatusCodes() - .provideArguments(extensionContext), - new ClientErrorStatusCodes().provideArguments(extensionContext)) - .map(args -> args.get()[0]) - .map(Integer.class::cast) - .collect(Collectors.toSet()); - - return IntStream.range(100, 600) - .filter(statusCode -> !errorCodes.contains(statusCode)) - .mapToObj(Arguments::of); - } + Arguments.of(99, true), + Arguments.of(600, true)); } } diff --git a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpStatusCodeConverterServerTest.java b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpStatusCodeConverterServerTest.java index adf552194a98..fb6750ca9b64 100644 --- a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpStatusCodeConverterServerTest.java +++ b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpStatusCodeConverterServerTest.java @@ -6,147 +6,88 @@ package io.opentelemetry.instrumentation.api.instrumenter.http; import static io.opentelemetry.instrumentation.api.instrumenter.http.HttpStatusCodeConverter.SERVER; -import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.params.provider.Arguments.arguments; -import io.opentelemetry.api.trace.StatusCode; -import java.util.Set; -import java.util.stream.Collectors; -import java.util.stream.IntStream; import java.util.stream.Stream; -import org.junit.jupiter.api.extension.ExtensionContext; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.ArgumentsProvider; -import org.junit.jupiter.params.provider.ArgumentsSource; import org.junit.jupiter.params.provider.MethodSource; class HttpStatusCodeConverterServerTest { @ParameterizedTest @MethodSource("spanStatusCodes") - void httpStatusCodeToOtelStatus(int numeric, StatusCode code) { - assertEquals(code, SERVER.getSpanStatus(numeric)); + void httpStatusCodeToOtelStatus(int numeric, boolean isError) { + assertEquals(isError, SERVER.isError(numeric)); } static Stream spanStatusCodes() { return Stream.of( - Arguments.of(100, StatusCode.UNSET), - Arguments.of(101, StatusCode.UNSET), - Arguments.of(102, StatusCode.UNSET), - Arguments.of(103, StatusCode.UNSET), - Arguments.of(200, StatusCode.UNSET), - Arguments.of(201, StatusCode.UNSET), - Arguments.of(202, StatusCode.UNSET), - Arguments.of(203, StatusCode.UNSET), - Arguments.of(204, StatusCode.UNSET), - Arguments.of(205, StatusCode.UNSET), - Arguments.of(206, StatusCode.UNSET), - Arguments.of(207, StatusCode.UNSET), - Arguments.of(208, StatusCode.UNSET), - Arguments.of(226, StatusCode.UNSET), - Arguments.of(300, StatusCode.UNSET), - Arguments.of(301, StatusCode.UNSET), - Arguments.of(302, StatusCode.UNSET), - Arguments.of(303, StatusCode.UNSET), - Arguments.of(304, StatusCode.UNSET), - Arguments.of(305, StatusCode.UNSET), - Arguments.of(306, StatusCode.UNSET), - Arguments.of(307, StatusCode.UNSET), - Arguments.of(308, StatusCode.UNSET), - Arguments.of(400, StatusCode.UNSET), - Arguments.of(401, StatusCode.UNSET), - Arguments.of(403, StatusCode.UNSET), - Arguments.of(404, StatusCode.UNSET), - Arguments.of(405, StatusCode.UNSET), - Arguments.of(406, StatusCode.UNSET), - Arguments.of(407, StatusCode.UNSET), - Arguments.of(408, StatusCode.UNSET), - Arguments.of(409, StatusCode.UNSET), - Arguments.of(410, StatusCode.UNSET), - Arguments.of(411, StatusCode.UNSET), - Arguments.of(412, StatusCode.UNSET), - Arguments.of(413, StatusCode.UNSET), - Arguments.of(414, StatusCode.UNSET), - Arguments.of(415, StatusCode.UNSET), - Arguments.of(416, StatusCode.UNSET), - Arguments.of(417, StatusCode.UNSET), - Arguments.of(418, StatusCode.UNSET), - Arguments.of(421, StatusCode.UNSET), - Arguments.of(422, StatusCode.UNSET), - Arguments.of(423, StatusCode.UNSET), - Arguments.of(424, StatusCode.UNSET), - Arguments.of(425, StatusCode.UNSET), - Arguments.of(426, StatusCode.UNSET), - Arguments.of(428, StatusCode.UNSET), - Arguments.of(429, StatusCode.UNSET), - Arguments.of(431, StatusCode.UNSET), - Arguments.of(451, StatusCode.UNSET), - Arguments.of(500, StatusCode.ERROR), - Arguments.of(501, StatusCode.ERROR), - Arguments.of(502, StatusCode.ERROR), - Arguments.of(503, StatusCode.ERROR), - Arguments.of(504, StatusCode.ERROR), - Arguments.of(505, StatusCode.ERROR), - Arguments.of(506, StatusCode.ERROR), - Arguments.of(507, StatusCode.ERROR), - Arguments.of(508, StatusCode.ERROR), - Arguments.of(510, StatusCode.ERROR), - Arguments.of(511, StatusCode.ERROR), + Arguments.of(100, false), + Arguments.of(101, false), + Arguments.of(102, false), + Arguments.of(103, false), + Arguments.of(200, false), + Arguments.of(201, false), + Arguments.of(202, false), + Arguments.of(203, false), + Arguments.of(204, false), + Arguments.of(205, false), + Arguments.of(206, false), + Arguments.of(207, false), + Arguments.of(208, false), + Arguments.of(226, false), + Arguments.of(300, false), + Arguments.of(301, false), + Arguments.of(302, false), + Arguments.of(303, false), + Arguments.of(304, false), + Arguments.of(305, false), + Arguments.of(306, false), + Arguments.of(307, false), + Arguments.of(308, false), + Arguments.of(400, false), + Arguments.of(401, false), + Arguments.of(403, false), + Arguments.of(404, false), + Arguments.of(405, false), + Arguments.of(406, false), + Arguments.of(407, false), + Arguments.of(408, false), + Arguments.of(409, false), + Arguments.of(410, false), + Arguments.of(411, false), + Arguments.of(412, false), + Arguments.of(413, false), + Arguments.of(414, false), + Arguments.of(415, false), + Arguments.of(416, false), + Arguments.of(417, false), + Arguments.of(418, false), + Arguments.of(421, false), + Arguments.of(422, false), + Arguments.of(423, false), + Arguments.of(424, false), + Arguments.of(425, false), + Arguments.of(426, false), + Arguments.of(428, false), + Arguments.of(429, false), + Arguments.of(431, false), + Arguments.of(451, false), + Arguments.of(500, false), + Arguments.of(501, false), + Arguments.of(502, false), + Arguments.of(503, false), + Arguments.of(504, false), + Arguments.of(505, false), + Arguments.of(506, false), + Arguments.of(507, false), + Arguments.of(508, false), + Arguments.of(510, false), + Arguments.of(511, false), // Don't exist - Arguments.of(99, StatusCode.ERROR), - Arguments.of(600, StatusCode.ERROR)); - } - - @ArgumentsSource(ServerErrorStatusCodes.class) - @ParameterizedTest - void shouldReturnErrorTypeForServerErrors(int httpStatusCode, String errorType) { - assertThat(SERVER.getErrorType(httpStatusCode)).isEqualTo(errorType); - } - - static class ServerErrorStatusCodes implements ArgumentsProvider { - - @Override - public Stream provideArguments(ExtensionContext extensionContext) { - return Stream.of( - arguments(500, "Internal Server Error"), - arguments(501, "Not Implemented"), - arguments(502, "Bad Gateway"), - arguments(503, "Service Unavailable"), - arguments(504, "Gateway Timeout"), - arguments(505, "HTTP Version Not Supported"), - arguments(506, "Variant Also Negotiates"), - arguments(507, "Insufficient Storage"), - arguments(508, "Loop Detected"), - arguments(510, "Not Extended"), - arguments(511, "Network Authentication Required")); - } - } - - @ArgumentsSource(OtherStatusCodes.class) - @ParameterizedTest - void noErrorTypeForOtherStatusCodes(int httpStatusCode) { - assertThat(SERVER.getErrorType(httpStatusCode)).isNull(); - } - - static class OtherStatusCodes implements ArgumentsProvider { - - @Override - public Stream provideArguments(ExtensionContext extensionContext) - throws Exception { - - Set serverErrorCodes = - new ServerErrorStatusCodes() - .provideArguments(extensionContext) - .map(args -> args.get()[0]) - .map(Integer.class::cast) - .collect(Collectors.toSet()); - - return IntStream.range(100, 600) - .filter(statusCode -> !serverErrorCodes.contains(statusCode)) - .mapToObj(Arguments::of); - } + Arguments.of(99, false), + Arguments.of(600, false)); } } diff --git a/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorStableSemconvTest.java b/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorStableSemconvTest.java index f0bdaad10a9d..a48cea562236 100644 --- a/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorStableSemconvTest.java +++ b/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorStableSemconvTest.java @@ -333,7 +333,7 @@ void shouldExtractErrorType_httpStatusCode() { assertThat(attributes.build()) .containsEntry(SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 400) - .containsEntry(HttpAttributes.ERROR_TYPE, "Bad Request"); + .containsEntry(HttpAttributes.ERROR_TYPE, "400"); } @Test diff --git a/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientExperimentalMetricsStableSemconvTest.java b/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientExperimentalMetricsStableSemconvTest.java index 8762719bbf99..a201f1958122 100644 --- a/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientExperimentalMetricsStableSemconvTest.java +++ b/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientExperimentalMetricsStableSemconvTest.java @@ -47,9 +47,9 @@ void collectsMetrics() { Attributes responseAttributes = Attributes.builder() .put(SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 200) + .put(HttpAttributes.ERROR_TYPE, "400") .put(SemanticAttributes.HTTP_REQUEST_BODY_SIZE, 100) .put(SemanticAttributes.HTTP_RESPONSE_BODY_SIZE, 200) - .put(HttpAttributes.ERROR_TYPE, "Bad Request") .put(SemanticAttributes.NETWORK_PROTOCOL_NAME, "http") .put(SemanticAttributes.NETWORK_PROTOCOL_VERSION, "2.0") .put(SemanticAttributes.SERVER_SOCKET_ADDRESS, "1.2.3.4") @@ -94,7 +94,7 @@ void collectsMetrics() { equalTo(SemanticAttributes.HTTP_REQUEST_METHOD, "GET"), equalTo( SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 200), - equalTo(HttpAttributes.ERROR_TYPE, "Bad Request"), + equalTo(HttpAttributes.ERROR_TYPE, "400"), equalTo( SemanticAttributes.NETWORK_PROTOCOL_NAME, "http"), equalTo( @@ -121,7 +121,7 @@ void collectsMetrics() { equalTo(SemanticAttributes.HTTP_REQUEST_METHOD, "GET"), equalTo( SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 200), - equalTo(HttpAttributes.ERROR_TYPE, "Bad Request"), + equalTo(HttpAttributes.ERROR_TYPE, "400"), equalTo( SemanticAttributes.NETWORK_PROTOCOL_NAME, "http"), equalTo( diff --git a/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientMetricsStableSemconvTest.java b/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientMetricsStableSemconvTest.java index 0d6f6c91a5fc..4e86eff57d65 100644 --- a/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientMetricsStableSemconvTest.java +++ b/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientMetricsStableSemconvTest.java @@ -49,9 +49,9 @@ void collectsMetrics() { Attributes responseAttributes = Attributes.builder() .put(SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 200) + .put(HttpAttributes.ERROR_TYPE, "400") .put(SemanticAttributes.HTTP_REQUEST_BODY_SIZE, 100) .put(SemanticAttributes.HTTP_RESPONSE_BODY_SIZE, 200) - .put(HttpAttributes.ERROR_TYPE, "Bad Request") .put(SemanticAttributes.NETWORK_PROTOCOL_NAME, "http") .put(SemanticAttributes.NETWORK_PROTOCOL_VERSION, "2.0") .put(SemanticAttributes.SERVER_SOCKET_ADDRESS, "1.2.3.4") @@ -96,7 +96,7 @@ void collectsMetrics() { equalTo(SemanticAttributes.HTTP_REQUEST_METHOD, "GET"), equalTo( SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 200), - equalTo(HttpAttributes.ERROR_TYPE, "Bad Request"), + equalTo(HttpAttributes.ERROR_TYPE, "400"), equalTo( SemanticAttributes.NETWORK_PROTOCOL_NAME, "http"), equalTo( diff --git a/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorStableSemconvTest.java b/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorStableSemconvTest.java index 57fe41881dc6..5f57a669f202 100644 --- a/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorStableSemconvTest.java +++ b/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorStableSemconvTest.java @@ -374,7 +374,7 @@ void shouldExtractErrorType_httpStatusCode() { assertThat(attributes.build()) .containsEntry(SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 500) - .containsEntry(HttpAttributes.ERROR_TYPE, "Internal Server Error"); + .containsEntry(HttpAttributes.ERROR_TYPE, "500"); } @Test diff --git a/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerExperimentalMetricsStableSemconvTest.java b/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerExperimentalMetricsStableSemconvTest.java index 2e2d6fe72d65..32ce0239a6ce 100644 --- a/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerExperimentalMetricsStableSemconvTest.java +++ b/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerExperimentalMetricsStableSemconvTest.java @@ -50,9 +50,9 @@ void collectsMetrics() { Attributes responseAttributes = Attributes.builder() .put(SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 200) + .put(HttpAttributes.ERROR_TYPE, "500") .put(SemanticAttributes.HTTP_REQUEST_BODY_SIZE, 100) .put(SemanticAttributes.HTTP_RESPONSE_BODY_SIZE, 200) - .put(HttpAttributes.ERROR_TYPE, "Internal Server Error") .put(SemanticAttributes.CLIENT_SOCKET_ADDRESS, "1.2.3.4") .put(SemanticAttributes.CLIENT_SOCKET_PORT, 8080) .put(SemanticAttributes.SERVER_SOCKET_ADDRESS, "4.3.2.1") @@ -156,8 +156,7 @@ void collectsMetrics() { equalTo(SemanticAttributes.HTTP_REQUEST_METHOD, "GET"), equalTo( SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 200), - equalTo( - HttpAttributes.ERROR_TYPE, "Internal Server Error"), + equalTo(HttpAttributes.ERROR_TYPE, "500"), equalTo( SemanticAttributes.NETWORK_PROTOCOL_NAME, "http"), equalTo( @@ -183,8 +182,7 @@ void collectsMetrics() { equalTo(SemanticAttributes.HTTP_REQUEST_METHOD, "GET"), equalTo( SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 200), - equalTo( - HttpAttributes.ERROR_TYPE, "Internal Server Error"), + equalTo(HttpAttributes.ERROR_TYPE, "500"), equalTo( SemanticAttributes.NETWORK_PROTOCOL_NAME, "http"), equalTo( diff --git a/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetricsStableSemconvTest.java b/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetricsStableSemconvTest.java index 7eab64ff5ccf..2ea4be9d7f29 100644 --- a/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetricsStableSemconvTest.java +++ b/instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetricsStableSemconvTest.java @@ -52,9 +52,9 @@ void collectsMetrics() { Attributes responseAttributes = Attributes.builder() .put(SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 200) + .put(HttpAttributes.ERROR_TYPE, "500") .put(SemanticAttributes.HTTP_REQUEST_BODY_SIZE, 100) .put(SemanticAttributes.HTTP_RESPONSE_BODY_SIZE, 200) - .put(HttpAttributes.ERROR_TYPE, "Internal Server Error") .put(SemanticAttributes.CLIENT_SOCKET_ADDRESS, "1.2.3.4") .put(SemanticAttributes.CLIENT_SOCKET_PORT, 8080) .put(SemanticAttributes.SERVER_SOCKET_ADDRESS, "4.3.2.1") @@ -99,8 +99,7 @@ void collectsMetrics() { equalTo(SemanticAttributes.HTTP_REQUEST_METHOD, "GET"), equalTo( SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 200), - equalTo( - HttpAttributes.ERROR_TYPE, "Internal Server Error"), + equalTo(HttpAttributes.ERROR_TYPE, "500"), equalTo( SemanticAttributes.NETWORK_PROTOCOL_NAME, "http"), equalTo( diff --git a/instrumentation/google-http-client-1.19/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/googlehttpclient/AbstractGoogleHttpClientTest.java b/instrumentation/google-http-client-1.19/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/googlehttpclient/AbstractGoogleHttpClientTest.java index d1e20a40d9aa..38449792419a 100644 --- a/instrumentation/google-http-client-1.19/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/googlehttpclient/AbstractGoogleHttpClientTest.java +++ b/instrumentation/google-http-client-1.19/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/googlehttpclient/AbstractGoogleHttpClientTest.java @@ -111,7 +111,7 @@ void errorTracesWhenExceptionIsNotThrown() throws Exception { getAttributeKey(SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH), AbstractLongAssert::isPositive))); if (SemconvStability.emitStableHttpSemconv()) { - attributes.add(equalTo(HttpAttributes.ERROR_TYPE, "Internal Server Error")); + attributes.add(equalTo(HttpAttributes.ERROR_TYPE, "500")); } testing.waitAndAssertTraces( diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java index ffc09e95851d..82b5681ed8a3 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java @@ -6,7 +6,6 @@ package io.opentelemetry.instrumentation.testing.junit.http; import static io.opentelemetry.api.common.AttributeKey.stringKey; -import static io.opentelemetry.instrumentation.testing.junit.http.HttpStatusCodeUtil.assertErrorTypeForStatusCode; import static io.opentelemetry.instrumentation.testing.util.TelemetryDataUtil.comparingRootSpanAttribute; import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat; import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo; @@ -1109,8 +1108,9 @@ SpanDataAssert assertClientSpan( getAttributeKey(SemanticAttributes.HTTP_STATUS_CODE); if (responseCode != null) { assertThat(attrs).containsEntry(httpResponseStatusKey, (long) responseCode); - if (responseCode >= 400) { - assertErrorTypeForStatusCode(attrs, responseCode); + if (responseCode >= 400 && SemconvStability.emitStableHttpSemconv()) { + assertThat(attrs) + .containsEntry(HttpAttributes.ERROR_TYPE, String.valueOf(responseCode)); } } else { assertThat(attrs).doesNotContainKey(httpResponseStatusKey); diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java index 016a1c2d6e39..b2c58ad10085 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java @@ -5,7 +5,6 @@ package io.opentelemetry.instrumentation.testing.junit.http; -import static io.opentelemetry.instrumentation.testing.junit.http.HttpStatusCodeUtil.assertErrorTypeForStatusCode; import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.CAPTURE_HEADERS; import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.CAPTURE_PARAMETERS; import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.ERROR; @@ -800,8 +799,8 @@ protected SpanDataAssert assertServerSpan( assertThat(attrs) .containsEntry(getAttributeKey(SemanticAttributes.HTTP_STATUS_CODE), statusCode); - if (statusCode >= 500) { - assertErrorTypeForStatusCode(attrs, statusCode); + if (statusCode >= 500 && SemconvStability.emitStableHttpSemconv()) { + assertThat(attrs).containsEntry(HttpAttributes.ERROR_TYPE, String.valueOf(statusCode)); } AttributeKey netProtocolKey = diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpStatusCodeUtil.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpStatusCodeUtil.java deleted file mode 100644 index f56554b7a6c7..000000000000 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpStatusCodeUtil.java +++ /dev/null @@ -1,84 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.instrumentation.testing.junit.http; - -import static io.opentelemetry.api.common.AttributeKey.stringKey; -import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat; -import static java.util.Collections.unmodifiableMap; - -import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.instrumentation.api.internal.SemconvStability; -import java.util.HashMap; -import java.util.Map; -import javax.annotation.Nullable; - -public final class HttpStatusCodeUtil { - - private static final Map errorTypes; - - static { - Map phrases = new HashMap<>(); - phrases.put(400, "Bad Request"); - phrases.put(401, "Unauthorized"); - phrases.put(402, "Payment Required"); - phrases.put(403, "Forbidden"); - phrases.put(404, "Not Found"); - phrases.put(405, "Method Not Allowed"); - phrases.put(406, "Not Acceptable"); - phrases.put(407, "Proxy Authentication Required"); - phrases.put(408, "Request Timeout"); - phrases.put(409, "Conflict"); - phrases.put(410, "Gone"); - phrases.put(411, "Length Required"); - phrases.put(412, "Precondition Failed"); - phrases.put(413, "Content Too Large"); - phrases.put(414, "URI Too Long"); - phrases.put(415, "Unsupported Media Type"); - phrases.put(416, "Range Not Satisfiable"); - phrases.put(417, "Expectation Failed"); - phrases.put(418, "I'm a teapot"); - phrases.put(421, "Misdirected Request"); - phrases.put(422, "Unprocessable Content"); - phrases.put(423, "Locked"); - phrases.put(424, "Failed Dependency"); - phrases.put(425, "Too Early"); - phrases.put(426, "Upgrade Required"); - phrases.put(428, "Precondition Required"); - phrases.put(429, "Too Many Requests"); - phrases.put(431, "Request Header Fields Too Large"); - phrases.put(451, "Unavailable For Legal Reasons"); - phrases.put(500, "Internal Server Error"); - phrases.put(501, "Not Implemented"); - phrases.put(502, "Bad Gateway"); - phrases.put(503, "Service Unavailable"); - phrases.put(504, "Gateway Timeout"); - phrases.put(505, "HTTP Version Not Supported"); - phrases.put(506, "Variant Also Negotiates"); - phrases.put(507, "Insufficient Storage"); - phrases.put(508, "Loop Detected"); - phrases.put(510, "Not Extended"); - phrases.put(511, "Network Authentication Required"); - errorTypes = unmodifiableMap(phrases); - } - - @Nullable - public static String getErrorType(Integer statusCode) { - return errorTypes.get(statusCode); - } - - public static void assertErrorTypeForStatusCode(Attributes attributes, int statusCode) { - if (SemconvStability.emitStableHttpSemconv()) { - String errorType = errorTypes.get(statusCode); - if (errorType == null) { - assertThat(attributes).doesNotContainKey(stringKey("error.type")); - } else { - assertThat(attributes).containsEntry(stringKey("error.type"), errorType); - } - } - } - - private HttpStatusCodeUtil() {} -} From 0b8bf3b2c3a3f72986a5d551db3fd92a80446655 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Wed, 20 Sep 2023 14:18:17 +0200 Subject: [PATCH 5/5] fixes after rebase --- .../http/HttpCommonAttributesExtractor.java | 8 +++--- .../HttpStatusCodeConverterServerTest.java | 26 +++++++++---------- .../junit/http/AbstractHttpClientTest.java | 1 + .../junit/http/AbstractHttpServerTest.java | 1 + 4 files changed, 20 insertions(+), 16 deletions(-) diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java index 142b203569c6..ab72f719346f 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java @@ -121,9 +121,11 @@ public void onEnd( } if (SemconvStability.emitStableHttpSemconv()) { - String errorType; - if (statusCode != null && statusCodeConverter.isError(statusCode)) { - errorType = statusCode.toString(); + String errorType = null; + if (statusCode != null && statusCode > 0) { + if (statusCodeConverter.isError(statusCode)) { + errorType = statusCode.toString(); + } } else { errorType = getter.getErrorType(request, response, error); // fall back to exception class name & _OTHER diff --git a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpStatusCodeConverterServerTest.java b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpStatusCodeConverterServerTest.java index fb6750ca9b64..59d58fe2afbb 100644 --- a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpStatusCodeConverterServerTest.java +++ b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpStatusCodeConverterServerTest.java @@ -74,20 +74,20 @@ static Stream spanStatusCodes() { Arguments.of(429, false), Arguments.of(431, false), Arguments.of(451, false), - Arguments.of(500, false), - Arguments.of(501, false), - Arguments.of(502, false), - Arguments.of(503, false), - Arguments.of(504, false), - Arguments.of(505, false), - Arguments.of(506, false), - Arguments.of(507, false), - Arguments.of(508, false), - Arguments.of(510, false), - Arguments.of(511, false), + Arguments.of(500, true), + Arguments.of(501, true), + Arguments.of(502, true), + Arguments.of(503, true), + Arguments.of(504, true), + Arguments.of(505, true), + Arguments.of(506, true), + Arguments.of(507, true), + Arguments.of(508, true), + Arguments.of(510, true), + Arguments.of(511, true), // Don't exist - Arguments.of(99, false), - Arguments.of(600, false)); + Arguments.of(99, true), + Arguments.of(600, true)); } } diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java index 82b5681ed8a3..672eff61478b 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java @@ -18,6 +18,7 @@ import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanKind; +import io.opentelemetry.instrumentation.api.instrumenter.http.internal.HttpAttributes; import io.opentelemetry.instrumentation.api.internal.HttpConstants; import io.opentelemetry.instrumentation.api.internal.SemconvStability; import io.opentelemetry.instrumentation.test.utils.PortUtils; diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java index b2c58ad10085..fe9ab4b12580 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java @@ -27,6 +27,7 @@ import io.opentelemetry.context.Context; import io.opentelemetry.context.propagation.TextMapPropagator; import io.opentelemetry.context.propagation.TextMapSetter; +import io.opentelemetry.instrumentation.api.instrumenter.http.internal.HttpAttributes; import io.opentelemetry.instrumentation.api.internal.HttpConstants; import io.opentelemetry.instrumentation.api.internal.SemconvStability; import io.opentelemetry.instrumentation.testing.GlobalTraceUtil;