From 9d1906727441312b369f804904f60052113580df Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Fri, 28 May 2021 11:47:55 +0200 Subject: [PATCH 01/12] Spring Integration library instrumentation --- .../api/instrumenter/Instrumenter.java | 15 +- .../api/instrumenter/InstrumenterBuilder.java | 8 + .../PropagatorBasedSpanLinkExtractor.java | 28 +++ .../api/instrumenter/SpanLinkExtractor.java | 27 ++ .../api/instrumenter/InstrumenterTest.java | 29 ++- .../PropagatorBasedSpanLinkExtractorTest.java | 61 +++++ .../spring-integration-4.1-library.gradle | 11 + .../spring/messaging/ContextAndScope.java | 30 +++ .../MessageChannelSpanNameExtractor.java | 24 ++ .../messaging/MessageHeadersGetter.java | 62 +++++ .../messaging/MessageHeadersSetter.java | 33 +++ .../messaging/MessageSpanLinkExtractor.java | 36 +++ .../spring/messaging/MessageWithChannel.java | 22 ++ .../messaging/SpringIntegrationTracing.java | 53 ++++ .../SpringIntegrationTracingBuilder.java | 56 +++++ .../messaging/TracingChannelInterceptor.java | 128 ++++++++++ .../groovy/CapturingMessageHandler.groovy | 26 ++ .../test/groovy/ComplexPropagationTest.groovy | 160 ++++++++++++ .../SpringIntegrationTracingTest.groovy | 231 ++++++++++++++++++ settings.gradle | 1 + .../test/asserts/SpanAssert.groovy | 4 + .../testing/LibraryTestRunner.java | 2 + testing-common/testing-common.gradle | 1 + 23 files changed, 1043 insertions(+), 5 deletions(-) create mode 100644 instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/PropagatorBasedSpanLinkExtractor.java create mode 100644 instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanLinkExtractor.java create mode 100644 instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/PropagatorBasedSpanLinkExtractorTest.java create mode 100644 instrumentation/spring/spring-integration-4.1/library/spring-integration-4.1-library.gradle create mode 100644 instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/ContextAndScope.java create mode 100644 instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageChannelSpanNameExtractor.java create mode 100644 instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageHeadersGetter.java create mode 100644 instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageHeadersSetter.java create mode 100644 instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageSpanLinkExtractor.java create mode 100644 instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageWithChannel.java create mode 100644 instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/SpringIntegrationTracing.java create mode 100644 instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/SpringIntegrationTracingBuilder.java create mode 100644 instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/TracingChannelInterceptor.java create mode 100644 instrumentation/spring/spring-integration-4.1/library/src/test/groovy/CapturingMessageHandler.groovy create mode 100644 instrumentation/spring/spring-integration-4.1/library/src/test/groovy/ComplexPropagationTest.groovy create mode 100644 instrumentation/spring/spring-integration-4.1/library/src/test/groovy/SpringIntegrationTracingTest.groovy diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java index 34da549985c9..34cd78175321 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java @@ -55,7 +55,9 @@ public static InstrumenterBuilder newBuil private final SpanNameExtractor spanNameExtractor; private final SpanKindExtractor spanKindExtractor; private final SpanStatusExtractor spanStatusExtractor; - private final List> extractors; + private final List> + attributesExtractors; + private final List> spanLinkExtractors; private final List requestListeners; private final ErrorCauseExtractor errorCauseExtractor; @Nullable private final StartTimeExtractor startTimeExtractor; @@ -68,7 +70,8 @@ public static InstrumenterBuilder newBuil this.spanNameExtractor = builder.spanNameExtractor; this.spanKindExtractor = builder.spanKindExtractor; this.spanStatusExtractor = builder.spanStatusExtractor; - this.extractors = new ArrayList<>(builder.attributesExtractors); + this.attributesExtractors = new ArrayList<>(builder.attributesExtractors); + this.spanLinkExtractors = new ArrayList<>(builder.spanLinkExtractors); this.requestListeners = new ArrayList<>(builder.requestListeners); this.errorCauseExtractor = builder.errorCauseExtractor; this.startTimeExtractor = builder.startTimeExtractor; @@ -119,8 +122,12 @@ public Context start(Context parentContext, REQUEST request) { spanBuilder.setStartTimestamp(startTimeExtractor.extract(request)); } + for (SpanLinkExtractor extractor : spanLinkExtractors) { + spanBuilder.addLink(extractor.extract(parentContext, request)); + } + UnsafeAttributes attributesBuilder = new UnsafeAttributes(); - for (AttributesExtractor extractor : extractors) { + for (AttributesExtractor extractor : attributesExtractors) { extractor.onStart(attributesBuilder, request); } Attributes attributes = attributesBuilder; @@ -154,7 +161,7 @@ public void end(Context context, REQUEST request, RESPONSE response, @Nullable T Span span = Span.fromContext(context); UnsafeAttributes attributesBuilder = new UnsafeAttributes(); - for (AttributesExtractor extractor : extractors) { + for (AttributesExtractor extractor : attributesExtractors) { extractor.onEnd(attributesBuilder, request, response); } Attributes attributes = attributesBuilder; diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterBuilder.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterBuilder.java index 60d502cf013d..83658da26d02 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterBuilder.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterBuilder.java @@ -33,6 +33,7 @@ public final class InstrumenterBuilder { final List> attributesExtractors = new ArrayList<>(); + final List> spanLinkExtractors = new ArrayList<>(); final List requestListeners = new ArrayList<>(); SpanKindExtractor spanKindExtractor = SpanKindExtractor.alwaysInternal(); @@ -83,6 +84,13 @@ public InstrumenterBuilder addAttributesExtractors( return addAttributesExtractors(Arrays.asList(attributesExtractors)); } + /** Adds a {@link SpanLinkExtractor} to extract span link from requests. */ + public InstrumenterBuilder addSpanLinkExtractor( + SpanLinkExtractor spanLinkExtractor) { + spanLinkExtractors.add(spanLinkExtractor); + return this; + } + /** Adds a {@link RequestMetrics} whose metrics will be recorded for request start and stop. */ @UnstableApi public InstrumenterBuilder addRequestMetrics(RequestMetrics factory) { diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/PropagatorBasedSpanLinkExtractor.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/PropagatorBasedSpanLinkExtractor.java new file mode 100644 index 000000000000..f0fabf11c40f --- /dev/null +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/PropagatorBasedSpanLinkExtractor.java @@ -0,0 +1,28 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.api.instrumenter; + +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.SpanContext; +import io.opentelemetry.context.Context; +import io.opentelemetry.context.propagation.ContextPropagators; +import io.opentelemetry.context.propagation.TextMapGetter; + +final class PropagatorBasedSpanLinkExtractor implements SpanLinkExtractor { + private final ContextPropagators propagators; + private final TextMapGetter getter; + + PropagatorBasedSpanLinkExtractor(ContextPropagators propagators, TextMapGetter getter) { + this.propagators = propagators; + this.getter = getter; + } + + @Override + public SpanContext extract(Context parentContext, REQUEST request) { + Context extracted = propagators.getTextMapPropagator().extract(parentContext, request, getter); + return Span.fromContext(extracted).getSpanContext(); + } +} diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanLinkExtractor.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanLinkExtractor.java new file mode 100644 index 000000000000..e0c5d3651a4d --- /dev/null +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanLinkExtractor.java @@ -0,0 +1,27 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.api.instrumenter; + +import io.opentelemetry.api.trace.SpanContext; +import io.opentelemetry.context.Context; +import io.opentelemetry.context.propagation.ContextPropagators; +import io.opentelemetry.context.propagation.TextMapGetter; + +/** Extractor of a span link for a request. */ +@FunctionalInterface +public interface SpanLinkExtractor { + /** Extract a {@link SpanContext} that should be linked to the newly created span. */ + SpanContext extract(Context parentContext, REQUEST request); + + /** + * Returns a new {@link SpanLinkExtractor} that will extract a {@link SpanContext} from the + * request using configured {@code propagators}. + */ + static SpanLinkExtractor fromUpstreamRequest( + ContextPropagators propagators, TextMapGetter getter) { + return new PropagatorBasedSpanLinkExtractor<>(propagators, getter); + } +} diff --git a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterTest.java b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterTest.java index 78a7fbabfa32..fe2e21d39958 100644 --- a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterTest.java +++ b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterTest.java @@ -15,6 +15,7 @@ import io.opentelemetry.api.trace.SpanId; import io.opentelemetry.api.trace.SpanKind; import io.opentelemetry.api.trace.TraceFlags; +import io.opentelemetry.api.trace.TraceId; import io.opentelemetry.api.trace.TraceState; import io.opentelemetry.api.trace.propagation.W3CTraceContextPropagator; import io.opentelemetry.context.Context; @@ -22,6 +23,7 @@ import io.opentelemetry.instrumentation.api.tracer.ServerSpan; import io.opentelemetry.sdk.common.InstrumentationLibraryInfo; import io.opentelemetry.sdk.testing.junit5.OpenTelemetryExtension; +import io.opentelemetry.sdk.trace.data.LinkData; import io.opentelemetry.sdk.trace.data.StatusData; import java.time.Instant; import java.util.Collections; @@ -33,6 +35,8 @@ import org.junit.jupiter.api.extension.RegisterExtension; class InstrumenterTest { + private static final String LINK_TRACE_ID = TraceId.fromLongs(0, 42); + private static final String LINK_SPAN_ID = SpanId.fromLong(123); private static final Map REQUEST = Collections.unmodifiableMap( @@ -40,7 +44,9 @@ class InstrumenterTest { entry("req1", "req1_value"), entry("req2", "req2_value"), entry("req2_2", "req2_2_value"), - entry("req3", "req3_value")) + entry("req3", "req3_value"), + entry("linkTraceId", LINK_TRACE_ID), + entry("linkSpanId", LINK_SPAN_ID)) .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue))); private static final Map RESPONSE = @@ -86,6 +92,17 @@ protected void onEnd( } } + static class LinkExtractor implements SpanLinkExtractor> { + @Override + public SpanContext extract(Context parentContext, Map request) { + return SpanContext.create( + request.get("linkTraceId"), + request.get("linkSpanId"), + TraceFlags.getSampled(), + TraceState.getDefault()); + } + } + static class MapGetter implements TextMapGetter> { @Override @@ -108,6 +125,7 @@ void server() { Instrumenter., Map>newBuilder( otelTesting.getOpenTelemetry(), "test", unused -> "span") .addAttributesExtractors(new AttributesExtractor1(), new AttributesExtractor2()) + .addSpanLinkExtractor(new LinkExtractor()) .newServerInstrumenter(new MapGetter()); Context context = instrumenter.start(Context.root(), REQUEST); @@ -132,6 +150,7 @@ void server() { .hasSpanId(spanContext.getSpanId()) .hasParentSpanId(SpanId.getInvalid()) .hasStatus(StatusData.unset()) + .hasLinks(expectedSpanLink()) .hasAttributesSatisfying( attributes -> assertThat(attributes) @@ -215,6 +234,7 @@ void client() { Instrumenter., Map>newBuilder( otelTesting.getOpenTelemetry(), "test", unused -> "span") .addAttributesExtractors(new AttributesExtractor1(), new AttributesExtractor2()) + .addSpanLinkExtractor(new LinkExtractor()) .newClientInstrumenter(Map::put); Map request = new HashMap<>(REQUEST); @@ -240,6 +260,7 @@ void client() { .hasSpanId(spanContext.getSpanId()) .hasParentSpanId(SpanId.getInvalid()) .hasStatus(StatusData.unset()) + .hasLinks(expectedSpanLink()) .hasAttributesSatisfying( attributes -> assertThat(attributes) @@ -340,4 +361,10 @@ void shouldStartSpanWithGivenStartTime() { trace.hasSpansSatisfyingExactly( span -> span.hasName("test span").startsAt(startTime).endsAt(endTime))); } + + private static LinkData expectedSpanLink() { + return LinkData.create( + SpanContext.create( + LINK_TRACE_ID, LINK_SPAN_ID, TraceFlags.getSampled(), TraceState.getDefault())); + } } diff --git a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/PropagatorBasedSpanLinkExtractorTest.java b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/PropagatorBasedSpanLinkExtractorTest.java new file mode 100644 index 000000000000..2b6d644694b9 --- /dev/null +++ b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/PropagatorBasedSpanLinkExtractorTest.java @@ -0,0 +1,61 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.api.instrumenter; + +import static java.util.Collections.singletonMap; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import io.opentelemetry.api.trace.SpanContext; +import io.opentelemetry.api.trace.SpanId; +import io.opentelemetry.api.trace.TraceFlags; +import io.opentelemetry.api.trace.TraceId; +import io.opentelemetry.api.trace.TraceState; +import io.opentelemetry.api.trace.propagation.W3CTraceContextPropagator; +import io.opentelemetry.context.Context; +import io.opentelemetry.context.propagation.ContextPropagators; +import io.opentelemetry.context.propagation.TextMapGetter; +import java.util.Map; +import org.junit.jupiter.api.Test; + +class PropagatorBasedSpanLinkExtractorTest { + private static final String TRACE_ID = TraceId.fromLongs(0, 123); + private static final String SPAN_ID = SpanId.fromLong(456); + + @Test + void shouldExtractSpanLink() { + // given + ContextPropagators propagators = + ContextPropagators.create(W3CTraceContextPropagator.getInstance()); + + SpanLinkExtractor> underTest = + SpanLinkExtractor.fromUpstreamRequest(propagators, new MapGetter()); + + Map request = + singletonMap("traceparent", String.format("00-%s-%s-01", TRACE_ID, SPAN_ID)); + + // when + SpanContext link = underTest.extract(Context.root(), request); + + // then + assertEquals( + SpanContext.createFromRemoteParent( + TRACE_ID, SPAN_ID, TraceFlags.getSampled(), TraceState.getDefault()), + link); + } + + static final class MapGetter implements TextMapGetter> { + + @Override + public Iterable keys(Map carrier) { + return carrier.keySet(); + } + + @Override + public String get(Map carrier, String key) { + return carrier.get(key); + } + } +} diff --git a/instrumentation/spring/spring-integration-4.1/library/spring-integration-4.1-library.gradle b/instrumentation/spring/spring-integration-4.1/library/spring-integration-4.1-library.gradle new file mode 100644 index 000000000000..607e16bbf536 --- /dev/null +++ b/instrumentation/spring/spring-integration-4.1/library/spring-integration-4.1-library.gradle @@ -0,0 +1,11 @@ +apply from: "$rootDir/gradle/instrumentation-library.gradle" + +dependencies { + compileOnly "com.google.auto.value:auto-value-annotations" + annotationProcessor "com.google.auto.value:auto-value" + + library 'org.springframework.integration:spring-integration-core:4.1.0.RELEASE' + + testImplementation "org.springframework.boot:spring-boot-starter-test:1.5.17.RELEASE" + testImplementation "org.springframework.boot:spring-boot-starter:1.5.17.RELEASE" +} diff --git a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/ContextAndScope.java b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/ContextAndScope.java new file mode 100644 index 000000000000..d42341ee9119 --- /dev/null +++ b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/ContextAndScope.java @@ -0,0 +1,30 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.spring.messaging; + +import com.google.auto.value.AutoValue; +import io.opentelemetry.context.Context; +import io.opentelemetry.context.Scope; + +@AutoValue +abstract class ContextAndScope { + + abstract Context getContext(); + + abstract Scope getScope(); + + void close() { + getScope().close(); + } + + static ContextAndScope makeCurrent(Context context) { + return create(context, context.makeCurrent()); + } + + static ContextAndScope create(Context context, Scope scope) { + return new AutoValue_ContextAndScope(context, scope); + } +} diff --git a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageChannelSpanNameExtractor.java b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageChannelSpanNameExtractor.java new file mode 100644 index 000000000000..20b569fae1c2 --- /dev/null +++ b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageChannelSpanNameExtractor.java @@ -0,0 +1,24 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.spring.messaging; + +import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor; +import org.springframework.integration.channel.AbstractMessageChannel; +import org.springframework.messaging.MessageChannel; + +final class MessageChannelSpanNameExtractor implements SpanNameExtractor { + @Override + public String extract(MessageWithChannel messageWithChannel) { + MessageChannel channel = messageWithChannel.getMessageChannel(); + if (channel instanceof AbstractMessageChannel) { + return ((AbstractMessageChannel) channel).getFullChannelName(); + } + if (channel instanceof org.springframework.messaging.support.AbstractMessageChannel) { + return ((org.springframework.messaging.support.AbstractMessageChannel) channel).getBeanName(); + } + return channel.getClass().getSimpleName(); + } +} diff --git a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageHeadersGetter.java b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageHeadersGetter.java new file mode 100644 index 000000000000..3a6d0b2d8f58 --- /dev/null +++ b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageHeadersGetter.java @@ -0,0 +1,62 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.spring.messaging; + +import io.opentelemetry.context.propagation.TextMapGetter; +import java.nio.charset.StandardCharsets; +import java.util.List; +import java.util.Map; +import org.checkerframework.checker.nullness.qual.Nullable; +import org.springframework.messaging.MessageHeaders; +import org.springframework.messaging.support.NativeMessageHeaderAccessor; + +// Native headers logic inspired by +// https://github.com/spring-cloud/spring-cloud-sleuth/blob/main/spring-cloud-sleuth-instrumentation/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/MessageHeaderPropagatorGetter.java +enum MessageHeadersGetter implements TextMapGetter { + INSTANCE; + + @Override + public Iterable keys(MessageWithChannel carrier) { + MessageHeaders headers = carrier.getMessage().getHeaders(); + Map> nativeHeaders = + headers.get(NativeMessageHeaderAccessor.NATIVE_HEADERS, Map.class); + if (nativeHeaders != null) { + return nativeHeaders.keySet(); + } + return headers.keySet(); + } + + @Override + public String get(MessageWithChannel carrier, String key) { + MessageHeaders headers = carrier.getMessage().getHeaders(); + String nativeHeaderValue = getNativeHeader(headers, key); + if (nativeHeaderValue != null) { + return nativeHeaderValue; + } + Object headerValue = headers.get(key); + if (headerValue == null) { + return null; + } + if (headerValue instanceof byte[]) { + return new String((byte[]) headerValue, StandardCharsets.UTF_8); + } + return headerValue.toString(); + } + + @Nullable + private static String getNativeHeader(MessageHeaders carrier, String key) { + Map> nativeMap = + carrier.get(NativeMessageHeaderAccessor.NATIVE_HEADERS, Map.class); + if (nativeMap == null) { + return null; + } + List values = nativeMap.get(key); + if (values == null || values.isEmpty()) { + return null; + } + return values.get(0); + } +} diff --git a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageHeadersSetter.java b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageHeadersSetter.java new file mode 100644 index 000000000000..170d33f7f8ee --- /dev/null +++ b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageHeadersSetter.java @@ -0,0 +1,33 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.spring.messaging; + +import static java.util.Collections.singletonList; + +import io.opentelemetry.context.propagation.TextMapSetter; +import java.util.List; +import java.util.Map; +import org.springframework.messaging.support.MessageHeaderAccessor; +import org.springframework.messaging.support.NativeMessageHeaderAccessor; + +// Native headers logic inspired by +// https://github.com/spring-cloud/spring-cloud-sleuth/blob/main/spring-cloud-sleuth-instrumentation/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/MessageHeaderPropagatorSetter.java +enum MessageHeadersSetter implements TextMapSetter { + INSTANCE; + + @Override + public void set(MessageHeaderAccessor carrier, String key, String value) { + carrier.setHeader(key, value); + setNativeHeader(carrier, key, value); + } + + private void setNativeHeader(MessageHeaderAccessor carrier, String key, String value) { + Object nativeMap = carrier.getHeader(NativeMessageHeaderAccessor.NATIVE_HEADERS); + if (nativeMap instanceof Map) { + ((Map>) nativeMap).put(key, singletonList(value)); + } + } +} diff --git a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageSpanLinkExtractor.java b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageSpanLinkExtractor.java new file mode 100644 index 000000000000..241c4256ea01 --- /dev/null +++ b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageSpanLinkExtractor.java @@ -0,0 +1,36 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.spring.messaging; + +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.SpanContext; +import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.instrumenter.SpanLinkExtractor; + +final class MessageSpanLinkExtractor implements SpanLinkExtractor { + private final SpanLinkExtractor delegate; + + MessageSpanLinkExtractor(SpanLinkExtractor delegate) { + this.delegate = delegate; + } + + @Override + public SpanContext extract(Context parentContext, MessageWithChannel messageWithChannel) { + SpanContext spanFromMessage = delegate.extract(parentContext, messageWithChannel); + SpanContext parentSpan = Span.fromContext(parentContext).getSpanContext(); + if (referencesSameSpan(spanFromMessage, parentSpan)) { + return SpanContext.getInvalid(); + } + return spanFromMessage; + } + + // SpanContext#equals() includes e.g. remote flag, which we don't really care about here + // we just want to avoid adding links to spans with the same id, flags don't matter at all + private boolean referencesSameSpan(SpanContext spanFromMessage, SpanContext parentSpan) { + return parentSpan.getTraceId().equals(spanFromMessage.getTraceId()) + && parentSpan.getSpanId().equals(spanFromMessage.getSpanId()); + } +} diff --git a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageWithChannel.java b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageWithChannel.java new file mode 100644 index 000000000000..159aaff680db --- /dev/null +++ b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageWithChannel.java @@ -0,0 +1,22 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.spring.messaging; + +import com.google.auto.value.AutoValue; +import org.springframework.messaging.Message; +import org.springframework.messaging.MessageChannel; + +@AutoValue +public abstract class MessageWithChannel { + + public abstract Message getMessage(); + + public abstract MessageChannel getMessageChannel(); + + static MessageWithChannel create(Message message, MessageChannel messageChannel) { + return new AutoValue_MessageWithChannel(message, messageChannel); + } +} diff --git a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/SpringIntegrationTracing.java b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/SpringIntegrationTracing.java new file mode 100644 index 000000000000..a9f38ee30b1d --- /dev/null +++ b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/SpringIntegrationTracing.java @@ -0,0 +1,53 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.spring.messaging; + +import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.context.propagation.ContextPropagators; +import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; +import org.springframework.messaging.Message; +import org.springframework.messaging.MessageChannel; +import org.springframework.messaging.support.ChannelInterceptor; + +/** Entrypoint for tracing Spring Integration {@link MessageChannel}s. */ +public final class SpringIntegrationTracing { + + /** + * Returns a new {@link SpringIntegrationTracing} configured with the given {@link OpenTelemetry}. + */ + public static SpringIntegrationTracing create(OpenTelemetry openTelemetry) { + return newBuilder(openTelemetry).build(); + } + + /** + * Returns a new {@link SpringIntegrationTracingBuilder} configured with the given {@link + * OpenTelemetry}. + */ + public static SpringIntegrationTracingBuilder newBuilder(OpenTelemetry openTelemetry) { + return new SpringIntegrationTracingBuilder(openTelemetry); + } + + private final ContextPropagators propagators; + private final Instrumenter instrumenter; + + SpringIntegrationTracing( + ContextPropagators propagators, Instrumenter instrumenter) { + this.propagators = propagators; + this.instrumenter = instrumenter; + } + + /** + * Returns a new {@link ChannelInterceptor} that traces {@link MessageChannel#send(Message)} calls + * and propagates context through {@link Message}s. + * + * @see org.springframework.integration.channel.ChannelInterceptorAware + * @see org.springframework.messaging.support.InterceptableChannel + * @see org.springframework.integration.config.GlobalChannelInterceptor + */ + public ChannelInterceptor newChannelInterceptor() { + return new TracingChannelInterceptor(propagators, instrumenter); + } +} diff --git a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/SpringIntegrationTracingBuilder.java b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/SpringIntegrationTracingBuilder.java new file mode 100644 index 000000000000..5499c17c8cfa --- /dev/null +++ b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/SpringIntegrationTracingBuilder.java @@ -0,0 +1,56 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.spring.messaging; + +import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; +import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; +import io.opentelemetry.instrumentation.api.instrumenter.SpanLinkExtractor; +import java.util.ArrayList; +import java.util.List; + +/** A builder of {@link SpringIntegrationTracing}. */ +public final class SpringIntegrationTracingBuilder { + private static final String INSTRUMENTATION_NAME = "io.opentelemetry.spring-integration-core-4.1"; + + private final OpenTelemetry openTelemetry; + private final List> additionalAttributeExtractors = + new ArrayList<>(); + + SpringIntegrationTracingBuilder(OpenTelemetry openTelemetry) { + this.openTelemetry = openTelemetry; + } + + /** + * Adds an additional {@link AttributesExtractor} to invoke to set attributes to instrumented + * items. + */ + public SpringIntegrationTracingBuilder addAttributeExtractor( + AttributesExtractor attributesExtractor) { + additionalAttributeExtractors.add(attributesExtractor); + return this; + } + + /** + * Returns a new {@link SpringIntegrationTracing} with the settings of this {@link + * SpringIntegrationTracingBuilder}. + */ + public SpringIntegrationTracing build() { + MessageChannelSpanNameExtractor spanNameExtractor = new MessageChannelSpanNameExtractor(); + MessageSpanLinkExtractor spanLinkExtractor = + new MessageSpanLinkExtractor( + SpanLinkExtractor.fromUpstreamRequest( + openTelemetry.getPropagators(), MessageHeadersGetter.INSTANCE)); + + Instrumenter instrumenter = + Instrumenter.newBuilder( + openTelemetry, INSTRUMENTATION_NAME, spanNameExtractor) + .addSpanLinkExtractor(spanLinkExtractor) + .addAttributesExtractors(additionalAttributeExtractors) + .newInstrumenter(); + return new SpringIntegrationTracing(openTelemetry.getPropagators(), instrumenter); + } +} diff --git a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/TracingChannelInterceptor.java b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/TracingChannelInterceptor.java new file mode 100644 index 000000000000..97c27df8089e --- /dev/null +++ b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/TracingChannelInterceptor.java @@ -0,0 +1,128 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.spring.messaging; + +import io.opentelemetry.context.Context; +import io.opentelemetry.context.Scope; +import io.opentelemetry.context.propagation.ContextPropagators; +import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; +import java.util.List; +import java.util.Map; +import org.springframework.messaging.Message; +import org.springframework.messaging.MessageChannel; +import org.springframework.messaging.MessageHandler; +import org.springframework.messaging.support.ExecutorChannelInterceptor; +import org.springframework.messaging.support.MessageBuilder; +import org.springframework.messaging.support.MessageHeaderAccessor; +import org.springframework.messaging.support.NativeMessageHeaderAccessor; +import org.springframework.util.LinkedMultiValueMap; + +final class TracingChannelInterceptor implements ExecutorChannelInterceptor { + private static final String CONTEXT_AND_SCOPE_KEY = ContextAndScope.class.getName(); + private static final String SCOPE_KEY = TracingChannelInterceptor.class.getName() + ".scope"; + + private final ContextPropagators propagators; + private final Instrumenter instrumenter; + + TracingChannelInterceptor( + ContextPropagators propagators, Instrumenter instrumenter) { + this.propagators = propagators; + this.instrumenter = instrumenter; + } + + @Override + public Message preSend(Message message, MessageChannel messageChannel) { + Context parentContext = Context.current(); + MessageWithChannel messageWithChannel = MessageWithChannel.create(message, messageChannel); + + if (!instrumenter.shouldStart(parentContext, messageWithChannel)) { + return message; + } + Context context = instrumenter.start(parentContext, messageWithChannel); + + MessageHeaderAccessor messageHeaderAccessor = createMutableHeaderAccessor(message); + propagators + .getTextMapPropagator() + .inject(context, messageHeaderAccessor, MessageHeadersSetter.INSTANCE); + messageHeaderAccessor.setHeader(CONTEXT_AND_SCOPE_KEY, ContextAndScope.makeCurrent(context)); + return createMessageWithHeaders(message, messageHeaderAccessor); + } + + @Override + public void postSend(Message message, MessageChannel messageChannel, boolean sent) {} + + @Override + public void afterSendCompletion( + Message message, MessageChannel messageChannel, boolean sent, Exception e) { + ContextAndScope contextAndScope = + message.getHeaders().get(CONTEXT_AND_SCOPE_KEY, ContextAndScope.class); + if (contextAndScope != null) { + contextAndScope.close(); + MessageWithChannel messageWithChannel = MessageWithChannel.create(message, messageChannel); + instrumenter.end(contextAndScope.getContext(), messageWithChannel, null, e); + } + } + + @Override + public boolean preReceive(MessageChannel messageChannel) { + return true; + } + + @Override + public Message postReceive(Message message, MessageChannel messageChannel) { + return message; + } + + @Override + public void afterReceiveCompletion( + Message message, MessageChannel messageChannel, Exception e) {} + + @Override + public Message beforeHandle( + Message message, MessageChannel channel, MessageHandler handler) { + MessageWithChannel messageWithChannel = MessageWithChannel.create(message, channel); + Context context = + propagators + .getTextMapPropagator() + .extract(Context.current(), messageWithChannel, MessageHeadersGetter.INSTANCE); + Scope scope = context.makeCurrent(); + MessageHeaderAccessor messageHeaderAccessor = MessageHeaderAccessor.getMutableAccessor(message); + messageHeaderAccessor.setHeader(SCOPE_KEY, scope); + return createMessageWithHeaders(message, messageHeaderAccessor); + } + + @Override + public void afterMessageHandled( + Message message, MessageChannel channel, MessageHandler handler, Exception ex) { + Scope scope = message.getHeaders().get(SCOPE_KEY, Scope.class); + if (scope != null) { + scope.close(); + } + } + + private static MessageHeaderAccessor createMutableHeaderAccessor(Message message) { + MessageHeaderAccessor headerAccessor = MessageHeaderAccessor.getMutableAccessor(message); + headerAccessor.setLeaveMutable(true); + ensureNativeHeadersAreMutable(headerAccessor); + return headerAccessor; + } + + private static void ensureNativeHeadersAreMutable(MessageHeaderAccessor headerAccessor) { + Object nativeMap = headerAccessor.getHeader(NativeMessageHeaderAccessor.NATIVE_HEADERS); + if (nativeMap != null && !(nativeMap instanceof LinkedMultiValueMap)) { + headerAccessor.setHeader( + NativeMessageHeaderAccessor.NATIVE_HEADERS, + new LinkedMultiValueMap<>((Map>) nativeMap)); + } + } + + private static Message createMessageWithHeaders( + Message message, MessageHeaderAccessor messageHeaderAccessor) { + return MessageBuilder.fromMessage(message) + .copyHeaders(messageHeaderAccessor.toMessageHeaders()) + .build(); + } +} diff --git a/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/CapturingMessageHandler.groovy b/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/CapturingMessageHandler.groovy new file mode 100644 index 000000000000..5fa03d05c8be --- /dev/null +++ b/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/CapturingMessageHandler.groovy @@ -0,0 +1,26 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +import static io.opentelemetry.instrumentation.test.utils.TraceUtils.runUnderTrace + +import java.util.concurrent.CompletableFuture +import org.springframework.messaging.Message +import org.springframework.messaging.MessageHandler +import org.springframework.messaging.MessagingException + +class CapturingMessageHandler implements MessageHandler { + final CompletableFuture> captured = new CompletableFuture<>() + + @Override + void handleMessage(Message message) throws MessagingException { + runUnderTrace("handler") { + captured.complete(message) + } + } + + Message join() { + captured.join() + } +} diff --git a/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/ComplexPropagationTest.groovy b/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/ComplexPropagationTest.groovy new file mode 100644 index 000000000000..2f7cba6839b8 --- /dev/null +++ b/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/ComplexPropagationTest.groovy @@ -0,0 +1,160 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +import static io.opentelemetry.instrumentation.test.utils.TraceUtils.runUnderTrace + +import io.opentelemetry.api.GlobalOpenTelemetry +import io.opentelemetry.instrumentation.spring.messaging.SpringIntegrationTracing +import io.opentelemetry.instrumentation.test.LibraryInstrumentationSpecification +import io.opentelemetry.sdk.trace.data.SpanData +import java.util.concurrent.BlockingQueue +import java.util.concurrent.ExecutorService +import java.util.concurrent.Executors +import java.util.concurrent.LinkedBlockingQueue +import java.util.stream.Collectors +import org.springframework.boot.SpringApplication +import org.springframework.boot.SpringBootConfiguration +import org.springframework.boot.autoconfigure.EnableAutoConfiguration +import org.springframework.boot.context.event.ApplicationReadyEvent +import org.springframework.context.ConfigurableApplicationContext +import org.springframework.context.annotation.Bean +import org.springframework.context.event.EventListener +import org.springframework.integration.channel.DirectChannel +import org.springframework.integration.channel.ExecutorChannel +import org.springframework.integration.config.GlobalChannelInterceptor +import org.springframework.messaging.Message +import org.springframework.messaging.SubscribableChannel +import org.springframework.messaging.support.ChannelInterceptor +import org.springframework.messaging.support.MessageBuilder +import spock.lang.Shared + +class ComplexPropagationTest extends LibraryInstrumentationSpecification { + + @Shared + ConfigurableApplicationContext applicationContext + + def setupSpec() { + def app = new SpringApplication(ExternalQueueConfig) + applicationContext = app.run() + } + + def cleanupSpec() { + applicationContext?.close() + } + + def "should propagate context through a custom message queue"() { + given: + def sendChannel = applicationContext.getBean("sendChannel", SubscribableChannel) + def receiveChannel = applicationContext.getBean("receiveChannel", SubscribableChannel) + + def messageHandler = new CapturingMessageHandler() + receiveChannel.subscribe(messageHandler) + + when: + runUnderTrace("parent") { + sendChannel.send(MessageBuilder.withPayload("test") + .setHeader("theAnswer", "42") + .build()) + } + + then: + messageHandler.join() + + assertTraces(2) { + SpanData sendChannelSpan + + trace(0, 2) { + sendChannelSpan = span(1) + + span(0) { + name "parent" + } + span(1) { + name "application.sendChannel" + childOf span(0) + hasNoLinks() + } + } + trace(1, 2) { + span(0) { + name "application.receiveChannel" + hasNoParent() + hasLink sendChannelSpan + } + span(1) { + name "handler" + childOf span(0) + } + } + } + + cleanup: + receiveChannel.unsubscribe(messageHandler) + } + + // this setup simulates separate producer/consumer and some "external" message queue in between + @SpringBootConfiguration + @EnableAutoConfiguration + static class ExternalQueueConfig { + @Bean + SubscribableChannel sendChannel() { + new ExecutorChannel(Executors.newSingleThreadExecutor()) + } + + @Bean + SubscribableChannel receiveChannel() { + new DirectChannel() + } + + @Bean + BlockingQueue externalQueue() { + new LinkedBlockingQueue() + } + + @Bean + ExecutorService consumerThread() { + Executors.newSingleThreadExecutor() + } + + @EventListener(ApplicationReadyEvent) + void initialize() { + sendChannel().subscribe { message -> + externalQueue().offer(Payload.from(message)) + } + + consumerThread().execute({ + while (!Thread.interrupted()) { + def payload = externalQueue().take() + receiveChannel().send(payload.toMessage()) + } + }) + } + + @GlobalChannelInterceptor + @Bean + ChannelInterceptor otelInterceptor() { + SpringIntegrationTracing.create(GlobalOpenTelemetry.get()).newChannelInterceptor() + } + } + + static class Payload { + String body + Map headers + + static Payload from(Message message) { + def body = message.payload as String + Map headers = message.headers.entrySet().stream() + .filter({ kv -> kv.value instanceof String }) + .collect(Collectors.toMap({ it.key }, {it.value })) + new Payload(body: body, headers: headers) + } + + Message toMessage() { + MessageBuilder.withPayload(body) + .copyHeaders(headers) + .build() + } + } +} diff --git a/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/SpringIntegrationTracingTest.groovy b/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/SpringIntegrationTracingTest.groovy new file mode 100644 index 000000000000..a70400900bff --- /dev/null +++ b/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/SpringIntegrationTracingTest.groovy @@ -0,0 +1,231 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +import static io.opentelemetry.instrumentation.test.utils.TraceUtils.runUnderTrace + +import io.opentelemetry.api.GlobalOpenTelemetry +import io.opentelemetry.api.trace.SpanId +import io.opentelemetry.api.trace.TraceId +import io.opentelemetry.instrumentation.spring.messaging.SpringIntegrationTracing +import io.opentelemetry.instrumentation.test.LibraryInstrumentationSpecification +import io.opentelemetry.sdk.trace.data.SpanData +import java.util.concurrent.Executors +import org.springframework.boot.SpringApplication +import org.springframework.boot.SpringBootConfiguration +import org.springframework.boot.autoconfigure.EnableAutoConfiguration +import org.springframework.boot.context.event.ApplicationReadyEvent +import org.springframework.context.ConfigurableApplicationContext +import org.springframework.context.annotation.Bean +import org.springframework.context.event.EventListener +import org.springframework.integration.channel.DirectChannel +import org.springframework.integration.config.GlobalChannelInterceptor +import org.springframework.messaging.Message +import org.springframework.messaging.SubscribableChannel +import org.springframework.messaging.support.ChannelInterceptor +import org.springframework.messaging.support.ExecutorSubscribableChannel +import org.springframework.messaging.support.MessageBuilder +import spock.lang.Shared +import spock.lang.Unroll + +@Unroll +class SpringIntegrationTracingTest extends LibraryInstrumentationSpecification { + static final String TRACE_ID = TraceId.fromLongs(0, 42) + static final String SPAN_ID = SpanId.fromLong(123) + + @Shared + ConfigurableApplicationContext applicationContext + + def setupSpec() { + def app = new SpringApplication(MessageChannelsConfig) + applicationContext = app.run() + } + + def cleanupSpec() { + applicationContext?.close() + } + + def "should propagate context (#channelName)"() { + given: + def channel = applicationContext.getBean(channelName, SubscribableChannel) + + def messageHandler = new CapturingMessageHandler() + channel.subscribe(messageHandler) + + when: + runUnderTrace("parent") { + channel.send(MessageBuilder.withPayload("test") + .build()) + } + + then: + def capturedMessage = messageHandler.join() + + assertTraces(1) { + trace(0, 3) { + span(0) { + name "parent" + } + span(1) { + name interceptorSpanName + childOf span(0) + hasNoLinks() + } + span(2) { + name "handler" + childOf span(1) + } + + def interceptorSpan = span(1) + verifyCorrectSpanWasPropagated(capturedMessage, interceptorSpan) + } + } + + cleanup: + channel.unsubscribe(messageHandler) + + where: + channelName | interceptorSpanName + "directChannel" | "application.directChannel" + "executorChannel" | "executorChannel" + } + + def "should add link to span received in message headers (#channelName)"() { + given: + def channel = applicationContext.getBean(channelName, SubscribableChannel) + + def messageHandler = new CapturingMessageHandler() + channel.subscribe(messageHandler) + + when: + runUnderTrace("parent") { + channel.send(MessageBuilder.withPayload("test") + .setHeader("traceparent", "00-$TRACE_ID-$SPAN_ID-01") + .build()) + } + + then: + def capturedMessage = messageHandler.join() + + assertTraces(1) { + trace(0, 3) { + span(0) { + name "parent" + } + span(1) { + name interceptorSpanName + childOf span(0) + hasLink(TRACE_ID, SPAN_ID) + } + span(2) { + name "handler" + childOf span(1) + } + + def interceptorSpan = span(1) + verifyCorrectSpanWasPropagated(capturedMessage, interceptorSpan) + } + } + + cleanup: + channel.unsubscribe(messageHandler) + + where: + channelName | interceptorSpanName + "directChannel" | "application.directChannel" + "executorChannel" | "executorChannel" + } + + def "should handle multiple message channels in a chain"() { + given: + def channel1 = applicationContext.getBean("linkedChannel1", SubscribableChannel) + def channel2 = applicationContext.getBean("linkedChannel2", SubscribableChannel) + + def messageHandler = new CapturingMessageHandler() + channel2.subscribe(messageHandler) + + when: + runUnderTrace("parent") { + channel1.send(MessageBuilder.withPayload("test") + .build()) + } + + then: + def capturedMessage = messageHandler.join() + + assertTraces(1) { + trace(0, 4) { + span(0) { + name "parent" + } + span(1) { + name "application.linkedChannel1" + childOf span(0) + hasNoLinks() + } + span(2) { + name "application.linkedChannel2" + childOf span(1) + hasNoLinks() + } + span(3) { + name "handler" + childOf span(2) + } + + def lastChannelSpan = span(2) + verifyCorrectSpanWasPropagated(capturedMessage, lastChannelSpan) + } + } + + cleanup: + channel2.unsubscribe(messageHandler) + } + + static void verifyCorrectSpanWasPropagated(Message capturedMessage, SpanData parentSpan) { + def propagatedSpan = capturedMessage.headers.get("traceparent") as String + assert propagatedSpan.contains(parentSpan.traceId), "wrong trace id" + assert propagatedSpan.contains(parentSpan.spanId), "wrong span id" + } + + @SpringBootConfiguration + @EnableAutoConfiguration + static class MessageChannelsConfig { + @Bean + SubscribableChannel directChannel() { + new DirectChannel() + } + + @Bean + SubscribableChannel executorChannel() { + def channel = new ExecutorSubscribableChannel(Executors.newSingleThreadExecutor()) + // ExecutorSubscribableChannel isn't ChannelInterceptorAware, so we'll inject the interceptor manually + channel.addInterceptor(otelInterceptor()) + channel + } + + @Bean + SubscribableChannel linkedChannel1() { + new DirectChannel() + } + + @Bean + SubscribableChannel linkedChannel2() { + new DirectChannel() + } + + @EventListener(ApplicationReadyEvent) + void initialize() { + linkedChannel1().subscribe { message -> + linkedChannel2().send(message) + } + } + + @GlobalChannelInterceptor + @Bean + ChannelInterceptor otelInterceptor() { + SpringIntegrationTracing.create(GlobalOpenTelemetry.get()).newChannelInterceptor() + } + } +} diff --git a/settings.gradle b/settings.gradle index dbf1df73d4a9..0ce9b820eff2 100644 --- a/settings.gradle +++ b/settings.gradle @@ -271,6 +271,7 @@ include ':instrumentation:spark-2.3:javaagent' include ':instrumentation:spring:spring-batch-3.0:javaagent' include ':instrumentation:spring:spring-core-2.0:javaagent' include ':instrumentation:spring:spring-data-1.8:javaagent' +include ':instrumentation:spring:spring-integration-4.1:library' include ':instrumentation:spring:spring-scheduling-3.1:javaagent' include ':instrumentation:spring:spring-web-3.1:library' include ':instrumentation:spring:spring-webmvc-3.1:javaagent' diff --git a/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/asserts/SpanAssert.groovy b/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/asserts/SpanAssert.groovy index 6aa62f2ff718..22cedc3adc06 100644 --- a/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/asserts/SpanAssert.groovy +++ b/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/asserts/SpanAssert.groovy @@ -116,6 +116,10 @@ class SpanAssert { assert found } + def hasNoLinks() { + assert span.links.empty + } + def status(StatusCode expected) { assert span.status.statusCode == expected checked.status = true diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/LibraryTestRunner.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/LibraryTestRunner.java index 429f05d73376..ee9e3f0f7374 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/LibraryTestRunner.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/LibraryTestRunner.java @@ -10,6 +10,7 @@ import io.opentelemetry.api.trace.propagation.W3CTraceContextPropagator; import io.opentelemetry.context.Context; import io.opentelemetry.context.propagation.ContextPropagators; +import io.opentelemetry.exporter.logging.LoggingSpanExporter; import io.opentelemetry.sdk.OpenTelemetrySdk; import io.opentelemetry.sdk.common.CompletableResultCode; import io.opentelemetry.sdk.metrics.data.MetricData; @@ -43,6 +44,7 @@ public final class LibraryTestRunner implements InstrumentationTestRunner { .setTracerProvider( SdkTracerProvider.builder() .addSpanProcessor(new FlushTrackingSpanProcessor()) + .addSpanProcessor(SimpleSpanProcessor.create(new LoggingSpanExporter())) .addSpanProcessor(SimpleSpanProcessor.create(testExporter)) .build()) .setPropagators(ContextPropagators.create(W3CTraceContextPropagator.getInstance())) diff --git a/testing-common/testing-common.gradle b/testing-common/testing-common.gradle index f34be8e6e2f5..16697f35e3ca 100644 --- a/testing-common/testing-common.gradle +++ b/testing-common/testing-common.gradle @@ -28,6 +28,7 @@ dependencies { implementation "org.slf4j:jcl-over-slf4j" implementation "org.slf4j:jul-to-slf4j" implementation "io.opentelemetry:opentelemetry-extension-annotations" + implementation "io.opentelemetry:opentelemetry-exporter-logging" implementation project(':instrumentation-api') api "com.squareup.okhttp3:okhttp:4.9.0" From 0696d89413a016cf3c4a0f6a93a270e807ac0a2c Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Mon, 31 May 2021 17:11:37 +0200 Subject: [PATCH 02/12] testLatestDeps --- .../library/spring-integration-4.1-library.gradle | 8 ++++++-- .../src/test/groovy/SpringIntegrationTracingTest.groovy | 7 +++++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/instrumentation/spring/spring-integration-4.1/library/spring-integration-4.1-library.gradle b/instrumentation/spring/spring-integration-4.1/library/spring-integration-4.1-library.gradle index 607e16bbf536..15a45cf593e2 100644 --- a/instrumentation/spring/spring-integration-4.1/library/spring-integration-4.1-library.gradle +++ b/instrumentation/spring/spring-integration-4.1/library/spring-integration-4.1-library.gradle @@ -6,6 +6,10 @@ dependencies { library 'org.springframework.integration:spring-integration-core:4.1.0.RELEASE' - testImplementation "org.springframework.boot:spring-boot-starter-test:1.5.17.RELEASE" - testImplementation "org.springframework.boot:spring-boot-starter:1.5.17.RELEASE" + testLibrary "org.springframework.boot:spring-boot-starter-test:1.5.22.RELEASE" + testLibrary "org.springframework.boot:spring-boot-starter:1.5.22.RELEASE" +} + +test { + systemProperty "testLatestDeps", testLatestDeps } diff --git a/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/SpringIntegrationTracingTest.groovy b/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/SpringIntegrationTracingTest.groovy index a70400900bff..f14baae831d0 100644 --- a/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/SpringIntegrationTracingTest.groovy +++ b/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/SpringIntegrationTracingTest.groovy @@ -200,8 +200,11 @@ class SpringIntegrationTracingTest extends LibraryInstrumentationSpecification { @Bean SubscribableChannel executorChannel() { def channel = new ExecutorSubscribableChannel(Executors.newSingleThreadExecutor()) - // ExecutorSubscribableChannel isn't ChannelInterceptorAware, so we'll inject the interceptor manually - channel.addInterceptor(otelInterceptor()) + if (!Boolean.getBoolean("testLatestDeps")) { + // spring does not inject the interceptor in 4.1 because ExecutorSubscribableChannel isn't ChannelInterceptorAware + // in later versions spring injects the global interceptor into InterceptableChannel (which ExecutorSubscribableChannel is) + channel.addInterceptor(otelInterceptor()) + } channel } From db43a5a1dd59f45fc9e8d2d0c65f6f280bb7ff61 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Tue, 1 Jun 2021 16:49:02 +0200 Subject: [PATCH 03/12] attributesExtractor --- .../spring/messaging/SpringIntegrationTracingBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/SpringIntegrationTracingBuilder.java b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/SpringIntegrationTracingBuilder.java index 5499c17c8cfa..d88878413e76 100644 --- a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/SpringIntegrationTracingBuilder.java +++ b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/SpringIntegrationTracingBuilder.java @@ -28,7 +28,7 @@ public final class SpringIntegrationTracingBuilder { * Adds an additional {@link AttributesExtractor} to invoke to set attributes to instrumented * items. */ - public SpringIntegrationTracingBuilder addAttributeExtractor( + public SpringIntegrationTracingBuilder addAttributesExtractor( AttributesExtractor attributesExtractor) { additionalAttributeExtractors.add(attributesExtractor); return this; From 2c17109f5f86a5455852053e93fb78ecc0367a40 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Tue, 1 Jun 2021 17:57:06 +0200 Subject: [PATCH 04/12] errorprone --- .../instrumentation/spring/messaging/MessageHeadersSetter.java | 2 +- .../spring/messaging/MessageSpanLinkExtractor.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageHeadersSetter.java b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageHeadersSetter.java index 170d33f7f8ee..e4ce6db3089f 100644 --- a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageHeadersSetter.java +++ b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageHeadersSetter.java @@ -24,7 +24,7 @@ public void set(MessageHeaderAccessor carrier, String key, String value) { setNativeHeader(carrier, key, value); } - private void setNativeHeader(MessageHeaderAccessor carrier, String key, String value) { + private static void setNativeHeader(MessageHeaderAccessor carrier, String key, String value) { Object nativeMap = carrier.getHeader(NativeMessageHeaderAccessor.NATIVE_HEADERS); if (nativeMap instanceof Map) { ((Map>) nativeMap).put(key, singletonList(value)); diff --git a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageSpanLinkExtractor.java b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageSpanLinkExtractor.java index 241c4256ea01..ad290f7ae352 100644 --- a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageSpanLinkExtractor.java +++ b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageSpanLinkExtractor.java @@ -29,7 +29,7 @@ public SpanContext extract(Context parentContext, MessageWithChannel messageWith // SpanContext#equals() includes e.g. remote flag, which we don't really care about here // we just want to avoid adding links to spans with the same id, flags don't matter at all - private boolean referencesSameSpan(SpanContext spanFromMessage, SpanContext parentSpan) { + private static boolean referencesSameSpan(SpanContext spanFromMessage, SpanContext parentSpan) { return parentSpan.getTraceId().equals(spanFromMessage.getTraceId()) && parentSpan.getSpanId().equals(spanFromMessage.getSpanId()); } From 1f276be0df83edcc2adb6ef835fa7f3a4f1f40fd Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Wed, 2 Jun 2021 13:13:50 +0200 Subject: [PATCH 05/12] Code review comments --- .../api/instrumenter/SpanLinkExtractor.java | 5 ++++- .../api/instrumenter/InstrumenterTest.java | 22 +++++++++++++++++++ .../messaging/MessageHeadersGetter.java | 2 ++ .../messaging/MessageHeadersSetter.java | 2 ++ 4 files changed, 30 insertions(+), 1 deletion(-) diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanLinkExtractor.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanLinkExtractor.java index e0c5d3651a4d..91020e94fc76 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanLinkExtractor.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanLinkExtractor.java @@ -13,7 +13,10 @@ /** Extractor of a span link for a request. */ @FunctionalInterface public interface SpanLinkExtractor { - /** Extract a {@link SpanContext} that should be linked to the newly created span. */ + /** + * Extract a {@link SpanContext} that should be linked to the newly created span. Returning {@code + * SpanContext.getInvalid()} will not add any link to the span. + */ SpanContext extract(Context parentContext, REQUEST request); /** diff --git a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterTest.java b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterTest.java index fe2e21d39958..b82e9aa37d02 100644 --- a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterTest.java +++ b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterTest.java @@ -362,6 +362,28 @@ void shouldStartSpanWithGivenStartTime() { span -> span.hasName("test span").startsAt(startTime).endsAt(endTime))); } + @Test + void shouldNotAddInvalidLink() { + // given + Instrumenter instrumenter = + Instrumenter.newBuilder( + otelTesting.getOpenTelemetry(), "test", request -> "test span") + .addSpanLinkExtractor((parentContext, request) -> SpanContext.getInvalid()) + .newInstrumenter(); + + // when + Context context = instrumenter.start(Context.root(), "request"); + instrumenter.end(context, "request", "response", null); + + // then + otelTesting + .assertTraces() + .hasTracesSatisfyingExactly( + trace -> + trace.hasSpansSatisfyingExactly( + span -> span.hasName("test span").hasTotalRecordedLinks(0))); + } + private static LinkData expectedSpanLink() { return LinkData.create( SpanContext.create( diff --git a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageHeadersGetter.java b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageHeadersGetter.java index 3a6d0b2d8f58..90cd0545140b 100644 --- a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageHeadersGetter.java +++ b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageHeadersGetter.java @@ -13,6 +13,8 @@ import org.springframework.messaging.MessageHeaders; import org.springframework.messaging.support.NativeMessageHeaderAccessor; +// Reading native headers is required by some protocols, e.g. STOMP +// see https://github.com/spring-cloud/spring-cloud-sleuth/issues/716 for more details // Native headers logic inspired by // https://github.com/spring-cloud/spring-cloud-sleuth/blob/main/spring-cloud-sleuth-instrumentation/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/MessageHeaderPropagatorGetter.java enum MessageHeadersGetter implements TextMapGetter { diff --git a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageHeadersSetter.java b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageHeadersSetter.java index e4ce6db3089f..05eff313a3ce 100644 --- a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageHeadersSetter.java +++ b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageHeadersSetter.java @@ -13,6 +13,8 @@ import org.springframework.messaging.support.MessageHeaderAccessor; import org.springframework.messaging.support.NativeMessageHeaderAccessor; +// Setting native headers is required by some protocols, e.g. STOMP +// see https://github.com/spring-cloud/spring-cloud-sleuth/issues/716 for more details // Native headers logic inspired by // https://github.com/spring-cloud/spring-cloud-sleuth/blob/main/spring-cloud-sleuth-instrumentation/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/MessageHeaderPropagatorSetter.java enum MessageHeadersSetter implements TextMapSetter { From 86e0a60918d8add3f44f6a32b4d728a8701d8230 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Wed, 2 Jun 2021 14:57:41 +0200 Subject: [PATCH 06/12] rename package messaging -> integration --- .../spring/{messaging => integration}/ContextAndScope.java | 2 +- .../MessageChannelSpanNameExtractor.java | 2 +- .../spring/{messaging => integration}/MessageHeadersGetter.java | 2 +- .../spring/{messaging => integration}/MessageHeadersSetter.java | 2 +- .../{messaging => integration}/MessageSpanLinkExtractor.java | 2 +- .../spring/{messaging => integration}/MessageWithChannel.java | 2 +- .../{messaging => integration}/SpringIntegrationTracing.java | 2 +- .../SpringIntegrationTracingBuilder.java | 2 +- .../{messaging => integration}/TracingChannelInterceptor.java | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) rename instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/{messaging => integration}/ContextAndScope.java (90%) rename instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/{messaging => integration}/MessageChannelSpanNameExtractor.java (93%) rename instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/{messaging => integration}/MessageHeadersGetter.java (97%) rename instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/{messaging => integration}/MessageHeadersSetter.java (95%) rename instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/{messaging => integration}/MessageSpanLinkExtractor.java (95%) rename instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/{messaging => integration}/MessageWithChannel.java (89%) rename instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/{messaging => integration}/SpringIntegrationTracing.java (96%) rename instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/{messaging => integration}/SpringIntegrationTracingBuilder.java (97%) rename instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/{messaging => integration}/TracingChannelInterceptor.java (98%) diff --git a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/ContextAndScope.java b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/ContextAndScope.java similarity index 90% rename from instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/ContextAndScope.java rename to instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/ContextAndScope.java index d42341ee9119..30b1117b04cd 100644 --- a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/ContextAndScope.java +++ b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/ContextAndScope.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.instrumentation.spring.messaging; +package io.opentelemetry.instrumentation.spring.integration; import com.google.auto.value.AutoValue; import io.opentelemetry.context.Context; diff --git a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageChannelSpanNameExtractor.java b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/MessageChannelSpanNameExtractor.java similarity index 93% rename from instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageChannelSpanNameExtractor.java rename to instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/MessageChannelSpanNameExtractor.java index 20b569fae1c2..4eec19a612af 100644 --- a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageChannelSpanNameExtractor.java +++ b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/MessageChannelSpanNameExtractor.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.instrumentation.spring.messaging; +package io.opentelemetry.instrumentation.spring.integration; import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor; import org.springframework.integration.channel.AbstractMessageChannel; diff --git a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageHeadersGetter.java b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/MessageHeadersGetter.java similarity index 97% rename from instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageHeadersGetter.java rename to instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/MessageHeadersGetter.java index 90cd0545140b..8df012e88e02 100644 --- a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageHeadersGetter.java +++ b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/MessageHeadersGetter.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.instrumentation.spring.messaging; +package io.opentelemetry.instrumentation.spring.integration; import io.opentelemetry.context.propagation.TextMapGetter; import java.nio.charset.StandardCharsets; diff --git a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageHeadersSetter.java b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/MessageHeadersSetter.java similarity index 95% rename from instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageHeadersSetter.java rename to instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/MessageHeadersSetter.java index 05eff313a3ce..1564b5013bfe 100644 --- a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageHeadersSetter.java +++ b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/MessageHeadersSetter.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.instrumentation.spring.messaging; +package io.opentelemetry.instrumentation.spring.integration; import static java.util.Collections.singletonList; diff --git a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageSpanLinkExtractor.java b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/MessageSpanLinkExtractor.java similarity index 95% rename from instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageSpanLinkExtractor.java rename to instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/MessageSpanLinkExtractor.java index ad290f7ae352..8a3d1e133dbd 100644 --- a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageSpanLinkExtractor.java +++ b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/MessageSpanLinkExtractor.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.instrumentation.spring.messaging; +package io.opentelemetry.instrumentation.spring.integration; import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanContext; diff --git a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageWithChannel.java b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/MessageWithChannel.java similarity index 89% rename from instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageWithChannel.java rename to instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/MessageWithChannel.java index 159aaff680db..ee959799e35e 100644 --- a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/MessageWithChannel.java +++ b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/MessageWithChannel.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.instrumentation.spring.messaging; +package io.opentelemetry.instrumentation.spring.integration; import com.google.auto.value.AutoValue; import org.springframework.messaging.Message; diff --git a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/SpringIntegrationTracing.java b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/SpringIntegrationTracing.java similarity index 96% rename from instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/SpringIntegrationTracing.java rename to instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/SpringIntegrationTracing.java index a9f38ee30b1d..7329b7f38ce2 100644 --- a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/SpringIntegrationTracing.java +++ b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/SpringIntegrationTracing.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.instrumentation.spring.messaging; +package io.opentelemetry.instrumentation.spring.integration; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.context.propagation.ContextPropagators; diff --git a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/SpringIntegrationTracingBuilder.java b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/SpringIntegrationTracingBuilder.java similarity index 97% rename from instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/SpringIntegrationTracingBuilder.java rename to instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/SpringIntegrationTracingBuilder.java index d88878413e76..e5942f5bfb74 100644 --- a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/SpringIntegrationTracingBuilder.java +++ b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/SpringIntegrationTracingBuilder.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.instrumentation.spring.messaging; +package io.opentelemetry.instrumentation.spring.integration; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; diff --git a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/TracingChannelInterceptor.java b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/TracingChannelInterceptor.java similarity index 98% rename from instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/TracingChannelInterceptor.java rename to instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/TracingChannelInterceptor.java index 97c27df8089e..46b1b250ca6e 100644 --- a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/messaging/TracingChannelInterceptor.java +++ b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/TracingChannelInterceptor.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.instrumentation.spring.messaging; +package io.opentelemetry.instrumentation.spring.integration; import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; From d5e817f091312f10939b04e350649ec9db387074 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Wed, 2 Jun 2021 15:48:51 +0200 Subject: [PATCH 07/12] move package in groovy files too --- .../library/src/test/groovy/ComplexPropagationTest.groovy | 2 +- .../library/src/test/groovy/SpringIntegrationTracingTest.groovy | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/ComplexPropagationTest.groovy b/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/ComplexPropagationTest.groovy index 2f7cba6839b8..4abd1fbebd16 100644 --- a/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/ComplexPropagationTest.groovy +++ b/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/ComplexPropagationTest.groovy @@ -6,7 +6,7 @@ import static io.opentelemetry.instrumentation.test.utils.TraceUtils.runUnderTrace import io.opentelemetry.api.GlobalOpenTelemetry -import io.opentelemetry.instrumentation.spring.messaging.SpringIntegrationTracing +import io.opentelemetry.instrumentation.spring.integration.SpringIntegrationTracing import io.opentelemetry.instrumentation.test.LibraryInstrumentationSpecification import io.opentelemetry.sdk.trace.data.SpanData import java.util.concurrent.BlockingQueue diff --git a/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/SpringIntegrationTracingTest.groovy b/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/SpringIntegrationTracingTest.groovy index f14baae831d0..d20b6e8dea81 100644 --- a/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/SpringIntegrationTracingTest.groovy +++ b/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/SpringIntegrationTracingTest.groovy @@ -8,7 +8,7 @@ import static io.opentelemetry.instrumentation.test.utils.TraceUtils.runUnderTra import io.opentelemetry.api.GlobalOpenTelemetry import io.opentelemetry.api.trace.SpanId import io.opentelemetry.api.trace.TraceId -import io.opentelemetry.instrumentation.spring.messaging.SpringIntegrationTracing +import io.opentelemetry.instrumentation.spring.integration.SpringIntegrationTracing import io.opentelemetry.instrumentation.test.LibraryInstrumentationSpecification import io.opentelemetry.sdk.trace.data.SpanData import java.util.concurrent.Executors From 7c8d614f4c9734580ab3b54fcd916383ffeffddd Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Tue, 8 Jun 2021 14:36:49 +0200 Subject: [PATCH 08/12] thread local map --- .../TracingChannelInterceptor.java | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/TracingChannelInterceptor.java b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/TracingChannelInterceptor.java index 46b1b250ca6e..7d35d4a65e26 100644 --- a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/TracingChannelInterceptor.java +++ b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/TracingChannelInterceptor.java @@ -6,9 +6,9 @@ package io.opentelemetry.instrumentation.spring.integration; import io.opentelemetry.context.Context; -import io.opentelemetry.context.Scope; import io.opentelemetry.context.propagation.ContextPropagators; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; +import java.util.IdentityHashMap; import java.util.List; import java.util.Map; import org.springframework.messaging.Message; @@ -21,8 +21,9 @@ import org.springframework.util.LinkedMultiValueMap; final class TracingChannelInterceptor implements ExecutorChannelInterceptor { - private static final String CONTEXT_AND_SCOPE_KEY = ContextAndScope.class.getName(); - private static final String SCOPE_KEY = TracingChannelInterceptor.class.getName() + ".scope"; + + private static final ThreadLocal> LOCAL_CONTEXT_AND_SCOPE = + ThreadLocal.withInitial(IdentityHashMap::new); private final ContextPropagators propagators; private final Instrumenter instrumenter; @@ -47,7 +48,7 @@ public Message preSend(Message message, MessageChannel messageChannel) { propagators .getTextMapPropagator() .inject(context, messageHeaderAccessor, MessageHeadersSetter.INSTANCE); - messageHeaderAccessor.setHeader(CONTEXT_AND_SCOPE_KEY, ContextAndScope.makeCurrent(context)); + LOCAL_CONTEXT_AND_SCOPE.get().put(messageChannel, ContextAndScope.makeCurrent(context)); return createMessageWithHeaders(message, messageHeaderAccessor); } @@ -57,8 +58,7 @@ public void postSend(Message message, MessageChannel messageChannel, boolean @Override public void afterSendCompletion( Message message, MessageChannel messageChannel, boolean sent, Exception e) { - ContextAndScope contextAndScope = - message.getHeaders().get(CONTEXT_AND_SCOPE_KEY, ContextAndScope.class); + ContextAndScope contextAndScope = LOCAL_CONTEXT_AND_SCOPE.get().remove(messageChannel); if (contextAndScope != null) { contextAndScope.close(); MessageWithChannel messageWithChannel = MessageWithChannel.create(message, messageChannel); @@ -88,18 +88,18 @@ public Message beforeHandle( propagators .getTextMapPropagator() .extract(Context.current(), messageWithChannel, MessageHeadersGetter.INSTANCE); - Scope scope = context.makeCurrent(); - MessageHeaderAccessor messageHeaderAccessor = MessageHeaderAccessor.getMutableAccessor(message); - messageHeaderAccessor.setHeader(SCOPE_KEY, scope); - return createMessageWithHeaders(message, messageHeaderAccessor); + // beforeHandle()/afterMessageHandles() always execute in a different thread than send(), so + // there's no real risk of overwriting the send() context + LOCAL_CONTEXT_AND_SCOPE.get().put(channel, ContextAndScope.makeCurrent(context)); + return message; } @Override public void afterMessageHandled( Message message, MessageChannel channel, MessageHandler handler, Exception ex) { - Scope scope = message.getHeaders().get(SCOPE_KEY, Scope.class); - if (scope != null) { - scope.close(); + ContextAndScope contextAndScope = LOCAL_CONTEXT_AND_SCOPE.get().remove(channel); + if (contextAndScope != null) { + contextAndScope.close(); } } From 3e25f12f3178c8fd2ab7fbd1c985a46b3440a185 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Tue, 8 Jun 2021 16:53:50 +0200 Subject: [PATCH 09/12] Revert "thread local map" This reverts commit 7c8d614f4c9734580ab3b54fcd916383ffeffddd. --- .../TracingChannelInterceptor.java | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/TracingChannelInterceptor.java b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/TracingChannelInterceptor.java index 7d35d4a65e26..46b1b250ca6e 100644 --- a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/TracingChannelInterceptor.java +++ b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/TracingChannelInterceptor.java @@ -6,9 +6,9 @@ package io.opentelemetry.instrumentation.spring.integration; import io.opentelemetry.context.Context; +import io.opentelemetry.context.Scope; import io.opentelemetry.context.propagation.ContextPropagators; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; -import java.util.IdentityHashMap; import java.util.List; import java.util.Map; import org.springframework.messaging.Message; @@ -21,9 +21,8 @@ import org.springframework.util.LinkedMultiValueMap; final class TracingChannelInterceptor implements ExecutorChannelInterceptor { - - private static final ThreadLocal> LOCAL_CONTEXT_AND_SCOPE = - ThreadLocal.withInitial(IdentityHashMap::new); + private static final String CONTEXT_AND_SCOPE_KEY = ContextAndScope.class.getName(); + private static final String SCOPE_KEY = TracingChannelInterceptor.class.getName() + ".scope"; private final ContextPropagators propagators; private final Instrumenter instrumenter; @@ -48,7 +47,7 @@ public Message preSend(Message message, MessageChannel messageChannel) { propagators .getTextMapPropagator() .inject(context, messageHeaderAccessor, MessageHeadersSetter.INSTANCE); - LOCAL_CONTEXT_AND_SCOPE.get().put(messageChannel, ContextAndScope.makeCurrent(context)); + messageHeaderAccessor.setHeader(CONTEXT_AND_SCOPE_KEY, ContextAndScope.makeCurrent(context)); return createMessageWithHeaders(message, messageHeaderAccessor); } @@ -58,7 +57,8 @@ public void postSend(Message message, MessageChannel messageChannel, boolean @Override public void afterSendCompletion( Message message, MessageChannel messageChannel, boolean sent, Exception e) { - ContextAndScope contextAndScope = LOCAL_CONTEXT_AND_SCOPE.get().remove(messageChannel); + ContextAndScope contextAndScope = + message.getHeaders().get(CONTEXT_AND_SCOPE_KEY, ContextAndScope.class); if (contextAndScope != null) { contextAndScope.close(); MessageWithChannel messageWithChannel = MessageWithChannel.create(message, messageChannel); @@ -88,18 +88,18 @@ public Message beforeHandle( propagators .getTextMapPropagator() .extract(Context.current(), messageWithChannel, MessageHeadersGetter.INSTANCE); - // beforeHandle()/afterMessageHandles() always execute in a different thread than send(), so - // there's no real risk of overwriting the send() context - LOCAL_CONTEXT_AND_SCOPE.get().put(channel, ContextAndScope.makeCurrent(context)); - return message; + Scope scope = context.makeCurrent(); + MessageHeaderAccessor messageHeaderAccessor = MessageHeaderAccessor.getMutableAccessor(message); + messageHeaderAccessor.setHeader(SCOPE_KEY, scope); + return createMessageWithHeaders(message, messageHeaderAccessor); } @Override public void afterMessageHandled( Message message, MessageChannel channel, MessageHandler handler, Exception ex) { - ContextAndScope contextAndScope = LOCAL_CONTEXT_AND_SCOPE.get().remove(channel); - if (contextAndScope != null) { - contextAndScope.close(); + Scope scope = message.getHeaders().get(SCOPE_KEY, Scope.class); + if (scope != null) { + scope.close(); } } From 4f6db46a41a84c663908e3a7e29366789c3c27a0 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Thu, 10 Jun 2021 10:21:04 +0200 Subject: [PATCH 10/12] Always extract parent SpanContext from the incoming message --- .../api/instrumenter/InstrumenterBuilder.java | 15 ++++-- .../integration/MessageSpanLinkExtractor.java | 36 ------------- .../SpringIntegrationTracingBuilder.java | 14 ++--- .../test/groovy/ComplexPropagationTest.groovy | 20 +++----- .../SpringIntegrationTracingTest.groovy | 51 ------------------- 5 files changed, 21 insertions(+), 115 deletions(-) delete mode 100644 instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/MessageSpanLinkExtractor.java diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterBuilder.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterBuilder.java index 83658da26d02..8446a73c8ac7 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterBuilder.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterBuilder.java @@ -134,8 +134,7 @@ public Instrumenter newClientInstrumenter(TextMapSetter newServerInstrumenter(TextMapGetter getter) { - return newInstrumenter( - InstrumenterConstructor.propagatingFromUpstream(getter), SpanKindExtractor.alwaysServer()); + return newUpstreamPropagatingInstrumenter(SpanKindExtractor.alwaysServer(), getter); } /** @@ -153,9 +152,17 @@ public Instrumenter newProducerInstrumenter(TextMapSetter newConsumerInstrumenter(TextMapGetter getter) { + return newUpstreamPropagatingInstrumenter(SpanKindExtractor.alwaysConsumer(), getter); + } + + /** + * Returns a new {@link Instrumenter} which will create spans with kind determined by the passed + * {@code spanKindExtractor} and extract context from requests + */ + public Instrumenter newUpstreamPropagatingInstrumenter( + SpanKindExtractor spanKindExtractor, TextMapGetter getter) { return newInstrumenter( - InstrumenterConstructor.propagatingFromUpstream(getter), - SpanKindExtractor.alwaysConsumer()); + InstrumenterConstructor.propagatingFromUpstream(getter), spanKindExtractor); } /** diff --git a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/MessageSpanLinkExtractor.java b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/MessageSpanLinkExtractor.java deleted file mode 100644 index 8a3d1e133dbd..000000000000 --- a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/MessageSpanLinkExtractor.java +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.instrumentation.spring.integration; - -import io.opentelemetry.api.trace.Span; -import io.opentelemetry.api.trace.SpanContext; -import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.api.instrumenter.SpanLinkExtractor; - -final class MessageSpanLinkExtractor implements SpanLinkExtractor { - private final SpanLinkExtractor delegate; - - MessageSpanLinkExtractor(SpanLinkExtractor delegate) { - this.delegate = delegate; - } - - @Override - public SpanContext extract(Context parentContext, MessageWithChannel messageWithChannel) { - SpanContext spanFromMessage = delegate.extract(parentContext, messageWithChannel); - SpanContext parentSpan = Span.fromContext(parentContext).getSpanContext(); - if (referencesSameSpan(spanFromMessage, parentSpan)) { - return SpanContext.getInvalid(); - } - return spanFromMessage; - } - - // SpanContext#equals() includes e.g. remote flag, which we don't really care about here - // we just want to avoid adding links to spans with the same id, flags don't matter at all - private static boolean referencesSameSpan(SpanContext spanFromMessage, SpanContext parentSpan) { - return parentSpan.getTraceId().equals(spanFromMessage.getTraceId()) - && parentSpan.getSpanId().equals(spanFromMessage.getSpanId()); - } -} diff --git a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/SpringIntegrationTracingBuilder.java b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/SpringIntegrationTracingBuilder.java index e5942f5bfb74..392ff5d9ed00 100644 --- a/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/SpringIntegrationTracingBuilder.java +++ b/instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/SpringIntegrationTracingBuilder.java @@ -5,10 +5,11 @@ package io.opentelemetry.instrumentation.spring.integration; +import static io.opentelemetry.instrumentation.api.instrumenter.SpanKindExtractor.alwaysInternal; + import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; -import io.opentelemetry.instrumentation.api.instrumenter.SpanLinkExtractor; import java.util.ArrayList; import java.util.List; @@ -39,18 +40,11 @@ public SpringIntegrationTracingBuilder addAttributesExtractor( * SpringIntegrationTracingBuilder}. */ public SpringIntegrationTracing build() { - MessageChannelSpanNameExtractor spanNameExtractor = new MessageChannelSpanNameExtractor(); - MessageSpanLinkExtractor spanLinkExtractor = - new MessageSpanLinkExtractor( - SpanLinkExtractor.fromUpstreamRequest( - openTelemetry.getPropagators(), MessageHeadersGetter.INSTANCE)); - Instrumenter instrumenter = Instrumenter.newBuilder( - openTelemetry, INSTRUMENTATION_NAME, spanNameExtractor) - .addSpanLinkExtractor(spanLinkExtractor) + openTelemetry, INSTRUMENTATION_NAME, new MessageChannelSpanNameExtractor()) .addAttributesExtractors(additionalAttributeExtractors) - .newInstrumenter(); + .newUpstreamPropagatingInstrumenter(alwaysInternal(), MessageHeadersGetter.INSTANCE); return new SpringIntegrationTracing(openTelemetry.getPropagators(), instrumenter); } } diff --git a/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/ComplexPropagationTest.groovy b/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/ComplexPropagationTest.groovy index 4abd1fbebd16..d3e9574fdee3 100644 --- a/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/ComplexPropagationTest.groovy +++ b/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/ComplexPropagationTest.groovy @@ -62,30 +62,22 @@ class ComplexPropagationTest extends LibraryInstrumentationSpecification { then: messageHandler.join() - assertTraces(2) { - SpanData sendChannelSpan - - trace(0, 2) { - sendChannelSpan = span(1) - + assertTraces(1) { + trace(0, 4) { span(0) { name "parent" } span(1) { name "application.sendChannel" childOf span(0) - hasNoLinks() } - } - trace(1, 2) { - span(0) { + span(2) { name "application.receiveChannel" - hasNoParent() - hasLink sendChannelSpan + childOf span(1) } - span(1) { + span(3) { name "handler" - childOf span(0) + childOf span(2) } } } diff --git a/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/SpringIntegrationTracingTest.groovy b/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/SpringIntegrationTracingTest.groovy index d20b6e8dea81..67392c2c6bfc 100644 --- a/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/SpringIntegrationTracingTest.groovy +++ b/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/SpringIntegrationTracingTest.groovy @@ -6,8 +6,6 @@ import static io.opentelemetry.instrumentation.test.utils.TraceUtils.runUnderTrace import io.opentelemetry.api.GlobalOpenTelemetry -import io.opentelemetry.api.trace.SpanId -import io.opentelemetry.api.trace.TraceId import io.opentelemetry.instrumentation.spring.integration.SpringIntegrationTracing import io.opentelemetry.instrumentation.test.LibraryInstrumentationSpecification import io.opentelemetry.sdk.trace.data.SpanData @@ -31,8 +29,6 @@ import spock.lang.Unroll @Unroll class SpringIntegrationTracingTest extends LibraryInstrumentationSpecification { - static final String TRACE_ID = TraceId.fromLongs(0, 42) - static final String SPAN_ID = SpanId.fromLong(123) @Shared ConfigurableApplicationContext applicationContext @@ -70,53 +66,6 @@ class SpringIntegrationTracingTest extends LibraryInstrumentationSpecification { span(1) { name interceptorSpanName childOf span(0) - hasNoLinks() - } - span(2) { - name "handler" - childOf span(1) - } - - def interceptorSpan = span(1) - verifyCorrectSpanWasPropagated(capturedMessage, interceptorSpan) - } - } - - cleanup: - channel.unsubscribe(messageHandler) - - where: - channelName | interceptorSpanName - "directChannel" | "application.directChannel" - "executorChannel" | "executorChannel" - } - - def "should add link to span received in message headers (#channelName)"() { - given: - def channel = applicationContext.getBean(channelName, SubscribableChannel) - - def messageHandler = new CapturingMessageHandler() - channel.subscribe(messageHandler) - - when: - runUnderTrace("parent") { - channel.send(MessageBuilder.withPayload("test") - .setHeader("traceparent", "00-$TRACE_ID-$SPAN_ID-01") - .build()) - } - - then: - def capturedMessage = messageHandler.join() - - assertTraces(1) { - trace(0, 3) { - span(0) { - name "parent" - } - span(1) { - name interceptorSpanName - childOf span(0) - hasLink(TRACE_ID, SPAN_ID) } span(2) { name "handler" From a60ac9839724f7547ec82e01e792949ed3fc52b7 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Thu, 10 Jun 2021 12:01:52 +0200 Subject: [PATCH 11/12] checkstyle --- .../instrumentation/api/instrumenter/InstrumenterBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterBuilder.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterBuilder.java index 8446a73c8ac7..2bad44b27ba0 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterBuilder.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterBuilder.java @@ -157,7 +157,7 @@ public Instrumenter newConsumerInstrumenter(TextMapGetter newUpstreamPropagatingInstrumenter( SpanKindExtractor spanKindExtractor, TextMapGetter getter) { From c0210dc0308b1cf11a30ba97c6fef90eca961198 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Thu, 10 Jun 2021 13:13:38 +0200 Subject: [PATCH 12/12] codenarc --- .../library/src/test/groovy/ComplexPropagationTest.groovy | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/ComplexPropagationTest.groovy b/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/ComplexPropagationTest.groovy index d3e9574fdee3..9db1d92e9a30 100644 --- a/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/ComplexPropagationTest.groovy +++ b/instrumentation/spring/spring-integration-4.1/library/src/test/groovy/ComplexPropagationTest.groovy @@ -8,7 +8,6 @@ import static io.opentelemetry.instrumentation.test.utils.TraceUtils.runUnderTra import io.opentelemetry.api.GlobalOpenTelemetry import io.opentelemetry.instrumentation.spring.integration.SpringIntegrationTracing import io.opentelemetry.instrumentation.test.LibraryInstrumentationSpecification -import io.opentelemetry.sdk.trace.data.SpanData import java.util.concurrent.BlockingQueue import java.util.concurrent.ExecutorService import java.util.concurrent.Executors @@ -139,7 +138,7 @@ class ComplexPropagationTest extends LibraryInstrumentationSpecification { def body = message.payload as String Map headers = message.headers.entrySet().stream() .filter({ kv -> kv.value instanceof String }) - .collect(Collectors.toMap({ it.key }, {it.value })) + .collect(Collectors.toMap({ it.key }, { it.value })) new Payload(body: body, headers: headers) }