diff --git a/instrumentation-api-semconv/build.gradle.kts b/instrumentation-api-semconv/build.gradle.kts index ff83cdcea972..fd51e449e7ac 100644 --- a/instrumentation-api-semconv/build.gradle.kts +++ b/instrumentation-api-semconv/build.gradle.kts @@ -75,10 +75,6 @@ tasks { dependsOn("generateJflex") } - test { - jvmArgs("-Dotel.instrumentation.http.prefer-forwarded-url-scheme=true") - } - check { dependsOn(testing.suites) } diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/AlternateUrlSchemeProvider.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/ForwardedUrlSchemeProvider.java similarity index 77% rename from instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/AlternateUrlSchemeProvider.java rename to instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/ForwardedUrlSchemeProvider.java index 4d33b8da0ee2..2b308cafd051 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/AlternateUrlSchemeProvider.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/ForwardedUrlSchemeProvider.java @@ -5,32 +5,20 @@ package io.opentelemetry.instrumentation.api.instrumenter.http; -import io.opentelemetry.instrumentation.api.internal.ConfigPropertiesUtil; import java.util.Locale; import java.util.function.Function; import javax.annotation.Nullable; -final class AlternateUrlSchemeProvider implements Function { - - // if set to true, the instrumentation will prefer the scheme from Forwarded/X-Forwarded-Proto - // headers over the one extracted from the URL - private static final boolean PREFER_FORWARDED_URL_SCHEME = - ConfigPropertiesUtil.getBoolean( - "otel.instrumentation.http.prefer-forwarded-url-scheme", false); +final class ForwardedUrlSchemeProvider implements Function { private final HttpServerAttributesGetter getter; - AlternateUrlSchemeProvider(HttpServerAttributesGetter getter) { + ForwardedUrlSchemeProvider(HttpServerAttributesGetter getter) { this.getter = getter; } @Override public String apply(REQUEST request) { - if (!PREFER_FORWARDED_URL_SCHEME) { - // don't parse headers, extract scheme from the URL - return null; - } - // try Forwarded for (String forwarded : getter.getHttpRequestHeader(request, "forwarded")) { String proto = extractProtoFromForwardedHeader(forwarded); diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorBuilder.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorBuilder.java index 9a672ddc0ba5..6fda831705a3 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorBuilder.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorBuilder.java @@ -128,7 +128,7 @@ public AttributesExtractor build() { InternalUrlAttributesExtractor buildUrlExtractor() { return new InternalUrlAttributesExtractor<>( httpAttributesGetter, - new AlternateUrlSchemeProvider<>(httpAttributesGetter), + new ForwardedUrlSchemeProvider<>(httpAttributesGetter), SemconvStability.emitStableHttpSemconv(), SemconvStability.emitOldHttpSemconv()); } diff --git a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/AlternateUrlSchemeProviderTest.java b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/ForwardedUrlSchemeProviderTest.java similarity index 97% rename from instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/AlternateUrlSchemeProviderTest.java rename to instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/ForwardedUrlSchemeProviderTest.java index c22c205b3ae8..3db247f201e6 100644 --- a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/AlternateUrlSchemeProviderTest.java +++ b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/ForwardedUrlSchemeProviderTest.java @@ -28,13 +28,13 @@ import org.mockito.junit.jupiter.MockitoExtension; @ExtendWith(MockitoExtension.class) -class AlternateUrlSchemeProviderTest { +class ForwardedUrlSchemeProviderTest { private static final String REQUEST = "request"; @Mock HttpServerAttributesGetter getter; - @InjectMocks AlternateUrlSchemeProvider underTest; + @InjectMocks ForwardedUrlSchemeProvider underTest; @Test void noHeaders() { diff --git a/instrumentation-api-semconv/src/testBothHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorBothSemconvTest.java b/instrumentation-api-semconv/src/testBothHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorBothSemconvTest.java index d8d854207f19..b1cafe142951 100644 --- a/instrumentation-api-semconv/src/testBothHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorBothSemconvTest.java +++ b/instrumentation-api-semconv/src/testBothHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorBothSemconvTest.java @@ -122,10 +122,10 @@ public Integer getServerPort(Map request) { void normal() { Map request = new HashMap<>(); request.put("method", "POST"); - request.put("urlFull", "http://github.com"); + request.put("urlFull", "https://github.com"); request.put("urlPath", "/repositories/1"); request.put("urlQuery", "details=true"); - request.put("urlScheme", "http"); + request.put("urlScheme", "https"); request.put("header.content-length", "10"); request.put("route", "/repositories/{id}"); request.put("header.user-agent", "okhttp 3.x"); @@ -159,9 +159,9 @@ void normal() { entry(SemanticAttributes.SERVER_ADDRESS, "github.com"), entry(SemanticAttributes.HTTP_METHOD, "POST"), entry(SemanticAttributes.HTTP_REQUEST_METHOD, "POST"), - entry(SemanticAttributes.HTTP_SCHEME, "http"), + entry(SemanticAttributes.HTTP_SCHEME, "https"), entry(SemanticAttributes.HTTP_TARGET, "/repositories/1?details=true"), - entry(SemanticAttributes.URL_SCHEME, "http"), + entry(SemanticAttributes.URL_SCHEME, "https"), entry(SemanticAttributes.URL_PATH, "/repositories/1"), entry(SemanticAttributes.URL_QUERY, "details=true"), entry(SemanticAttributes.USER_AGENT_ORIGINAL, "okhttp 3.x"), 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 5f57a669f202..ba73b94a0b7a 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 @@ -159,10 +159,10 @@ public String getErrorType( void normal() { Map request = new HashMap<>(); request.put("method", "POST"); - request.put("urlFull", "http://github.com"); + request.put("urlFull", "https://github.com"); request.put("urlPath", "/repositories/1"); request.put("urlQuery", "details=true"); - request.put("urlScheme", "http"); + request.put("urlScheme", "https"); request.put("header.content-length", "10"); request.put("route", "/repositories/{id}"); request.put("header.user-agent", "okhttp 3.x"); @@ -196,7 +196,7 @@ void normal() { .containsOnly( entry(SemanticAttributes.SERVER_ADDRESS, "github.com"), entry(SemanticAttributes.HTTP_REQUEST_METHOD, "POST"), - entry(SemanticAttributes.URL_SCHEME, "http"), + entry(SemanticAttributes.URL_SCHEME, "https"), entry(SemanticAttributes.URL_PATH, "/repositories/1"), entry(SemanticAttributes.URL_QUERY, "details=true"), entry(SemanticAttributes.USER_AGENT_ORIGINAL, "okhttp 3.x"), @@ -417,4 +417,26 @@ void shouldExtractErrorType_other() { assertThat(attributes.build()).containsEntry(HttpAttributes.ERROR_TYPE, HttpConstants._OTHER); } + + @Test + void shouldPreferUrlSchemeFromForwardedHeader() { + Map request = new HashMap<>(); + request.put("urlScheme", "http"); + request.put("header.forwarded", "proto=https"); + + Map response = new HashMap<>(); + response.put("statusCode", "202"); + + AttributesExtractor, Map> extractor = + HttpServerAttributesExtractor.create(new TestHttpServerAttributesGetter()); + + AttributesBuilder startAttributes = Attributes.builder(); + extractor.onStart(startAttributes, Context.root(), request); + assertThat(startAttributes.build()).containsOnly(entry(SemanticAttributes.URL_SCHEME, "https")); + + AttributesBuilder endAttributes = Attributes.builder(); + extractor.onEnd(endAttributes, Context.root(), request, response, null); + assertThat(endAttributes.build()) + .containsOnly(entry(SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 202L)); + } } diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java index eafe2dd14b52..27d20e39eb26 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java @@ -200,7 +200,6 @@ private static void copyNecessaryConfigToSystemProperties(ConfigProperties confi for (String property : asList( "otel.instrumentation.experimental.span-suppression-strategy", - "otel.instrumentation.http.prefer-forwarded-url-scheme", "otel.semconv-stability.opt-in")) { String value = config.getString(property); if (value != null) {