diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java index 5e66e81c8fa9..234f532cfb66 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java @@ -183,11 +183,10 @@ public void end( span.recordException(error); } - UnsafeAttributes attributesBuilder = new UnsafeAttributes(); + UnsafeAttributes attributes = new UnsafeAttributes(); for (AttributesExtractor extractor : attributesExtractors) { - extractor.onEnd(attributesBuilder, request, response, error); + extractor.onEnd(attributes, request, response, error); } - Attributes attributes = attributesBuilder; span.setAllAttributes(attributes); Instant endTime = null; diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpSpanStatusExtractor.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpSpanStatusExtractor.java index 47d9c5fcbc95..a004ae6e1791 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpSpanStatusExtractor.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpSpanStatusExtractor.java @@ -19,22 +19,36 @@ public final class HttpSpanStatusExtractor implements SpanStatusExtractor { + private final HttpStatusConverter statusConverter; + + /** + * Returns the {@link SpanStatusExtractor} for HTTP requests, which will use the HTTP status code + * to determine the {@link StatusCode} if available or fallback to {@linkplain #getDefault() the + * default status} otherwise. + */ + public static SpanStatusExtractor create( + HttpClientAttributesExtractor attributesExtractor) { + return new HttpSpanStatusExtractor<>(attributesExtractor, HttpStatusConverter.CLIENT); + } + /** * Returns the {@link SpanStatusExtractor} for HTTP requests, which will use the HTTP status code * to determine the {@link StatusCode} if available or fallback to {@linkplain #getDefault() the * default status} otherwise. */ public static SpanStatusExtractor create( - HttpCommonAttributesExtractor attributesExtractor) { - return new HttpSpanStatusExtractor<>(attributesExtractor); + HttpServerAttributesExtractor attributesExtractor) { + return new HttpSpanStatusExtractor<>(attributesExtractor, HttpStatusConverter.SERVER); } private final HttpCommonAttributesExtractor attributesExtractor; private HttpSpanStatusExtractor( - HttpCommonAttributesExtractor attributesExtractor) { + HttpCommonAttributesExtractor attributesExtractor, + HttpStatusConverter statusConverter) { this.attributesExtractor = attributesExtractor; + this.statusConverter = statusConverter; } @Override @@ -42,7 +56,7 @@ public StatusCode extract(REQUEST request, @Nullable RESPONSE response, Throwabl if (response != null) { Integer statusCode = attributesExtractor.statusCode(request, response); if (statusCode != null) { - StatusCode statusCodeObj = HttpStatusConverter.statusFromHttpStatus(statusCode); + StatusCode statusCodeObj = statusConverter.statusFromHttpStatus(statusCode); if (statusCodeObj == StatusCode.ERROR) { return statusCodeObj; } diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpClientStatusConverter.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpClientStatusConverter.java new file mode 100644 index 000000000000..681affece6de --- /dev/null +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpClientStatusConverter.java @@ -0,0 +1,25 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.api.tracer; + +import io.opentelemetry.api.trace.StatusCode; + +final class HttpClientStatusConverter implements HttpStatusConverter { + + static final HttpStatusConverter INSTANCE = new HttpClientStatusConverter(); + + // https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/http.md#status + @Override + public StatusCode statusFromHttpStatus(int httpStatus) { + if (httpStatus >= 100 && httpStatus < 400) { + return StatusCode.UNSET; + } + + return StatusCode.ERROR; + } + + private HttpClientStatusConverter() {} +} diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpClientTracer.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpClientTracer.java index 3005040b0ace..a9ab37b504fe 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpClientTracer.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpClientTracer.java @@ -211,7 +211,7 @@ protected void onResponse(Span span, RESPONSE response) { Integer status = status(response); if (status != null) { span.setAttribute(SemanticAttributes.HTTP_STATUS_CODE, (long) status); - StatusCode statusCode = HttpStatusConverter.statusFromHttpStatus(status); + StatusCode statusCode = HttpStatusConverter.CLIENT.statusFromHttpStatus(status); if (statusCode != StatusCode.UNSET) { span.setStatus(statusCode); } diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpServerStatusConverter.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpServerStatusConverter.java new file mode 100644 index 000000000000..f6db3757d5de --- /dev/null +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpServerStatusConverter.java @@ -0,0 +1,25 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.api.tracer; + +import io.opentelemetry.api.trace.StatusCode; + +final class HttpServerStatusConverter implements HttpStatusConverter { + + static final HttpStatusConverter INSTANCE = new HttpServerStatusConverter(); + + // https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/http.md#status + @Override + public StatusCode statusFromHttpStatus(int httpStatus) { + if (httpStatus >= 100 && httpStatus < 500) { + return StatusCode.UNSET; + } + + return StatusCode.ERROR; + } + + private HttpServerStatusConverter() {} +} diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpServerTracer.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpServerTracer.java index 97d32c8a1114..f44258c71240 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpServerTracer.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpServerTracer.java @@ -276,7 +276,7 @@ private static String extractIpAddress(String forwarded, int start) { private static void setStatus(Span span, int status) { span.setAttribute(SemanticAttributes.HTTP_STATUS_CODE, (long) status); - StatusCode statusCode = HttpStatusConverter.statusFromHttpStatus(status); + StatusCode statusCode = HttpStatusConverter.SERVER.statusFromHttpStatus(status); if (statusCode != StatusCode.UNSET) { span.setStatus(statusCode); } diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpStatusConverter.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpStatusConverter.java index 4709fbc2196c..d53ed64739d2 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpStatusConverter.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpStatusConverter.java @@ -7,16 +7,10 @@ import io.opentelemetry.api.trace.StatusCode; -public final class HttpStatusConverter { +public interface HttpStatusConverter { - // https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/http.md#status - public static StatusCode statusFromHttpStatus(int httpStatus) { - if (httpStatus >= 100 && httpStatus < 400) { - return StatusCode.UNSET; - } + HttpStatusConverter SERVER = HttpServerStatusConverter.INSTANCE; + HttpStatusConverter CLIENT = HttpClientStatusConverter.INSTANCE; - return StatusCode.ERROR; - } - - private HttpStatusConverter() {} + StatusCode statusFromHttpStatus(int httpStatus); } diff --git a/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/HttpStatusConverterTest.groovy b/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/HttpClientStatusConverterTest.groovy similarity index 95% rename from instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/HttpStatusConverterTest.groovy rename to instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/HttpClientStatusConverterTest.groovy index 7b2b37abaf58..fc11f271c019 100644 --- a/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/HttpStatusConverterTest.groovy +++ b/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/HttpClientStatusConverterTest.groovy @@ -8,11 +8,11 @@ package io.opentelemetry.instrumentation.api.tracer import io.opentelemetry.api.trace.StatusCode import spock.lang.Specification -class HttpStatusConverterTest extends Specification { +class HttpClientStatusConverterTest extends Specification { def "test HTTP #httpStatus to OTel #expectedStatus"() { when: - def status = HttpStatusConverter.statusFromHttpStatus(httpStatus) + def status = HttpStatusConverter.CLIENT.statusFromHttpStatus(httpStatus) then: status == expectedStatus diff --git a/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/HttpClientTracerTest.groovy b/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/HttpClientTracerTest.groovy index a9a3a1c661b9..f72b85d85877 100644 --- a/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/HttpClientTracerTest.groovy +++ b/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/HttpClientTracerTest.groovy @@ -111,7 +111,7 @@ class HttpClientTracerTest extends BaseTracerTest { def "test onResponse"() { setup: def tracer = newTracer() - def statusCode = status != null ? HttpStatusConverter.statusFromHttpStatus(status) : null + def statusCode = status != null ? HttpStatusConverter.CLIENT.statusFromHttpStatus(status) : null when: tracer.onResponse(span, resp) diff --git a/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/HttpServerStatusConverterTest.groovy b/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/HttpServerStatusConverterTest.groovy new file mode 100644 index 000000000000..dd1ba9f73302 --- /dev/null +++ b/instrumentation-api/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/HttpServerStatusConverterTest.groovy @@ -0,0 +1,94 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.api.tracer + +import io.opentelemetry.api.trace.StatusCode +import spock.lang.Specification + +class HttpServerStatusConverterTest extends Specification { + + def "test HTTP #httpStatus to OTel #expectedStatus"() { + when: + def status = HttpStatusConverter.SERVER.statusFromHttpStatus(httpStatus) + + then: + status == expectedStatus + + // https://en.wikipedia.org/wiki/List_of_HTTP_status_codes + where: + httpStatus | expectedStatus + 100 | StatusCode.UNSET + 101 | StatusCode.UNSET + 102 | StatusCode.UNSET + 103 | StatusCode.UNSET + + 200 | StatusCode.UNSET + 201 | StatusCode.UNSET + 202 | StatusCode.UNSET + 203 | StatusCode.UNSET + 204 | StatusCode.UNSET + 205 | StatusCode.UNSET + 206 | StatusCode.UNSET + 207 | StatusCode.UNSET + 208 | StatusCode.UNSET + 226 | StatusCode.UNSET + + 300 | StatusCode.UNSET + 301 | StatusCode.UNSET + 302 | StatusCode.UNSET + 303 | StatusCode.UNSET + 304 | StatusCode.UNSET + 305 | StatusCode.UNSET + 306 | StatusCode.UNSET + 307 | StatusCode.UNSET + 308 | StatusCode.UNSET + + 400 | StatusCode.UNSET + 401 | StatusCode.UNSET + 403 | StatusCode.UNSET + 404 | StatusCode.UNSET + 405 | StatusCode.UNSET + 406 | StatusCode.UNSET + 407 | StatusCode.UNSET + 408 | StatusCode.UNSET + 409 | StatusCode.UNSET + 410 | StatusCode.UNSET + 411 | StatusCode.UNSET + 412 | StatusCode.UNSET + 413 | StatusCode.UNSET + 414 | StatusCode.UNSET + 415 | StatusCode.UNSET + 416 | StatusCode.UNSET + 417 | StatusCode.UNSET + 418 | StatusCode.UNSET + 421 | StatusCode.UNSET + 422 | StatusCode.UNSET + 423 | StatusCode.UNSET + 424 | StatusCode.UNSET + 425 | StatusCode.UNSET + 426 | StatusCode.UNSET + 428 | StatusCode.UNSET + 429 | StatusCode.UNSET + 431 | StatusCode.UNSET + 451 | StatusCode.UNSET + + 500 | StatusCode.ERROR + 501 | StatusCode.ERROR + 502 | StatusCode.ERROR + 503 | StatusCode.ERROR + 504 | StatusCode.ERROR + 505 | StatusCode.ERROR + 506 | StatusCode.ERROR + 507 | StatusCode.ERROR + 508 | StatusCode.ERROR + 510 | StatusCode.ERROR + 511 | StatusCode.ERROR + + // Don't exist + 99 | StatusCode.ERROR + 600 | StatusCode.ERROR + } +} diff --git a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientSpanStatusExtractorTest.java b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientSpanStatusExtractorTest.java new file mode 100644 index 000000000000..39e7b7fad1c0 --- /dev/null +++ b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientSpanStatusExtractorTest.java @@ -0,0 +1,70 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.api.instrumenter.http; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.anyMap; +import static org.mockito.Mockito.when; + +import io.opentelemetry.api.trace.StatusCode; +import io.opentelemetry.instrumentation.api.tracer.HttpStatusConverter; +import java.util.Collections; +import java.util.Map; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class HttpClientSpanStatusExtractorTest { + @Mock private HttpClientAttributesExtractor, Map> extractor; + + @ParameterizedTest + @ValueSource(ints = {1, 100, 101, 200, 201, 300, 301, 400, 401, 500, 501, 600, 601}) + void hasStatus(int statusCode) { + when(extractor.statusCode(anyMap(), anyMap())).thenReturn(statusCode); + assertThat( + HttpSpanStatusExtractor.create(extractor) + .extract(Collections.emptyMap(), Collections.emptyMap(), null)) + .isEqualTo(HttpStatusConverter.CLIENT.statusFromHttpStatus(statusCode)); + } + + @ParameterizedTest + @ValueSource(ints = {1, 100, 101, 200, 201, 300, 301, 400, 401, 500, 501, 600, 601}) + void hasStatusAndException(int statusCode) { + when(extractor.statusCode(anyMap(), anyMap())).thenReturn(statusCode); + + // Presence of exception has no effect. + assertThat( + HttpSpanStatusExtractor.create(extractor) + .extract( + Collections.emptyMap(), Collections.emptyMap(), new IllegalStateException())) + .isEqualTo(StatusCode.ERROR); + } + + @Test + void hasNoStatus_fallsBackToDefault_unset() { + when(extractor.statusCode(anyMap(), anyMap())).thenReturn(null); + + assertThat( + HttpSpanStatusExtractor.create(extractor) + .extract(Collections.emptyMap(), Collections.emptyMap(), null)) + .isEqualTo(StatusCode.UNSET); + } + + @Test + void hasNoStatus_fallsBackToDefault_error() { + when(extractor.statusCode(anyMap(), anyMap())).thenReturn(null); + + assertThat( + HttpSpanStatusExtractor.create(extractor) + .extract( + Collections.emptyMap(), Collections.emptyMap(), new IllegalStateException())) + .isEqualTo(StatusCode.ERROR); + } +} diff --git a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpSpanStatusExtractorTest.java b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpSpanStatusExtractorTest.java index 50e808f9c36e..a4a3714d09fb 100644 --- a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpSpanStatusExtractorTest.java +++ b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpSpanStatusExtractorTest.java @@ -22,27 +22,53 @@ @ExtendWith(MockitoExtension.class) class HttpSpanStatusExtractorTest { - @Mock private HttpCommonAttributesExtractor, Map> extractor; + @Mock + private HttpServerAttributesExtractor, Map> serverExtractor; + + @Mock + private HttpClientAttributesExtractor, Map> clientExtractor; @ParameterizedTest - @ValueSource(ints = {1, 100, 101, 200, 201, 300, 301, 400, 401, 500, 501, 600, 601}) - void hasStatus(int statusCode) { - when(extractor.statusCode(anyMap(), anyMap())).thenReturn(statusCode); + @ValueSource(ints = {1, 100, 101, 200, 201, 300, 301, 500, 501, 600, 601}) + void hasServerStatus(int statusCode) { + when(serverExtractor.statusCode(anyMap(), anyMap())).thenReturn(statusCode); + assertThat( + HttpSpanStatusExtractor.create(serverExtractor) + .extract(Collections.emptyMap(), Collections.emptyMap(), null)) + .isEqualTo(HttpStatusConverter.SERVER.statusFromHttpStatus(statusCode)); + } + @ParameterizedTest + @ValueSource(ints = {1, 100, 101, 200, 201, 300, 301, 400, 401, 500, 501, 600, 601}) + void hasClientStatus(int statusCode) { + when(clientExtractor.statusCode(anyMap(), anyMap())).thenReturn(statusCode); assertThat( - HttpSpanStatusExtractor.create(extractor) + HttpSpanStatusExtractor.create(clientExtractor) .extract(Collections.emptyMap(), Collections.emptyMap(), null)) - .isEqualTo(HttpStatusConverter.statusFromHttpStatus(statusCode)); + .isEqualTo(HttpStatusConverter.CLIENT.statusFromHttpStatus(statusCode)); } @ParameterizedTest @ValueSource(ints = {1, 100, 101, 200, 201, 300, 301, 400, 401, 500, 501, 600, 601}) - void hasStatusAndException(int statusCode) { - when(extractor.statusCode(anyMap(), anyMap())).thenReturn(statusCode); + void hasServerStatusAndException(int statusCode) { + when(serverExtractor.statusCode(anyMap(), anyMap())).thenReturn(statusCode); // Presence of exception has no effect. assertThat( - HttpSpanStatusExtractor.create(extractor) + HttpSpanStatusExtractor.create(serverExtractor) + .extract( + Collections.emptyMap(), Collections.emptyMap(), new IllegalStateException())) + .isEqualTo(StatusCode.ERROR); + } + + @ParameterizedTest + @ValueSource(ints = {1, 100, 101, 200, 201, 300, 301, 400, 401, 500, 501, 600, 601}) + void hasClientStatusAndException(int statusCode) { + when(clientExtractor.statusCode(anyMap(), anyMap())).thenReturn(statusCode); + + // Presence of exception has no effect. + assertThat( + HttpSpanStatusExtractor.create(clientExtractor) .extract( Collections.emptyMap(), Collections.emptyMap(), new IllegalStateException())) .isEqualTo(StatusCode.ERROR); @@ -50,22 +76,32 @@ void hasStatusAndException(int statusCode) { @Test void hasNoStatus_fallsBackToDefault_unset() { - when(extractor.statusCode(anyMap(), anyMap())).thenReturn(null); + when(clientExtractor.statusCode(anyMap(), anyMap())).thenReturn(null); + when(serverExtractor.statusCode(anyMap(), anyMap())).thenReturn(null); assertThat( - HttpSpanStatusExtractor.create(extractor) + HttpSpanStatusExtractor.create(serverExtractor) + .extract(Collections.emptyMap(), Collections.emptyMap(), null)) + .isEqualTo(StatusCode.UNSET); + assertThat( + HttpSpanStatusExtractor.create(clientExtractor) .extract(Collections.emptyMap(), Collections.emptyMap(), null)) .isEqualTo(StatusCode.UNSET); } @Test void hasNoStatus_fallsBackToDefault_error() { - when(extractor.statusCode(anyMap(), anyMap())).thenReturn(null); + when(clientExtractor.statusCode(anyMap(), anyMap())).thenReturn(null); + when(serverExtractor.statusCode(anyMap(), anyMap())).thenReturn(null); - assertThat( - HttpSpanStatusExtractor.create(extractor) - .extract( - Collections.emptyMap(), Collections.emptyMap(), new IllegalStateException())) - .isEqualTo(StatusCode.ERROR); + StatusCode serverStatusCode = + HttpSpanStatusExtractor.create(serverExtractor) + .extract(Collections.emptyMap(), Collections.emptyMap(), new IllegalStateException()); + assertThat(serverStatusCode).isEqualTo(StatusCode.ERROR); + + StatusCode clientStatusCode = + HttpSpanStatusExtractor.create(clientExtractor) + .extract(Collections.emptyMap(), Collections.emptyMap(), new IllegalStateException()); + assertThat(clientStatusCode).isEqualTo(StatusCode.ERROR); } } diff --git a/instrumentation/http-url-connection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpurlconnection/HttpUrlConnectionInstrumentation.java b/instrumentation/http-url-connection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpurlconnection/HttpUrlConnectionInstrumentation.java index 1178dd085590..107eadbeb4b2 100644 --- a/instrumentation/http-url-connection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpurlconnection/HttpUrlConnectionInstrumentation.java +++ b/instrumentation/http-url-connection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpurlconnection/HttpUrlConnectionInstrumentation.java @@ -169,7 +169,7 @@ public static void methodExit( if (httpUrlState != null) { Span span = Java8BytecodeBridge.spanFromContext(httpUrlState.context); span.setAttribute(SemanticAttributes.HTTP_STATUS_CODE, returnValue); - StatusCode statusCode = HttpStatusConverter.statusFromHttpStatus(returnValue); + StatusCode statusCode = HttpStatusConverter.CLIENT.statusFromHttpStatus(returnValue); if (statusCode != StatusCode.UNSET) { span.setStatus(statusCode); } diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsFilterTest.groovy b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsFilterTest.groovy index 49289545d1d8..da7c291a1f86 100644 --- a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsFilterTest.groovy +++ b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsFilterTest.groovy @@ -18,7 +18,7 @@ import javax.ws.rs.ext.Provider import static io.opentelemetry.api.trace.SpanKind.INTERNAL import static io.opentelemetry.api.trace.SpanKind.SERVER -import static io.opentelemetry.api.trace.StatusCode.ERROR +import static io.opentelemetry.api.trace.StatusCode.UNSET import static io.opentelemetry.instrumentation.test.utils.TraceUtils.runUnderServerTrace @Unroll @@ -77,7 +77,7 @@ abstract class JaxRsFilterTest extends AgentInstrumentationSpecification { name parentSpanName != null ? parentSpanName : "test.span" kind SERVER if (runsOnServer() && abortNormal) { - status ERROR + status UNSET } } span(1) { diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsHttpServerTest.groovy b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsHttpServerTest.groovy index 665477e69297..2df38b723e9a 100644 --- a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsHttpServerTest.groovy +++ b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsHttpServerTest.groovy @@ -241,7 +241,6 @@ abstract class JaxRsHttpServerTest extends HttpServerTest implements Agent serverSpan(trace, index, traceID, parentID, method, endpoint == PATH_PARAM ? getContextPath() + "/path/{id}/param" : endpoint.resolvePath(address).path, endpoint.resolve(address), - endpoint.errored, endpoint.status, endpoint.query) } @@ -254,7 +253,6 @@ abstract class JaxRsHttpServerTest extends HttpServerTest implements Agent serverSpan(trace, index, null, null, "GET", rawUrl.path, rawUrl.toURI(), - statusCode >= 500, statusCode, null) } @@ -266,13 +264,12 @@ abstract class JaxRsHttpServerTest extends HttpServerTest implements Agent String method, String path, URI fullUrl, - boolean isError, int statusCode, String query) { trace.span(index) { name path kind SERVER - if (isError) { + if (statusCode >= 500) { status ERROR } if (parentID != null) { diff --git a/instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/src/main/java/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/internal/JettyClientInstrumenterBuilder.java b/instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/src/main/java/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/internal/JettyClientInstrumenterBuilder.java index 3ac455b7405c..ef3ab9dfcc13 100644 --- a/instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/src/main/java/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/internal/JettyClientInstrumenterBuilder.java +++ b/instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/src/main/java/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/internal/JettyClientInstrumenterBuilder.java @@ -57,15 +57,13 @@ public Instrumenter build() { JettyHttpClientNetAttributesExtractor netAttributesExtractor = new JettyHttpClientNetAttributesExtractor(); - Instrumenter instrumenter = - Instrumenter.newBuilder( - this.openTelemetry, INSTRUMENTATION_NAME, spanNameExtractor) - .setSpanStatusExtractor(spanStatusExtractor) - .addAttributesExtractor(httpAttributesExtractor) - .addAttributesExtractor(netAttributesExtractor) - .addAttributesExtractors(additionalExtractors) - .addRequestMetrics(HttpClientMetrics.get()) - .newClientInstrumenter(new HttpHeaderSetter()); - return instrumenter; + return Instrumenter.newBuilder( + this.openTelemetry, INSTRUMENTATION_NAME, spanNameExtractor) + .setSpanStatusExtractor(spanStatusExtractor) + .addAttributesExtractor(httpAttributesExtractor) + .addAttributesExtractor(netAttributesExtractor) + .addAttributesExtractors(additionalExtractors) + .addRequestMetrics(HttpClientMetrics.get()) + .newClientInstrumenter(new HttpHeaderSetter()); } } diff --git a/instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationForwardTests.groovy b/instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationForwardTests.groovy index 5c5188fde900..9d69e0a84876 100644 --- a/instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationForwardTests.groovy +++ b/instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationForwardTests.groovy @@ -20,6 +20,7 @@ import java.nio.file.Files import static io.opentelemetry.api.trace.SpanKind.SERVER import static io.opentelemetry.api.trace.StatusCode.ERROR +import static io.opentelemetry.api.trace.StatusCode.UNSET class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { @@ -425,7 +426,7 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification { hasNoParent() name "/$jspWebappContext/forwards/forwardToNonExistent.jsp" kind SERVER - status ERROR + status UNSET attributes { "${SemanticAttributes.NET_PEER_IP.key}" "127.0.0.1" "${SemanticAttributes.NET_PEER_PORT.key}" Long diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/AbstractServlet3MappingTest.groovy b/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/AbstractServlet3MappingTest.groovy index ed6b02924021..955cbf3d6d08 100644 --- a/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/AbstractServlet3MappingTest.groovy +++ b/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/AbstractServlet3MappingTest.groovy @@ -48,7 +48,7 @@ abstract class AbstractServlet3MappingTest extends AgentInstrum span(0) { name getContextPath() + spanName kind SpanKind.SERVER - if (!success) { + if (!success && response.status().code() >= 500) { status ERROR } } diff --git a/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/AbstractServlet5MappingTest.groovy b/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/AbstractServlet5MappingTest.groovy index 7b033959f66d..337df5e9b1c4 100644 --- a/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/AbstractServlet5MappingTest.groovy +++ b/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/AbstractServlet5MappingTest.groovy @@ -47,7 +47,7 @@ abstract class AbstractServlet5MappingTest extends AgentInstrum span(0) { name getContextPath() + spanName kind SpanKind.SERVER - if (!success) { + if (!success && response.status().code() >= 500) { status ERROR } } diff --git a/instrumentation/spring/spring-webflux-5.0/javaagent/src/test/groovy/SpringWebfluxTest.groovy b/instrumentation/spring/spring-webflux-5.0/javaagent/src/test/groovy/SpringWebfluxTest.groovy index be05708de527..21717d8c581e 100644 --- a/instrumentation/spring/spring-webflux-5.0/javaagent/src/test/groovy/SpringWebfluxTest.groovy +++ b/instrumentation/spring/spring-webflux-5.0/javaagent/src/test/groovy/SpringWebfluxTest.groovy @@ -27,6 +27,7 @@ import spock.lang.Unroll import static io.opentelemetry.api.trace.SpanKind.INTERNAL import static io.opentelemetry.api.trace.SpanKind.SERVER import static io.opentelemetry.api.trace.StatusCode.ERROR +import static io.opentelemetry.api.trace.StatusCode.UNSET @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, classes = [SpringWebFluxTestApplication, ForceNettyAutoConfiguration]) class SpringWebfluxTest extends AgentInstrumentationSpecification { @@ -286,7 +287,7 @@ class SpringWebfluxTest extends AgentInstrumentationSpecification { name "/**" kind SERVER hasNoParent() - status ERROR + status UNSET attributes { "${SemanticAttributes.NET_PEER_NAME}" { it == null || it == "localhost" } "${SemanticAttributes.NET_PEER_IP.key}" "127.0.0.1" diff --git a/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy b/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy index bd5a7eacddaa..e0e86ea3d59c 100644 --- a/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy +++ b/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy @@ -171,7 +171,6 @@ abstract class HttpServerTest extends InstrumentationSpecification imple final String fragment final int status final String body - final Boolean errored ServerEndpoint(String uri, int status, String body) { this.uriObj = URI.create(uri) @@ -180,7 +179,6 @@ abstract class HttpServerTest extends InstrumentationSpecification imple this.fragment = uriObj.fragment this.status = status this.body = body - this.errored = status >= 400 } String getPath() { @@ -594,7 +592,7 @@ abstract class HttpServerTest extends InstrumentationSpecification imple trace.span(index) { name expectedServerSpanName(endpoint) kind SpanKind.SERVER // can't use static import because of SERVER type parameter - if (endpoint.errored) { + if (endpoint.status >= 500) { status StatusCode.ERROR } if (parentID != null) {