From 1f5628548df33f11ddaa509638c5a69f598bf397 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Thu, 19 May 2022 18:03:11 +0200 Subject: [PATCH] Remove TimeExtractor and use internal API for setting start/end timestamps (#6051) * Remove TimeExtractor and use internal API for setting start/end timestamps * code review comments --- docs/contributing/using-instrumenter-api.md | 35 --------- .../api/instrumenter/Instrumenter.java | 61 ++++++++++------ .../api/instrumenter/InstrumenterBuilder.java | 7 +- .../api/instrumenter/TimeExtractor.java | 3 + .../api/internal/InstrumenterUtil.java | 71 +++++++++++++++++++ .../api/instrumenter/InstrumenterTest.java | 40 ----------- .../jms/MessageWithDestinationTest.java | 32 +++------ .../JmsMessageConsumerInstrumentation.java | 13 +++- .../jms/JmsMessageTimeExtractor.java | 24 ------- .../instrumentation/jms/JmsSingletons.java | 1 - .../jms/MessageWithDestination.java | 32 ++------- .../javaagent/instrumentation/jms/Timer.java | 2 +- .../KafkaConsumerInstrumentation.java | 16 +++-- .../kafkaclients/KafkaSingletons.java | 6 +- .../internal/KafkaConsumerTimeExtractor.java | 24 ------- .../internal/KafkaInstrumenterFactory.java | 12 +--- .../KafkaReceiveAttributesGetter.java | 27 +++---- .../kafka/internal/ReceivedRecords.java | 34 --------- .../v3_8/NettyChannelInstrumentation.java | 21 ++++-- .../netty/v3_8/client/ConnectionListener.java | 16 ++++- .../v3_8/client/NettyClientSingletons.java | 1 - .../client/NettyConnectionTimeExtractor.java | 26 ------- .../NettyClientInstrumenterFactory.java | 4 +- .../client/NettyConnectionTimeExtractor.java | 26 ------- .../NettyErrorOnlyConnectionInstrumenter.java | 9 ++- .../client/NettySslErrorOnlyInstrumenter.java | 9 ++- .../v4/common/client/NettySslRequest.java | 5 +- .../common/client/NettySslTimeExtractor.java | 24 ------- .../netty/common/NettyConnectionRequest.java | 6 +- .../instrumentation/netty/common/Timer.java | 16 ++++- .../RabbitChannelInstrumentation.java | 14 ++-- .../rabbitmq/RabbitReceiveTimeExtractor.java | 25 ------- .../rabbitmq/RabbitSingletons.java | 1 - .../rabbitmq/ReceiveRequest.java | 16 +---- 34 files changed, 244 insertions(+), 415 deletions(-) create mode 100644 instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/InstrumenterUtil.java delete mode 100644 instrumentation/jms-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jms/JmsMessageTimeExtractor.java delete mode 100644 instrumentation/kafka/kafka-clients/kafka-clients-common/library/src/main/java/io/opentelemetry/instrumentation/kafka/internal/KafkaConsumerTimeExtractor.java delete mode 100644 instrumentation/kafka/kafka-clients/kafka-clients-common/library/src/main/java/io/opentelemetry/instrumentation/kafka/internal/ReceivedRecords.java delete mode 100644 instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/client/NettyConnectionTimeExtractor.java delete mode 100644 instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4/common/client/NettyConnectionTimeExtractor.java delete mode 100644 instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4/common/client/NettySslTimeExtractor.java delete mode 100644 instrumentation/rabbitmq-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rabbitmq/RabbitReceiveTimeExtractor.java diff --git a/docs/contributing/using-instrumenter-api.md b/docs/contributing/using-instrumenter-api.md index 15402803ddc7..bd2bd4ce9037 100644 --- a/docs/contributing/using-instrumenter-api.md +++ b/docs/contributing/using-instrumenter-api.md @@ -319,41 +319,6 @@ default `jdk()` implementation that removes the known JDK wrapper exception type You can set the `ErrorCauseExtractor` in the `InstrumenterBuilder` using the `setErrorCauseExtractor()` method. -### Provide custom operation start and end times using the `TimeExtractor` - -In some cases, the instrumented library provides a way to retrieve accurate timestamps of when the -operation starts and ends. The `TimeExtractor` interface can be used to -feed this information into OpenTelemetry trace and metrics data. - -`extractStartTime()` can only extract the timestamp from the request. `extractEndTime()` -accepts the request, an optional response, and an optional `Throwable` error. Consider the following -example: - -```java -class MyTimeExtractor implements TimeExtractor { - - @Override - public Instant extractStartTime(Request request) { - return request.startTimestamp(); - } - - @Override - public Instant extractEndTime(Request request, @Nullable Response response, @Nullable Throwable error) { - if (response != null) { - return response.endTimestamp(); - } - return request.clock().now(); - } -} -``` - -The sample implementations above use the request to retrieve the start timestamp. The response is -used to compute the end time if it is available; in case it is missing (for example, when an error -occurs) the same time source is used to compute the current timestamp. - -You can set the time extractor in the `InstrumenterBuilder` using the `setTimeExtractor()` -method. - ### Register metrics by implementing the `OperationMetrics` and `OperationListener` If you need to add metrics to the `Instrumenter` you can implement the `OperationMetrics` 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 ebb9f1f0911f..0a0497ffb685 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 @@ -76,7 +76,6 @@ public static InstrumenterBuilder builder private final List> contextCustomizers; private final List operationListeners; private final ErrorCauseExtractor errorCauseExtractor; - @Nullable private final TimeExtractor timeExtractor; private final boolean enabled; private final SpanSuppressor spanSuppressor; @@ -91,7 +90,6 @@ public static InstrumenterBuilder builder this.contextCustomizers = new ArrayList<>(builder.contextCustomizers); this.operationListeners = builder.buildOperationListeners(); this.errorCauseExtractor = builder.errorCauseExtractor; - this.timeExtractor = builder.timeExtractor; this.enabled = builder.enabled; this.spanSuppressor = builder.buildSpanSuppressor(); } @@ -129,6 +127,38 @@ public boolean shouldStart(Context parentContext, REQUEST request) { * object of this operation. */ public Context start(Context parentContext, REQUEST request) { + return doStart(parentContext, request, null); + } + + /** + * Ends an instrumented operation. It is of extreme importance for this method to be always called + * after {@link #start(Context, Object) start()}. Calling {@code start()} without later {@code + * end()} will result in inaccurate or wrong telemetry and context leaks. + * + *

The {@code context} must be the same value that was returned from {@link #start(Context, + * Object)}. The {@code request} parameter is the request object of the operation, {@code + * response} is the response object of the operation, and {@code error} is an exception that was + * thrown by the operation or {@code null} if no error occurred. + */ + public void end( + Context context, REQUEST request, @Nullable RESPONSE response, @Nullable Throwable error) { + doEnd(context, request, response, error, null); + } + + /** Internal method for creating spans with given start/end timestamps. */ + Context startAndEnd( + Context parentContext, + REQUEST request, + @Nullable RESPONSE response, + @Nullable Throwable error, + Instant startTime, + Instant endTime) { + Context context = doStart(parentContext, request, startTime); + doEnd(context, request, response, error, endTime); + return context; + } + + private Context doStart(Context parentContext, REQUEST request, @Nullable Instant startTime) { SpanKind spanKind = spanKindExtractor.extract(request); SpanBuilder spanBuilder = tracer @@ -136,9 +166,7 @@ public Context start(Context parentContext, REQUEST request) { .setSpanKind(spanKind) .setParent(parentContext); - Instant startTime = null; - if (timeExtractor != null) { - startTime = timeExtractor.extractStartTime(request); + if (startTime != null) { spanBuilder.setStartTimestamp(startTime); } @@ -176,18 +204,12 @@ public Context start(Context parentContext, REQUEST request) { return spanSuppressor.storeInContext(context, spanKind, span); } - /** - * Ends an instrumented operation. It is of extreme importance for this method to be always called - * after {@link #start(Context, Object) start()}. Calling {@code start()} without later {@code - * end()} will result in inaccurate or wrong telemetry and context leaks. - * - *

The {@code context} must be the same value that was returned from {@link #start(Context, - * Object)}. The {@code request} parameter is the request object of the operation, {@code - * response} is the response object of the operation, and {@code error} is an exception that was - * thrown by the operation or {@code null} if no error occurred. - */ - public void end( - Context context, REQUEST request, @Nullable RESPONSE response, @Nullable Throwable error) { + private void doEnd( + Context context, + REQUEST request, + @Nullable RESPONSE response, + @Nullable Throwable error, + @Nullable Instant endTime) { Span span = Span.fromContext(context); if (error != null) { @@ -201,11 +223,6 @@ public void end( } span.setAllAttributes(attributes); - Instant endTime = null; - if (timeExtractor != null) { - endTime = timeExtractor.extractEndTime(request, response, error); - } - if (!operationListeners.isEmpty()) { long endNanos = getNanos(endTime); ListIterator i = 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 aff79810d2da..25d7e8cca985 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 @@ -5,8 +5,6 @@ package io.opentelemetry.instrumentation.api.instrumenter; -import static java.util.Objects.requireNonNull; - import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.metrics.Meter; import io.opentelemetry.api.metrics.MeterBuilder; @@ -57,7 +55,6 @@ public final class InstrumenterBuilder { SpanStatusExtractor spanStatusExtractor = SpanStatusExtractor.getDefault(); ErrorCauseExtractor errorCauseExtractor = ErrorCauseExtractor.jdk(); - @Nullable TimeExtractor timeExtractor = null; boolean enabled = true; InstrumenterBuilder( @@ -181,10 +178,12 @@ public InstrumenterBuilder setErrorCauseExtractor( * {@link TimeExtractor} will be used to generate any duration metrics, but the internal metric * timestamp (when it occurred) will always be stamped with "now" when the metric is recorded * (i.e. there is no way to back date a metric recording). + * + * @deprecated Setting operation start and end times is not currently supported. */ + @Deprecated public InstrumenterBuilder setTimeExtractor( TimeExtractor timeExtractor) { - this.timeExtractor = requireNonNull(timeExtractor); return this; } diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/TimeExtractor.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/TimeExtractor.java index cf95ab9ebe5b..29eff4340321 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/TimeExtractor.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/TimeExtractor.java @@ -15,7 +15,10 @@ * {@link TimeExtractor} will be used to generate any duration metrics, but the internal metric * timestamp (when it occurred) will always be stamped with "now" when the metric is recorded (i.e. * there is no way to back date a metric recording). + * + * @deprecated Setting operation start and end times is not currently supported. */ +@Deprecated public interface TimeExtractor { /** Returns the timestamp marking the start of the request processing. */ diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/InstrumenterUtil.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/InstrumenterUtil.java new file mode 100644 index 000000000000..94b7f9148a2a --- /dev/null +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/InstrumenterUtil.java @@ -0,0 +1,71 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.api.internal; + +import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.time.Instant; +import java.util.logging.Level; +import java.util.logging.Logger; +import javax.annotation.Nullable; + +/** + * This class is internal and is hence not for public use. Its APIs are unstable and can change at + * any time. + */ +public final class InstrumenterUtil { + + private static final Logger logger = Logger.getLogger(InstrumenterUtil.class.getName()); + + private static final Method startAndEndMethod; + + static { + Method method = null; + try { + method = + Instrumenter.class.getDeclaredMethod( + "startAndEnd", + Context.class, + Object.class, + Object.class, + Throwable.class, + Instant.class, + Instant.class); + method.setAccessible(true); + } catch (NoSuchMethodException e) { + logger.log( + Level.WARNING, "Could not get Instrumenter#startAndEnd() method with reflection", e); + } + startAndEndMethod = method; + } + + public static Context startAndEnd( + Instrumenter instrumenter, + Context parentContext, + REQUEST request, + @Nullable RESPONSE response, + @Nullable Throwable error, + Instant startTime, + Instant endTime) { + + if (startAndEndMethod == null) { + // already logged a warning when this class initialized + return parentContext; + } + try { + return (Context) + startAndEndMethod.invoke( + instrumenter, parentContext, request, response, error, startTime, endTime); + } catch (InvocationTargetException | IllegalAccessException e) { + logger.log(Level.WARNING, "Error occurred when calling Instrumenter#startAndEnd()", e); + return parentContext; + } + } + + private InstrumenterUtil() {} +} 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 5750ff30fe68..b7620a81816c 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 @@ -29,7 +29,6 @@ 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; import java.util.HashMap; import java.util.Map; @@ -126,20 +125,6 @@ public void extract( } } - static class TestTimeExtractor implements TimeExtractor { - - @Override - public Instant extractStartTime(Instant request) { - return request; - } - - @Override - public Instant extractEndTime( - Instant request, @Nullable Instant response, @Nullable Throwable error) { - return response; - } - } - static class MapGetter implements TextMapGetter> { @Override @@ -439,31 +424,6 @@ public void onEnd(Context context, Attributes endAttributes, long endNanos) { assertThat(Span.fromContext(endContext.get()).getSpanContext().isValid()).isTrue(); } - @Test - void shouldStartSpanWithGivenStartTime() { - // given - Instrumenter instrumenter = - Instrumenter.builder( - otelTesting.getOpenTelemetry(), "test", request -> "test span") - .setTimeExtractor(new TestTimeExtractor()) - .newInstrumenter(); - - Instant startTime = Instant.ofEpochSecond(100); - Instant endTime = Instant.ofEpochSecond(123); - - // when - Context context = instrumenter.start(Context.root(), startTime); - instrumenter.end(context, startTime, endTime, null); - - // then - otelTesting - .assertTraces() - .hasTracesSatisfyingExactly( - trace -> - trace.hasSpansSatisfyingExactly( - span -> span.hasName("test span").startsAt(startTime).endsAt(endTime))); - } - @Test void shouldNotAddInvalidLink() { // given diff --git a/instrumentation/jms-1.1/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/jms/MessageWithDestinationTest.java b/instrumentation/jms-1.1/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/jms/MessageWithDestinationTest.java index e00883eaa64f..bd8f7333fc6b 100644 --- a/instrumentation/jms-1.1/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/jms/MessageWithDestinationTest.java +++ b/instrumentation/jms-1.1/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/jms/MessageWithDestinationTest.java @@ -10,8 +10,6 @@ import static org.junit.jupiter.api.Assertions.assertSame; import static org.mockito.BDDMockito.given; -import io.opentelemetry.instrumentation.api.instrumenter.messaging.MessageOperation; -import java.time.Instant; import java.util.stream.Stream; import javax.jms.Destination; import javax.jms.JMSException; @@ -20,7 +18,6 @@ import javax.jms.TemporaryQueue; import javax.jms.TemporaryTopic; import javax.jms.Topic; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.params.ParameterizedTest; @@ -31,7 +28,6 @@ @ExtendWith(MockitoExtension.class) class MessageWithDestinationTest { - private static final Instant START_TIME = Instant.ofEpochSecond(42); @Mock Message message; @Mock Topic topic; @@ -39,12 +35,6 @@ class MessageWithDestinationTest { @Mock Queue queue; @Mock TemporaryQueue temporaryQueue; @Mock Destination destination; - @Mock Timer timer; - - @BeforeEach - void setUp() { - given(timer.startTime()).willReturn(START_TIME); - } @Test void shouldCreateMessageWithUnknownDestination() throws JMSException { @@ -52,11 +42,10 @@ void shouldCreateMessageWithUnknownDestination() throws JMSException { given(message.getJMSDestination()).willReturn(destination); // when - MessageWithDestination result = MessageWithDestination.create(message, null, timer); + MessageWithDestination result = MessageWithDestination.create(message, null); // then - assertMessage( - MessageOperation.SEND, "unknown", "unknown", /* expectedTemporary= */ false, result); + assertMessage("unknown", "unknown", /* expectedTemporary= */ false, result); } @Test @@ -65,11 +54,10 @@ void shouldUseFallbackDestinationToCreateMessage() throws JMSException { given(message.getJMSDestination()).willThrow(JMSException.class); // when - MessageWithDestination result = MessageWithDestination.create(message, destination, timer); + MessageWithDestination result = MessageWithDestination.create(message, destination); // then - assertMessage( - MessageOperation.SEND, "unknown", "unknown", /* expectedTemporary= */ false, result); + assertMessage("unknown", "unknown", /* expectedTemporary= */ false, result); } @ParameterizedTest @@ -91,11 +79,10 @@ void shouldCreateMessageWithQueue( } // when - MessageWithDestination result = MessageWithDestination.create(message, null, timer); + MessageWithDestination result = MessageWithDestination.create(message, null); // then - assertMessage( - MessageOperation.RECEIVE, "queue", expectedDestinationName, expectedTemporary, result); + assertMessage("queue", expectedDestinationName, expectedTemporary, result); } @ParameterizedTest @@ -117,11 +104,10 @@ void shouldCreateMessageWithTopic( } // when - MessageWithDestination result = MessageWithDestination.create(message, null, timer); + MessageWithDestination result = MessageWithDestination.create(message, null); // then - assertMessage( - MessageOperation.RECEIVE, "topic", expectedDestinationName, expectedTemporary, result); + assertMessage("topic", expectedDestinationName, expectedTemporary, result); } static Stream destinations() { @@ -133,7 +119,6 @@ static Stream destinations() { } private void assertMessage( - MessageOperation expectedMessageOperation, String expectedDestinationKind, String expectedDestinationName, boolean expectedTemporary, @@ -143,6 +128,5 @@ private void assertMessage( assertEquals(expectedDestinationKind, actual.destinationKind()); assertEquals(expectedDestinationName, actual.destinationName()); assertEquals(expectedTemporary, actual.isTemporaryDestination()); - assertEquals(START_TIME, actual.startTime()); } } diff --git a/instrumentation/jms-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jms/JmsMessageConsumerInstrumentation.java b/instrumentation/jms-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jms/JmsMessageConsumerInstrumentation.java index e24831cd716c..c204ee6be7e5 100644 --- a/instrumentation/jms-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jms/JmsMessageConsumerInstrumentation.java +++ b/instrumentation/jms-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jms/JmsMessageConsumerInstrumentation.java @@ -13,6 +13,7 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.internal.InstrumenterUtil; import io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; @@ -62,11 +63,17 @@ public static void stopSpan( } Context parentContext = Java8BytecodeBridge.currentContext(); - MessageWithDestination request = MessageWithDestination.create(message, null, timer); + MessageWithDestination request = MessageWithDestination.create(message, null); if (consumerInstrumenter().shouldStart(parentContext, request)) { - Context context = consumerInstrumenter().start(parentContext, request); - consumerInstrumenter().end(context, request, null, throwable); + InstrumenterUtil.startAndEnd( + consumerInstrumenter(), + parentContext, + request, + null, + throwable, + timer.startTime(), + timer.now()); } } } diff --git a/instrumentation/jms-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jms/JmsMessageTimeExtractor.java b/instrumentation/jms-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jms/JmsMessageTimeExtractor.java deleted file mode 100644 index 9ddce9d2c36f..000000000000 --- a/instrumentation/jms-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jms/JmsMessageTimeExtractor.java +++ /dev/null @@ -1,24 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.instrumentation.jms; - -import io.opentelemetry.instrumentation.api.instrumenter.TimeExtractor; -import java.time.Instant; -import javax.annotation.Nullable; - -class JmsMessageTimeExtractor implements TimeExtractor { - - @Override - public Instant extractStartTime(MessageWithDestination request) { - return request.startTime(); - } - - @Override - public Instant extractEndTime( - MessageWithDestination request, @Nullable Void unused, @Nullable Throwable error) { - return request.endTime(); - } -} diff --git a/instrumentation/jms-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jms/JmsSingletons.java b/instrumentation/jms-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jms/JmsSingletons.java index a2441437d269..18e365833b59 100644 --- a/instrumentation/jms-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jms/JmsSingletons.java +++ b/instrumentation/jms-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jms/JmsSingletons.java @@ -45,7 +45,6 @@ private static Instrumenter buildConsumerInstrumen INSTRUMENTATION_NAME, MessagingSpanNameExtractor.create(getter, operation)) .addAttributesExtractor(MessagingAttributesExtractor.create(getter, operation)) - .setTimeExtractor(new JmsMessageTimeExtractor()) .setEnabled(ExperimentalConfig.get().messagingReceiveInstrumentationEnabled()) .newInstrumenter(SpanKindExtractor.alwaysConsumer()); } diff --git a/instrumentation/jms-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jms/MessageWithDestination.java b/instrumentation/jms-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jms/MessageWithDestination.java index becaee4d6396..3928f32c16f8 100644 --- a/instrumentation/jms-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jms/MessageWithDestination.java +++ b/instrumentation/jms-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jms/MessageWithDestination.java @@ -6,7 +6,6 @@ package io.opentelemetry.javaagent.instrumentation.jms; import com.google.auto.value.AutoValue; -import java.time.Instant; import javax.jms.Destination; import javax.jms.JMSException; import javax.jms.Message; @@ -29,22 +28,7 @@ public abstract class MessageWithDestination { public abstract boolean isTemporaryDestination(); - abstract Timer timer(); - - public Instant startTime() { - return timer().startTime(); - } - - public Instant endTime() { - return timer().endTime(); - } - public static MessageWithDestination create(Message message, Destination fallbackDestination) { - return create(message, fallbackDestination, Timer.start()); - } - - public static MessageWithDestination create( - Message message, Destination fallbackDestination, Timer timer) { Destination jmsDestination = null; try { jmsDestination = message.getJMSDestination(); @@ -56,17 +40,16 @@ public static MessageWithDestination create( } if (jmsDestination instanceof Queue) { - return createMessageWithQueue(message, (Queue) jmsDestination, timer); + return createMessageWithQueue(message, (Queue) jmsDestination); } if (jmsDestination instanceof Topic) { - return createMessageWithTopic(message, (Topic) jmsDestination, timer); + return createMessageWithTopic(message, (Topic) jmsDestination); } return new AutoValue_MessageWithDestination( - message, "unknown", "unknown", /* isTemporaryDestination= */ false, timer); + message, "unknown", "unknown", /* isTemporaryDestination= */ false); } - private static MessageWithDestination createMessageWithQueue( - Message message, Queue destination, Timer timer) { + private static MessageWithDestination createMessageWithQueue(Message message, Queue destination) { String queueName; try { queueName = destination.getQueueName(); @@ -77,11 +60,10 @@ private static MessageWithDestination createMessageWithQueue( boolean temporary = destination instanceof TemporaryQueue || queueName.startsWith(TIBCO_TMP_PREFIX); - return new AutoValue_MessageWithDestination(message, queueName, "queue", temporary, timer); + return new AutoValue_MessageWithDestination(message, queueName, "queue", temporary); } - private static MessageWithDestination createMessageWithTopic( - Message message, Topic destination, Timer timer) { + private static MessageWithDestination createMessageWithTopic(Message message, Topic destination) { String topicName; try { topicName = destination.getTopicName(); @@ -92,6 +74,6 @@ private static MessageWithDestination createMessageWithTopic( boolean temporary = destination instanceof TemporaryTopic || topicName.startsWith(TIBCO_TMP_PREFIX); - return new AutoValue_MessageWithDestination(message, topicName, "topic", temporary, timer); + return new AutoValue_MessageWithDestination(message, topicName, "topic", temporary); } } diff --git a/instrumentation/jms-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jms/Timer.java b/instrumentation/jms-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jms/Timer.java index b7d96719c33d..b71de1da9ecb 100644 --- a/instrumentation/jms-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jms/Timer.java +++ b/instrumentation/jms-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jms/Timer.java @@ -25,7 +25,7 @@ public Instant startTime() { return startTime; } - public Instant endTime() { + public Instant now() { long durationNanos = System.nanoTime() - startNanoTime; return startTime().plusNanos(durationNanos); } diff --git a/instrumentation/kafka/kafka-clients/kafka-clients-0.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kafkaclients/KafkaConsumerInstrumentation.java b/instrumentation/kafka/kafka-clients/kafka-clients-0.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kafkaclients/KafkaConsumerInstrumentation.java index eada1ffdfe5f..c2820c4190f1 100644 --- a/instrumentation/kafka/kafka-clients/kafka-clients-0.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kafkaclients/KafkaConsumerInstrumentation.java +++ b/instrumentation/kafka/kafka-clients/kafka-clients-0.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kafkaclients/KafkaConsumerInstrumentation.java @@ -14,8 +14,8 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.internal.InstrumenterUtil; import io.opentelemetry.instrumentation.api.util.VirtualField; -import io.opentelemetry.instrumentation.kafka.internal.ReceivedRecords; import io.opentelemetry.instrumentation.kafka.internal.Timer; import io.opentelemetry.javaagent.bootstrap.kafka.KafkaClientsConsumerProcessTracing; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; @@ -64,10 +64,16 @@ public static void onExit( } Context parentContext = currentContext(); - ReceivedRecords receivedRecords = ReceivedRecords.create(records, timer); - if (consumerReceiveInstrumenter().shouldStart(parentContext, receivedRecords)) { - Context context = consumerReceiveInstrumenter().start(parentContext, receivedRecords); - consumerReceiveInstrumenter().end(context, receivedRecords, null, error); + if (consumerReceiveInstrumenter().shouldStart(parentContext, records)) { + Context context = + InstrumenterUtil.startAndEnd( + consumerReceiveInstrumenter(), + parentContext, + records, + null, + error, + timer.startTime(), + timer.now()); // we're storing the context of the receive span so that process spans can use it as parent // context even though the span has ended diff --git a/instrumentation/kafka/kafka-clients/kafka-clients-0.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kafkaclients/KafkaSingletons.java b/instrumentation/kafka/kafka-clients/kafka-clients-0.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kafkaclients/KafkaSingletons.java index 69f4a79216c2..05f6f7b8e721 100644 --- a/instrumentation/kafka/kafka-clients/kafka-clients-0.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kafkaclients/KafkaSingletons.java +++ b/instrumentation/kafka/kafka-clients/kafka-clients-0.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kafkaclients/KafkaSingletons.java @@ -8,15 +8,15 @@ import io.opentelemetry.api.GlobalOpenTelemetry; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; import io.opentelemetry.instrumentation.kafka.internal.KafkaInstrumenterFactory; -import io.opentelemetry.instrumentation.kafka.internal.ReceivedRecords; import org.apache.kafka.clients.consumer.ConsumerRecord; +import org.apache.kafka.clients.consumer.ConsumerRecords; import org.apache.kafka.clients.producer.ProducerRecord; public final class KafkaSingletons { private static final String INSTRUMENTATION_NAME = "io.opentelemetry.kafka-clients-0.11"; private static final Instrumenter, Void> PRODUCER_INSTRUMENTER; - private static final Instrumenter CONSUMER_RECEIVE_INSTRUMENTER; + private static final Instrumenter, Void> CONSUMER_RECEIVE_INSTRUMENTER; private static final Instrumenter, Void> CONSUMER_PROCESS_INSTRUMENTER; static { @@ -31,7 +31,7 @@ public final class KafkaSingletons { return PRODUCER_INSTRUMENTER; } - public static Instrumenter consumerReceiveInstrumenter() { + public static Instrumenter, Void> consumerReceiveInstrumenter() { return CONSUMER_RECEIVE_INSTRUMENTER; } diff --git a/instrumentation/kafka/kafka-clients/kafka-clients-common/library/src/main/java/io/opentelemetry/instrumentation/kafka/internal/KafkaConsumerTimeExtractor.java b/instrumentation/kafka/kafka-clients/kafka-clients-common/library/src/main/java/io/opentelemetry/instrumentation/kafka/internal/KafkaConsumerTimeExtractor.java deleted file mode 100644 index 056ce81b16a0..000000000000 --- a/instrumentation/kafka/kafka-clients/kafka-clients-common/library/src/main/java/io/opentelemetry/instrumentation/kafka/internal/KafkaConsumerTimeExtractor.java +++ /dev/null @@ -1,24 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.instrumentation.kafka.internal; - -import io.opentelemetry.instrumentation.api.instrumenter.TimeExtractor; -import java.time.Instant; -import javax.annotation.Nullable; - -class KafkaConsumerTimeExtractor implements TimeExtractor { - - @Override - public Instant extractStartTime(ReceivedRecords request) { - return request.startTime(); - } - - @Override - public Instant extractEndTime( - ReceivedRecords request, @Nullable Void unused, @Nullable Throwable error) { - return request.now(); - } -} diff --git a/instrumentation/kafka/kafka-clients/kafka-clients-common/library/src/main/java/io/opentelemetry/instrumentation/kafka/internal/KafkaInstrumenterFactory.java b/instrumentation/kafka/kafka-clients/kafka-clients-common/library/src/main/java/io/opentelemetry/instrumentation/kafka/internal/KafkaInstrumenterFactory.java index 99da63ac512a..2101c43d45dc 100644 --- a/instrumentation/kafka/kafka-clients/kafka-clients-common/library/src/main/java/io/opentelemetry/instrumentation/kafka/internal/KafkaInstrumenterFactory.java +++ b/instrumentation/kafka/kafka-clients/kafka-clients-common/library/src/main/java/io/opentelemetry/instrumentation/kafka/internal/KafkaInstrumenterFactory.java @@ -62,24 +62,16 @@ public KafkaInstrumenterFactory setErrorCauseExtractor(ErrorCauseExtractor error .newInstrumenter(SpanKindExtractor.alwaysProducer()); } - public Instrumenter createConsumerReceiveInstrumenter() { - return createConsumerReceiveInstrumenter(Collections.emptyList()); - } - - public Instrumenter createConsumerReceiveInstrumenter( - Iterable> extractors) { - + public Instrumenter, Void> createConsumerReceiveInstrumenter() { KafkaReceiveAttributesGetter getter = KafkaReceiveAttributesGetter.INSTANCE; MessageOperation operation = MessageOperation.RECEIVE; - return Instrumenter.builder( + return Instrumenter., Void>builder( openTelemetry, instrumentationName, MessagingSpanNameExtractor.create(getter, operation)) .addAttributesExtractor(MessagingAttributesExtractor.create(getter, operation)) - .addAttributesExtractors(extractors) .setErrorCauseExtractor(errorCauseExtractor) - .setTimeExtractor(new KafkaConsumerTimeExtractor()) .setEnabled(ExperimentalConfig.get().messagingReceiveInstrumentationEnabled()) .newInstrumenter(SpanKindExtractor.alwaysConsumer()); } diff --git a/instrumentation/kafka/kafka-clients/kafka-clients-common/library/src/main/java/io/opentelemetry/instrumentation/kafka/internal/KafkaReceiveAttributesGetter.java b/instrumentation/kafka/kafka-clients/kafka-clients-common/library/src/main/java/io/opentelemetry/instrumentation/kafka/internal/KafkaReceiveAttributesGetter.java index bf5e517fcf68..4365ba09f607 100644 --- a/instrumentation/kafka/kafka-clients/kafka-clients-common/library/src/main/java/io/opentelemetry/instrumentation/kafka/internal/KafkaReceiveAttributesGetter.java +++ b/instrumentation/kafka/kafka-clients/kafka-clients-common/library/src/main/java/io/opentelemetry/instrumentation/kafka/internal/KafkaReceiveAttributesGetter.java @@ -10,6 +10,7 @@ import java.util.Set; import java.util.stream.Collectors; import javax.annotation.Nullable; +import org.apache.kafka.clients.consumer.ConsumerRecords; import org.apache.kafka.common.TopicPartition; /** @@ -17,24 +18,24 @@ * any time. */ public enum KafkaReceiveAttributesGetter - implements MessagingAttributesGetter { + implements MessagingAttributesGetter, Void> { INSTANCE; @Override - public String system(ReceivedRecords receivedRecords) { + public String system(ConsumerRecords consumerRecords) { return "kafka"; } @Override - public String destinationKind(ReceivedRecords receivedRecords) { + public String destinationKind(ConsumerRecords consumerRecords) { return SemanticAttributes.MessagingDestinationKindValues.TOPIC; } @Override @Nullable - public String destination(ReceivedRecords receivedRecords) { + public String destination(ConsumerRecords consumerRecords) { Set topics = - receivedRecords.records().partitions().stream() + consumerRecords.partitions().stream() .map(TopicPartition::topic) .collect(Collectors.toSet()); // only return topic when there's exactly one in the batch @@ -42,49 +43,49 @@ public String destination(ReceivedRecords receivedRecords) { } @Override - public boolean temporaryDestination(ReceivedRecords receivedRecords) { + public boolean temporaryDestination(ConsumerRecords consumerRecords) { return false; } @Override @Nullable - public String protocol(ReceivedRecords receivedRecords) { + public String protocol(ConsumerRecords consumerRecords) { return null; } @Override @Nullable - public String protocolVersion(ReceivedRecords receivedRecords) { + public String protocolVersion(ConsumerRecords consumerRecords) { return null; } @Override @Nullable - public String url(ReceivedRecords receivedRecords) { + public String url(ConsumerRecords consumerRecords) { return null; } @Override @Nullable - public String conversationId(ReceivedRecords receivedRecords) { + public String conversationId(ConsumerRecords consumerRecords) { return null; } @Override @Nullable - public Long messagePayloadSize(ReceivedRecords receivedRecords) { + public Long messagePayloadSize(ConsumerRecords consumerRecords) { return null; } @Override @Nullable - public Long messagePayloadCompressedSize(ReceivedRecords receivedRecords) { + public Long messagePayloadCompressedSize(ConsumerRecords consumerRecords) { return null; } @Override @Nullable - public String messageId(ReceivedRecords receivedRecords, @Nullable Void unused) { + public String messageId(ConsumerRecords consumerRecords, @Nullable Void unused) { return null; } } diff --git a/instrumentation/kafka/kafka-clients/kafka-clients-common/library/src/main/java/io/opentelemetry/instrumentation/kafka/internal/ReceivedRecords.java b/instrumentation/kafka/kafka-clients/kafka-clients-common/library/src/main/java/io/opentelemetry/instrumentation/kafka/internal/ReceivedRecords.java deleted file mode 100644 index 5b02d4688ebf..000000000000 --- a/instrumentation/kafka/kafka-clients/kafka-clients-common/library/src/main/java/io/opentelemetry/instrumentation/kafka/internal/ReceivedRecords.java +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.instrumentation.kafka.internal; - -import com.google.auto.value.AutoValue; -import java.time.Instant; -import org.apache.kafka.clients.consumer.ConsumerRecords; - -/** - * This class is internal and is hence not for public use. Its APIs are unstable and can change at - * any time. - */ -@AutoValue -public abstract class ReceivedRecords { - - public static ReceivedRecords create(ConsumerRecords records, Timer timer) { - return new AutoValue_ReceivedRecords(records, timer); - } - - public abstract ConsumerRecords records(); - - abstract Timer timer(); - - public Instant startTime() { - return timer().startTime(); - } - - public Instant now() { - return timer().now(); - } -} diff --git a/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/NettyChannelInstrumentation.java b/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/NettyChannelInstrumentation.java index 902c59a287e6..7140b5fe179f 100644 --- a/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/NettyChannelInstrumentation.java +++ b/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/NettyChannelInstrumentation.java @@ -15,11 +15,13 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.internal.InstrumenterUtil; import io.opentelemetry.instrumentation.api.util.VirtualField; import io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import io.opentelemetry.javaagent.instrumentation.netty.common.NettyConnectionRequest; +import io.opentelemetry.javaagent.instrumentation.netty.common.Timer; import io.opentelemetry.javaagent.instrumentation.netty.v3_8.client.ConnectionListener; import java.net.SocketAddress; import net.bytebuddy.asm.Advice; @@ -58,7 +60,8 @@ public static void onEnter( @Advice.This Channel channel, @Advice.Argument(0) SocketAddress remoteAddress, @Advice.Local("otelParentContext") Context parentContext, - @Advice.Local("otelRequest") NettyConnectionRequest request) { + @Advice.Local("otelRequest") NettyConnectionRequest request, + @Advice.Local("otelTimer") Timer timer) { parentContext = Java8BytecodeBridge.currentContext(); Span span = Java8BytecodeBridge.spanFromContext(parentContext); @@ -74,6 +77,7 @@ public static void onEnter( virtualField.set(channel, new NettyConnectionContext(parentContext)); request = NettyConnectionRequest.connect(remoteAddress); + timer = Timer.start(); } @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) @@ -81,7 +85,8 @@ public static void onExit( @Advice.Return ChannelFuture channelFuture, @Advice.Thrown Throwable error, @Advice.Local("otelParentContext") Context parentContext, - @Advice.Local("otelRequest") NettyConnectionRequest request) { + @Advice.Local("otelRequest") NettyConnectionRequest request, + @Advice.Local("otelTimer") Timer timer) { if (request == null) { return; @@ -89,11 +94,17 @@ public static void onExit( if (error != null) { if (connectionInstrumenter().shouldStart(parentContext, request)) { - Context context = connectionInstrumenter().start(parentContext, request); - connectionInstrumenter().end(context, request, null, error); + InstrumenterUtil.startAndEnd( + connectionInstrumenter(), + parentContext, + request, + null, + error, + timer.startTime(), + timer.now()); } } else { - channelFuture.addListener(new ConnectionListener(parentContext, request)); + channelFuture.addListener(new ConnectionListener(parentContext, request, timer)); } } } diff --git a/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/client/ConnectionListener.java b/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/client/ConnectionListener.java index cd1208f54c3c..e95b2b18b7bc 100644 --- a/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/client/ConnectionListener.java +++ b/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/client/ConnectionListener.java @@ -8,7 +8,9 @@ import static io.opentelemetry.javaagent.instrumentation.netty.v3_8.client.NettyClientSingletons.connectionInstrumenter; import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.internal.InstrumenterUtil; import io.opentelemetry.javaagent.instrumentation.netty.common.NettyConnectionRequest; +import io.opentelemetry.javaagent.instrumentation.netty.common.Timer; import org.jboss.netty.channel.ChannelFuture; import org.jboss.netty.channel.ChannelFutureListener; @@ -16,18 +18,26 @@ public final class ConnectionListener implements ChannelFutureListener { private final Context parentContext; private final NettyConnectionRequest request; + private final Timer timer; - public ConnectionListener(Context parentContext, NettyConnectionRequest request) { + public ConnectionListener(Context parentContext, NettyConnectionRequest request, Timer timer) { this.parentContext = parentContext; this.request = request; + this.timer = timer; } @Override public void operationComplete(ChannelFuture future) { Throwable cause = future.getCause(); if (cause != null && connectionInstrumenter().shouldStart(parentContext, request)) { - Context context = connectionInstrumenter().start(parentContext, request); - connectionInstrumenter().end(context, request, future.getChannel(), cause); + InstrumenterUtil.startAndEnd( + connectionInstrumenter(), + parentContext, + request, + future.getChannel(), + cause, + timer.startTime(), + timer.now()); } } } diff --git a/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/client/NettyClientSingletons.java b/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/client/NettyClientSingletons.java index 677a62963372..20d3ed501387 100644 --- a/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/client/NettyClientSingletons.java +++ b/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/client/NettyClientSingletons.java @@ -58,7 +58,6 @@ public final class NettyClientSingletons { .addAttributesExtractor(nettyConnectAttributesExtractor) .addAttributesExtractor( PeerServiceAttributesExtractor.create(nettyConnectAttributesGetter)) - .setTimeExtractor(new NettyConnectionTimeExtractor()) .newInstrumenter(SpanKindExtractor.alwaysClient()); } diff --git a/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/client/NettyConnectionTimeExtractor.java b/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/client/NettyConnectionTimeExtractor.java deleted file mode 100644 index f541ba818506..000000000000 --- a/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/client/NettyConnectionTimeExtractor.java +++ /dev/null @@ -1,26 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.instrumentation.netty.v3_8.client; - -import io.opentelemetry.instrumentation.api.instrumenter.TimeExtractor; -import io.opentelemetry.javaagent.instrumentation.netty.common.NettyConnectionRequest; -import java.time.Instant; -import javax.annotation.Nullable; -import org.jboss.netty.channel.Channel; - -class NettyConnectionTimeExtractor implements TimeExtractor { - - @Override - public Instant extractStartTime(NettyConnectionRequest request) { - return request.timer().startTime(); - } - - @Override - public Instant extractEndTime( - NettyConnectionRequest request, @Nullable Channel channel, @Nullable Throwable error) { - return request.timer().now(); - } -} diff --git a/instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4/common/client/NettyClientInstrumenterFactory.java b/instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4/common/client/NettyClientInstrumenterFactory.java index fa044116d044..bf2ed54acccd 100644 --- a/instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4/common/client/NettyClientInstrumenterFactory.java +++ b/instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4/common/client/NettyClientInstrumenterFactory.java @@ -57,8 +57,7 @@ public NettyConnectionInstrumenter createConnectionInstrumenter() { Instrumenter.builder( GlobalOpenTelemetry.get(), instrumentationName, NettyConnectionRequest::spanName) .addAttributesExtractor(NetClientAttributesExtractor.create(netAttributesGetter)) - .addAttributesExtractor(PeerServiceAttributesExtractor.create(netAttributesGetter)) - .setTimeExtractor(new NettyConnectionTimeExtractor()); + .addAttributesExtractor(PeerServiceAttributesExtractor.create(netAttributesGetter)); if (!connectionTelemetryEnabled) { // when the connection telemetry is not enabled, netty creates CONNECT spans whenever a // connection error occurs - because there is no HTTP span in that scenario, if raw netty @@ -88,7 +87,6 @@ public NettySslInstrumenter createSslInstrumenter() { GlobalOpenTelemetry.get(), instrumentationName, NettySslRequest::spanName) .addAttributesExtractor(NetClientAttributesExtractor.create(netAttributesGetter)) .addAttributesExtractor(PeerServiceAttributesExtractor.create(netAttributesGetter)) - .setTimeExtractor(new NettySslTimeExtractor()) .newInstrumenter( sslTelemetryEnabled ? SpanKindExtractor.alwaysInternal() diff --git a/instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4/common/client/NettyConnectionTimeExtractor.java b/instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4/common/client/NettyConnectionTimeExtractor.java deleted file mode 100644 index 6acfa159e01b..000000000000 --- a/instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4/common/client/NettyConnectionTimeExtractor.java +++ /dev/null @@ -1,26 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.instrumentation.netty.v4.common.client; - -import io.netty.channel.Channel; -import io.opentelemetry.instrumentation.api.instrumenter.TimeExtractor; -import io.opentelemetry.javaagent.instrumentation.netty.common.NettyConnectionRequest; -import java.time.Instant; -import javax.annotation.Nullable; - -class NettyConnectionTimeExtractor implements TimeExtractor { - - @Override - public Instant extractStartTime(NettyConnectionRequest request) { - return request.timer().startTime(); - } - - @Override - public Instant extractEndTime( - NettyConnectionRequest request, @Nullable Channel channel, @Nullable Throwable error) { - return request.timer().now(); - } -} diff --git a/instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4/common/client/NettyErrorOnlyConnectionInstrumenter.java b/instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4/common/client/NettyErrorOnlyConnectionInstrumenter.java index 4bfe551c5465..19d88e91b074 100644 --- a/instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4/common/client/NettyErrorOnlyConnectionInstrumenter.java +++ b/instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4/common/client/NettyErrorOnlyConnectionInstrumenter.java @@ -8,7 +8,9 @@ import io.netty.channel.Channel; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; +import io.opentelemetry.instrumentation.api.internal.InstrumenterUtil; import io.opentelemetry.javaagent.instrumentation.netty.common.NettyConnectionRequest; +import io.opentelemetry.javaagent.instrumentation.netty.common.Timer; import javax.annotation.Nullable; final class NettyErrorOnlyConnectionInstrumenter implements NettyConnectionInstrumenter { @@ -27,15 +29,16 @@ public boolean shouldStart(Context parentContext, NettyConnectionRequest request @Override public Context start(Context parentContext, NettyConnectionRequest request) { - return parentContext; + return parentContext.with(Timer.start()); } @Override public void end( Context context, NettyConnectionRequest request, Channel channel, @Nullable Throwable error) { if (error != null && instrumenter.shouldStart(context, request)) { - Context connectContext = instrumenter.start(context, request); - instrumenter.end(connectContext, request, channel, error); + Timer timer = Timer.get(context); + InstrumenterUtil.startAndEnd( + instrumenter, context, request, channel, error, timer.startTime(), timer.now()); } } } diff --git a/instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4/common/client/NettySslErrorOnlyInstrumenter.java b/instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4/common/client/NettySslErrorOnlyInstrumenter.java index 3c3f253b78c4..cf7fdc16d468 100644 --- a/instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4/common/client/NettySslErrorOnlyInstrumenter.java +++ b/instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4/common/client/NettySslErrorOnlyInstrumenter.java @@ -7,6 +7,8 @@ import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; +import io.opentelemetry.instrumentation.api.internal.InstrumenterUtil; +import io.opentelemetry.javaagent.instrumentation.netty.common.Timer; import javax.annotation.Nullable; final class NettySslErrorOnlyInstrumenter implements NettySslInstrumenter { @@ -25,14 +27,15 @@ public boolean shouldStart(Context parentContext, NettySslRequest request) { @Override public Context start(Context parentContext, NettySslRequest request) { - return parentContext; + return parentContext.with(Timer.start()); } @Override public void end(Context context, NettySslRequest request, @Nullable Throwable error) { if (error != null && instrumenter.shouldStart(context, request)) { - Context connectContext = instrumenter.start(context, request); - instrumenter.end(connectContext, request, null, error); + Timer timer = Timer.get(context); + InstrumenterUtil.startAndEnd( + instrumenter, context, request, null, error, timer.startTime(), timer.now()); } } } diff --git a/instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4/common/client/NettySslRequest.java b/instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4/common/client/NettySslRequest.java index 47ce43ffef8b..50a88887d292 100644 --- a/instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4/common/client/NettySslRequest.java +++ b/instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4/common/client/NettySslRequest.java @@ -7,7 +7,6 @@ import com.google.auto.value.AutoValue; import io.netty.channel.Channel; -import io.opentelemetry.javaagent.instrumentation.netty.common.Timer; import java.net.SocketAddress; import javax.annotation.Nullable; @@ -15,15 +14,13 @@ public abstract class NettySslRequest { static NettySslRequest create(Channel channel) { - return new AutoValue_NettySslRequest(Timer.start(), channel, channel.remoteAddress()); + return new AutoValue_NettySslRequest(channel, channel.remoteAddress()); } String spanName() { return "SSL handshake"; } - abstract Timer timer(); - abstract Channel channel(); @Nullable diff --git a/instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4/common/client/NettySslTimeExtractor.java b/instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4/common/client/NettySslTimeExtractor.java deleted file mode 100644 index 04129cdb4263..000000000000 --- a/instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4/common/client/NettySslTimeExtractor.java +++ /dev/null @@ -1,24 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.instrumentation.netty.v4.common.client; - -import io.opentelemetry.instrumentation.api.instrumenter.TimeExtractor; -import java.time.Instant; -import javax.annotation.Nullable; - -class NettySslTimeExtractor implements TimeExtractor { - - @Override - public Instant extractStartTime(NettySslRequest request) { - return request.timer().startTime(); - } - - @Override - public Instant extractEndTime( - NettySslRequest request, @Nullable Void unused, @Nullable Throwable error) { - return request.timer().now(); - } -} diff --git a/instrumentation/netty/netty-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/common/NettyConnectionRequest.java b/instrumentation/netty/netty-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/common/NettyConnectionRequest.java index a8e214f35209..046f4741f66c 100644 --- a/instrumentation/netty/netty-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/common/NettyConnectionRequest.java +++ b/instrumentation/netty/netty-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/common/NettyConnectionRequest.java @@ -13,17 +13,15 @@ public abstract class NettyConnectionRequest { public static NettyConnectionRequest resolve(SocketAddress remoteAddress) { - return new AutoValue_NettyConnectionRequest("RESOLVE", Timer.start(), remoteAddress); + return new AutoValue_NettyConnectionRequest("RESOLVE", remoteAddress); } public static NettyConnectionRequest connect(SocketAddress remoteAddress) { - return new AutoValue_NettyConnectionRequest("CONNECT", Timer.start(), remoteAddress); + return new AutoValue_NettyConnectionRequest("CONNECT", remoteAddress); } public abstract String spanName(); - public abstract Timer timer(); - @Nullable public abstract SocketAddress remoteAddressOnStart(); } diff --git a/instrumentation/netty/netty-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/common/Timer.java b/instrumentation/netty/netty-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/common/Timer.java index 6d1e0104edc4..c570bbfc0482 100644 --- a/instrumentation/netty/netty-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/common/Timer.java +++ b/instrumentation/netty/netty-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/common/Timer.java @@ -5,9 +5,14 @@ package io.opentelemetry.javaagent.instrumentation.netty.common; +import io.opentelemetry.context.Context; +import io.opentelemetry.context.ContextKey; +import io.opentelemetry.context.ImplicitContextKeyed; import java.time.Instant; -public final class Timer { +public final class Timer implements ImplicitContextKeyed { + + private static final ContextKey KEY = ContextKey.named("opentelemetry-timer-key"); public static Timer start() { return new Timer(Instant.now(), System.nanoTime()); @@ -29,4 +34,13 @@ public Instant now() { long durationNanos = System.nanoTime() - startNanoTime; return startTime().plusNanos(durationNanos); } + + @Override + public Context storeInContext(Context context) { + return context.with(KEY, this); + } + + public static Timer get(Context context) { + return context.get(KEY); + } } diff --git a/instrumentation/rabbitmq-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rabbitmq/RabbitChannelInstrumentation.java b/instrumentation/rabbitmq-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rabbitmq/RabbitChannelInstrumentation.java index 4173efc1715f..61fd01492292 100644 --- a/instrumentation/rabbitmq-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rabbitmq/RabbitChannelInstrumentation.java +++ b/instrumentation/rabbitmq-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rabbitmq/RabbitChannelInstrumentation.java @@ -31,6 +31,7 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; +import io.opentelemetry.instrumentation.api.internal.InstrumenterUtil; import io.opentelemetry.javaagent.bootstrap.CallDepth; import io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; @@ -209,16 +210,21 @@ public static void extractAndStartSpan( } Context parentContext = Java8BytecodeBridge.currentContext(); - ReceiveRequest request = - ReceiveRequest.create(queue, timer, response, channel.getConnection()); + ReceiveRequest request = ReceiveRequest.create(queue, response, channel.getConnection()); if (!receiveInstrumenter().shouldStart(parentContext, request)) { return; } // can't create span and put into scope in method enter above, because can't add parent after // span creation - Context context = receiveInstrumenter().start(parentContext, request); - receiveInstrumenter().end(context, request, null, throwable); + InstrumenterUtil.startAndEnd( + receiveInstrumenter(), + parentContext, + request, + null, + throwable, + timer.startTime(), + timer.now()); } } diff --git a/instrumentation/rabbitmq-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rabbitmq/RabbitReceiveTimeExtractor.java b/instrumentation/rabbitmq-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rabbitmq/RabbitReceiveTimeExtractor.java deleted file mode 100644 index 812ff2c00a1c..000000000000 --- a/instrumentation/rabbitmq-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rabbitmq/RabbitReceiveTimeExtractor.java +++ /dev/null @@ -1,25 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.instrumentation.rabbitmq; - -import com.rabbitmq.client.GetResponse; -import io.opentelemetry.instrumentation.api.instrumenter.TimeExtractor; -import java.time.Instant; -import javax.annotation.Nullable; - -class RabbitReceiveTimeExtractor implements TimeExtractor { - - @Override - public Instant extractStartTime(ReceiveRequest request) { - return request.startTime(); - } - - @Override - public Instant extractEndTime( - ReceiveRequest request, @Nullable GetResponse response, @Nullable Throwable error) { - return request.now(); - } -} diff --git a/instrumentation/rabbitmq-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rabbitmq/RabbitSingletons.java b/instrumentation/rabbitmq-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rabbitmq/RabbitSingletons.java index 6e9e684c442d..8cc44d98511c 100644 --- a/instrumentation/rabbitmq-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rabbitmq/RabbitSingletons.java +++ b/instrumentation/rabbitmq-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rabbitmq/RabbitSingletons.java @@ -72,7 +72,6 @@ private static Instrumenter createReceiveInstrument return Instrumenter.builder( GlobalOpenTelemetry.get(), instrumentationName, ReceiveRequest::spanName) .addAttributesExtractors(extractors) - .setTimeExtractor(new RabbitReceiveTimeExtractor()) .newInstrumenter(SpanKindExtractor.alwaysClient()); } diff --git a/instrumentation/rabbitmq-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rabbitmq/ReceiveRequest.java b/instrumentation/rabbitmq-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rabbitmq/ReceiveRequest.java index fc143be0aa13..af7267dfec3d 100644 --- a/instrumentation/rabbitmq-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rabbitmq/ReceiveRequest.java +++ b/instrumentation/rabbitmq-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rabbitmq/ReceiveRequest.java @@ -8,21 +8,17 @@ import com.google.auto.value.AutoValue; import com.rabbitmq.client.Connection; import com.rabbitmq.client.GetResponse; -import java.time.Instant; import javax.annotation.Nullable; @AutoValue public abstract class ReceiveRequest { - public static ReceiveRequest create( - String queue, Timer timer, GetResponse response, Connection connection) { - return new AutoValue_ReceiveRequest(queue, timer, response, connection); + public static ReceiveRequest create(String queue, GetResponse response, Connection connection) { + return new AutoValue_ReceiveRequest(queue, response, connection); } public abstract String getQueue(); - public abstract Timer getTimer(); - @Nullable public abstract GetResponse getResponse(); @@ -32,12 +28,4 @@ String spanName() { String queue = getQueue(); return (queue.startsWith("amq.gen-") ? "" : queue) + " receive"; } - - Instant startTime() { - return getTimer().startTime(); - } - - Instant now() { - return getTimer().now(); - } }