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

Don't normalize the '-' character in HTTP header names (#9735)

This commit is contained in:
Mateusz Rzeszutek 2023-10-24 18:16:13 +02:00 committed by GitHub
parent 5a2f52978f
commit 8bc5297d6d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 148 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/CapturedHttpHeadersUtil.java
View File

@ -17,31 +17,53 @@ import java.util.stream.Collectors;
final class CapturedHttpHeadersUtil {
// these are naturally bounded because they only store keys listed in
// otel.instrumentation.http.capture-headers.server.request and
// otel.instrumentation.http.capture-headers.server.response
private static final ConcurrentMap<String, AttributeKey<List<String>>> requestKeysCache =
new ConcurrentHashMap<>();
private static final ConcurrentMap<String, AttributeKey<List<String>>> responseKeysCache =
new ConcurrentHashMap<>();
// otel.instrumentation.http.server.capture-request-headers and
// otel.instrumentation.http.server.capture-response-headers
private static final ConcurrentMap<String, AttributeKey<List<String>>>
oldSemconvRequestKeysCache = new ConcurrentHashMap<>();
private static final ConcurrentMap<String, AttributeKey<List<String>>>
stableSemconvRequestKeysCache = new ConcurrentHashMap<>();
private static final ConcurrentMap<String, AttributeKey<List<String>>>
oldSemconvResponseKeysCache = new ConcurrentHashMap<>();
private static final ConcurrentMap<String, AttributeKey<List<String>>>
stableSemconvResponseKeysCache = new ConcurrentHashMap<>();
static List<String> lowercase(List<String> names) {
return unmodifiableList(
names.stream().map(s -> s.toLowerCase(Locale.ROOT)).collect(Collectors.toList()));
}
static AttributeKey<List<String>> requestAttributeKey(String headerName) {
return requestKeysCache.computeIfAbsent(headerName, n -> createKey("request", n));
static AttributeKey<List<String>> oldSemconvRequestAttributeKey(String headerName) {
return oldSemconvRequestKeysCache.computeIfAbsent(
headerName, n -> createOldSemconvKey("request", n));
}
static AttributeKey<List<String>> responseAttributeKey(String headerName) {
return responseKeysCache.computeIfAbsent(headerName, n -> createKey("response", n));
static AttributeKey<List<String>> stableSemconvRequestAttributeKey(String headerName) {
return stableSemconvRequestKeysCache.computeIfAbsent(
headerName, n -> createStableSemconvKey("request", n));
}
private static AttributeKey<List<String>> createKey(String type, String headerName) {
// headerName is always lowercase, see CapturedHttpHeaders
static AttributeKey<List<String>> oldSemconvResponseAttributeKey(String headerName) {
return oldSemconvResponseKeysCache.computeIfAbsent(
headerName, n -> createOldSemconvKey("response", n));
}
static AttributeKey<List<String>> stableSemconvResponseAttributeKey(String headerName) {
return stableSemconvResponseKeysCache.computeIfAbsent(
headerName, n -> createStableSemconvKey("response", n));
}
private static AttributeKey<List<String>> createOldSemconvKey(String type, String headerName) {
// headerName is always lowercase, see CapturedHttpHeadersUtil#lowercase
String key = "http." + type + ".header." + headerName.replace('-', '_');
return AttributeKey.stringArrayKey(key);
}
private static AttributeKey<List<String>> createStableSemconvKey(String type, String headerName) {
// headerName is always lowercase, see CapturedHttpHeadersUtil#lowercase
String key = "http." + type + ".header." + headerName;
return AttributeKey.stringArrayKey(key);
}
private CapturedHttpHeadersUtil() {}
}

20
instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java
View File

@ -6,8 +6,10 @@
package io.opentelemetry.instrumentation.api.instrumenter.http;
import static io.opentelemetry.instrumentation.api.instrumenter.http.CapturedHttpHeadersUtil.lowercase;
import static io.opentelemetry.instrumentation.api.instrumenter.http.CapturedHttpHeadersUtil.requestAttributeKey;
import static io.opentelemetry.instrumentation.api.instrumenter.http.CapturedHttpHeadersUtil.responseAttributeKey;
import static io.opentelemetry.instrumentation.api.instrumenter.http.CapturedHttpHeadersUtil.oldSemconvRequestAttributeKey;
import static io.opentelemetry.instrumentation.api.instrumenter.http.CapturedHttpHeadersUtil.oldSemconvResponseAttributeKey;
import static io.opentelemetry.instrumentation.api.instrumenter.http.CapturedHttpHeadersUtil.stableSemconvRequestAttributeKey;
import static io.opentelemetry.instrumentation.api.instrumenter.http.CapturedHttpHeadersUtil.stableSemconvResponseAttributeKey;
import static io.opentelemetry.instrumentation.api.internal.AttributesExtractorUtil.internalSet;
import static io.opentelemetry.instrumentation.api.internal.HttpConstants._OTHER;
@ -70,7 +72,12 @@ abstract class HttpCommonAttributesExtractor<
for (String name : capturedRequestHeaders) {
List<String> values = getter.getHttpRequestHeader(request, name);
if (!values.isEmpty()) {
internalSet(attributes, requestAttributeKey(name), values);
if (SemconvStability.emitOldHttpSemconv()) {
internalSet(attributes, oldSemconvRequestAttributeKey(name), values);
}
if (SemconvStability.emitStableHttpSemconv()) {
internalSet(attributes, stableSemconvRequestAttributeKey(name), values);
}
}
}
}
@ -115,7 +122,12 @@ abstract class HttpCommonAttributesExtractor<
for (String name : capturedResponseHeaders) {
List<String> values = getter.getHttpResponseHeader(request, response, name);
if (!values.isEmpty()) {
internalSet(attributes, responseAttributeKey(name), values);
if (SemconvStability.emitOldHttpSemconv()) {
internalSet(attributes, oldSemconvResponseAttributeKey(name), values);
}
if (SemconvStability.emitStableHttpSemconv()) {
internalSet(attributes, stableSemconvResponseAttributeKey(name), values);
}
}
}
}

6
instrumentation-api-semconv/src/testBothHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorBothSemconvTest.java
View File

@ -143,6 +143,9 @@ class HttpClientAttributesExtractorBothSemconvTest {
entry(
AttributeKey.stringArrayKey("http.request.header.custom_request_header"),
asList("123", "456")),
entry(
AttributeKey.stringArrayKey("http.request.header.custom-request-header"),
asList("123", "456")),
entry(SemanticAttributes.NET_PEER_NAME, "github.com"),
entry(SemanticAttributes.NET_PEER_PORT, 123L),
entry(SemanticAttributes.SERVER_ADDRESS, "github.com"),
@ -162,6 +165,9 @@ class HttpClientAttributesExtractorBothSemconvTest {
entry(
AttributeKey.stringArrayKey("http.response.header.custom_response_header"),
asList("654", "321")),
entry(
AttributeKey.stringArrayKey("http.response.header.custom-response-header"),
asList("654", "321")),
entry(SemanticAttributes.NET_PROTOCOL_NAME, "http"),
entry(SemanticAttributes.NET_PROTOCOL_VERSION, "1.1"),
entry(SemanticAttributes.NETWORK_PROTOCOL_NAME, "http"),

6
instrumentation-api-semconv/src/testBothHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorBothSemconvTest.java
View File

@ -170,6 +170,9 @@ class HttpServerAttributesExtractorBothSemconvTest {
entry(SemanticAttributes.CLIENT_ADDRESS, "1.1.1.1"),
entry(
AttributeKey.stringArrayKey("http.request.header.custom_request_header"),
asList("123", "456")),
entry(
AttributeKey.stringArrayKey("http.request.header.custom-request-header"),
asList("123", "456")));
AttributesBuilder endAttributes = Attributes.builder();
@ -189,6 +192,9 @@ class HttpServerAttributesExtractorBothSemconvTest {
entry(SemanticAttributes.HTTP_RESPONSE_BODY_SIZE, 20L),
entry(
AttributeKey.stringArrayKey("http.response.header.custom_response_header"),
asList("654", "321")),
entry(
AttributeKey.stringArrayKey("http.response.header.custom-response-header"),
asList("654", "321")));
}
}

4
instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorStableSemconvTest.java
View File

@ -173,7 +173,7 @@ class HttpClientAttributesExtractorStableSemconvTest {
entry(SemanticAttributes.URL_FULL, "http://github.com"),
entry(SemanticAttributes.USER_AGENT_ORIGINAL, "okhttp 3.x"),
entry(
AttributeKey.stringArrayKey("http.request.header.custom_request_header"),
AttributeKey.stringArrayKey("http.request.header.custom-request-header"),
asList("123", "456")),
entry(SemanticAttributes.SERVER_ADDRESS, "github.com"),
entry(SemanticAttributes.SERVER_PORT, 123L),
@ -187,7 +187,7 @@ class HttpClientAttributesExtractorStableSemconvTest {
entry(SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 202L),
entry(SemanticAttributes.HTTP_RESPONSE_BODY_SIZE, 20L),
entry(
AttributeKey.stringArrayKey("http.response.header.custom_response_header"),
AttributeKey.stringArrayKey("http.response.header.custom-response-header"),
asList("654", "321")),
entry(SemanticAttributes.NETWORK_PROTOCOL_NAME, "http"),
entry(SemanticAttributes.NETWORK_PROTOCOL_VERSION, "1.1"),

4
instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorStableSemconvTest.java
View File

@ -219,7 +219,7 @@ class HttpServerAttributesExtractorStableSemconvTest {
entry(SemanticAttributes.HTTP_ROUTE, "/repositories/{id}"),
entry(SemanticAttributes.CLIENT_ADDRESS, "1.1.1.1"),
entry(
AttributeKey.stringArrayKey("http.request.header.custom_request_header"),
AttributeKey.stringArrayKey("http.request.header.custom-request-header"),
asList("123", "456")));
AttributesBuilder endAttributes = Attributes.builder();
@ -235,7 +235,7 @@ class HttpServerAttributesExtractorStableSemconvTest {
entry(SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 202L),
entry(SemanticAttributes.HTTP_RESPONSE_BODY_SIZE, 20L),
entry(
AttributeKey.stringArrayKey("http.response.header.custom_response_header"),
AttributeKey.stringArrayKey("http.response.header.custom-response-header"),
asList("654", "321")));
}

11
instrumentation/jaxrs/jaxrs-common/testing/src/main/groovy/AbstractJaxRsHttpServerTest.groovy
View File

@ -3,6 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
import io.opentelemetry.instrumentation.api.internal.SemconvStability
import io.opentelemetry.instrumentation.test.AgentTestTrait
import io.opentelemetry.instrumentation.test.asserts.TraceAssert
import io.opentelemetry.instrumentation.test.base.HttpServerTest
@ -296,8 +297,14 @@ abstract class AbstractJaxRsHttpServerTest<S> extends HttpServerTest<S> implemen
"$SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH" { it == null || it instanceof Long }
"$SemanticAttributes.HTTP_ROUTE" path
if (fullUrl.getPath().endsWith(ServerEndpoint.CAPTURE_HEADERS.getPath())) {
"http.request.header.x_test_request" { it == ["test"] }
"http.response.header.x_test_response" { it == ["test"] }
if (SemconvStability.emitOldHttpSemconv()) {
"http.request.header.x_test_request" { it == ["test"] }
"http.response.header.x_test_response" { it == ["test"] }
}
if (SemconvStability.emitStableHttpSemconv()) {
"http.request.header.x-test-request" { it == ["test"] }
"http.response.header.x-test-response" { it == ["test"] }
}
}
}
}

