Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Reduce allocations by using same UnsafeAttributes in Instrumenter.onEnd as onStart. #11092

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Reduce allocations by using same UnsafeAttributes in Instrumenter.onEnd as onStart. #11092

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f5e4fee
First attempt at removing UnsafeAttributes allocation from Instrument…
johnbley Apr 5, 2024
d270def
Remove UnsafeAttributes allocation from onEnd, reusing the one from o…
johnbley Apr 10, 2024
b43fa39
Merge branch 'main' into collapse_unsafeattribute_allocations
johnbley Apr 10, 2024
f769db6
Missed a test line in the merge conflicts
johnbley Apr 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 14 additions & 1 deletion docs/apidiffs/current_vs_latest/opentelemetry-instrumentation-api.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,15 @@
Comparing source compatibility of against
No changes.
***! 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)
35 changes: 8 additions & 27 deletions ...entelemetry/instrumentation/api/incubator/semconv/http/HttpClientExperimentalMetrics.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 <a
Expand All @@ -30,13 +27,6 @@
* the response size</a>.
*/
public final class HttpClientExperimentalMetrics implements OperationListener {

private static final ContextKey<Attributes> 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
Expand Down Expand Up @@ -70,31 +60,22 @@ 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);
}
}
}
21 changes: 4 additions & 17 deletions .../io/opentelemetry/instrumentation/api/incubator/semconv/http/HttpMessageBodySizeUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,32 +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.incubating.HttpIncubatingAttributes;
import javax.annotation.Nullable;

final class HttpMessageBodySizeUtil {

@Nullable
static Long getHttpRequestBodySize(Attributes... attributesList) {
return getAttribute(HttpIncubatingAttributes.HTTP_REQUEST_BODY_SIZE, attributesList);
}

@Nullable
static Long getHttpResponseBodySize(Attributes... attributesList) {
return getAttribute(HttpIncubatingAttributes.HTTP_RESPONSE_BODY_SIZE, attributesList);
static Long getHttpRequestBodySize(Attributes attributes) {
return attributes.get(HttpIncubatingAttributes.HTTP_REQUEST_BODY_SIZE);
}

@Nullable
private static <T> T getAttribute(AttributeKey<T> 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(HttpIncubatingAttributes.HTTP_RESPONSE_BODY_SIZE);
}

private HttpMessageBodySizeUtil() {}
Expand Down
17 changes: 9 additions & 8 deletions ...entelemetry/instrumentation/api/incubator/semconv/http/HttpServerExperimentalMetrics.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,16 @@ 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(
Expand All @@ -102,16 +105,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);
}
}
}
38 changes: 5 additions & 33 deletions ...ain/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcClientMetrics.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 <a
Expand All @@ -29,11 +24,6 @@ public final class RpcClientMetrics implements OperationListener {

private static final double NANOS_PER_MS = TimeUnit.MILLISECONDS.toNanos(1);

private static final ContextKey<RpcClientMetrics.State> 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) {
Expand All @@ -56,33 +46,15 @@ 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(),
context);
}

@AutoValue
abstract static class State {

abstract Attributes startAttributes();

abstract long startTimeNanos();
(endNanos - startNanos) / NANOS_PER_MS, startAndEndAttributes, context);
}
}
38 changes: 5 additions & 33 deletions ...ain/java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcServerMetrics.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 <a
Expand All @@ -29,11 +24,6 @@ public final class RpcServerMetrics implements OperationListener {

private static final double NANOS_PER_MS = TimeUnit.MILLISECONDS.toNanos(1);

private static final ContextKey<RpcServerMetrics.State> 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) {
Expand All @@ -56,33 +46,15 @@ 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(),
context);
}

@AutoValue
abstract static class State {

abstract Attributes startAttributes();

abstract long startTimeNanos();
(endNanos - startNanos) / NANOS_PER_MS, startAndEndAttributes, context);
}
}
5 changes: 3 additions & 2 deletions ...lemetry/instrumentation/api/incubator/semconv/http/HttpClientExperimentalMetricsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ void collectsMetrics() {

Attributes responseAttributes =
Attributes.builder()
.putAll(requestAttributes)
.put(HttpAttributes.HTTP_RESPONSE_STATUS_CODE, 200)
.put(ErrorAttributes.ERROR_TYPE, "400")
.put(HttpIncubatingAttributes.HTTP_REQUEST_BODY_SIZE, 100)
Expand Down Expand Up @@ -77,7 +78,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(
Expand Down Expand Up @@ -134,7 +135,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(
Expand Down
5 changes: 3 additions & 2 deletions ...lemetry/instrumentation/api/incubator/semconv/http/HttpServerExperimentalMetricsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ void collectsMetrics() {

Attributes responseAttributes =
Attributes.builder()
.putAll(requestAttributes)
.put(HttpAttributes.HTTP_RESPONSE_STATUS_CODE, 200)
.put(ErrorAttributes.ERROR_TYPE, "500")
.put(HttpIncubatingAttributes.HTTP_REQUEST_BODY_SIZE, 100)
Expand Down Expand Up @@ -124,7 +125,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(
Expand Down Expand Up @@ -196,7 +197,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(
Expand Down
6 changes: 4 additions & 2 deletions ...java/io/opentelemetry/instrumentation/api/incubator/semconv/rpc/RpcClientMetricsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -50,6 +51,7 @@ void collectsMetrics() {

Attributes responseAttributes2 =
Attributes.builder()
.putAll(requestAttributes)
.put(ServerAttributes.SERVER_PORT, 8080)
.put(NetworkAttributes.NETWORK_TRANSPORT, "tcp")
.build();
Expand All @@ -72,7 +74,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(
Expand Down Expand Up @@ -104,7 +106,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(
Expand Down