From d4e27fb4f3f1e3bbe9b9ff8e6ffe3d481bb725ef Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 30 Oct 2026 03:43:26 +0000 Subject: [PATCH] Replace gRPC request header selectors with exact-name capture lists Co-authored-by: trask <218610+trask@users.noreply.github.com> --- CHANGELOG.md | 7 +- ...BackedDeclarativeConfigPropertiesTest.java | 33 +++++++ docs/declarative-configuration-example.yaml | 32 ++----- instrumentation/grpc-1.6/README.md | 14 ++- .../grpc-1.6/javaagent/build.gradle.kts | 6 +- .../grpc/v1_6/GrpcSingletons.java | 19 +--- .../v1_6/CapturedGrpcRequestHeadersUtil.java | 65 ------------- .../grpc/v1_6/GrpcAttributesExtractor.java | 60 ++++-------- .../grpc/v1_6/GrpcTelemetryBuilder.java | 40 ++++---- .../grpc/v1_6/internal/GrpcConfig.java | 40 ++------ .../CapturedGrpcRequestHeadersUtilTest.java | 39 -------- .../v1_6/GrpcAttributesExtractorTest.java | 94 +++++++++---------- .../instrumentation/grpc/v1_6/GrpcTest.java | 76 +++++++++++++-- .../grpc/v1_6/internal/GrpcConfigTest.java | 58 +++++++----- instrumentation/grpc-1.6/metadata.yaml | 48 ++-------- .../grpc/v1_6/AbstractGrpcTest.java | 40 ++++++-- 16 files changed, 288 insertions(+), 383 deletions(-) delete mode 100644 instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/CapturedGrpcRequestHeadersUtil.java delete mode 100644 instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/CapturedGrpcRequestHeadersUtilTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index f45224abd0b3..eb22d5e35392 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,11 @@ ### ⚠️ Breaking changes -- Rename `otel.instrumentation.grpc.{client,server}.request-metadata.{included,excluded}` to - `otel.instrumentation.grpc.{client,server}.request-headers.{included,excluded}`. +- Replace gRPC request metadata/header selectors with exact-name lists: + `otel.instrumentation.grpc.{client,server}.request-captured-headers`, + declarative `java.grpc.{client,server}.request_captured_headers`, and + `GrpcTelemetryBuilder.set{Client,Server}RequestCapturedHeaders(Collection)`. + Wildcard and exclusion selection are no longer supported; unset or empty lists capture no headers. - Captured gRPC request attributes now use `rpc.request.header.` instead of `rpc.request.metadata.`. - Rename `messaging.system` to `messaging.system.name` (`aws_sqs` becomes `aws.sqs`) and adopt diff --git a/declarative-config-bridge/src/test/java/io/opentelemetry/instrumentation/config/bridge/ConfigPropertiesBackedDeclarativeConfigPropertiesTest.java b/declarative-config-bridge/src/test/java/io/opentelemetry/instrumentation/config/bridge/ConfigPropertiesBackedDeclarativeConfigPropertiesTest.java index dd362c1f6dd2..6d23353f4e91 100644 --- a/declarative-config-bridge/src/test/java/io/opentelemetry/instrumentation/config/bridge/ConfigPropertiesBackedDeclarativeConfigPropertiesTest.java +++ b/declarative-config-bridge/src/test/java/io/opentelemetry/instrumentation/config/bridge/ConfigPropertiesBackedDeclarativeConfigPropertiesTest.java @@ -5,6 +5,7 @@ package io.opentelemetry.instrumentation.config.bridge; +import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; import static java.util.Collections.singletonMap; import static org.assertj.core.api.Assertions.assertThat; @@ -191,6 +192,38 @@ void testComponentConfigDoesNotUseInstrumentationSpecialCases() { .isNull(); } + @ParameterizedTest + @ValueSource(strings = {"client", "server"}) + void testGrpcRequestCapturedHeadersMapping(String side) { + DeclarativeConfigProperties config = + createConfig( + "otel.instrumentation.grpc." + side + ".request-captured-headers", + "X-Request-Id,some-*-key,some-?-key"); + + assertThat( + config + .getStructured("java") + .getStructured("grpc") + .getStructured(side) + .getScalarList("request_captured_headers", String.class)) + .containsExactly("X-Request-Id", "some-*-key", "some-?-key"); + } + + @ParameterizedTest + @ValueSource(strings = {"client", "server"}) + void testGrpcRequestCapturedHeadersEmpty(String side) { + DeclarativeConfigProperties config = + createConfig("otel.instrumentation.grpc." + side + ".request-captured-headers", ""); + + assertThat( + config + .getStructured("java") + .getStructured("grpc") + .getStructured(side) + .getScalarList("request_captured_headers", String.class, emptyList())) + .isEmpty(); + } + @Test void testMessagingHeadersSelectorMapping() { Map properties = new HashMap<>(); diff --git a/docs/declarative-configuration-example.yaml b/docs/declarative-configuration-example.yaml index b67080a8b13d..b3bca1526343 100644 --- a/docs/declarative-configuration-example.yaml +++ b/docs/declarative-configuration-example.yaml @@ -255,18 +255,10 @@ instrumentation/development: enabled: false grpc: client: - request_headers: - # A comma-separated list of ASCII request header key patterns to exclude from client - # spans. Excluded patterns take precedence over included patterns. Matching is - # case-insensitive. `?` matches one character and `*` matches zero or more characters. If - # included is not configured, all non-excluded ASCII headers are captured, which may - # expose sensitive information. - excluded: [] - - # A comma-separated list of ASCII request header key patterns to capture on client spans. - # Matching is case-insensitive. `?` matches one character and `*` matches zero or more - # characters. - included: [] + # A comma-separated list of ASCII request header names to capture on client spans. Matching + # is case-insensitive and exact; wildcards are not supported. No headers are captured when + # unset or empty. + request_captured_headers: [] # Determines whether to emit a span event for each individual message received and sent. emit_message_events: true @@ -275,18 +267,10 @@ instrumentation/development: # `grpc.sent.message_count` and `grpc.canceled`. experimental_span_attributes/development: false server: - request_headers: - # A comma-separated list of ASCII request header key patterns to exclude from server - # spans. Excluded patterns take precedence over included patterns. Matching is - # case-insensitive. `?` matches one character and `*` matches zero or more characters. If - # included is not configured, all non-excluded ASCII headers are captured, which may - # expose sensitive information. - excluded: [] - - # A comma-separated list of ASCII request header key patterns to capture on server spans. - # Matching is case-insensitive. `?` matches one character and `*` matches zero or more - # characters. - included: [] + # A comma-separated list of ASCII request header names to capture on server spans. Matching + # is case-insensitive and exact; wildcards are not supported. No headers are captured when + # unset or empty. + request_captured_headers: [] guava: # Enables experimental span attribute `guava.canceled` for cancelled operations. experimental_span_attributes/development: false diff --git a/instrumentation/grpc-1.6/README.md b/instrumentation/grpc-1.6/README.md index c8ea13203e04..73c5a2fb4218 100644 --- a/instrumentation/grpc-1.6/README.md +++ b/instrumentation/grpc-1.6/README.md @@ -1,13 +1,11 @@ # Settings for the gRPC instrumentation -| System property | Type | Default | Description | -| ----------------------------------------------------------- | ------- | ------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| `otel.instrumentation.grpc.emit-message-events` | Boolean | `true` | Determines whether to emit span event for each individual message received and sent. | -| `otel.instrumentation.grpc.experimental-span-attributes` | Boolean | `false` | Enable the capture of experimental span attributes. | -| `otel.instrumentation.grpc.client.request-headers.included` | String | | A comma-separated list of ASCII request header key patterns to capture on client spans. Matching is case-insensitive. `?` matches one character and `*` matches zero or more characters. | -| `otel.instrumentation.grpc.client.request-headers.excluded` | String | | A comma-separated list of ASCII request header key patterns to exclude from client spans. Excluded patterns take precedence over included patterns. Matching is case-insensitive. `?` matches one character and `*` matches zero or more characters. If included is not configured, all non-excluded ASCII headers are captured, which may expose sensitive information. | -| `otel.instrumentation.grpc.server.request-headers.included` | String | | A comma-separated list of ASCII request header key patterns to capture on server spans. Matching is case-insensitive. `?` matches one character and `*` matches zero or more characters. | -| `otel.instrumentation.grpc.server.request-headers.excluded` | String | | A comma-separated list of ASCII request header key patterns to exclude from server spans. Excluded patterns take precedence over included patterns. Matching is case-insensitive. `?` matches one character and `*` matches zero or more characters. If included is not configured, all non-excluded ASCII headers are captured, which may expose sensitive information. | +| System property | Type | Default | Description | +| ----------------------------------------------------------- | ------- | ------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `otel.instrumentation.grpc.emit-message-events` | Boolean | `true` | Determines whether to emit span event for each individual message received and sent. | +| `otel.instrumentation.grpc.experimental-span-attributes` | Boolean | `false` | Enable the capture of experimental span attributes. | +| `otel.instrumentation.grpc.client.request-captured-headers` | String | | A comma-separated list of ASCII request header names to capture on client spans. Matching is case-insensitive and exact; wildcards are not supported. No headers are captured when unset or empty. | +| `otel.instrumentation.grpc.server.request-captured-headers` | String | | A comma-separated list of ASCII request header names to capture on server spans. Matching is case-insensitive and exact; wildcards are not supported. No headers are captured when unset or empty. | Captured headers use `rpc.request.header.`, with lowercase keys and string-array values. Binary metadata and HTTP/2 pseudo-headers are not captured. diff --git a/instrumentation/grpc-1.6/javaagent/build.gradle.kts b/instrumentation/grpc-1.6/javaagent/build.gradle.kts index 2ca586e6a049..ac90a9c5f387 100644 --- a/instrumentation/grpc-1.6/javaagent/build.gradle.kts +++ b/instrumentation/grpc-1.6/javaagent/build.gradle.kts @@ -40,10 +40,8 @@ testing { // The agent context debug mechanism isn't compatible with the bridge approach which may add a // gRPC context to the root. jvmArgs("-Dotel.javaagent.experimental.thread-propagation-debugger.enabled=false") - jvmArgs("-Dotel.instrumentation.grpc.client.request-headers.included=some-*-key") - jvmArgs("-Dotel.instrumentation.grpc.client.request-headers.excluded=*-excluded-key") - jvmArgs("-Dotel.instrumentation.grpc.server.request-headers.included=some-*-key") - jvmArgs("-Dotel.instrumentation.grpc.server.request-headers.excluded=*-excluded-key") + jvmArgs("-Dotel.instrumentation.grpc.client.request-captured-headers=SOME-CLIENT-KEY,*,some-*-key,some-?lient-key") + jvmArgs("-Dotel.instrumentation.grpc.server.request-captured-headers=SOME-SERVER-KEY,*,some-*-key,some-?erver-key") // latest dep test occasionally fails because network type is ipv6 instead of the expected ipv4 // and peer address is 0:0:0:0:0:0:0:1 instead of 127.0.0.1 jvmArgs("-Djava.net.preferIPv4Stack=true") diff --git a/instrumentation/grpc-1.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grpc/v1_6/GrpcSingletons.java b/instrumentation/grpc-1.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grpc/v1_6/GrpcSingletons.java index 33ca4fa5d7d6..b1e33de824a5 100644 --- a/instrumentation/grpc-1.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grpc/v1_6/GrpcSingletons.java +++ b/instrumentation/grpc-1.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grpc/v1_6/GrpcSingletons.java @@ -11,11 +11,9 @@ import io.opentelemetry.api.GlobalOpenTelemetry; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties; -import io.opentelemetry.instrumentation.api.config.IncludeExclude; import io.opentelemetry.instrumentation.api.incubator.config.internal.DeclarativeConfigUtil; import io.opentelemetry.instrumentation.api.util.VirtualField; import io.opentelemetry.instrumentation.grpc.v1_6.GrpcTelemetry; -import io.opentelemetry.instrumentation.grpc.v1_6.GrpcTelemetryBuilder; import io.opentelemetry.instrumentation.grpc.v1_6.internal.ContextStorageBridge; import io.opentelemetry.instrumentation.grpc.v1_6.internal.Experimental; import io.opentelemetry.instrumentation.grpc.v1_6.internal.GrpcConfig; @@ -46,20 +44,13 @@ public class GrpcSingletons { boolean experimentalSpanAttributes = config.getBoolean("experimental_span_attributes/development", false); - GrpcTelemetryBuilder telemetryBuilder = + telemetry = GrpcTelemetry.builder(openTelemetry) .setEmitMessageEvents(emitMessageEvents) - .setCaptureExperimentalSpanAttributes(experimentalSpanAttributes); - IncludeExclude clientRequestHeaders = grpcConfig.getClientRequestHeaders(); - if (clientRequestHeaders != null) { - telemetryBuilder.setClientRequestHeaders(clientRequestHeaders); - } - - IncludeExclude serverRequestHeaders = grpcConfig.getServerRequestHeaders(); - if (serverRequestHeaders != null) { - telemetryBuilder.setServerRequestHeaders(serverRequestHeaders); - } - telemetry = telemetryBuilder.build(); + .setCaptureExperimentalSpanAttributes(experimentalSpanAttributes) + .setClientRequestCapturedHeaders(grpcConfig.getClientRequestCapturedHeaders()) + .setServerRequestCapturedHeaders(grpcConfig.getServerRequestCapturedHeaders()) + .build(); } @Nullable diff --git a/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/CapturedGrpcRequestHeadersUtil.java b/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/CapturedGrpcRequestHeadersUtil.java deleted file mode 100644 index 11718f76ebd8..000000000000 --- a/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/CapturedGrpcRequestHeadersUtil.java +++ /dev/null @@ -1,65 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.instrumentation.grpc.v1_6; - -import static java.util.Collections.unmodifiableList; -import static java.util.Collections.unmodifiableMap; -import static java.util.stream.Collectors.toList; - -import io.opentelemetry.api.common.AttributeKey; -import io.opentelemetry.instrumentation.api.config.IncludeExclude; -import java.util.HashMap; -import java.util.List; -import java.util.Locale; -import java.util.Map; - -final class CapturedGrpcRequestHeadersUtil { - private static final String RPC_REQUEST_HEADER_KEY_ATTRIBUTE_PREFIX = "rpc.request.header."; - - static List lowercase(List names) { - return unmodifiableList(names.stream().map(s -> s.toLowerCase(Locale.ROOT)).collect(toList())); - } - - static IncludeExclude lowercase(IncludeExclude selector) { - return IncludeExclude.builder() - .setIncluded(lowercase(selector.getIncluded())) - .setExcluded(lowercase(selector.getExcluded())) - .build(); - } - - static Map>> createLiteralRequestHeaderAttributeKeys( - IncludeExclude selector) { - return createLiteralAttributeKeys(selector, RPC_REQUEST_HEADER_KEY_ATTRIBUTE_PREFIX); - } - - static AttributeKey> requestHeaderAttributeKey( - String metadataKey, - Map>> literalRequestHeaderAttributeKeys) { - return attributeKey( - metadataKey, literalRequestHeaderAttributeKeys, RPC_REQUEST_HEADER_KEY_ATTRIBUTE_PREFIX); - } - - private static Map>> createLiteralAttributeKeys( - IncludeExclude selector, String prefix) { - Map>> result = new HashMap<>(); - for (String pattern : selector.getIncluded()) { - if (pattern.indexOf('*') == -1 && pattern.indexOf('?') == -1) { - result.put(pattern, AttributeKey.stringArrayKey(prefix + pattern)); - } - } - return unmodifiableMap(result); - } - - private static AttributeKey> attributeKey( - String metadataKey, - Map>> literalRequestHeaderAttributeKeys, - String prefix) { - AttributeKey> attributeKey = literalRequestHeaderAttributeKeys.get(metadataKey); - return attributeKey != null ? attributeKey : AttributeKey.stringArrayKey(prefix + metadataKey); - } - - private CapturedGrpcRequestHeadersUtil() {} -} diff --git a/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcAttributesExtractor.java b/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcAttributesExtractor.java index 05b51acf07e9..ac57e66b9d89 100644 --- a/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcAttributesExtractor.java +++ b/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcAttributesExtractor.java @@ -5,18 +5,14 @@ package io.opentelemetry.instrumentation.grpc.v1_6; -import static io.opentelemetry.instrumentation.grpc.v1_6.CapturedGrpcRequestHeadersUtil.createLiteralRequestHeaderAttributeKeys; -import static io.opentelemetry.instrumentation.grpc.v1_6.CapturedGrpcRequestHeadersUtil.lowercase; -import static io.opentelemetry.instrumentation.grpc.v1_6.CapturedGrpcRequestHeadersUtil.requestHeaderAttributeKey; -import static java.util.Collections.emptyMap; - import io.grpc.Metadata; import io.grpc.Status; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.api.config.IncludeExclude; import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; +import java.util.Collection; +import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; @@ -28,24 +24,18 @@ final class GrpcAttributesExtractor implements AttributesExtractor RPC_STATUS_CODE = AttributeKey.stringKey("rpc.status_code"); private final GrpcRpcAttributesGetter getter; - @Nullable private final IncludeExclude requestHeaders; - - // Only literal included names are stored here. We intentionally do not cache wildcard or - // exclude-only matches, even in a bounded cache: peer-controlled names could churn its entries, - // making allocation behavior depend on prior traffic. Creating those attribute keys on demand - // provides a fixed per-capture cost with no peer-controlled retained state, while literal - // configured keys remain allocation-free. - private final Map>> literalRequestHeaderAttributeKeys; + private final Map>> requestHeaderAttributeKeys = + new HashMap<>(); - GrpcAttributesExtractor(GrpcRpcAttributesGetter getter, @Nullable IncludeExclude requestHeaders) { + GrpcAttributesExtractor( + GrpcRpcAttributesGetter getter, Collection requestCapturedHeaders) { this.getter = getter; - if (requestHeaders == null || requestHeaders.isEmpty()) { - this.requestHeaders = null; - literalRequestHeaderAttributeKeys = emptyMap(); - } else { - this.requestHeaders = lowercase(requestHeaders); - literalRequestHeaderAttributeKeys = - createLiteralRequestHeaderAttributeKeys(this.requestHeaders); + for (String name : requestCapturedHeaders) { + String key = name.toLowerCase(Locale.ROOT); + if (isAsciiMetadataKey(key)) { + requestHeaderAttributeKeys.put( + key, AttributeKey.stringArrayKey("rpc.request.header." + key)); + } } } @@ -64,33 +54,19 @@ public void onEnd( if (status != null) { attributes.put(RPC_STATUS_CODE, status.getCode().name()); } - if (requestHeaders == null || request.getMetadata() == null) { + if (request.getMetadata() == null) { return; } - for (String key : request.getMetadata().keys()) { - // binary metadata and HTTP/2 pseudo-headers are never captured, even when the selector - // matches, because reading them below with Metadata.Key.of() would throw - if (!isAsciiMetadataKey(key)) { - continue; - } - // gRPC lowercases metadata key names, but normalize defensively so that matching is - // case-insensitive as documented and an excluded key cannot be captured because a transport - // surfaced its name with different casing - String metadataKey = key.toLowerCase(Locale.ROOT); - if (!requestHeaders.matches(metadataKey)) { - continue; - } - List value = getter.metadataValue(request, metadataKey); + for (Map.Entry>> entry : + requestHeaderAttributeKeys.entrySet()) { + List value = getter.metadataValue(request, entry.getKey()); if (!value.isEmpty()) { - attributes.put( - requestHeaderAttributeKey(metadataKey, literalRequestHeaderAttributeKeys), value); + attributes.put(entry.getValue(), value); } } } - // Returns whether the key names ASCII metadata that Metadata.Key.of() accepts. Metadata received - // from a peer can contain HTTP/2 pseudo-headers and header names built from HTTP token characters - // that gRPC does not allow in a metadata key. + // Metadata.Key.of() rejects binary and pseudo-headers as well as invalid ASCII metadata names. private static boolean isAsciiMetadataKey(String key) { if (key.isEmpty() || key.endsWith(Metadata.BINARY_HEADER_SUFFIX)) { return false; diff --git a/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcTelemetryBuilder.java b/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcTelemetryBuilder.java index 5c07e1da0fc8..2e9c73ef7c08 100644 --- a/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcTelemetryBuilder.java +++ b/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcTelemetryBuilder.java @@ -8,12 +8,12 @@ import static io.opentelemetry.instrumentation.api.incubator.semconv.rpc.internal.RpcExceptionEventExtractors.setRpcClientExceptionEventExtractor; import static io.opentelemetry.instrumentation.api.incubator.semconv.rpc.internal.RpcExceptionEventExtractors.setRpcServerExceptionEventExtractor; import static java.util.Arrays.asList; +import static java.util.Collections.emptyList; import static java.util.Collections.unmodifiableList; import com.google.errorprone.annotations.CanIgnoreReturnValue; import io.grpc.Status; import io.opentelemetry.api.OpenTelemetry; -import io.opentelemetry.instrumentation.api.config.IncludeExclude; import io.opentelemetry.instrumentation.api.incubator.semconv.rpc.RpcClientAttributesExtractor; import io.opentelemetry.instrumentation.api.incubator.semconv.rpc.RpcClientMetrics; import io.opentelemetry.instrumentation.api.incubator.semconv.rpc.RpcServerAttributesExtractor; @@ -30,9 +30,9 @@ import io.opentelemetry.instrumentation.api.semconv.network.ServerAttributesExtractor; import io.opentelemetry.instrumentation.grpc.v1_6.internal.GrpcClientNetworkAttributesGetter; import java.util.ArrayList; +import java.util.Collection; import java.util.List; import java.util.function.UnaryOperator; -import javax.annotation.Nullable; /** A builder of {@link GrpcTelemetry}. */ public final class GrpcTelemetryBuilder { @@ -61,8 +61,8 @@ public final class GrpcTelemetryBuilder { private boolean captureExperimentalSpanAttributes; private boolean emitMessageEvents = true; - @Nullable private IncludeExclude clientRequestHeaders; - @Nullable private IncludeExclude serverRequestHeaders; + private List clientRequestCapturedHeaders = emptyList(); + private List serverRequestCapturedHeaders = emptyList(); GrpcTelemetryBuilder(OpenTelemetry openTelemetry) { this.openTelemetry = openTelemetry; @@ -150,30 +150,30 @@ public GrpcTelemetryBuilder setCaptureExperimentalSpanAttributes( /** * Sets which ASCII request headers should be captured as span attributes on client spans. * - *

