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

Put http.route attribute onto http.server.duration on Play framework request processing #7801

Merged
merged 8 commits into from
Feb 14, 2023
Merged

Put http.route attribute onto http.server.duration on Play framework request processing #7801

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
451c07b
Put `http_route` attribute onto `http_server_duration` on Play framew…
moznion Feb 11, 2023
dbc69e4
Leave a note for `Play` auto instrumentation about `http.route` attri…
moznion Feb 11, 2023
5151da5
Add a smoke test case to check whether the `http.route` attribute is …
moznion Feb 13, 2023
871ce5f
Fix a violation which is pointed out by `:smoke-tests:codenarcTest`
moznion Feb 13, 2023
ff97ca1
Simplify the span updating for Play2.6 MVC
moznion Feb 13, 2023
5be95b8
Simplify a smoke test case for Play framework
moznion Feb 13, 2023
6109ae3
Support `http.route` attribute in `http.server.duration` on Play 2.4 MVC
moznion Feb 13, 2023
d52b750
Tidy up the method names for consistency
moznion Feb 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/supported-libraries.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ These are the supported libraries and frameworks:
| [OkHttp](https://github.com/square/okhttp/) | 2.2+ | [opentelemetry-okhttp-3.0](../instrumentation/okhttp/okhttp-3.0/library) | [HTTP Client Spans], [HTTP Client Metrics] |
| [Oracle UCP](https://docs.oracle.com/database/121/JJUCP/) | 11.2+ | [opentelemetry-oracle-ucp-11.2](../instrumentation/oracle-ucp-11.2/library) | [Database Pool Metrics] |
| [OSHI](https://github.com/oshi/oshi/) | 5.3.1+ | [opentelemetry-oshi](../instrumentation/oshi/library) | [System Metrics] (partial support) |
| [Play](https://github.com/playframework/playframework) | 2.4+ | N/A | [HTTP Client Spans], [HTTP Client Metrics] |
| [Play](https://github.com/playframework/playframework) | 2.4+ | N/A | [HTTP Client Spans], [HTTP Client Metrics], Provides `http.route` [2] |
| [Play WS](https://github.com/playframework/play-ws) | 1.0+ | N/A | [HTTP Client Spans], [HTTP Client Metrics] |
| [Quartz](https://www.quartz-scheduler.org/) | 2.0+ | [opentelemetry-quartz-2.0](../instrumentation/quartz-2.0/library) | none |
| [RabbitMQ Client](https://github.com/rabbitmq/rabbitmq-java-client) | 2.7+ | N/A | [Messaging Spans] |
Expand Down
4 changes: 2 additions & 2 deletions ...main/java/io/opentelemetry/javaagent/instrumentation/play/v2_4/ActionInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface;
import static io.opentelemetry.javaagent.instrumentation.play.v2_4.Play24Singletons.instrumenter;
import static io.opentelemetry.javaagent.instrumentation.play.v2_4.Play24Singletons.updateSpanNames;
import static io.opentelemetry.javaagent.instrumentation.play.v2_4.Play24Singletons.updateSpan;
import static net.bytebuddy.matcher.ElementMatchers.named;
import static net.bytebuddy.matcher.ElementMatchers.returns;
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
Expand Down Expand Up @@ -73,7 +73,7 @@ public static void stopTraceOnResponse(
@Advice.Local("otelScope") Scope scope) {
scope.close();

updateSpanNames(context, req);
updateSpan(context, req);
// span finished in RequestCompleteCallback
if (throwable == null) {
responseFuture.onComplete(
Expand Down
11 changes: 4 additions & 7 deletions .../src/main/java/io/opentelemetry/javaagent/instrumentation/play/v2_4/Play24Singletons.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
import io.opentelemetry.api.trace.Span;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteHolder;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteSource;
import play.api.mvc.Request;
import scala.Option;

Expand All @@ -25,18 +26,14 @@ public static Instrumenter<Void, Void> instrumenter() {
return INSTRUMENTER;
}

public static void updateSpanNames(Context context, Request<?> request) {
public static void updateSpan(Context context, Request<?> request) {
String route = getRoute(request);
if (route == null) {
return;
}

Span.fromContext(context).updateName(route);
// set the span name on the upstream akka/netty span
Span serverSpan = LocalRootSpan.fromContextOrNull(context);
if (serverSpan != null) {
serverSpan.updateName(route);
}
HttpRouteHolder.updateHttpRoute(context, HttpRouteSource.CONTROLLER, route);
}

private static String getRoute(Request<?> request) {
Expand Down
4 changes: 2 additions & 2 deletions ...main/java/io/opentelemetry/javaagent/instrumentation/play/v2_6/ActionInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface;
import static io.opentelemetry.javaagent.instrumentation.play.v2_6.Play26Singletons.instrumenter;
import static io.opentelemetry.javaagent.instrumentation.play.v2_6.Play26Singletons.updateSpanNames;
import static io.opentelemetry.javaagent.instrumentation.play.v2_6.Play26Singletons.updateSpan;
import static net.bytebuddy.matcher.ElementMatchers.named;
import static net.bytebuddy.matcher.ElementMatchers.returns;
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
Expand Down Expand Up @@ -73,7 +73,7 @@ public static void stopTraceOnResponse(
@Advice.Local("otelScope") Scope scope) {
scope.close();

updateSpanNames(context, req);
updateSpan(context, req);
if (throwable == null) {
// span is finished when future completes
// not using responseFuture.onComplete() because that doesn't guarantee this handler span
Expand Down
11 changes: 4 additions & 7 deletions .../src/main/java/io/opentelemetry/javaagent/instrumentation/play/v2_6/Play26Singletons.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
import io.opentelemetry.api.trace.Span;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteHolder;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteSource;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -52,18 +53,14 @@ public static Instrumenter<Void, Void> instrumenter() {
return INSTRUMENTER;
}

public static void updateSpanNames(Context context, Request<?> request) {
public static void updateSpan(Context context, Request<?> request) {
String route = getRoute(request);
if (route == null) {
return;
}

Span.fromContext(context).updateName(route);
// set the span name on the upstream akka/netty span
Span serverSpan = LocalRootSpan.fromContextOrNull(context);
if (serverSpan != null) {
serverSpan.updateName(route);
}
HttpRouteHolder.updateHttpRoute(context, HttpRouteSource.CONTROLLER, route);
}

private static String getRoute(Request<?> request) {
Expand Down
3 changes: 3 additions & 0 deletions smoke-tests/src/test/groovy/io/opentelemetry/smoketest/PlaySmokeTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package io.opentelemetry.smoketest

import io.opentelemetry.proto.collector.trace.v1.ExportTraceServiceRequest
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
import spock.lang.IgnoreIf

import java.time.Duration
Expand Down Expand Up @@ -37,6 +38,8 @@ class PlaySmokeTest extends SmokeTest {
//One internal, one SERVER
countSpansByName(traces, '/welcome') == 2

new TraceInspector(traces).countFilteredAttributes(SemanticAttributes.HTTP_ROUTE.key, "/welcome") == 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️ this is good enough for me, thx!


cleanup:
stopTarget()

Expand Down