34
instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/ServletRequestParametersExtractor.java
View File

@ -11,6 +11,7 @@ import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.api.common.AttributesBuilder;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
import io.opentelemetry.instrumentation.api.internal.SemconvStability;
import io.opentelemetry.javaagent.bootstrap.internal.InstrumentationConfig;
import java.util.List;
import java.util.Locale;
@ -27,8 +28,10 @@ public class ServletRequestParametersExtractor<REQUEST, RESPONSE>
.getList(
"otel.instrumentation.servlet.experimental.capture-request-parameters", emptyList());
private static final ConcurrentMap<String, AttributeKey<List<String>>> parameterKeysCache =
new ConcurrentHashMap<>();
private static final ConcurrentMap<String, AttributeKey<List<String>>>
oldSemconvParameterKeysCache = new ConcurrentHashMap<>();
private static final ConcurrentMap<String, AttributeKey<List<String>>>
stableSemconvParameterKeysCache = new ConcurrentHashMap<>();
private final ServletAccessor<REQUEST, RESPONSE> accessor;
@ -45,7 +48,12 @@ public class ServletRequestParametersExtractor<REQUEST, RESPONSE>
for (String name : CAPTURE_REQUEST_PARAMETERS) {
List<String> values = accessor.getRequestParameterValues(request, name);
if (!values.isEmpty()) {
consumer.accept(parameterAttributeKey(name), values);
if (SemconvStability.emitOldHttpSemconv()) {
consumer.accept(oldSemconvParameterAttributeKey(name), values);
}
if (SemconvStability.emitStableHttpSemconv()) {
consumer.accept(stableSemconvParameterAttributeKey(name), values);
}
}
}
}
@ -69,15 +77,29 @@ public class ServletRequestParametersExtractor<REQUEST, RESPONSE>
setAttributes(request, attributes::put);
}
private static AttributeKey<List<String>> parameterAttributeKey(String headerName) {
return parameterKeysCache.computeIfAbsent(headerName, n -> createKey(n));
private static AttributeKey<List<String>> oldSemconvParameterAttributeKey(String headerName) {
return oldSemconvParameterKeysCache.computeIfAbsent(
headerName, ServletRequestParametersExtractor::createOldSemconvKey);
}
private static AttributeKey<List<String>> createKey(String parameterName) {
private static AttributeKey<List<String>> stableSemconvParameterAttributeKey(String headerName) {
return stableSemconvParameterKeysCache.computeIfAbsent(
headerName, ServletRequestParametersExtractor::createStableSemconvKey);
}
private static AttributeKey<List<String>> createOldSemconvKey(String parameterName) {
// normalize parameter name similarly as is done with header names when header values are
// captured as span attributes
parameterName = parameterName.toLowerCase(Locale.ROOT);
String key = "servlet.request.parameter." + parameterName.replace('-', '_');
return AttributeKey.stringArrayKey(key);
}
private static AttributeKey<List<String>> createStableSemconvKey(String parameterName) {
// normalize parameter name similarly as is done with header names when header values are
// captured as span attributes
parameterName = parameterName.toLowerCase(Locale.ROOT);
String key = "servlet.request.parameter." + parameterName;
return AttributeKey.stringArrayKey(key);
}
}

30
testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java
View File

@ -23,6 +23,7 @@ import io.opentelemetry.instrumentation.api.internal.HttpConstants;
import io.opentelemetry.instrumentation.api.internal.SemconvStability;
import io.opentelemetry.instrumentation.test.utils.PortUtils;
import io.opentelemetry.instrumentation.testing.InstrumentationTestRunner;
import io.opentelemetry.sdk.testing.assertj.AttributeAssertion;
import io.opentelemetry.sdk.testing.assertj.SpanDataAssert;
import io.opentelemetry.sdk.testing.assertj.TraceAssert;
import io.opentelemetry.sdk.trace.data.SpanData;
@ -538,13 +539,28 @@ public abstract class AbstractHttpClientTest<REQUEST> implements HttpClientTypeA
trace.hasSpansSatisfyingExactly(
span -> {
assertClientSpan(span, uri, method, responseCode, null).hasNoParent();
span.hasAttributesSatisfying(
equalTo(
AttributeKey.stringArrayKey("http.request.header.x_test_request"),
singletonList("test")),
equalTo(
AttributeKey.stringArrayKey("http.response.header.x_test_response"),
singletonList("test")));
List<AttributeAssertion> attributeAssertions = new ArrayList<>();
if (SemconvStability.emitOldHttpSemconv()) {
attributeAssertions.add(
equalTo(
AttributeKey.stringArrayKey("http.request.header.x_test_request"),
singletonList("test")));
attributeAssertions.add(
equalTo(
AttributeKey.stringArrayKey("http.response.header.x_test_response"),
singletonList("test")));
}
if (SemconvStability.emitStableHttpSemconv()) {
attributeAssertions.add(
equalTo(
AttributeKey.stringArrayKey("http.request.header.x-test-request"),
singletonList("test")));
attributeAssertions.add(
equalTo(
AttributeKey.stringArrayKey("http.response.header.x-test-response"),
singletonList("test")));
}
span.hasAttributesSatisfying(attributeAssertions);
},
span -> assertServerSpan(span).hasParent(trace.getSpan(0)));
});