Header keys and selector patterns are matched case-insensitively. {@code ?} matches any - * single character and {@code *} matches any number of characters, including none. Excluded - * patterns take precedence over included patterns. No headers are captured unless a selector is - * configured; a selector with only excluded patterns captures every ASCII header key that it does - * not exclude. + *

Header names are matched exactly, ignoring case. Wildcards are not supported. By default, no + * headers are captured. An empty collection disables capture. Binary metadata and HTTP/2 + * pseudo-headers are never captured. */ @CanIgnoreReturnValue - public GrpcTelemetryBuilder setClientRequestHeaders(IncludeExclude clientRequestHeaders) { - this.clientRequestHeaders = clientRequestHeaders; + public GrpcTelemetryBuilder setClientRequestCapturedHeaders( + Collection clientRequestCapturedHeaders) { + this.clientRequestCapturedHeaders = + unmodifiableList(new ArrayList<>(clientRequestCapturedHeaders)); return this; } /** * Sets which ASCII request headers should be captured as span attributes on server spans. * - *

Header keys and selector patterns are matched case-insensitively. {@code ?} matches any - * single character and {@code *} matches any number of characters, including none. Excluded - * patterns take precedence over included patterns. No headers are captured unless a selector is - * configured; a selector with only excluded patterns captures every ASCII header key that it does - * not exclude. + *

Header names are matched exactly, ignoring case. Wildcards are not supported. By default, no + * headers are captured. An empty collection disables capture. Binary metadata and HTTP/2 + * pseudo-headers are never captured. */ @CanIgnoreReturnValue - public GrpcTelemetryBuilder setServerRequestHeaders(IncludeExclude serverRequestHeaders) { - this.serverRequestHeaders = serverRequestHeaders; + public GrpcTelemetryBuilder setServerRequestCapturedHeaders( + Collection serverRequestCapturedHeaders) { + this.serverRequestCapturedHeaders = + unmodifiableList(new ArrayList<>(serverRequestCapturedHeaders)); return this; } @@ -206,7 +206,7 @@ public GrpcTelemetry build() { .addAttributesExtractor(NetworkAttributesExtractor.create(netClientAttributesGetter)) .addAttributesExtractors(additionalClientExtractors) .addAttributesExtractor( - new GrpcAttributesExtractor(clientRpcAttributesGetter, clientRequestHeaders)) + new GrpcAttributesExtractor(clientRpcAttributesGetter, clientRequestCapturedHeaders)) .addOperationMetrics(RpcClientMetrics.get(GRPC_DURATION_SECONDS_BUCKETS)); setRpcClientExceptionEventExtractor(clientInstrumenterBuilder); Experimental.addOperationListenerAttributesExtractor( @@ -219,7 +219,7 @@ public GrpcTelemetry build() { .addAttributesExtractor(ServerAttributesExtractor.create(netServerAttributesGetter)) .addAttributesExtractor(NetworkAttributesExtractor.create(netServerAttributesGetter)) .addAttributesExtractor( - new GrpcAttributesExtractor(serverRpcAttributesGetter, serverRequestHeaders)) + new GrpcAttributesExtractor(serverRpcAttributesGetter, serverRequestCapturedHeaders)) .addAttributesExtractors(additionalServerExtractors) .addOperationMetrics(RpcServerMetrics.get(GRPC_DURATION_SECONDS_BUCKETS)); setRpcServerExceptionEventExtractor(serverInstrumenterBuilder); diff --git a/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/internal/GrpcConfig.java b/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/internal/GrpcConfig.java index 464439f17793..5c8b6a96feb4 100644 --- a/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/internal/GrpcConfig.java +++ b/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/internal/GrpcConfig.java @@ -9,10 +9,8 @@ import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties; -import io.opentelemetry.instrumentation.api.config.IncludeExclude; import io.opentelemetry.instrumentation.api.incubator.config.internal.DeclarativeConfigUtil; import java.util.List; -import javax.annotation.Nullable; /** * This class is internal and is hence not for public use. Its APIs are unstable and can change at @@ -20,43 +18,25 @@ */ public class GrpcConfig { - @Nullable private final IncludeExclude clientRequestHeaders; - @Nullable private final IncludeExclude serverRequestHeaders; + private final List clientRequestCapturedHeaders; + private final List serverRequestCapturedHeaders; public static GrpcConfig create(OpenTelemetry openTelemetry) { return new GrpcConfig(DeclarativeConfigUtil.getInstrumentationConfig(openTelemetry, "grpc")); } GrpcConfig(DeclarativeConfigProperties config) { - clientRequestHeaders = getRequestHeaders(config, "client"); - serverRequestHeaders = getRequestHeaders(config, "server"); + clientRequestCapturedHeaders = + config.get("client").getScalarList("request_captured_headers", String.class, emptyList()); + serverRequestCapturedHeaders = + config.get("server").getScalarList("request_captured_headers", String.class, emptyList()); } - @Nullable - public IncludeExclude getClientRequestHeaders() { - return clientRequestHeaders; + public List getClientRequestCapturedHeaders() { + return clientRequestCapturedHeaders; } - @Nullable - public IncludeExclude getServerRequestHeaders() { - return serverRequestHeaders; - } - - @Nullable - private static IncludeExclude getRequestHeaders(DeclarativeConfigProperties config, String side) { - DeclarativeConfigProperties requestHeaders = config.get(side).get("request_headers"); - List included = requestHeaders.getScalarList("included", String.class); - List excluded = requestHeaders.getScalarList("excluded", String.class); - IncludeExclude selector = - IncludeExclude.builder() - .setIncluded(included == null ? emptyList() : included) - .setExcluded(excluded == null ? emptyList() : excluded) - .build(); - // an empty selector is equivalent to no selector at all, matching flat configuration where - // empty property values cannot be distinguished from unset ones - if (!selector.isEmpty()) { - return selector; - } - return null; + public List getServerRequestCapturedHeaders() { + return serverRequestCapturedHeaders; } } diff --git a/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/CapturedGrpcRequestHeadersUtilTest.java b/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/CapturedGrpcRequestHeadersUtilTest.java deleted file mode 100644 index e756e4c88c71..000000000000 --- a/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/CapturedGrpcRequestHeadersUtilTest.java +++ /dev/null @@ -1,39 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.instrumentation.grpc.v1_6; - -import static io.opentelemetry.instrumentation.grpc.v1_6.CapturedGrpcRequestHeadersUtil.createLiteralRequestHeaderAttributeKeys; -import static io.opentelemetry.instrumentation.grpc.v1_6.CapturedGrpcRequestHeadersUtil.requestHeaderAttributeKey; -import static java.util.Arrays.asList; -import static org.assertj.core.api.Assertions.assertThat; - -import io.opentelemetry.api.common.AttributeKey; -import io.opentelemetry.instrumentation.api.config.IncludeExclude; -import java.util.List; -import java.util.Map; -import org.junit.jupiter.api.Test; - -class CapturedGrpcRequestHeadersUtilTest { - - @Test - void retainsOnlyLiteralRequestHeaderAttributeKeys() { - IncludeExclude selector = - IncludeExclude.builder() - .setIncluded(asList("literal-key", "wildcard-*", "single-?")) - .build(); - - Map>> literalRequestHeaderAttributeKeys = - createLiteralRequestHeaderAttributeKeys(selector); - - assertThat(literalRequestHeaderAttributeKeys).containsOnlyKeys("literal-key"); - assertThat(requestHeaderAttributeKey("literal-key", literalRequestHeaderAttributeKeys)) - .isSameAs(literalRequestHeaderAttributeKeys.get("literal-key")); - assertThat(requestHeaderAttributeKey("wildcard-key", literalRequestHeaderAttributeKeys)) - .isNotSameAs(requestHeaderAttributeKey("wildcard-key", literalRequestHeaderAttributeKeys)); - assertThat(requestHeaderAttributeKey("literal-key", literalRequestHeaderAttributeKeys).getKey()) - .isEqualTo("rpc.request.header.literal-key"); - } -} diff --git a/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcAttributesExtractorTest.java b/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcAttributesExtractorTest.java index cc6eccefccaa..d0c85778dee2 100644 --- a/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcAttributesExtractorTest.java +++ b/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcAttributesExtractorTest.java @@ -8,21 +8,18 @@ import static io.opentelemetry.api.common.AttributeKey.stringArrayKey; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Arrays.asList; -import static java.util.Collections.singleton; +import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.verifyNoInteractions; import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.api.config.IncludeExclude; -import java.util.HashSet; -import javax.annotation.Nullable; +import java.util.Collection; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -30,8 +27,8 @@ class GrpcAttributesExtractorTest { @ParameterizedTest - @ValueSource(strings = {"INCLUDED-KEY", "INCLUDED-*", "INCLUDED-?EY"}) - void capturesSelectedRequestHeaders(String includedPattern) { + @ValueSource(strings = {"included-key", "INCLUDED-KEY", "Included-Key"}) + void capturesExactRequestHeadersIgnoringCase(String includedName) { Metadata metadata = new Metadata(); metadata.put( Metadata.Key.of("included-key", Metadata.ASCII_STRING_MARSHALLER), "included-value"); @@ -39,19 +36,17 @@ void capturesSelectedRequestHeaders(String includedPattern) { metadata.put( Metadata.Key.of("included-excluded", Metadata.ASCII_STRING_MARSHALLER), "excluded-value"); metadata.put(Metadata.Key.of("other-key", Metadata.ASCII_STRING_MARSHALLER), "other-value"); + metadata.put(Metadata.Key.of("authorization", Metadata.ASCII_STRING_MARSHALLER), "******"); + metadata.put(Metadata.Key.of("cookie", Metadata.ASCII_STRING_MARSHALLER), "session=secret"); metadata.put( Metadata.Key.of("included-bin", Metadata.BINARY_BYTE_MARSHALLER), "binary-value".getBytes(UTF_8)); GrpcRequest request = GrpcRequest.createServerRequest(mock(MethodDescriptor.class), metadata, null, null); - IncludeExclude selector = - IncludeExclude.builder() - .setIncluded(asList(includedPattern, "missing-key")) - .setExcluded(singletonList("*-EXCLUDED")) - .build(); AttributesBuilder attributes = Attributes.builder(); - createExtractor(selector).onEnd(attributes, Context.root(), request, null, null); + createExtractor(asList(includedName, "missing-key")) + .onEnd(attributes, Context.root(), request, null, null); Attributes result = attributes.build(); assertThat(result) @@ -62,76 +57,71 @@ void capturesSelectedRequestHeaders(String includedPattern) { } @Test - void absentSelectorCapturesNothing() { + void emptyListCapturesNothing() { Metadata metadata = new Metadata(); metadata.put(Metadata.Key.of("some-key", Metadata.ASCII_STRING_MARSHALLER), "some-value"); GrpcRequest request = GrpcRequest.createServerRequest(mock(MethodDescriptor.class), metadata, null, null); AttributesBuilder attributes = Attributes.builder(); - createExtractor(null).onEnd(attributes, Context.root(), request, null, null); + createExtractor(emptyList()).onEnd(attributes, Context.root(), request, null, null); assertThat(attributes.build()).isEqualTo(Attributes.empty()); } - @Test - void emptySelectorCapturesNothing() { + @ParameterizedTest + @ValueSource(strings = {"*", "?", "included-*", "included-?ey"}) + void doesNotExpandWildcardsOrCaptureUnlistedSensitiveHeaders(String name) { Metadata metadata = new Metadata(); - metadata.put(Metadata.Key.of("some-key", Metadata.ASCII_STRING_MARSHALLER), "some-value"); + metadata.put( + Metadata.Key.of("included-key", Metadata.ASCII_STRING_MARSHALLER), "included-value"); + metadata.put(Metadata.Key.of("authorization", Metadata.ASCII_STRING_MARSHALLER), "******"); + metadata.put(Metadata.Key.of("cookie", Metadata.ASCII_STRING_MARSHALLER), "session=secret"); GrpcRequest request = GrpcRequest.createServerRequest(mock(MethodDescriptor.class), metadata, null, null); AttributesBuilder attributes = Attributes.builder(); - createExtractor(IncludeExclude.builder().build()) - .onEnd(attributes, Context.root(), request, null, null); + createExtractor(singletonList(name)).onEnd(attributes, Context.root(), request, null, null); assertThat(attributes.build()).isEqualTo(Attributes.empty()); } - @Test - void matchesRequestHeadersCaseInsensitively() { - Metadata delegate = new Metadata(); - delegate.put( - Metadata.Key.of("included-key", Metadata.ASCII_STRING_MARSHALLER), "included-value"); - delegate.put( - Metadata.Key.of("excluded-key", Metadata.ASCII_STRING_MARSHALLER), "excluded-value"); - // a transport could surface metadata key names with casing that Metadata.Key would normalize - Metadata metadata = spy(delegate); - when(metadata.keys()).thenReturn(new HashSet<>(asList("Included-Key", "Excluded-Key"))); + @ParameterizedTest + @ValueSource( + strings = { + "", + ":authority", + "included-bin", + "INCLUDED-BIN", + "x-foo!bar", + "x_foo+bar", + "x-foo~bar" + }) + void skipsBinaryPseudoAndInvalidHeaders(String key) { + Metadata metadata = mock(Metadata.class); GrpcRequest request = GrpcRequest.createServerRequest(mock(MethodDescriptor.class), metadata, null, null); - IncludeExclude selector = - IncludeExclude.builder() - .setIncluded(singletonList("*-key")) - .setExcluded(singletonList("excluded-key")) - .build(); AttributesBuilder attributes = Attributes.builder(); - createExtractor(selector).onEnd(attributes, Context.root(), request, null, null); + createExtractor(singletonList(key)).onEnd(attributes, Context.root(), request, null, null); - assertThat(attributes.build()) - .isEqualTo( - Attributes.of( - stringArrayKey("rpc.request.header.included-key"), - singletonList("included-value"))); + assertThat(attributes.build()).isEqualTo(Attributes.empty()); + verifyNoInteractions(metadata); } - @ParameterizedTest - @ValueSource(strings = {":authority", "x-foo!bar", "x_foo+bar", "x-foo~bar"}) - void skipsKeysThatGrpcMetadataCannotRepresent(String key) { - Metadata metadata = mock(Metadata.class); - when(metadata.keys()).thenReturn(singleton(key)); + @Test + void missingMetadataCapturesNothing() { GrpcRequest request = - GrpcRequest.createServerRequest(mock(MethodDescriptor.class), metadata, null, null); + GrpcRequest.createServerRequest(mock(MethodDescriptor.class), null, null, null); AttributesBuilder attributes = Attributes.builder(); - IncludeExclude selector = IncludeExclude.builder().setIncluded(singleton("*")).build(); - createExtractor(selector).onEnd(attributes, Context.root(), request, null, null); + createExtractor(singletonList("included-key")) + .onEnd(attributes, Context.root(), request, null, null); assertThat(attributes.build()).isEqualTo(Attributes.empty()); } - private static GrpcAttributesExtractor createExtractor(@Nullable IncludeExclude selector) { - return new GrpcAttributesExtractor(new GrpcRpcAttributesGetter(false), selector); + private static GrpcAttributesExtractor createExtractor(Collection capturedHeaders) { + return new GrpcAttributesExtractor(new GrpcRpcAttributesGetter(false), capturedHeaders); } } diff --git a/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcTest.java b/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcTest.java index 600dc5550f63..adfaa91e1031 100644 --- a/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcTest.java +++ b/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcTest.java @@ -9,6 +9,8 @@ import static io.opentelemetry.instrumentation.testing.util.TestLatestDeps.testLatestDeps; import static io.opentelemetry.semconv.ServerAttributes.SERVER_ADDRESS; import static io.opentelemetry.semconv.ServerAttributes.SERVER_PORT; +import static java.util.Arrays.asList; +import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static java.util.concurrent.TimeUnit.SECONDS; import static org.assertj.core.api.Assertions.assertThat; @@ -39,7 +41,6 @@ import io.opentelemetry.api.trace.SpanKind; import io.opentelemetry.context.Context; import io.opentelemetry.context.propagation.ContextPropagators; -import io.opentelemetry.instrumentation.api.config.IncludeExclude; import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; import io.opentelemetry.instrumentation.grpc.v1_6.internal.Experimental; @@ -63,10 +64,8 @@ class GrpcTest extends AbstractGrpcTest { protected ServerBuilder configureServer(ServerBuilder server) { GrpcTelemetry telemetry = GrpcTelemetry.builder(testing.getOpenTelemetry()) - .setServerRequestHeaders( - IncludeExclude.builder() - .setIncluded(singletonList(SERVER_REQUEST_HEADER_KEY)) - .build()) + .setServerRequestCapturedHeaders( + asList("SOME-SERVER-KEY", "*", "some-*-key", "some-?erver-key")) .build(); telemetry.configureServerBuilder(server); // A server can be configured more than once. @@ -78,10 +77,8 @@ protected ServerBuilder configureServer(ServerBuilder server) { protected ManagedChannelBuilder configureClient(ManagedChannelBuilder client) { Experimental.addClientInterceptor( GrpcTelemetry.builder(testing.getOpenTelemetry()) - .setClientRequestHeaders( - IncludeExclude.builder() - .setIncluded(singletonList(CLIENT_REQUEST_HEADER_KEY)) - .build()) + .setClientRequestCapturedHeaders( + asList("SOME-CLIENT-KEY", "*", "some-*-key", "some-?lient-key")) .build(), client); return client; @@ -97,6 +94,67 @@ protected InstrumentationExtension testing() { return testing; } + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void defaultAndEmptyCapturedHeadersCaptureNothing(boolean explicitlyEmpty) throws Exception { + GrpcTelemetryBuilder builder = GrpcTelemetry.builder(testing.getOpenTelemetry()); + if (explicitlyEmpty) { + builder + .setClientRequestCapturedHeaders(singletonList("authorization")) + .setServerRequestCapturedHeaders(singletonList("authorization")) + .setClientRequestCapturedHeaders(emptyList()) + .setServerRequestCapturedHeaders(emptyList()); + } + GrpcTelemetry telemetry = builder.build(); + ServerBuilder serverBuilder = + ServerBuilder.forPort(0) + .addService( + new GreeterGrpc.GreeterImplBase() { + @Override + public void sayHello( + Helloworld.Request request, + StreamObserver responseObserver) { + responseObserver.onNext(Helloworld.Response.getDefaultInstance()); + responseObserver.onCompleted(); + } + }); + telemetry.configureServerBuilder(serverBuilder); + Server server = serverBuilder.build().start(); + closer.add(() -> server.shutdownNow().awaitTermination()); + + ManagedChannelBuilder channelBuilder = + ManagedChannelBuilder.forAddress("localhost", server.getPort()); + Experimental.addClientInterceptor(telemetry, channelBuilder); + ManagedChannel channel = createChannel(channelBuilder); + closer.add(() -> channel.shutdownNow().awaitTermination(10, SECONDS)); + + Metadata headers = new Metadata(); + headers.put(Metadata.Key.of("authorization", Metadata.ASCII_STRING_MARSHALLER), "******"); + headers.put(Metadata.Key.of("some-key", Metadata.ASCII_STRING_MARSHALLER), "value"); + GreeterGrpc.newBlockingStub(channel) + .withInterceptors(MetadataUtils.newAttachHeadersInterceptor(headers)) + .sayHello(Helloworld.Request.getDefaultInstance()); + + testing.waitAndAssertTraces( + trace -> + trace.hasSpansSatisfyingExactly( + span -> + span.hasKind(SpanKind.CLIENT) + .satisfies( + spanData -> + assertThat(spanData.getAttributes().asMap().keySet()) + .noneMatch( + key -> key.getKey().startsWith("rpc.request.header."))), + span -> + span.hasKind(SpanKind.SERVER) + .hasParent(trace.getSpan(0)) + .satisfies( + spanData -> + assertThat(spanData.getAttributes().asMap().keySet()) + .noneMatch( + key -> key.getKey().startsWith("rpc.request.header."))))); + } + @Test void registeredMethodShortCircuitedBeforeTracingInterceptor() throws Exception { ServerBuilder serverBuilder = diff --git a/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/internal/GrpcConfigTest.java b/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/internal/GrpcConfigTest.java index d72cc13198ba..8a35691941ed 100644 --- a/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/internal/GrpcConfigTest.java +++ b/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/internal/GrpcConfigTest.java @@ -13,55 +13,61 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties; -import io.opentelemetry.instrumentation.api.config.IncludeExclude; import org.junit.jupiter.api.Test; class GrpcConfigTest { @Test - void readsRequestHeaderSelectors() { + void readsRequestCapturedHeaderLists() { DeclarativeConfigProperties config = mockConfig(); - when(config.get("client").get("request_headers").getScalarList("included", String.class)) - .thenReturn(asList("client-*", "other")); - when(config.get("client").get("request_headers").getScalarList("excluded", String.class)) - .thenReturn(singletonList("*-secret")); - when(config.get("server").get("request_headers").getScalarList("excluded", String.class)) - .thenReturn(singletonList("server-secret")); + when(config.get("client").getScalarList("request_captured_headers", String.class, emptyList())) + .thenReturn(asList("client-key", "OTHER")); + when(config.get("server").getScalarList("request_captured_headers", String.class, emptyList())) + .thenReturn(singletonList("server-key")); GrpcConfig grpcConfig = new GrpcConfig(config); - IncludeExclude client = grpcConfig.getClientRequestHeaders(); - assertThat(client).isNotNull(); - assertThat(client.getIncluded()).containsExactly("client-*", "other"); - assertThat(client.getExcluded()).containsExactly("*-secret"); - IncludeExclude server = grpcConfig.getServerRequestHeaders(); - assertThat(server).isNotNull(); - assertThat(server.getIncluded()).isEmpty(); - assertThat(server.getExcluded()).containsExactly("server-secret"); + assertThat(grpcConfig.getClientRequestCapturedHeaders()).containsExactly("client-key", "OTHER"); + assertThat(grpcConfig.getServerRequestCapturedHeaders()).containsExactly("server-key"); } @Test - void emptySelectorCapturesNothing() { + void defaultsToNoCapturedHeaders() { DeclarativeConfigProperties config = mockConfig(); - when(config.get("client").get("request_headers").getScalarList("included", String.class)) - .thenReturn(emptyList()); - when(config.get("client").get("request_headers").getScalarList("excluded", String.class)) - .thenReturn(emptyList()); GrpcConfig grpcConfig = new GrpcConfig(config); - assertThat(grpcConfig.getClientRequestHeaders()).isNull(); + assertThat(grpcConfig.getClientRequestCapturedHeaders()).isEmpty(); + assertThat(grpcConfig.getServerRequestCapturedHeaders()).isEmpty(); + } + + @Test + void unavailableDeclarativeConfigurationCapturesNothing() { + GrpcConfig grpcConfig = GrpcConfig.create(OpenTelemetry.noop()); + + assertThat(grpcConfig.getClientRequestCapturedHeaders()).isEmpty(); + assertThat(grpcConfig.getServerRequestCapturedHeaders()).isEmpty(); + } + + @Test + void preservesLiteralWildcardNames() { + DeclarativeConfigProperties config = mockConfig(); + when(config.get("client").getScalarList("request_captured_headers", String.class, emptyList())) + .thenReturn(asList("*", "some-?ey")); + + GrpcConfig grpcConfig = new GrpcConfig(config); + + assertThat(grpcConfig.getClientRequestCapturedHeaders()).containsExactly("*", "some-?ey"); } private static DeclarativeConfigProperties mockConfig() { DeclarativeConfigProperties config = mock(DeclarativeConfigProperties.class, RETURNS_DEEP_STUBS); for (String side : asList("client", "server")) { - when(config.get(side).get("request_headers").getScalarList("included", String.class)) - .thenReturn(null); - when(config.get(side).get("request_headers").getScalarList("excluded", String.class)) - .thenReturn(null); + when(config.get(side).getScalarList("request_captured_headers", String.class, emptyList())) + .thenReturn(emptyList()); } return config; } diff --git a/instrumentation/grpc-1.6/metadata.yaml b/instrumentation/grpc-1.6/metadata.yaml index daffe742ed93..f7ff1e391745 100644 --- a/instrumentation/grpc-1.6/metadata.yaml +++ b/instrumentation/grpc-1.6/metadata.yaml @@ -19,47 +19,19 @@ configurations: Enable the capture of experimental span attributes `grpc.received.message_count`, `grpc.sent.message_count` and `grpc.canceled`. default: false - - name: otel.instrumentation.grpc.client.request-headers.included - declarative_name: java.grpc.client.request_headers.included + - name: otel.instrumentation.grpc.client.request-captured-headers + declarative_name: java.grpc.client.request_captured_headers type: list description: > - A comma-separated list of ASCII request header key patterns to capture on client spans. - Matching is case-insensitive. `?` matches one character and `*` matches zero or more - characters. + A comma-separated list of ASCII request header names to capture on client spans. + Matching is case-insensitive and exact; wildcards are not supported. + No headers are captured when unset or empty. default: '' - examples: - - "custom-request-header" - - "my-*-key,another-header-key" - - name: otel.instrumentation.grpc.client.request-headers.excluded - declarative_name: java.grpc.client.request_headers.excluded + - name: otel.instrumentation.grpc.server.request-captured-headers + declarative_name: java.grpc.server.request_captured_headers type: list description: > - A comma-separated list of ASCII request header key patterns to exclude from client spans. - Excluded patterns take precedence over included patterns. Matching is case-insensitive. `?` matches one character and - `*` matches zero or more characters. If included is not configured, all non-excluded ASCII - headers are captured, which may expose sensitive information. + A comma-separated list of ASCII request header names to capture on server spans. + Matching is case-insensitive and exact; wildcards are not supported. + No headers are captured when unset or empty. default: '' - examples: - - "authorization,*-token" - - name: otel.instrumentation.grpc.server.request-headers.included - declarative_name: java.grpc.server.request_headers.included - type: list - description: > - A comma-separated list of ASCII request header key patterns to capture on server spans. - Matching is case-insensitive. `?` matches one character and `*` matches zero or more - characters. - default: '' - examples: - - "custom-request-header" - - "my-*-key,another-header-key" - - name: otel.instrumentation.grpc.server.request-headers.excluded - declarative_name: java.grpc.server.request_headers.excluded - type: list - description: > - A comma-separated list of ASCII request header key patterns to exclude from server spans. - Excluded patterns take precedence over included patterns. Matching is case-insensitive. `?` matches one character and - `*` matches zero or more characters. If included is not configured, all non-excluded ASCII - headers are captured, which may expose sensitive information. - default: '' - examples: - - "authorization,*-token" diff --git a/instrumentation/grpc-1.6/testing/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/AbstractGrpcTest.java b/instrumentation/grpc-1.6/testing/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/AbstractGrpcTest.java index 92757c66dd73..ed30d1071f0e 100644 --- a/instrumentation/grpc-1.6/testing/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/AbstractGrpcTest.java +++ b/instrumentation/grpc-1.6/testing/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/AbstractGrpcTest.java @@ -28,7 +28,6 @@ import static io.opentelemetry.semconv.incubating.RpcIncubatingAttributes.RPC_METHOD_ORIGINAL; import static io.opentelemetry.semconv.incubating.RpcIncubatingAttributes.RPC_SYSTEM_NAME; import static java.util.Arrays.asList; -import static java.util.Collections.singletonList; import static java.util.concurrent.TimeUnit.SECONDS; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -102,7 +101,7 @@ public abstract class AbstractGrpcTest { protected static final String SERVER_REQUEST_HEADER_KEY = "some-server-key"; - private static final String EXCLUDED_REQUEST_HEADER_KEY = "some-excluded-key"; + private static final String UNLISTED_REQUEST_HEADER_KEY = "some-unlisted-key"; protected abstract ServerBuilder configureServer(ServerBuilder server); @@ -1592,12 +1591,21 @@ public void sayHello( extraMetadata.put( Metadata.Key.of(SERVER_REQUEST_HEADER_KEY, Metadata.ASCII_STRING_MARSHALLER), serverHeaderValue); + extraMetadata.put( + Metadata.Key.of(SERVER_REQUEST_HEADER_KEY, Metadata.ASCII_STRING_MARSHALLER), + "second-server-value"); extraMetadata.put( Metadata.Key.of(CLIENT_REQUEST_HEADER_KEY, Metadata.ASCII_STRING_MARSHALLER), clientHeaderValue); extraMetadata.put( - Metadata.Key.of(EXCLUDED_REQUEST_HEADER_KEY, Metadata.ASCII_STRING_MARSHALLER), - "excluded-value"); + Metadata.Key.of(CLIENT_REQUEST_HEADER_KEY, Metadata.ASCII_STRING_MARSHALLER), + "second-client-value"); + extraMetadata.put( + Metadata.Key.of(UNLISTED_REQUEST_HEADER_KEY, Metadata.ASCII_STRING_MARSHALLER), + "unlisted-value"); + extraMetadata.put(Metadata.Key.of("authorization", Metadata.ASCII_STRING_MARSHALLER), "******"); + extraMetadata.put( + Metadata.Key.of("cookie", Metadata.ASCII_STRING_MARSHALLER), "session=secret"); GreeterGrpc.GreeterBlockingStub client = GreeterGrpc.newBlockingStub(channel) @@ -1624,11 +1632,17 @@ public void sayHello( equalTo( stringArrayKey( "rpc.request.header." + CLIENT_REQUEST_HEADER_KEY), - singletonList(clientHeaderValue)), + asList(clientHeaderValue, "second-client-value")), equalTo( stringArrayKey( - "rpc.request.header." + EXCLUDED_REQUEST_HEADER_KEY), - null)), + "rpc.request.header." + UNLISTED_REQUEST_HEADER_KEY), + null), + equalTo( + stringArrayKey( + "rpc.request.header." + SERVER_REQUEST_HEADER_KEY), + null), + equalTo(stringArrayKey("rpc.request.header.authorization"), null), + equalTo(stringArrayKey("rpc.request.header.cookie"), null)), span -> span.hasName("example.Greeter/SayHello") .hasKind(SpanKind.SERVER) @@ -1637,11 +1651,17 @@ public void sayHello( equalTo( stringArrayKey( "rpc.request.header." + SERVER_REQUEST_HEADER_KEY), - singletonList(serverHeaderValue)), + asList(serverHeaderValue, "second-server-value")), equalTo( stringArrayKey( - "rpc.request.header." + EXCLUDED_REQUEST_HEADER_KEY), - null)))); + "rpc.request.header." + UNLISTED_REQUEST_HEADER_KEY), + null), + equalTo( + stringArrayKey( + "rpc.request.header." + CLIENT_REQUEST_HEADER_KEY), + null), + equalTo(stringArrayKey("rpc.request.header.authorization"), null), + equalTo(stringArrayKey("rpc.request.header.cookie"), null)))); } private ManagedChannel createChannel(Server server) throws Exception {