diff --git a/CHANGELOG.md b/CHANGELOG.md index afd9647fb22d..43bcb0448e62 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -293,6 +293,9 @@ ### ⚠️ Breaking changes to non-stable APIs +- In `opentelemetry-instrumentation-api-incubator`, custom + `MessagingAttributesGetter` implementations must implement `getMessageHeaderNames`, returning + supported header names or an empty iterable for carriers without supported headers. - Remove deprecated `ProcessResource.get()`. Replace it with `ProcessResource.create()` to omit command-line attributes, or `ProcessResource.create(true)` to retain them. - The `TracingSqsMessageHandler` customization constructor now accepts an diff --git a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/messaging/MessagingAttributesGetter.java b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/messaging/MessagingAttributesGetter.java index f24453cf9c07..9100a4f00170 100644 --- a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/messaging/MessagingAttributesGetter.java +++ b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/messaging/MessagingAttributesGetter.java @@ -91,16 +91,12 @@ default List getMessageHeader(REQUEST request, String name) { *

This is used to resolve header selectors that cannot be turned into a list of exact header * names, such as selectors containing wildcard patterns or selectors that only exclude headers. * Selectors that only list exact header names are resolved with {@link #getMessageHeader(Object, - * String)} alone. To preserve compatibility with existing implementations, overriding this method - * is optional until 3.0. + * String)} alone. * *

Implementations of this method must not return a null value; an empty iterable should * be returned instead. The returned iterable is only read, so implementations may return a view * over the underlying header names as long as that view cannot change while the returned iterable * is being read. */ - // TODO: remove the default implementation and make this required to implement in 3.0 - default Iterable getMessageHeaderNames(REQUEST request) { - return emptyList(); - } + Iterable getMessageHeaderNames(REQUEST request); } diff --git a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/messaging/MessagingAttributesExtractorTest.java b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/messaging/MessagingAttributesExtractorTest.java index 5a19e729d6de..98a23bcd1802 100644 --- a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/messaging/MessagingAttributesExtractorTest.java +++ b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/messaging/MessagingAttributesExtractorTest.java @@ -18,6 +18,7 @@ import static io.opentelemetry.semconv.incubating.MessagingIncubatingAttributes.MESSAGING_OPERATION_NAME; import static io.opentelemetry.semconv.incubating.MessagingIncubatingAttributes.MESSAGING_OPERATION_TYPE; import static io.opentelemetry.semconv.incubating.MessagingIncubatingAttributes.MESSAGING_SYSTEM; +import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.Assertions.entry; @@ -290,5 +291,10 @@ public Long getBatchMessageCount(Map request, @Nullable String r public String getErrorType(Map request, String response, Throwable error) { return "failure".equals(response) ? response : null; } + + @Override + public Iterable getMessageHeaderNames(Map request) { + return emptyList(); + } } } diff --git a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/messaging/MessagingHeadersTest.java b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/messaging/MessagingHeadersTest.java index 4b60c7924aaa..a8fe881eb9fa 100644 --- a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/messaging/MessagingHeadersTest.java +++ b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/messaging/MessagingHeadersTest.java @@ -8,6 +8,7 @@ import static io.opentelemetry.api.common.AttributeKey.stringArrayKey; import static java.util.Arrays.asList; import static java.util.Collections.emptyList; +import static java.util.Collections.emptyMap; import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; @@ -94,8 +95,7 @@ void matchingHeaderNameIsCapturedOnlyOnce() { @Test void exactNamesAreCapturedWithoutHeaderNameEnumeration() { - // simulates a third party getter that implements only getMessageHeader - MapGetter getter = new MapGetter(false); + MapGetter getter = new MapGetter(true); AttributesBuilder attributes = Attributes.builder(); extractor(getter, selector(singletonList("Test-Message-Header"), emptyList())) @@ -105,17 +105,28 @@ void exactNamesAreCapturedWithoutHeaderNameEnumeration() { } @Test - void wildcardCapturesNothingWithoutHeaderNameEnumeration() { - // a third party getter that implements only getMessageHeader cannot resolve wildcards - MapGetter getter = new MapGetter(false); + void wildcardCapturesNothingWithoutHeaders() { + AttributesBuilder attributes = Attributes.builder(); + extractor(new MapGetter(false), selector(singletonList("Test-Message-*"), emptyList())) + .onEnd(attributes, Context.root(), emptyMap(), null, null); + assertThat(attributes.build().asMap()).isEmpty(); + } + + @Test + void excludeOnlyCapturesNothingWithoutHeaders() { AttributesBuilder attributes = Attributes.builder(); - extractor(getter, selector(singletonList("Test-Message-*"), emptyList())) - .onEnd(attributes, Context.root(), MESSAGE, null, null); + extractor(new MapGetter(false), selector(emptyList(), singletonList("Authorization"))) + .onEnd(attributes, Context.root(), emptyMap(), null, null); assertThat(attributes.build().asMap()).isEmpty(); } + @Test + void exactNamesAreNotNormalized() { + assertThat(capture(selector(singletonList("test-message-header"), emptyList()))).isEmpty(); + } + @SuppressWarnings("deprecation") // testing deprecated API @Test void deprecatedCapturedHeadersSetterSelectsExactNames() { @@ -166,7 +177,7 @@ void deprecatedCapturedHeadersSetterWithOnlyWildcardsCapturesNothing() { private static Map, Object> capture(IncludeExclude headers) { AttributesBuilder attributes = Attributes.builder(); - extractor(new MapGetter(true), headers).onEnd(attributes, Context.root(), MESSAGE, null, null); + extractor(new MapGetter(false), headers).onEnd(attributes, Context.root(), MESSAGE, null, null); return attributes.build().asMap(); } @@ -182,7 +193,7 @@ private static AttributesExtractor, Void> extractor( private static MessagingAttributesExtractorBuilder, Void> builder() { return MessagingAttributesExtractor.builder( - new MapGetter(true), MessagingOperationType.PROCESS, "process"); + new MapGetter(false), MessagingOperationType.PROCESS, "process"); } private static IncludeExclude selector(List included, List excluded) { @@ -196,10 +207,10 @@ private static Map.Entry, Object> headerEntry(String name, Strin private static final class MapGetter implements MessagingAttributesGetter, Void> { - private final boolean enumerateNames; + private final boolean failOnEnumeration; - MapGetter(boolean enumerateNames) { - this.enumerateNames = enumerateNames; + MapGetter(boolean failOnEnumeration) { + this.failOnEnumeration = failOnEnumeration; } @Override @@ -255,7 +266,10 @@ public List getMessageHeader(Map request, String name) { @Override public Collection getMessageHeaderNames(Map request) { - return enumerateNames ? new ArrayList<>(request.keySet()) : emptyList(); + if (failOnEnumeration) { + throw new AssertionError("Exact header names must not require enumeration"); + } + return new ArrayList<>(request.keySet()); } } } diff --git a/instrumentation/aws-lambda/aws-lambda-events-common-2.2/library/src/main/java/io/opentelemetry/instrumentation/awslambdaevents/common/v2_2/internal/SqsEventAttributesGetter.java b/instrumentation/aws-lambda/aws-lambda-events-common-2.2/library/src/main/java/io/opentelemetry/instrumentation/awslambdaevents/common/v2_2/internal/SqsEventAttributesGetter.java index 452f819e4278..d2b280bbd1ac 100644 --- a/instrumentation/aws-lambda/aws-lambda-events-common-2.2/library/src/main/java/io/opentelemetry/instrumentation/awslambdaevents/common/v2_2/internal/SqsEventAttributesGetter.java +++ b/instrumentation/aws-lambda/aws-lambda-events-common-2.2/library/src/main/java/io/opentelemetry/instrumentation/awslambdaevents/common/v2_2/internal/SqsEventAttributesGetter.java @@ -5,6 +5,8 @@ package io.opentelemetry.instrumentation.awslambdaevents.common.v2_2.internal; +import static java.util.Collections.emptyList; + import com.amazonaws.services.lambda.runtime.events.SQSEvent; import com.amazonaws.services.lambda.runtime.events.SQSEvent.SQSMessage; import java.util.List; @@ -24,4 +26,10 @@ public Long getBatchMessageCount(SQSEvent event, @Nullable Void unused) { List records = event.getRecords(); return records == null ? null : (long) records.size(); } + + @Override + public Iterable getMessageHeaderNames(SQSEvent event) { + // Headers belong to individual records, not the batch event. + return emptyList(); + } } diff --git a/instrumentation/aws-lambda/aws-lambda-events-common-2.2/library/src/main/java/io/opentelemetry/instrumentation/awslambdaevents/common/v2_2/internal/SqsMessageAttributesGetter.java b/instrumentation/aws-lambda/aws-lambda-events-common-2.2/library/src/main/java/io/opentelemetry/instrumentation/awslambdaevents/common/v2_2/internal/SqsMessageAttributesGetter.java index d175c7c08bdc..dac082627b95 100644 --- a/instrumentation/aws-lambda/aws-lambda-events-common-2.2/library/src/main/java/io/opentelemetry/instrumentation/awslambdaevents/common/v2_2/internal/SqsMessageAttributesGetter.java +++ b/instrumentation/aws-lambda/aws-lambda-events-common-2.2/library/src/main/java/io/opentelemetry/instrumentation/awslambdaevents/common/v2_2/internal/SqsMessageAttributesGetter.java @@ -5,7 +5,14 @@ package io.opentelemetry.instrumentation.awslambdaevents.common.v2_2.internal; +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; + +import com.amazonaws.services.lambda.runtime.events.SQSEvent.MessageAttribute; import com.amazonaws.services.lambda.runtime.events.SQSEvent.SQSMessage; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; import javax.annotation.Nullable; final class SqsMessageAttributesGetter extends SqsAttributesGetter { @@ -21,4 +28,18 @@ public String getDestination(SQSMessage message) { public String getMessageId(SQSMessage message, @Nullable Boolean unused) { return message.getMessageId(); } + + @Override + public List getMessageHeader(SQSMessage message, String name) { + Map attributes = message.getMessageAttributes(); + MessageAttribute attribute = attributes == null ? null : attributes.get(name); + String value = attribute == null ? null : attribute.getStringValue(); + return value == null ? emptyList() : singletonList(value); + } + + @Override + public Iterable getMessageHeaderNames(SQSMessage message) { + Map attributes = message.getMessageAttributes(); + return attributes == null ? emptyList() : new ArrayList<>(attributes.keySet()); + } } diff --git a/instrumentation/aws-lambda/aws-lambda-events-common-2.2/library/src/test/java/io/opentelemetry/instrumentation/awslambdaevents/common/v2_2/internal/SqsMessageAttributesGetterTest.java b/instrumentation/aws-lambda/aws-lambda-events-common-2.2/library/src/test/java/io/opentelemetry/instrumentation/awslambdaevents/common/v2_2/internal/SqsMessageAttributesGetterTest.java new file mode 100644 index 000000000000..ba0982b0770c --- /dev/null +++ b/instrumentation/aws-lambda/aws-lambda-events-common-2.2/library/src/test/java/io/opentelemetry/instrumentation/awslambdaevents/common/v2_2/internal/SqsMessageAttributesGetterTest.java @@ -0,0 +1,55 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.awslambdaevents.common.v2_2.internal; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.amazonaws.services.lambda.runtime.events.SQSEvent; +import com.amazonaws.services.lambda.runtime.events.SQSEvent.MessageAttribute; +import com.amazonaws.services.lambda.runtime.events.SQSEvent.SQSMessage; +import java.nio.ByteBuffer; +import java.util.HashMap; +import java.util.Map; +import org.junit.jupiter.api.Test; + +class SqsMessageAttributesGetterTest { + + private final SqsMessageAttributesGetter getter = new SqsMessageAttributesGetter(); + + @Test + void headers() { + SQSMessage message = new SQSMessage(); + MessageAttribute attribute = new MessageAttribute(); + attribute.setStringValue("value"); + MessageAttribute binary = new MessageAttribute(); + binary.setBinaryValue(ByteBuffer.wrap(new byte[] {1})); + Map attributes = new HashMap<>(); + attributes.put("Test-Header", attribute); + attributes.put("Binary-Header", binary); + attributes.put("Null-Header", null); + message.setMessageAttributes(attributes); + + assertThat(getter.getMessageHeaderNames(message)) + .containsExactlyInAnyOrder("Test-Header", "Binary-Header", "Null-Header"); + assertThat(getter.getMessageHeader(message, "Test-Header")).containsExactly("value"); + assertThat(getter.getMessageHeader(message, "test-header")).isEmpty(); + assertThat(getter.getMessageHeader(message, "Binary-Header")).isEmpty(); + assertThat(getter.getMessageHeader(message, "Null-Header")).isEmpty(); + assertThat(getter.getMessageHeader(message, "missing")).isEmpty(); + + Iterable names = getter.getMessageHeaderNames(message); + attributes.clear(); + assertThat(names).containsExactlyInAnyOrder("Test-Header", "Binary-Header", "Null-Header"); + } + + @Test + void noHeaders() { + SQSMessage message = new SQSMessage(); + assertThat(getter.getMessageHeaderNames(message)).isEmpty(); + assertThat(getter.getMessageHeader(message, "missing")).isEmpty(); + assertThat(new SqsEventAttributesGetter().getMessageHeaderNames(new SQSEvent())).isEmpty(); + } +} diff --git a/instrumentation/camel-2.20/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/camel/v2_20/CamelMessagingAttributesGetterTest.java b/instrumentation/camel-2.20/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/camel/v2_20/CamelMessagingAttributesGetterTest.java new file mode 100644 index 000000000000..570351514368 --- /dev/null +++ b/instrumentation/camel-2.20/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/camel/v2_20/CamelMessagingAttributesGetterTest.java @@ -0,0 +1,90 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.camel.v2_20; + +import static io.opentelemetry.api.common.AttributeKey.stringArrayKey; +import static java.util.Collections.singletonList; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.entry; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.common.AttributesBuilder; +import io.opentelemetry.api.trace.SpanKind; +import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.config.IncludeExclude; +import io.opentelemetry.instrumentation.api.incubator.semconv.messaging.MessagingAttributesExtractor; +import io.opentelemetry.instrumentation.api.incubator.semconv.messaging.MessagingOperationType; +import io.opentelemetry.javaagent.instrumentation.camel.v2_20.decorators.DecoratorRegistry; +import java.util.HashMap; +import java.util.Map; +import org.apache.camel.Endpoint; +import org.apache.camel.Exchange; +import org.apache.camel.impl.DefaultCamelContext; +import org.apache.camel.impl.DefaultExchange; +import org.junit.jupiter.api.Test; + +class CamelMessagingAttributesGetterTest { + + private final CamelMessagingAttributesGetter getter = new CamelMessagingAttributesGetter(); + + @Test + void headers() { + CamelRequest request = request(); + Map headers = new HashMap<>(); + headers.put("Test-Header", "value"); + headers.put("Number", 42); + headers.put("Null", null); + request.getExchange().getIn().setHeaders(headers); + + assertThat(getter.getMessageHeaderNames(request)) + .containsExactlyInAnyOrder("Test-Header", "Number", "Null"); + assertThat(getter.getMessageHeader(request, "Test-Header")).containsExactly("value"); + assertThat(getter.getMessageHeader(request, "Number")).containsExactly("42"); + assertThat(getter.getMessageHeader(request, "Null")).isEmpty(); + assertThat(getter.getMessageHeader(request, "missing")).isEmpty(); + + Iterable names = getter.getMessageHeaderNames(request); + request.getExchange().getIn().getHeaders().clear(); + assertThat(names).containsExactlyInAnyOrder("Test-Header", "Number", "Null"); + assertThat(getter.getMessageHeaderNames(request)).isEmpty(); + } + + @Test + void selectors() { + CamelRequest request = request(); + request.getExchange().getIn().setHeader("Test-Header", "value"); + request.getExchange().getIn().setHeader("Authorization", "private"); + + IncludeExclude wildcard = IncludeExclude.builder().setIncluded(singletonList("Test-*")).build(); + IncludeExclude excludeOnly = + IncludeExclude.builder().setExcluded(singletonList("Authorization")).build(); + for (IncludeExclude selector : new IncludeExclude[] {wildcard, excludeOnly}) { + AttributesBuilder attributes = Attributes.builder(); + MessagingAttributesExtractor.builder(getter, MessagingOperationType.PROCESS, "process") + .setHeaders(selector) + .build() + .onEnd(attributes, Context.root(), request, null, null); + + assertThat(attributes.build().asMap()) + .containsOnly( + entry(stringArrayKey("messaging.header.Test-Header"), singletonList("value"))); + } + } + + private static CamelRequest request() { + Endpoint endpoint = mock(Endpoint.class); + when(endpoint.getEndpointUri()).thenReturn("jms:queue:test"); + Exchange exchange = new DefaultExchange(new DefaultCamelContext()); + return CamelRequest.create( + new DecoratorRegistry().forComponent("jms"), + exchange, + endpoint, + CamelDirection.INBOUND, + SpanKind.CONSUMER); + } +} diff --git a/instrumentation/camel-2.20/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/camel/v2_20/CamelMessagingAttributesGetter.java b/instrumentation/camel-2.20/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/camel/v2_20/CamelMessagingAttributesGetter.java index 4373473272fb..9ae9d0c41e02 100644 --- a/instrumentation/camel-2.20/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/camel/v2_20/CamelMessagingAttributesGetter.java +++ b/instrumentation/camel-2.20/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/camel/v2_20/CamelMessagingAttributesGetter.java @@ -5,8 +5,13 @@ package io.opentelemetry.javaagent.instrumentation.camel.v2_20; +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; + import io.opentelemetry.instrumentation.api.incubator.semconv.messaging.MessagingAttributesGetter; import io.opentelemetry.javaagent.instrumentation.camel.v2_20.decorators.MessagingSpanDecorator; +import java.util.ArrayList; +import java.util.List; import javax.annotation.Nullable; final class CamelMessagingAttributesGetter @@ -65,6 +70,17 @@ public Long getBatchMessageCount(CamelRequest request, @Nullable Void unused) { return null; } + @Override + public List getMessageHeader(CamelRequest request, String name) { + Object value = request.getExchange().getIn().getHeaders().get(name); + return value == null ? emptyList() : singletonList(value.toString()); + } + + @Override + public Iterable getMessageHeaderNames(CamelRequest request) { + return new ArrayList<>(request.getExchange().getIn().getHeaders().keySet()); + } + @Nullable @Override public String getDestinationPartitionId(CamelRequest request) {