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.

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

Add code attributes to play mvc controller spans #13140

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.instrumentation.play.v2_4;

import io.opentelemetry.instrumentation.api.incubator.semconv.code.CodeAttributesGetter;
import javax.annotation.Nullable;

public class ActionCodeAttributesGetter implements CodeAttributesGetter<ActionData> {
@Nullable
@Override
public Class<?> getCodeClass(ActionData actionData) {
return actionData.codeClass();
}

@Nullable
@Override
public String getMethodName(ActionData actionData) {
return actionData.methodName();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.instrumentation.play.v2_4;

import java.lang.reflect.Method;

public class ActionData {
private final Class<?> target;
private final Method method;

public ActionData(Class<?> target, Method method) {
this.target = target;
this.method = method;
}

public Class<?> codeClass() {
return target;
}

public String methodName() {
return method.getName();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,15 @@
import static io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge.currentContext;
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.updateSpan;
import static net.bytebuddy.matcher.ElementMatchers.named;
import static net.bytebuddy.matcher.ElementMatchers.returns;
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;

import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import java.lang.reflect.Method;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.matcher.ElementMatcher;
Expand Down Expand Up @@ -50,17 +49,14 @@ 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) {
public static ActionScope onEnter(
@Advice.This Object target,
@Advice.Origin Method method,
@Advice.Argument(0) Request<?> req) {
Context parentContext = currentContext();
if (!instrumenter().shouldStart(parentContext, null)) {
return;
}

context = instrumenter().start(parentContext, null);
scope = context.makeCurrent();
ActionData actionData = new ActionData(target.getClass(), method);
return ActionScope.start(parentContext, actionData);
}

@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
Expand All @@ -69,20 +65,21 @@ 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 ActionScope actionScope) {
if (actionScope == null || actionScope.getScope() == null) {
return;
}
scope.close();
actionScope.closeScope();

updateSpan(actionScope.getContext(), req);

updateSpan(context, req);
// span finished in RequestCompleteCallback
if (throwable == null) {
responseFuture.onComplete(
new RequestCompleteCallback(context), ((Action<?>) thisAction).executionContext());
new RequestCompleteCallback(actionScope.getContext()),
((Action<?>) thisAction).executionContext());
} else {
instrumenter().end(context, null, null, throwable);
actionScope.end(throwable);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.instrumentation.play.v2_4;

import static io.opentelemetry.javaagent.instrumentation.play.v2_4.Play24Singletons.instrumenter;

import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;

/** Container used to carry state between enter and exit advices */
public final class ActionScope {

private final ActionData actionData;
private final Context context;
private final Scope scope;

public ActionScope(Context context, Scope scope, ActionData actionData) {
this.actionData = actionData;
this.context = context;
this.scope = scope;
}

public Context getContext() {
return context;
}

public Scope getScope() {
return scope;
}

public static ActionScope start(Context parentContext, ActionData actionData) {
if (!instrumenter().shouldStart(parentContext, actionData)) {
return null;
}

Context context = instrumenter().start(parentContext, actionData);
return new ActionScope(context, context.makeCurrent(), actionData);
}

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

public void end(Throwable throwable) {
closeScope();
instrumenter().end(context, actionData, null, throwable);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.api.trace.Span;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.incubator.semconv.code.CodeAttributesExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import io.opentelemetry.instrumentation.api.semconv.http.HttpServerRoute;
import io.opentelemetry.instrumentation.api.semconv.http.HttpServerRouteSource;
Expand All @@ -16,15 +17,20 @@
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(
GlobalOpenTelemetry.get(), "io.opentelemetry.play-mvc-2.4", s -> SPAN_NAME)
.setEnabled(ExperimentalConfig.get().controllerTelemetryEnabled())
.buildInstrumenter();
private static final Instrumenter<ActionData, Void> INSTRUMENTER;

static {
INSTRUMENTER =
Instrumenter.<ActionData, Void>builder(
GlobalOpenTelemetry.get(), "io.opentelemetry.play-mvc-2.4", s -> SPAN_NAME)
.addAttributesExtractor(
CodeAttributesExtractor.create(new ActionCodeAttributesGetter()))
.setEnabled(ExperimentalConfig.get().controllerTelemetryEnabled())
.buildInstrumenter();
}

public static Instrumenter<Void, Void> instrumenter() {
public static Instrumenter<ActionData, Void> instrumenter() {
return INSTRUMENTER;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@
import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.QUERY_PARAM;
import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.REDIRECT;
import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.SUCCESS;
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;
import static io.opentelemetry.semconv.HttpAttributes.HTTP_ROUTE;
import static io.opentelemetry.semconv.incubating.CodeIncubatingAttributes.CODE_FUNCTION;
import static io.opentelemetry.semconv.incubating.CodeIncubatingAttributes.CODE_NAMESPACE;

import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
Expand Down Expand Up @@ -112,9 +115,15 @@ protected void configure(HttpServerTestOptions options) {
}

@Override
@SuppressWarnings("deprecation") // uses deprecated semconv
public SpanDataAssert assertHandlerSpan(
SpanDataAssert span, String method, ServerEndpoint endpoint) {
span.hasName("play.request").hasKind(INTERNAL);
span.hasName("play.request")
.hasKind(INTERNAL)
.hasAttributesSatisfyingExactly(
equalTo(CODE_NAMESPACE, "play.api.mvc.ActionBuilder$$anon$2"),
equalTo(CODE_FUNCTION, "apply"));
Comment on lines +124 to +125
Copy link
Contributor

Choose a reason for hiding this comment

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

In my opinion this is not useful. It points to framework code and is the same for all endpoints.


if (endpoint == EXCEPTION) {
span.hasStatus(StatusData.error());
span.hasException(new IllegalArgumentException(EXCEPTION.getBody()));
Expand Down