From 92266c74be2dafa940ea39c6aea5f5ecc3b6ff90 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Sun, 12 Sep 2021 12:02:57 -0700 Subject: [PATCH 1/3] Add more args to RequestListener --- .../api/instrumenter/Instrumenter.java | 10 +++++----- .../api/instrumenter/InstrumenterBuilder.java | 2 +- .../api/instrumenter/RequestListener.java | 18 ++++++++++++------ .../api/instrumenter/RequestMetrics.java | 2 +- .../instrumenter/http/HttpClientMetrics.java | 13 ++++++++++--- .../instrumenter/http/HttpServerMetrics.java | 13 ++++++++++--- 6 files changed, 39 insertions(+), 19 deletions(-) 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 4143a0408dcd..30e30e59c8b3 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 @@ -69,7 +69,7 @@ public static InstrumenterBuilder newBuil private final List> spanLinksExtractors; private final List> attributesExtractors; - private final List requestListeners; + private final List> requestListeners; private final ErrorCauseExtractor errorCauseExtractor; @Nullable private final StartTimeExtractor startTimeExtractor; @Nullable private final EndTimeExtractor endTimeExtractor; @@ -144,8 +144,8 @@ public Context start(Context parentContext, REQUEST request) { Context context = parentContext; - for (RequestListener requestListener : requestListeners) { - context = requestListener.start(context, attributes); + for (RequestListener requestListener : requestListeners) { + context = requestListener.start(context, attributes, request); } spanBuilder.setAllAttributes(attributes); @@ -177,8 +177,8 @@ public void end( Attributes attributes = attributesBuilder; span.setAllAttributes(attributes); - for (RequestListener requestListener : requestListeners) { - requestListener.end(context, attributes); + for (RequestListener requestListener : requestListeners) { + requestListener.end(context, attributes, request, response, error); } StatusCode statusCode = spanStatusExtractor.extract(request, response, error); 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 378a0561a81e..756d8c9cf3d5 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 @@ -42,7 +42,7 @@ public final class InstrumenterBuilder { final List> spanLinksExtractors = new ArrayList<>(); final List> attributesExtractors = new ArrayList<>(); - final List requestListeners = new ArrayList<>(); + final List> requestListeners = new ArrayList<>(); SpanKindExtractor spanKindExtractor = SpanKindExtractor.alwaysInternal(); SpanStatusExtractor spanStatusExtractor = diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/RequestListener.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/RequestListener.java index eaa56295ac01..60fce7e49f64 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/RequestListener.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/RequestListener.java @@ -7,22 +7,28 @@ import io.opentelemetry.api.common.Attributes; import io.opentelemetry.context.Context; +import org.checkerframework.checker.nullness.qual.Nullable; /** * A listener of the start and end of a request. Instrumented libraries will call {@link - * #start(Context, Attributes)} as early as possible in the processing of a request and {@link - * #end(Context, Attributes)} as late as possible when finishing the request. These correspond to - * the start and end of a span when tracing. + * #start(Context, Attributes, Object)} as early as possible in the processing of a request and + * {@link #end(Context, Attributes, Object, Throwable)} as late as possible when finishing the + * request. These correspond to the start and end of a span when tracing. */ -public interface RequestListener { +public interface RequestListener { /** * Listener method that is called at the start of a request. If any state needs to be kept between * the start and end of the request, e.g., an in-progress span, it should be added to the passed * in {@link Context} and returned. */ - Context start(Context context, Attributes startAttributes); + Context start(Context context, Attributes startAttributes, REQUEST request); /** Listener method that is called at the end of a request. */ - void end(Context context, Attributes endAttributes); + void end( + Context context, + Attributes endAttributes, + REQUEST request, + @Nullable RESPONSE response, + @Nullable Throwable error); } diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/RequestMetrics.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/RequestMetrics.java index 126c6e031094..86f4a27c8d7c 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/RequestMetrics.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/RequestMetrics.java @@ -13,5 +13,5 @@ @UnstableApi public interface RequestMetrics { /** Returns a {@link RequestListener} for recording metrics using the given {@link Meter}. */ - RequestListener create(Meter meter); + RequestListener create(Meter meter); } diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientMetrics.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientMetrics.java index 85fb64621c0e..85218d4a9160 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientMetrics.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientMetrics.java @@ -17,6 +17,7 @@ import io.opentelemetry.instrumentation.api.instrumenter.RequestListener; import io.opentelemetry.instrumentation.api.instrumenter.RequestMetrics; import java.util.concurrent.TimeUnit; +import org.checkerframework.checker.nullness.qual.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -29,7 +30,8 @@ * dependencies. */ @UnstableApi -public final class HttpClientMetrics implements RequestListener { +public final class HttpClientMetrics + implements RequestListener { private static final double NANOS_PER_MS = TimeUnit.MILLISECONDS.toNanos(1); @@ -60,7 +62,7 @@ private HttpClientMetrics(Meter meter) { } @Override - public Context start(Context context, Attributes startAttributes) { + public Context start(Context context, Attributes startAttributes, REQUEST request) { long startTimeNanos = System.nanoTime(); return context.with( @@ -69,7 +71,12 @@ public Context start(Context context, Attributes startAttributes) { } @Override - public void end(Context context, Attributes endAttributes) { + public void end( + Context context, + Attributes endAttributes, + REQUEST request, + @Nullable RESPONSE response, + @Nullable Throwable error) { State state = context.get(HTTP_CLIENT_REQUEST_METRICS_STATE); if (state == null) { logger.debug( diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetrics.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetrics.java index 401e33284cc0..85604732f33f 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetrics.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetrics.java @@ -19,6 +19,7 @@ import io.opentelemetry.instrumentation.api.instrumenter.RequestListener; import io.opentelemetry.instrumentation.api.instrumenter.RequestMetrics; import java.util.concurrent.TimeUnit; +import org.checkerframework.checker.nullness.qual.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -31,7 +32,8 @@ * dependencies. */ @UnstableApi -public final class HttpServerMetrics implements RequestListener { +public final class HttpServerMetrics + implements RequestListener { private static final double NANOS_PER_MS = TimeUnit.MILLISECONDS.toNanos(1); @@ -70,7 +72,7 @@ private HttpServerMetrics(Meter meter) { } @Override - public Context start(Context context, Attributes startAttributes) { + public Context start(Context context, Attributes startAttributes, REQUEST request) { long startTimeNanos = System.nanoTime(); activeRequests.add(1, applyActiveRequestsView(startAttributes)); @@ -80,7 +82,12 @@ public Context start(Context context, Attributes startAttributes) { } @Override - public void end(Context context, Attributes endAttributes) { + public void end( + Context context, + Attributes endAttributes, + REQUEST request, + @Nullable RESPONSE response, + @Nullable Throwable error) { State state = context.get(HTTP_SERVER_REQUEST_METRICS_STATE); if (state == null) { logger.debug( From 37fb446a937cf41e01bd9ff0bda3e053b632f1b1 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Sun, 12 Sep 2021 13:29:54 -0700 Subject: [PATCH 2/3] Fix javadoc --- .../instrumentation/api/instrumenter/RequestListener.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/RequestListener.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/RequestListener.java index 60fce7e49f64..55a8c34a78b2 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/RequestListener.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/RequestListener.java @@ -12,8 +12,8 @@ /** * A listener of the start and end of a request. Instrumented libraries will call {@link * #start(Context, Attributes, Object)} as early as possible in the processing of a request and - * {@link #end(Context, Attributes, Object, Throwable)} as late as possible when finishing the - * request. These correspond to the start and end of a span when tracing. + * {@link #end(Context, Attributes, Object, Object, Throwable)} as late as possible when finishing + * the request. These correspond to the start and end of a span when tracing. */ public interface RequestListener { From 6cc5c62f2750af6d361946fd2e887ccd6b194d2a Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Sun, 12 Sep 2021 13:31:52 -0700 Subject: [PATCH 3/3] Update tests --- .../api/instrumenter/http/HttpClientMetricsTest.java | 8 ++++---- .../api/instrumenter/http/HttpServerMetricsTest.java | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientMetricsTest.java b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientMetricsTest.java index ffa7f591cfeb..e6e59812fb57 100644 --- a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientMetricsTest.java +++ b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientMetricsTest.java @@ -41,17 +41,17 @@ void collectsMetrics() { .put("http.status_code", 200) .build(); - Context context1 = listener.start(Context.current(), requestAttributes); + Context context1 = listener.start(Context.current(), requestAttributes, null); Collection metrics = meterProvider.collectAllMetrics(); assertThat(metrics).isEmpty(); - Context context2 = listener.start(Context.current(), requestAttributes); + Context context2 = listener.start(Context.current(), requestAttributes, null); metrics = meterProvider.collectAllMetrics(); assertThat(metrics).isEmpty(); - listener.end(context1, responseAttributes); + listener.end(context1, responseAttributes, null, null, null); metrics = meterProvider.collectAllMetrics(); assertThat(metrics).hasSize(1); @@ -75,7 +75,7 @@ void collectsMetrics() { attributeEntry("net.host.port", 1234L)); })); - listener.end(context2, responseAttributes); + listener.end(context2, responseAttributes, null, null, null); metrics = meterProvider.collectAllMetrics(); assertThat(metrics).hasSize(1); diff --git a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetricsTest.java b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetricsTest.java index a54eaadca5da..63bacc46e74a 100644 --- a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetricsTest.java +++ b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetricsTest.java @@ -41,7 +41,7 @@ void collectsMetrics() { .put("http.status_code", 200) .build(); - Context context1 = listener.start(Context.current(), requestAttributes); + Context context1 = listener.start(Context.current(), requestAttributes, null); Collection metrics = meterProvider.collectAllMetrics(); assertThat(metrics).hasSize(1); @@ -65,7 +65,7 @@ void collectsMetrics() { attributeEntry("http.method", "GET"), attributeEntry("http.scheme", "https")))); - Context context2 = listener.start(Context.current(), requestAttributes); + Context context2 = listener.start(Context.current(), requestAttributes, null); metrics = meterProvider.collectAllMetrics(); assertThat(metrics).hasSize(1); @@ -78,7 +78,7 @@ void collectsMetrics() { .points() .satisfiesExactly(point -> assertThat(point).hasValue(2))); - listener.end(context1, responseAttributes); + listener.end(context1, responseAttributes, null, null, null); metrics = meterProvider.collectAllMetrics(); assertThat(metrics).hasSize(2); @@ -110,7 +110,7 @@ void collectsMetrics() { attributeEntry("net.host.port", 1234L)); })); - listener.end(context2, responseAttributes); + listener.end(context2, responseAttributes, null, null, null); metrics = meterProvider.collectAllMetrics(); assertThat(metrics).hasSize(2);