Skip to content

Commit

Permalink
Add a ClassAndMethod class to Instrumentation API (#4619)
Browse files Browse the repository at this point in the history
* Add a ClassAndMethod class to Instrumentation API

* remove sentence

* Update docs/contributing/writing-instrumentation.md

Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>

* address review comment

Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
  • Loading branch information
laurit and trask authored Nov 10, 2021
1 parent 4c39b21 commit 16728e2
Show file tree
Hide file tree
Showing 18 changed files with 93 additions and 60 deletions.
15 changes: 15 additions & 0 deletions docs/contributing/writing-instrumentation-module.md
Original file line number Diff line number Diff line change
Expand Up @@ -340,3 +340,18 @@ bytecode tweaks to optimize it. Because of this, retrieving a `VirtualField` ins
limited: the `VirtualField#get()` method must receive class references as its parameters; it won't
work with variables, method params, etc. Both the owner class and the field class must be known at
compile time for it to work.

### Why we don't use ByteBuddy @Advice.Origin Method

Instead of
```
@Advice.Origin Method method
```
we prefer to use
```
@Advice.Origin("#t") Class<?> declaringClass,
@Advice.Origin("#m") String methodName
```
because the former inserts a call to `Class.getMethod(...)` in transformed method. In contrast,
getting the declaring class and method name is just loading constants from constant pool, which is
a much simpler operation.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

package io.opentelemetry.instrumentation.api.tracer;

import io.opentelemetry.instrumentation.api.util.ClassAndMethod;
import java.lang.reflect.Method;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
Expand Down Expand Up @@ -37,6 +38,14 @@ public static String fromMethod(Class<?> clazz, @Nullable Method method) {
return fromMethod(clazz, method == null ? "<unknown>" : method.getName());
}

/**
* This method is used to generate a span name based on a method. Anonymous classes are named
* based on their parent.
*/
public static String fromMethod(ClassAndMethod classAndMethod) {
return fromMethod(classAndMethod.declaringClass(), classAndMethod.methodName());
}

/**
* This method is used to generate a span name based on a method. Anonymous classes are named
* based on their parent.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.instrumentation.rmi.server;
package io.opentelemetry.instrumentation.api.util;

import com.google.auto.value.AutoValue;

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package io.opentelemetry.javaagent.instrumentation.extannotations;

import io.opentelemetry.instrumentation.api.instrumenter.code.CodeAttributesExtractor;
import io.opentelemetry.instrumentation.api.util.ClassAndMethod;
import javax.annotation.Nullable;

final class ExternalAnnotationAttributesExtractor
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.instrumentation.api.config.Config;
import io.opentelemetry.instrumentation.api.util.ClassAndMethod;
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import io.opentelemetry.instrumentation.api.instrumenter.code.CodeSpanNameExtractor;
import io.opentelemetry.instrumentation.api.util.ClassAndMethod;

public final class ExternalAnnotationSingletons {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.instrumentation.api.annotation.support.async.AsyncOperationEndSupport;
import io.opentelemetry.instrumentation.api.util.ClassAndMethod;
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import java.lang.reflect.Method;
import java.util.Set;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.type.TypeDescription;
Expand Down Expand Up @@ -55,30 +55,34 @@ public static class MethodAdvice {

@Advice.OnMethodEnter(suppress = Throwable.class)
public static void onEnter(
@Advice.Origin Method method,
@Advice.Origin("#t") Class<?> declaringClass,
@Advice.Origin("#m") String methodName,
@Advice.Local("otelMethod") ClassAndMethod classAndMethod,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) {
Context parentContext = currentContext();
if (!instrumenter().shouldStart(parentContext, method)) {
classAndMethod = ClassAndMethod.create(declaringClass, methodName);
if (!instrumenter().shouldStart(parentContext, classAndMethod)) {
return;
}

context = instrumenter().start(parentContext, method);
context = instrumenter().start(parentContext, classAndMethod);
scope = context.makeCurrent();
}

@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void stopSpan(
@Advice.Origin Method method,
@Advice.Origin("#r") Class<?> returnType,
@Advice.Local("otelMethod") ClassAndMethod classAndMethod,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope,
@Advice.Return(typing = Assigner.Typing.DYNAMIC, readOnly = false) Object returnValue,
@Advice.Thrown Throwable throwable) {
scope.close();

returnValue =
AsyncOperationEndSupport.create(instrumenter(), Void.class, method.getReturnType())
.asyncEnd(context, method, returnValue, throwable);
AsyncOperationEndSupport.create(instrumenter(), Void.class, returnType)
.asyncEnd(context, classAndMethod, returnValue, throwable);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,23 @@
import io.opentelemetry.instrumentation.api.instrumenter.SpanKindExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor;
import io.opentelemetry.instrumentation.api.tracer.SpanNames;
import java.lang.reflect.Method;
import io.opentelemetry.instrumentation.api.util.ClassAndMethod;

public final class MethodSingletons {
private static final String INSTRUMENTATION_NAME = "io.opentelemetry.methods";

private static final Instrumenter<Method, Void> INSTRUMENTER;
private static final Instrumenter<ClassAndMethod, Void> INSTRUMENTER;

static {
SpanNameExtractor<Method> spanName = SpanNames::fromMethod;
SpanNameExtractor<ClassAndMethod> spanName = SpanNames::fromMethod;

INSTRUMENTER =
Instrumenter.<Method, Void>builder(
Instrumenter.<ClassAndMethod, Void>builder(
GlobalOpenTelemetry.get(), INSTRUMENTATION_NAME, spanName)
.newInstrumenter(SpanKindExtractor.alwaysInternal());
}

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,17 @@ public static class WithSpanAdvice {

@Advice.OnMethodEnter(suppress = Throwable.class)
public static void onEnter(
@Advice.Origin Method method,
@Advice.Origin Method originMethod,
@Advice.Local("otelMethod") Method method,
@Advice.Local("otelOperationEndSupport")
AsyncOperationEndSupport<Method, Object> operationEndSupport,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) {

// Every usage of @Advice.Origin Method is replaced with a call to Class.getMethod, copy it
// to local variable so that there would be only one call to Class.getMethod.
method = originMethod;

Instrumenter<Method, Object> instrumenter = instrumenter();
Context current = Java8BytecodeBridge.currentContext();

Expand All @@ -134,7 +139,7 @@ public static void onEnter(

@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void stopSpan(
@Advice.Origin Method method,
@Advice.Local("otelMethod") Method method,
@Advice.Local("otelOperationEndSupport")
AsyncOperationEndSupport<Method, Object> operationEndSupport,
@Advice.Local("otelContext") Context context,
Expand All @@ -154,14 +159,19 @@ public static class WithSpanAttributesAdvice {

@Advice.OnMethodEnter(suppress = Throwable.class)
public static void onEnter(
@Advice.Origin Method method,
@Advice.Origin Method originMethod,
@Advice.Local("otelMethod") Method method,
@Advice.AllArguments(typing = Assigner.Typing.DYNAMIC) Object[] args,
@Advice.Local("otelOperationEndSupport")
AsyncOperationEndSupport<MethodRequest, Object> operationEndSupport,
@Advice.Local("otelRequest") MethodRequest request,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) {

// Every usage of @Advice.Origin Method is replaced with a call to Class.getMethod, copy it
// to local variable so that there would be only one call to Class.getMethod.
method = originMethod;

Instrumenter<MethodRequest, Object> instrumenter = instrumenterWithAttributes();
Context current = Java8BytecodeBridge.currentContext();
request = new MethodRequest(method, args);
Expand All @@ -176,7 +186,7 @@ public static void onEnter(

@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void stopSpan(
@Advice.Origin Method method,
@Advice.Local("otelMethod") Method method,
@Advice.Local("otelOperationEndSupport")
AsyncOperationEndSupport<MethodRequest, Object> operationEndSupport,
@Advice.Local("otelRequest") MethodRequest request,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.instrumentation.api.util.ClassAndMethod;
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import io.opentelemetry.javaagent.instrumentation.api.CallDepth;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package io.opentelemetry.javaagent.instrumentation.rmi.server;

import io.opentelemetry.instrumentation.api.instrumenter.rpc.RpcAttributesExtractor;
import io.opentelemetry.instrumentation.api.util.ClassAndMethod;

final class RmiServerAttributesExtractor extends RpcAttributesExtractor<ClassAndMethod, Void> {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import io.opentelemetry.instrumentation.api.instrumenter.SpanKindExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.rpc.RpcSpanNameExtractor;
import io.opentelemetry.instrumentation.api.util.ClassAndMethod;

public final class RmiServerSingletons {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,22 @@

import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.instrumentation.api.util.ClassAndMethod;
import io.opentelemetry.javaagent.instrumentation.api.CallDepth;
import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge;
import io.opentelemetry.javaagent.instrumentation.servlet.common.response.HttpServletResponseAdviceHelper;
import jakarta.servlet.http.HttpServletResponse;
import java.lang.reflect.Method;
import net.bytebuddy.asm.Advice;

@SuppressWarnings("unused")
public class ResponseSendAdvice {

@Advice.OnMethodEnter(suppress = Throwable.class)
public static void start(
@Advice.Origin Method method,
@Advice.This Object response,
@Advice.Origin("#t") Class<?> declaringClass,
@Advice.Origin("#m") String methodName,
@Advice.Local("otelMethod") ClassAndMethod classAndMethod,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope,
@Advice.Local("otelCallDepth") CallDepth callDepth) {
Expand All @@ -33,23 +36,25 @@ public static void start(
Context parentContext = Java8BytecodeBridge.currentContext();
// Don't want to generate a new top-level span
if (Java8BytecodeBridge.spanFromContext(parentContext).getSpanContext().isValid()) {
if (instrumenter().shouldStart(parentContext, method)) {
context = instrumenter().start(parentContext, method);
classAndMethod = ClassAndMethod.create(declaringClass, methodName);
if (instrumenter().shouldStart(parentContext, classAndMethod)) {
context = instrumenter().start(parentContext, classAndMethod);
scope = context.makeCurrent();
}
}
}

@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void stopSpan(
@Advice.Origin Method method,
@Advice.Thrown Throwable throwable,
@Advice.Local("otelMethod") ClassAndMethod classAndMethod,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope,
@Advice.Local("otelCallDepth") CallDepth callDepth) {
if (callDepth.decrementAndGet() > 0) {
return;
}
HttpServletResponseAdviceHelper.stopSpan(instrumenter(), throwable, context, scope, method);
HttpServletResponseAdviceHelper.stopSpan(
instrumenter(), throwable, context, scope, classAndMethod);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,20 @@
import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import io.opentelemetry.instrumentation.api.tracer.SpanNames;
import java.lang.reflect.Method;
import io.opentelemetry.instrumentation.api.util.ClassAndMethod;

public class ResponseSingletons {

private static final Instrumenter<Method, Void> INSTRUMENTER;
private static final Instrumenter<ClassAndMethod, Void> INSTRUMENTER;

static {
INSTRUMENTER =
Instrumenter.<Method, Void>builder(
Instrumenter.<ClassAndMethod, Void>builder(
GlobalOpenTelemetry.get(), "io.opentelemetry.servlet-5.0", SpanNames::fromMethod)
.newInstrumenter();
}

public static Instrumenter<Method, Void> instrumenter() {
public static Instrumenter<ClassAndMethod, Void> instrumenter() {
return INSTRUMENTER;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@
import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import java.lang.reflect.Method;
import io.opentelemetry.instrumentation.api.util.ClassAndMethod;

public class HttpServletResponseAdviceHelper {
public static void stopSpan(
Instrumenter<Method, Void> instrumenter,
Instrumenter<ClassAndMethod, Void> instrumenter,
Throwable throwable,
Context context,
Scope scope,
Method request) {
ClassAndMethod request) {
if (scope != null) {
scope.close();

Expand Down
Loading

0 comments on commit 16728e2

Please sign in to comment.