29
testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java
View File

@ -854,15 +854,30 @@ public abstract class AbstractHttpServerTest<SERVER> extends AbstractHttpServerU
}
if (endpoint == CAPTURE_HEADERS) {
assertThat(attrs)
.containsEntry("http.request.header.x_test_request", new String[] {"test"});
assertThat(attrs)
.containsEntry("http.response.header.x_test_response", new String[] {"test"});
if (SemconvStability.emitOldHttpSemconv()) {
assertThat(attrs)
.containsEntry("http.request.header.x_test_request", new String[] {"test"});
assertThat(attrs)
.containsEntry("http.response.header.x_test_response", new String[] {"test"});
}
if (SemconvStability.emitStableHttpSemconv()) {
assertThat(attrs)
.containsEntry("http.request.header.x-test-request", new String[] {"test"});
assertThat(attrs)
.containsEntry("http.response.header.x-test-response", new String[] {"test"});
}
}
if (endpoint == CAPTURE_PARAMETERS) {
assertThat(attrs)
.containsEntry(
"servlet.request.parameter.test_parameter", new String[] {"test value õäöü"});
if (SemconvStability.emitOldHttpSemconv()) {
assertThat(attrs)
.containsEntry(
"servlet.request.parameter.test_parameter", new String[] {"test value õäöü"});
}
if (SemconvStability.emitStableHttpSemconv()) {
assertThat(attrs)
.containsEntry(
"servlet.request.parameter.test-parameter", new String[] {"test value õäöü"});
}
}
});