From f5e4fee465eaaac705cfa0523246506bdcfcbb1f Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 5 Apr 2024 16:29:44 -0400 Subject: [PATCH 1/3] First attempt at removing UnsafeAttributes allocation from Instrumenter.onEnd --- .../opentelemetry-instrumentation-api.txt | 15 +++++++- .../http/HttpClientExperimentalMetrics.java | 34 ++++------------- .../semconv/http/HttpMessageBodySizeUtil.java | 1 + .../http/HttpServerExperimentalMetrics.java | 15 ++++---- .../semconv/rpc/RpcClientMetrics.java | 37 +++--------------- .../semconv/rpc/RpcServerMetrics.java | 37 +++--------------- .../HttpClientExperimentalMetricsTest.java | 5 ++- .../HttpServerExperimentalMetricsTest.java | 5 ++- .../semconv/rpc/RpcClientMetricsTest.java | 6 ++- .../semconv/rpc/RpcServerMetricsTest.java | 6 ++- .../api/instrumenter/Instrumenter.java | 29 +++++++++++--- .../api/instrumenter/OperationListener.java | 5 ++- .../api/internal/OperationMetricsUtil.java | 2 +- .../api/semconv/http/HttpClientMetrics.java | 38 ++----------------- .../api/semconv/http/HttpServerMetrics.java | 38 ++----------------- .../api/instrumenter/InstrumenterTest.java | 4 +- .../semconv/http/HttpClientMetricsTest.java | 5 ++- .../semconv/http/HttpServerMetricsTest.java | 9 +++-- 18 files changed, 99 insertions(+), 192 deletions(-) diff --git a/docs/apidiffs/current_vs_latest/opentelemetry-instrumentation-api.txt b/docs/apidiffs/current_vs_latest/opentelemetry-instrumentation-api.txt index df26146497bf..d02ef61f6c40 100644 --- a/docs/apidiffs/current_vs_latest/opentelemetry-instrumentation-api.txt +++ b/docs/apidiffs/current_vs_latest/opentelemetry-instrumentation-api.txt @@ -1,2 +1,15 @@ Comparing source compatibility of against -No changes. \ No newline at end of file +***! MODIFIED INTERFACE: PUBLIC ABSTRACT io.opentelemetry.instrumentation.api.instrumenter.OperationListener (not serializable) + === CLASS FILE FORMAT VERSION: 52.0 <- 52.0 + ---! REMOVED METHOD: PUBLIC(-) ABSTRACT(-) void onEnd(io.opentelemetry.context.Context, io.opentelemetry.api.common.Attributes, long) + +++* NEW METHOD: PUBLIC(+) ABSTRACT(+) void onEnd(io.opentelemetry.context.Context, io.opentelemetry.api.common.Attributes, long, long) +*** MODIFIED CLASS: PUBLIC FINAL io.opentelemetry.instrumentation.api.semconv.http.HttpClientMetrics (not serializable) + === CLASS FILE FORMAT VERSION: 52.0 <- 52.0 + ===! UNCHANGED INTERFACE: io.opentelemetry.instrumentation.api.instrumenter.OperationListener + --- REMOVED METHOD: PUBLIC(-) void onEnd(io.opentelemetry.context.Context, io.opentelemetry.api.common.Attributes, long) + +++ NEW METHOD: PUBLIC(+) void onEnd(io.opentelemetry.context.Context, io.opentelemetry.api.common.Attributes, long, long) +*** MODIFIED CLASS: PUBLIC FINAL io.opentelemetry.instrumentation.api.semconv.http.HttpServerMetrics (not serializable) + === CLASS FILE FORMAT VERSION: 52.0 <- 52.0 + ===! UNCHANGED INTERFACE: io.opentelemetry.instrumentation.api.instrumenter.OperationListener + --- REMOVED METHOD: PUBLIC(-) void onEnd(io.opentelemetry.context.Context, io.opentelemetry.api.common.Attributes, long) + +++ NEW METHOD: PUBLIC(+) void onEnd(io.opentelemetry.context.Context, io.opentelemetry.api.common.Attributes, long, long) diff --git a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpClientExperimentalMetrics.java b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpClientExperimentalMetrics.java index b3e94d89f065..030dd31237cd 100644 --- a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpClientExperimentalMetrics.java +++ b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpClientExperimentalMetrics.java @@ -7,18 +7,15 @@ import static io.opentelemetry.instrumentation.api.incubator.semconv.http.HttpMessageBodySizeUtil.getHttpRequestBodySize; import static io.opentelemetry.instrumentation.api.incubator.semconv.http.HttpMessageBodySizeUtil.getHttpResponseBodySize; -import static java.util.logging.Level.FINE; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.metrics.LongHistogram; import io.opentelemetry.api.metrics.LongHistogramBuilder; import io.opentelemetry.api.metrics.Meter; import io.opentelemetry.context.Context; -import io.opentelemetry.context.ContextKey; import io.opentelemetry.instrumentation.api.instrumenter.OperationListener; import io.opentelemetry.instrumentation.api.instrumenter.OperationMetrics; import io.opentelemetry.instrumentation.api.internal.OperationMetricsUtil; -import java.util.logging.Logger; /** * {@link OperationListener} which keeps track of . */ public final class HttpClientExperimentalMetrics implements OperationListener { - - private static final ContextKey HTTP_CLIENT_REQUEST_METRICS_START_ATTRIBUTES = - ContextKey.named("http-client-experimental-metrics-start-attributes"); - - private static final Logger logger = - Logger.getLogger(HttpClientExperimentalMetrics.class.getName()); - /** * Returns a {@link OperationMetrics} which can be used to enable recording of {@link * HttpClientExperimentalMetrics} on an {@link @@ -70,31 +60,21 @@ private HttpClientExperimentalMetrics(Meter meter) { } @Override + @SuppressWarnings("OtelCanIgnoreReturnValueSuggester") public Context onStart(Context context, Attributes startAttributes, long startNanos) { - return context.with(HTTP_CLIENT_REQUEST_METRICS_START_ATTRIBUTES, startAttributes); + return context; } @Override - public void onEnd(Context context, Attributes endAttributes, long endNanos) { - Attributes startAttributes = context.get(HTTP_CLIENT_REQUEST_METRICS_START_ATTRIBUTES); - if (startAttributes == null) { - logger.log( - FINE, - "No state present when ending context {0}. Cannot record HTTP request metrics.", - context); - return; - } - - Attributes sizeAttributes = startAttributes.toBuilder().putAll(endAttributes).build(); - - Long requestBodySize = getHttpRequestBodySize(endAttributes, startAttributes); + public void onEnd(Context context, Attributes startAndEndAttributes, long startNanos, long endNanos) { + Long requestBodySize = getHttpRequestBodySize(startAndEndAttributes); if (requestBodySize != null) { - requestSize.record(requestBodySize, sizeAttributes, context); + requestSize.record(requestBodySize, startAndEndAttributes, context); } - Long responseBodySize = getHttpResponseBodySize(endAttributes, startAttributes); + Long responseBodySize = getHttpResponseBodySize(startAndEndAttributes); if (responseBodySize != null) { - responseSize.record(responseBodySize, sizeAttributes, context); + responseSize.record(responseBodySize, startAndEndAttributes, context); } } } diff --git a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpMessageBodySizeUtil.java b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpMessageBodySizeUtil.java index 11ebeb9873c1..3f78797a4f8f 100644 --- a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpMessageBodySizeUtil.java +++ b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpMessageBodySizeUtil.java @@ -11,6 +11,7 @@ import javax.annotation.Nullable; final class HttpMessageBodySizeUtil { + // FIXME JBLEY can be inlined and removed I think @Nullable static Long getHttpRequestBodySize(Attributes... attributesList) { diff --git a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpServerExperimentalMetrics.java b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpServerExperimentalMetrics.java index 9459293ae1ab..f3b851aef345 100644 --- a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpServerExperimentalMetrics.java +++ b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpServerExperimentalMetrics.java @@ -83,13 +83,14 @@ private HttpServerExperimentalMetrics(Meter meter) { @Override public Context onStart(Context context, Attributes startAttributes, long startNanos) { - activeRequests.add(1, startAttributes, context); + // Have to clone the startAttributes because the same object is reused for startAndEndAttributes in onEnd + activeRequests.add(1, startAttributes.toBuilder().build(), context); return context.with(HTTP_SERVER_EXPERIMENTAL_METRICS_START_ATTRIBUTES, startAttributes); } @Override - public void onEnd(Context context, Attributes endAttributes, long endNanos) { + public void onEnd(Context context, Attributes startAndEndAttributes, long startNanos, long endNanos) { Attributes startAttributes = context.get(HTTP_SERVER_EXPERIMENTAL_METRICS_START_ATTRIBUTES); if (startAttributes == null) { logger.log( @@ -102,16 +103,14 @@ public void onEnd(Context context, Attributes endAttributes, long endNanos) { // request count (otherwise it will split the timeseries) activeRequests.add(-1, startAttributes, context); - Attributes sizeAttributes = startAttributes.toBuilder().putAll(endAttributes).build(); - - Long requestBodySize = getHttpRequestBodySize(endAttributes, startAttributes); + Long requestBodySize = getHttpRequestBodySize(startAndEndAttributes); if (requestBodySize != null) { - requestSize.record(requestBodySize, sizeAttributes, context); + requestSize.record(requestBodySize, startAndEndAttributes, context); } - Long responseBodySize = getHttpResponseBodySize(endAttributes, startAttributes); + Long responseBodySize = getHttpResponseBodySize(startAndEndAttributes); if (responseBodySize != null) { - responseSize.record(responseBodySize, sizeAttributes, context); + responseSize.record(responseBodySize, startAndEndAttributes, context); } } } diff --git a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcClientMetrics.java b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcClientMetrics.java index 72244fe698f4..c08ba502b69e 100644 --- a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcClientMetrics.java +++ b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcClientMetrics.java @@ -5,20 +5,15 @@ package io.opentelemetry.instrumentation.api.incubator.semconv.rpc; -import static java.util.logging.Level.FINE; - -import com.google.auto.value.AutoValue; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.metrics.DoubleHistogram; import io.opentelemetry.api.metrics.DoubleHistogramBuilder; import io.opentelemetry.api.metrics.Meter; import io.opentelemetry.context.Context; -import io.opentelemetry.context.ContextKey; import io.opentelemetry.instrumentation.api.instrumenter.OperationListener; import io.opentelemetry.instrumentation.api.instrumenter.OperationMetrics; import io.opentelemetry.instrumentation.api.internal.OperationMetricsUtil; import java.util.concurrent.TimeUnit; -import java.util.logging.Logger; /** * {@link OperationListener} which keeps track of RPC_CLIENT_REQUEST_METRICS_STATE = - ContextKey.named("rpc-client-request-metrics-state"); - - private static final Logger logger = Logger.getLogger(RpcClientMetrics.class.getName()); - private final DoubleHistogram clientDurationHistogram; private RpcClientMetrics(Meter meter) { @@ -56,33 +46,16 @@ public static OperationMetrics get() { } @Override + @SuppressWarnings("OtelCanIgnoreReturnValueSuggester") public Context onStart(Context context, Attributes startAttributes, long startNanos) { - return context.with( - RPC_CLIENT_REQUEST_METRICS_STATE, - new AutoValue_RpcClientMetrics_State(startAttributes, startNanos)); + return context; } @Override - public void onEnd(Context context, Attributes endAttributes, long endNanos) { - State state = context.get(RPC_CLIENT_REQUEST_METRICS_STATE); - if (state == null) { - logger.log( - FINE, - "No state present when ending context {0}. Cannot record RPC request metrics.", - context); - return; - } + public void onEnd(Context context, Attributes startAndEndAttributes, long startNanos, long endNanos) { clientDurationHistogram.record( - (endNanos - state.startTimeNanos()) / NANOS_PER_MS, - state.startAttributes().toBuilder().putAll(endAttributes).build(), + (endNanos - startNanos) / NANOS_PER_MS, + startAndEndAttributes, context); } - - @AutoValue - abstract static class State { - - abstract Attributes startAttributes(); - - abstract long startTimeNanos(); - } } diff --git a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcServerMetrics.java b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcServerMetrics.java index c4ca4265d793..c6a025e7a8b0 100644 --- a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcServerMetrics.java +++ b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcServerMetrics.java @@ -5,20 +5,15 @@ package io.opentelemetry.instrumentation.api.incubator.semconv.rpc; -import static java.util.logging.Level.FINE; - -import com.google.auto.value.AutoValue; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.metrics.DoubleHistogram; import io.opentelemetry.api.metrics.DoubleHistogramBuilder; import io.opentelemetry.api.metrics.Meter; import io.opentelemetry.context.Context; -import io.opentelemetry.context.ContextKey; import io.opentelemetry.instrumentation.api.instrumenter.OperationListener; import io.opentelemetry.instrumentation.api.instrumenter.OperationMetrics; import io.opentelemetry.instrumentation.api.internal.OperationMetricsUtil; import java.util.concurrent.TimeUnit; -import java.util.logging.Logger; /** * {@link OperationListener} which keeps track of RPC_SERVER_REQUEST_METRICS_STATE = - ContextKey.named("rpc-server-request-metrics-state"); - - private static final Logger logger = Logger.getLogger(RpcServerMetrics.class.getName()); - private final DoubleHistogram serverDurationHistogram; private RpcServerMetrics(Meter meter) { @@ -56,33 +46,16 @@ public static OperationMetrics get() { } @Override + @SuppressWarnings("OtelCanIgnoreReturnValueSuggester") public Context onStart(Context context, Attributes startAttributes, long startNanos) { - return context.with( - RPC_SERVER_REQUEST_METRICS_STATE, - new AutoValue_RpcServerMetrics_State(startAttributes, startNanos)); + return context; } @Override - public void onEnd(Context context, Attributes endAttributes, long endNanos) { - State state = context.get(RPC_SERVER_REQUEST_METRICS_STATE); - if (state == null) { - logger.log( - FINE, - "No state present when ending context {0}. Cannot record RPC request metrics.", - context); - return; - } + public void onEnd(Context context, Attributes startAndEndAttributes, long startNanos, long endNanos) { serverDurationHistogram.record( - (endNanos - state.startTimeNanos()) / NANOS_PER_MS, - state.startAttributes().toBuilder().putAll(endAttributes).build(), + (endNanos - startNanos) / NANOS_PER_MS, + startAndEndAttributes, context); } - - @AutoValue - abstract static class State { - - abstract Attributes startAttributes(); - - abstract long startTimeNanos(); - } } diff --git a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpClientExperimentalMetricsTest.java b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpClientExperimentalMetricsTest.java index 28655c16946b..ff22185acd32 100644 --- a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpClientExperimentalMetricsTest.java +++ b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpClientExperimentalMetricsTest.java @@ -45,6 +45,7 @@ void collectsMetrics() { Attributes responseAttributes = Attributes.builder() + .putAll(requestAttributes) .put(SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 200) .put(SemanticAttributes.ERROR_TYPE, "400") .put(SemanticAttributes.HTTP_REQUEST_BODY_SIZE, 100) @@ -73,7 +74,7 @@ void collectsMetrics() { assertThat(metricReader.collectAllMetrics()).isEmpty(); - listener.onEnd(context1, responseAttributes, nanos(250)); + listener.onEnd(context1, responseAttributes, nanos(100), nanos(250)); assertThat(metricReader.collectAllMetrics()) .satisfiesExactlyInAnyOrder( @@ -132,7 +133,7 @@ void collectsMetrics() { .hasTraceId("ff01020304050600ff0a0b0c0d0e0f00") .hasSpanId("090a0b0c0d0e0f00"))))); - listener.onEnd(context2, responseAttributes, nanos(300)); + listener.onEnd(context2, responseAttributes, nanos(150), nanos(300)); assertThat(metricReader.collectAllMetrics()) .satisfiesExactlyInAnyOrder( diff --git a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpServerExperimentalMetricsTest.java b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpServerExperimentalMetricsTest.java index c35567a07fc9..609374784266 100644 --- a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpServerExperimentalMetricsTest.java +++ b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpServerExperimentalMetricsTest.java @@ -49,6 +49,7 @@ void collectsMetrics() { Attributes responseAttributes = Attributes.builder() + .putAll(requestAttributes) .put(SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 200) .put(SemanticAttributes.ERROR_TYPE, "500") .put(SemanticAttributes.HTTP_REQUEST_BODY_SIZE, 100) @@ -120,7 +121,7 @@ void collectsMetrics() { .hasTraceId(spanContext2.getTraceId()) .hasSpanId(spanContext2.getSpanId()))))); - listener.onEnd(context1, responseAttributes, nanos(250)); + listener.onEnd(context1, responseAttributes, nanos(100), nanos(250)); assertThat(metricReader.collectAllMetrics()) .satisfiesExactlyInAnyOrder( @@ -194,7 +195,7 @@ void collectsMetrics() { .hasTraceId(spanContext1.getTraceId()) .hasSpanId(spanContext1.getSpanId()))))); - listener.onEnd(context2, responseAttributes, nanos(300)); + listener.onEnd(context2, responseAttributes, nanos(150), nanos(300)); assertThat(metricReader.collectAllMetrics()) .satisfiesExactlyInAnyOrder( diff --git a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcClientMetricsTest.java b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcClientMetricsTest.java index c24e55a9c2bb..d5aac492f5c0 100644 --- a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcClientMetricsTest.java +++ b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcClientMetricsTest.java @@ -40,6 +40,7 @@ void collectsMetrics() { Attributes responseAttributes1 = Attributes.builder() + .putAll(requestAttributes) .put(SemanticAttributes.SERVER_ADDRESS, "example.com") .put(SemanticAttributes.SERVER_PORT, 8080) .put(SemanticAttributes.NETWORK_TRANSPORT, "tcp") @@ -48,6 +49,7 @@ void collectsMetrics() { Attributes responseAttributes2 = Attributes.builder() + .putAll(requestAttributes) .put(SemanticAttributes.SERVER_PORT, 8080) .put(SemanticAttributes.NETWORK_TRANSPORT, "tcp") .build(); @@ -70,7 +72,7 @@ void collectsMetrics() { assertThat(metricReader.collectAllMetrics()).isEmpty(); - listener.onEnd(context1, responseAttributes1, nanos(250)); + listener.onEnd(context1, responseAttributes1, nanos(100), nanos(250)); assertThat(metricReader.collectAllMetrics()) .satisfiesExactlyInAnyOrder( @@ -101,7 +103,7 @@ void collectsMetrics() { .hasTraceId("ff01020304050600ff0a0b0c0d0e0f00") .hasSpanId("090a0b0c0d0e0f00"))))); - listener.onEnd(context2, responseAttributes2, nanos(300)); + listener.onEnd(context2, responseAttributes2, nanos(150), nanos(300)); assertThat(metricReader.collectAllMetrics()) .satisfiesExactlyInAnyOrder( diff --git a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcServerMetricsTest.java b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcServerMetricsTest.java index 1d683180b7b1..8dc28652cbe0 100644 --- a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcServerMetricsTest.java +++ b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcServerMetricsTest.java @@ -41,6 +41,7 @@ void collectsMetrics() { Attributes responseAttributes1 = Attributes.builder() + .putAll(requestAttributes) .put(SemanticAttributes.SERVER_ADDRESS, "example.com") .put(SemanticAttributes.SERVER_PORT, 8080) .put(NetworkAttributes.NETWORK_LOCAL_ADDRESS, "127.0.0.1") @@ -50,6 +51,7 @@ void collectsMetrics() { Attributes responseAttributes2 = Attributes.builder() + .putAll(requestAttributes) .put(SemanticAttributes.SERVER_PORT, 8080) .put(NetworkAttributes.NETWORK_LOCAL_ADDRESS, "127.0.0.1") .put(SemanticAttributes.NETWORK_TRANSPORT, "tcp") @@ -73,7 +75,7 @@ void collectsMetrics() { assertThat(metricReader.collectAllMetrics()).isEmpty(); - listener.onEnd(context1, responseAttributes1, nanos(250)); + listener.onEnd(context1, responseAttributes1, nanos(100), nanos(250)); assertThat(metricReader.collectAllMetrics()) .satisfiesExactlyInAnyOrder( @@ -103,7 +105,7 @@ void collectsMetrics() { .hasTraceId("ff01020304050600ff0a0b0c0d0e0f00") .hasSpanId("090a0b0c0d0e0f00"))))); - listener.onEnd(context2, responseAttributes2, nanos(300)); + listener.onEnd(context2, responseAttributes2, nanos(150), nanos(300)); assertThat(metricReader.collectAllMetrics()) .satisfiesExactlyInAnyOrder( 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 cee163f4a12e..ac5ca7aa51cb 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 @@ -5,18 +5,22 @@ package io.opentelemetry.instrumentation.api.instrumenter; +import com.google.auto.value.AutoValue; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanBuilder; import io.opentelemetry.api.trace.SpanKind; import io.opentelemetry.api.trace.Tracer; import io.opentelemetry.context.Context; +import io.opentelemetry.context.ContextKey; import io.opentelemetry.instrumentation.api.internal.HttpRouteState; import io.opentelemetry.instrumentation.api.internal.InstrumenterAccess; import io.opentelemetry.instrumentation.api.internal.InstrumenterUtil; import io.opentelemetry.instrumentation.api.internal.SupportabilityMetrics; import java.time.Instant; import java.util.concurrent.TimeUnit; +import java.util.logging.Level; +import java.util.logging.Logger; import javax.annotation.Nullable; /** @@ -63,6 +67,9 @@ public static InstrumenterBuilder builder } private static final SupportabilityMetrics supportability = SupportabilityMetrics.instance(); + private static final ContextKey INSTRUMENTER_STATE_CONTEXT_KEY = + ContextKey.named("instrumenter-state"); + private static final Logger logger = Logger.getLogger(Instrumenter.class.getName()); private final String instrumentationName; private final Tracer tracer; @@ -186,14 +193,15 @@ private Context doStart(Context parentContext, REQUEST request, @Nullable Instan boolean localRoot = LocalRootSpan.isLocalRoot(context); - spanBuilder.setAllAttributes(attributes); Span span = spanBuilder.setParent(context).startSpan(); context = context.with(span); + long startNanos = getNanos(startTime); + context = context.with(INSTRUMENTER_STATE_CONTEXT_KEY, new AutoValue_Instrumenter_State(attributes, startNanos)); + if (operationListeners.length != 0) { // operation listeners run after span start, so that they have access to the current span // for capturing exemplars - long startNanos = getNanos(startTime); for (int i = 0; i < operationListeners.length; i++) { context = operationListeners[i].onStart(context, attributes, startNanos); } @@ -222,16 +230,20 @@ private void doEnd( span.recordException(error); } - UnsafeAttributes attributes = new UnsafeAttributes(); + State state = context.get(INSTRUMENTER_STATE_CONTEXT_KEY); + UnsafeAttributes attributes = state == null ? new UnsafeAttributes() : state.attributes(); + if (state == null) { + logger.log(Level.FINE, "No instrumenter state present when ending context {0}. Cannot run operationListeners; metrics may not be recorded", context); + } for (AttributesExtractor extractor : attributesExtractors) { extractor.onEnd(attributes, context, request, response, error); } span.setAllAttributes(attributes); - if (operationListeners.length != 0) { + if (operationListeners.length != 0 && state != null) { long endNanos = getNanos(endTime); for (int i = operationListeners.length - 1; i >= 0; i--) { - operationListeners[i].onEnd(context, attributes, endNanos); + operationListeners[i].onEnd(context, attributes, state.startTimeNanos(), endNanos); } } @@ -252,6 +264,13 @@ private static long getNanos(@Nullable Instant time) { return TimeUnit.SECONDS.toNanos(time.getEpochSecond()) + time.getNano(); } + @AutoValue + abstract static class State { + abstract UnsafeAttributes attributes(); + abstract long startTimeNanos(); + } + + static { InstrumenterUtil.setInstrumenterAccess( new InstrumenterAccess() { diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/OperationListener.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/OperationListener.java index 66f942ee72e2..7a8479d885e8 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/OperationListener.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/OperationListener.java @@ -11,7 +11,7 @@ /** * A listener of the start and end of an instrumented operation. The {@link #onStart(Context, * Attributes, long)} methods will be called as early as possible in the processing of a request; - * and the {@link #onEnd(Context, Attributes, long)} method will be called as late as possible when + * and the {@link #onEnd(Context, Attributes, long, long)} method will be called as late as possible when * finishing the processing of a response. These correspond to the start and end of a span when * tracing. */ @@ -30,8 +30,9 @@ public interface OperationListener { /** * Listener method that is called at the end of an instrumented operation. * + * @param startAndEndAttributes The same object as startAttributes from onStart. * @param endNanos The nanosecond timestamp marking the end of the operation. Can be used to * compute the duration of the entire operation. */ - void onEnd(Context context, Attributes endAttributes, long endNanos); + void onEnd(Context context, Attributes startAndEndAttributes, long startNanos, long endNanos); } diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/OperationMetricsUtil.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/OperationMetricsUtil.java index 90a2c6fa595f..f9a5c0450a1a 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/OperationMetricsUtil.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/OperationMetricsUtil.java @@ -32,7 +32,7 @@ public Context onStart(Context context, Attributes startAttributes, long startNa } @Override - public void onEnd(Context context, Attributes endAttributes, long endNanos) {} + public void onEnd(Context context, Attributes startAndEndAttributes, long startNanos, long endNanos) {} }; public static OperationMetrics create( diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpClientMetrics.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpClientMetrics.java index 6f9974206e3d..c1661a9ce142 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpClientMetrics.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpClientMetrics.java @@ -5,21 +5,16 @@ package io.opentelemetry.instrumentation.api.semconv.http; -import static java.util.logging.Level.FINE; - -import com.google.auto.value.AutoValue; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.metrics.DoubleHistogram; import io.opentelemetry.api.metrics.DoubleHistogramBuilder; import io.opentelemetry.api.metrics.Meter; import io.opentelemetry.context.Context; -import io.opentelemetry.context.ContextKey; import io.opentelemetry.instrumentation.api.instrumenter.InstrumenterBuilder; import io.opentelemetry.instrumentation.api.instrumenter.OperationListener; import io.opentelemetry.instrumentation.api.instrumenter.OperationMetrics; import io.opentelemetry.instrumentation.api.internal.OperationMetricsUtil; import java.util.concurrent.TimeUnit; -import java.util.logging.Logger; /** * {@link OperationListener} which keeps track of HTTP_CLIENT_REQUEST_METRICS_STATE = - ContextKey.named("http-client-metrics-state"); - - private static final Logger logger = Logger.getLogger(HttpClientMetrics.class.getName()); - /** * Returns an {@link OperationMetrics} instance which can be used to enable recording of {@link * HttpClientMetrics}. @@ -61,33 +51,13 @@ private HttpClientMetrics(Meter meter) { } @Override + @SuppressWarnings("OtelCanIgnoreReturnValueSuggester") public Context onStart(Context context, Attributes startAttributes, long startNanos) { - return context.with( - HTTP_CLIENT_REQUEST_METRICS_STATE, - new AutoValue_HttpClientMetrics_State(startAttributes, startNanos)); + return context; } @Override - public void onEnd(Context context, Attributes endAttributes, long endNanos) { - State state = context.get(HTTP_CLIENT_REQUEST_METRICS_STATE); - if (state == null) { - logger.log( - FINE, - "No state present when ending context {0}. Cannot record HTTP request metrics.", - context); - return; - } - - Attributes attributes = state.startAttributes().toBuilder().putAll(endAttributes).build(); - - duration.record((endNanos - state.startTimeNanos()) / NANOS_PER_S, attributes, context); - } - - @AutoValue - abstract static class State { - - abstract Attributes startAttributes(); - - abstract long startTimeNanos(); + public void onEnd(Context context, Attributes startAndEndAttributes, long startNanos, long endNanos) { + duration.record((endNanos - startNanos) / NANOS_PER_S, startAndEndAttributes, context); } } diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerMetrics.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerMetrics.java index 58cd3abf5caf..9b1409adb811 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerMetrics.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerMetrics.java @@ -5,21 +5,16 @@ package io.opentelemetry.instrumentation.api.semconv.http; -import static java.util.logging.Level.FINE; - -import com.google.auto.value.AutoValue; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.metrics.DoubleHistogram; import io.opentelemetry.api.metrics.DoubleHistogramBuilder; import io.opentelemetry.api.metrics.Meter; import io.opentelemetry.context.Context; -import io.opentelemetry.context.ContextKey; import io.opentelemetry.instrumentation.api.instrumenter.InstrumenterBuilder; import io.opentelemetry.instrumentation.api.instrumenter.OperationListener; import io.opentelemetry.instrumentation.api.instrumenter.OperationMetrics; import io.opentelemetry.instrumentation.api.internal.OperationMetricsUtil; import java.util.concurrent.TimeUnit; -import java.util.logging.Logger; /** * {@link OperationListener} which keeps track of HTTP_SERVER_METRICS_STATE = - ContextKey.named("http-server-metrics-state"); - - private static final Logger logger = Logger.getLogger(HttpServerMetrics.class.getName()); - /** * Returns an {@link OperationMetrics} instance which can be used to enable recording of {@link * HttpServerMetrics}. @@ -61,33 +51,13 @@ private HttpServerMetrics(Meter meter) { } @Override + @SuppressWarnings("OtelCanIgnoreReturnValueSuggester") public Context onStart(Context context, Attributes startAttributes, long startNanos) { - return context.with( - HTTP_SERVER_METRICS_STATE, - new AutoValue_HttpServerMetrics_State(startAttributes, startNanos)); + return context; } @Override - public void onEnd(Context context, Attributes endAttributes, long endNanos) { - State state = context.get(HTTP_SERVER_METRICS_STATE); - if (state == null) { - logger.log( - FINE, - "No state present when ending context {0}. Cannot record HTTP request metrics.", - context); - return; - } - - Attributes attributes = state.startAttributes().toBuilder().putAll(endAttributes).build(); - - duration.record((endNanos - state.startTimeNanos()) / NANOS_PER_S, attributes, context); - } - - @AutoValue - abstract static class State { - - abstract Attributes startAttributes(); - - abstract long startTimeNanos(); + public void onEnd(Context context, Attributes startAndEndAttributes, long startNanos, long endNanos) { + duration.record((endNanos - startNanos) / NANOS_PER_S, startAndEndAttributes, context); } } 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 fb9ae8622d8d..9cb200d700db 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 @@ -453,7 +453,7 @@ public Context onStart(Context context, Attributes startAttributes, long startNa } @Override - public void onEnd(Context context, Attributes endAttributes, long endNanos) { + public void onEnd(Context context, Attributes startAndEndAttributes, long startNanos, long endNanos) { endContext.set(true); } }; @@ -485,7 +485,7 @@ public Context onStart(Context context, Attributes startAttributes, long startNa } @Override - public void onEnd(Context context, Attributes endAttributes, long endNanos) { + public void onEnd(Context context, Attributes startAndEndAttributes, long startNanos, long endNanos) { endContext.set(context); } }; diff --git a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/semconv/http/HttpClientMetricsTest.java b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/semconv/http/HttpClientMetricsTest.java index d0e67cd5f9e0..2e35690c79e1 100644 --- a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/semconv/http/HttpClientMetricsTest.java +++ b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/semconv/http/HttpClientMetricsTest.java @@ -47,6 +47,7 @@ void collectsMetrics() { Attributes responseAttributes = Attributes.builder() + .putAll(requestAttributes) .put(SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 200) .put(SemanticAttributes.ERROR_TYPE, "400") .put(SemanticAttributes.HTTP_REQUEST_BODY_SIZE, 100) @@ -75,7 +76,7 @@ void collectsMetrics() { assertThat(metricReader.collectAllMetrics()).isEmpty(); - listener.onEnd(context1, responseAttributes, nanos(250)); + listener.onEnd(context1, responseAttributes, nanos(100), nanos(250)); assertThat(metricReader.collectAllMetrics()) .satisfiesExactlyInAnyOrder( @@ -108,7 +109,7 @@ void collectsMetrics() { .hasSpanId("090a0b0c0d0e0f00")) .hasBucketBoundaries(DURATION_BUCKETS)))); - listener.onEnd(context2, responseAttributes, nanos(300)); + listener.onEnd(context2, responseAttributes, nanos(150), nanos(300)); assertThat(metricReader.collectAllMetrics()) .satisfiesExactlyInAnyOrder( diff --git a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerMetricsTest.java b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerMetricsTest.java index f3c6c2e0f160..ebdbbb4e6047 100644 --- a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerMetricsTest.java +++ b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerMetricsTest.java @@ -51,6 +51,7 @@ void collectsMetrics() { Attributes responseAttributes = Attributes.builder() + .putAll(requestAttributes) .put(SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 200) .put(SemanticAttributes.ERROR_TYPE, "500") .put(SemanticAttributes.HTTP_REQUEST_BODY_SIZE, 100) @@ -80,7 +81,7 @@ void collectsMetrics() { Context parent2 = Context.root().with(Span.wrap(spanContext2)); Context context2 = listener.onStart(parent2, requestAttributes, nanos(150)); - listener.onEnd(context1, responseAttributes, nanos(250)); + listener.onEnd(context1, responseAttributes, nanos(100), nanos(250)); assertThat(metricReader.collectAllMetrics()) .satisfiesExactlyInAnyOrder( @@ -112,7 +113,7 @@ void collectsMetrics() { .hasSpanId(spanContext1.getSpanId())) .hasBucketBoundaries(DURATION_BUCKETS)))); - listener.onEnd(context2, responseAttributes, nanos(300)); + listener.onEnd(context2, responseAttributes, nanos(150), nanos(300)); assertThat(metricReader.collectAllMetrics()) .satisfiesExactlyInAnyOrder( @@ -148,13 +149,13 @@ void collectsHttpRouteFromEndAttributes() { .build(); Attributes responseAttributes = - Attributes.builder().put(SemanticAttributes.HTTP_ROUTE, "/test/{id}").build(); + Attributes.builder().putAll(requestAttributes).put(SemanticAttributes.HTTP_ROUTE, "/test/{id}").build(); Context parentContext = Context.root(); // when Context context = listener.onStart(parentContext, requestAttributes, nanos(100)); - listener.onEnd(context, responseAttributes, nanos(200)); + listener.onEnd(context, responseAttributes, nanos(100), nanos(200)); // then assertThat(metricReader.collectAllMetrics()) From d270defcadf8ad65c4de55b4dd4a618b20377437 Mon Sep 17 00:00:00 2001 From: John Bley Date: Wed, 10 Apr 2024 10:05:53 -0400 Subject: [PATCH 2/3] Remove UnsafeAttributes allocation from onEnd, reusing the one from onStart --- .../http/HttpClientExperimentalMetrics.java | 3 ++- .../semconv/http/HttpMessageBodySizeUtil.java | 22 ++++--------------- .../http/HttpServerExperimentalMetrics.java | 6 +++-- .../semconv/rpc/RpcClientMetrics.java | 7 +++--- .../semconv/rpc/RpcServerMetrics.java | 7 +++--- .../api/instrumenter/Instrumenter.java | 14 ++++++++---- .../api/instrumenter/OperationListener.java | 4 ++-- .../api/internal/OperationMetricsUtil.java | 3 ++- .../api/semconv/http/HttpClientMetrics.java | 3 ++- .../api/semconv/http/HttpServerMetrics.java | 3 ++- .../api/instrumenter/InstrumenterTest.java | 6 +++-- .../semconv/http/HttpServerMetricsTest.java | 5 ++++- 12 files changed, 42 insertions(+), 41 deletions(-) diff --git a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpClientExperimentalMetrics.java b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpClientExperimentalMetrics.java index 030dd31237cd..082ac5ce5450 100644 --- a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpClientExperimentalMetrics.java +++ b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpClientExperimentalMetrics.java @@ -66,7 +66,8 @@ public Context onStart(Context context, Attributes startAttributes, long startNa } @Override - public void onEnd(Context context, Attributes startAndEndAttributes, long startNanos, long endNanos) { + public void onEnd( + Context context, Attributes startAndEndAttributes, long startNanos, long endNanos) { Long requestBodySize = getHttpRequestBodySize(startAndEndAttributes); if (requestBodySize != null) { requestSize.record(requestBodySize, startAndEndAttributes, context); diff --git a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpMessageBodySizeUtil.java b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpMessageBodySizeUtil.java index 3f78797a4f8f..9d45295b64f8 100644 --- a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpMessageBodySizeUtil.java +++ b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpMessageBodySizeUtil.java @@ -5,33 +5,19 @@ package io.opentelemetry.instrumentation.api.incubator.semconv.http; -import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.semconv.SemanticAttributes; import javax.annotation.Nullable; final class HttpMessageBodySizeUtil { - // FIXME JBLEY can be inlined and removed I think - - @Nullable - static Long getHttpRequestBodySize(Attributes... attributesList) { - return getAttribute(SemanticAttributes.HTTP_REQUEST_BODY_SIZE, attributesList); - } - @Nullable - static Long getHttpResponseBodySize(Attributes... attributesList) { - return getAttribute(SemanticAttributes.HTTP_RESPONSE_BODY_SIZE, attributesList); + static Long getHttpRequestBodySize(Attributes attributes) { + return attributes.get(SemanticAttributes.HTTP_REQUEST_BODY_SIZE); } @Nullable - private static T getAttribute(AttributeKey key, Attributes... attributesList) { - for (Attributes attributes : attributesList) { - T value = attributes.get(key); - if (value != null) { - return value; - } - } - return null; + static Long getHttpResponseBodySize(Attributes attributes) { + return attributes.get(SemanticAttributes.HTTP_RESPONSE_BODY_SIZE); } private HttpMessageBodySizeUtil() {} diff --git a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpServerExperimentalMetrics.java b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpServerExperimentalMetrics.java index f3b851aef345..29da33f05300 100644 --- a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpServerExperimentalMetrics.java +++ b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpServerExperimentalMetrics.java @@ -83,14 +83,16 @@ private HttpServerExperimentalMetrics(Meter meter) { @Override public Context onStart(Context context, Attributes startAttributes, long startNanos) { - // Have to clone the startAttributes because the same object is reused for startAndEndAttributes in onEnd + // Have to clone the startAttributes because the same object is reused for startAndEndAttributes + // in onEnd activeRequests.add(1, startAttributes.toBuilder().build(), context); return context.with(HTTP_SERVER_EXPERIMENTAL_METRICS_START_ATTRIBUTES, startAttributes); } @Override - public void onEnd(Context context, Attributes startAndEndAttributes, long startNanos, long endNanos) { + public void onEnd( + Context context, Attributes startAndEndAttributes, long startNanos, long endNanos) { Attributes startAttributes = context.get(HTTP_SERVER_EXPERIMENTAL_METRICS_START_ATTRIBUTES); if (startAttributes == null) { logger.log( diff --git a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcClientMetrics.java b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcClientMetrics.java index c08ba502b69e..32a4338b1bd7 100644 --- a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcClientMetrics.java +++ b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcClientMetrics.java @@ -52,10 +52,9 @@ public Context onStart(Context context, Attributes startAttributes, long startNa } @Override - public void onEnd(Context context, Attributes startAndEndAttributes, long startNanos, long endNanos) { + public void onEnd( + Context context, Attributes startAndEndAttributes, long startNanos, long endNanos) { clientDurationHistogram.record( - (endNanos - startNanos) / NANOS_PER_MS, - startAndEndAttributes, - context); + (endNanos - startNanos) / NANOS_PER_MS, startAndEndAttributes, context); } } diff --git a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcServerMetrics.java b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcServerMetrics.java index c6a025e7a8b0..35b8b515b22a 100644 --- a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcServerMetrics.java +++ b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcServerMetrics.java @@ -52,10 +52,9 @@ public Context onStart(Context context, Attributes startAttributes, long startNa } @Override - public void onEnd(Context context, Attributes startAndEndAttributes, long startNanos, long endNanos) { + public void onEnd( + Context context, Attributes startAndEndAttributes, long startNanos, long endNanos) { serverDurationHistogram.record( - (endNanos - startNanos) / NANOS_PER_MS, - startAndEndAttributes, - context); + (endNanos - startNanos) / NANOS_PER_MS, startAndEndAttributes, context); } } 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 ac5ca7aa51cb..36afb7e19ad1 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 @@ -197,7 +197,10 @@ private Context doStart(Context parentContext, REQUEST request, @Nullable Instan context = context.with(span); long startNanos = getNanos(startTime); - context = context.with(INSTRUMENTER_STATE_CONTEXT_KEY, new AutoValue_Instrumenter_State(attributes, startNanos)); + context = + context.with( + INSTRUMENTER_STATE_CONTEXT_KEY, + new AutoValue_Instrumenter_State(attributes, startNanos)); if (operationListeners.length != 0) { // operation listeners run after span start, so that they have access to the current span @@ -233,7 +236,10 @@ private void doEnd( State state = context.get(INSTRUMENTER_STATE_CONTEXT_KEY); UnsafeAttributes attributes = state == null ? new UnsafeAttributes() : state.attributes(); if (state == null) { - logger.log(Level.FINE, "No instrumenter state present when ending context {0}. Cannot run operationListeners; metrics may not be recorded", context); + logger.log( + Level.FINE, + "No instrumenter state present when ending context {0}. Cannot run operationListeners; metrics may not be recorded", + context); } for (AttributesExtractor extractor : attributesExtractors) { extractor.onEnd(attributes, context, request, response, error); @@ -267,9 +273,9 @@ private static long getNanos(@Nullable Instant time) { @AutoValue abstract static class State { abstract UnsafeAttributes attributes(); - abstract long startTimeNanos(); - } + abstract long startTimeNanos(); + } static { InstrumenterUtil.setInstrumenterAccess( diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/OperationListener.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/OperationListener.java index 7a8479d885e8..ff542ceb5898 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/OperationListener.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/OperationListener.java @@ -11,8 +11,8 @@ /** * A listener of the start and end of an instrumented operation. The {@link #onStart(Context, * Attributes, long)} methods will be called as early as possible in the processing of a request; - * and the {@link #onEnd(Context, Attributes, long, long)} method will be called as late as possible when - * finishing the processing of a response. These correspond to the start and end of a span when + * and the {@link #onEnd(Context, Attributes, long, long)} method will be called as late as possible + * when finishing the processing of a response. These correspond to the start and end of a span when * tracing. */ public interface OperationListener { diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/OperationMetricsUtil.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/OperationMetricsUtil.java index f9a5c0450a1a..1cef344cca83 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/OperationMetricsUtil.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/OperationMetricsUtil.java @@ -32,7 +32,8 @@ public Context onStart(Context context, Attributes startAttributes, long startNa } @Override - public void onEnd(Context context, Attributes startAndEndAttributes, long startNanos, long endNanos) {} + public void onEnd( + Context context, Attributes startAndEndAttributes, long startNanos, long endNanos) {} }; public static OperationMetrics create( diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpClientMetrics.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpClientMetrics.java index c1661a9ce142..e6087c646d21 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpClientMetrics.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpClientMetrics.java @@ -57,7 +57,8 @@ public Context onStart(Context context, Attributes startAttributes, long startNa } @Override - public void onEnd(Context context, Attributes startAndEndAttributes, long startNanos, long endNanos) { + public void onEnd( + Context context, Attributes startAndEndAttributes, long startNanos, long endNanos) { duration.record((endNanos - startNanos) / NANOS_PER_S, startAndEndAttributes, context); } } diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerMetrics.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerMetrics.java index 9b1409adb811..9bda7b3e8bef 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerMetrics.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerMetrics.java @@ -57,7 +57,8 @@ public Context onStart(Context context, Attributes startAttributes, long startNa } @Override - public void onEnd(Context context, Attributes startAndEndAttributes, long startNanos, long endNanos) { + public void onEnd( + Context context, Attributes startAndEndAttributes, long startNanos, long endNanos) { duration.record((endNanos - startNanos) / NANOS_PER_S, startAndEndAttributes, context); } } 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 9cb200d700db..2a49fa353076 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 @@ -453,7 +453,8 @@ public Context onStart(Context context, Attributes startAttributes, long startNa } @Override - public void onEnd(Context context, Attributes startAndEndAttributes, long startNanos, long endNanos) { + public void onEnd( + Context context, Attributes startAndEndAttributes, long startNanos, long endNanos) { endContext.set(true); } }; @@ -485,7 +486,8 @@ public Context onStart(Context context, Attributes startAttributes, long startNa } @Override - public void onEnd(Context context, Attributes startAndEndAttributes, long startNanos, long endNanos) { + public void onEnd( + Context context, Attributes startAndEndAttributes, long startNanos, long endNanos) { endContext.set(context); } }; diff --git a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerMetricsTest.java b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerMetricsTest.java index ebdbbb4e6047..0b045c17e0c1 100644 --- a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerMetricsTest.java +++ b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerMetricsTest.java @@ -149,7 +149,10 @@ void collectsHttpRouteFromEndAttributes() { .build(); Attributes responseAttributes = - Attributes.builder().putAll(requestAttributes).put(SemanticAttributes.HTTP_ROUTE, "/test/{id}").build(); + Attributes.builder() + .putAll(requestAttributes) + .put(SemanticAttributes.HTTP_ROUTE, "/test/{id}") + .build(); Context parentContext = Context.root(); From f769db654b1cbb940d66110609acc79127ea4349 Mon Sep 17 00:00:00 2001 From: John Bley Date: Wed, 10 Apr 2024 10:56:38 -0400 Subject: [PATCH 3/3] Missed a test line in the merge conflicts --- .../api/incubator/semconv/rpc/RpcClientMetricsTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcClientMetricsTest.java b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcClientMetricsTest.java index 3311e97e69fd..4673bb1c5b4d 100644 --- a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcClientMetricsTest.java +++ b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcClientMetricsTest.java @@ -42,6 +42,7 @@ void collectsMetrics() { Attributes responseAttributes1 = Attributes.builder() + .putAll(requestAttributes) .put(ServerAttributes.SERVER_ADDRESS, "example.com") .put(ServerAttributes.SERVER_PORT, 8080) .put(NetworkAttributes.NETWORK_TRANSPORT, "tcp")