Skip to content

Update play mvc to use AdviceScope #13140

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

Merged
merged 15 commits into from
Jul 18, 2025
Merged

Update play mvc to use AdviceScope #13140

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
b802d15
add code attributes to play mvc
jaydeluca Jan 31, 2025
1546f93
Merge branch 'main' of github.com:jaydeluca/opentelemetry-java-instru…
jaydeluca Jul 11, 2025
0450f64
make AdviceScope class consistent, update test and remove deprecated …
jaydeluca Jul 11, 2025
a6220e8
code review notes
jaydeluca Jul 15, 2025
f2315e3
finish moving logic into advicescope class
jaydeluca Jul 15, 2025
f7d1652
remove code attributes
jaydeluca Jul 15, 2025
ed566b9
no longer need actiondata
jaydeluca Jul 15, 2025
760e04d
revert instrumenter changes, move advicescope into advice class
jaydeluca Jul 16, 2025
091d26e
inline scope within advicescope
jaydeluca Jul 16, 2025
a24f019
fix nesting and cast
jaydeluca Jul 16, 2025
4fe2bdd
migrate 2.6 too
jaydeluca Jul 16, 2025
94ebe69
remove cast
jaydeluca Jul 16, 2025
cb48f8f
move updateSpan into future wrapper
jaydeluca Jul 17, 2025
9324fbc
Merge branch 'main' into add-code-attributes-play-mvc
laurit Jul 18, 2025
e585f56
fix indy convertsion
laurit Jul 18, 2025
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
58 changes: 41 additions & 17 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 @@ -18,6 +18,7 @@
import io.opentelemetry.context.Scope;
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import javax.annotation.Nullable;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.matcher.ElementMatcher;
Expand Down Expand Up @@ -50,17 +51,8 @@ public void transform(TypeTransformer transformer) {
public static class ApplyAdvice {

@Advice.OnMethodEnter(suppress = Throwable.class)
public static void onEnter(
@Advice.Argument(0) Request<?> req,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) {
Context parentContext = currentContext();
if (!instrumenter().shouldStart(parentContext, null)) {
return;
}

context = instrumenter().start(parentContext, null);
scope = context.makeCurrent();
public static AdviceScope onEnter(@Advice.Argument(0) Request<?> req) {
return AdviceScope.start(currentContext());
}

@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
Expand All @@ -69,18 +61,50 @@ public static void stopTraceOnResponse(
@Advice.Thrown Throwable throwable,
@Advice.Argument(0) Request<?> req,
@Advice.Return(readOnly = false) Future<Result> responseFuture,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) {
if (scope == null) {
@Advice.Enter @Nullable AdviceScope actionScope) {
if (actionScope == null) {
return;
}
scope.close();

actionScope.end(throwable, responseFuture, (Action<?>) thisAction, req);
Copy link
Contributor

Choose a reason for hiding this comment

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

casting to (Action<?>) is a bit weird. What if you changed @Advice.This Object thisAction to @Advice.This Action<?> thisAction, would that work?

Copy link
Member Author

Choose a reason for hiding this comment

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

nice, yea that seems to work, thanks

}
}

public static class AdviceScope {

private final Context context;
private final Scope scope;

public AdviceScope(Context context, Scope scope) {
this.context = context;
this.scope = scope;
}

@Nullable
public static AdviceScope start(Context parentContext) {
if (!instrumenter().shouldStart(parentContext, null)) {
return null;
}

Context context = instrumenter().start(parentContext, null);
return new AdviceScope(context, context.makeCurrent());
}

public void closeScope() {
if (scope != null) {
scope.close();
}
}

public void end(
Throwable throwable, Future<Result> responseFuture, Action<?> thisAction, Request<?> req) {
closeScope();
updateSpan(context, req);
// span finished in RequestCompleteCallback

if (throwable == null) {
// span finished in RequestCompleteCallback
responseFuture.onComplete(
new RequestCompleteCallback(context), ((Action<?>) thisAction).executionContext());
new RequestCompleteCallback(context), thisAction.executionContext());
} else {
instrumenter().end(context, null, null, throwable);
}
Expand Down
1 change: 0 additions & 1 deletion .../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 @@ -16,7 +16,6 @@
import scala.Option;

public final class Play24Singletons {

private static final String SPAN_NAME = "play.request";
private static final Instrumenter<Void, Void> INSTRUMENTER =
Instrumenter.<Void, Void>builder(
Expand Down
Loading