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 a ClassAndMethod class to Instrumentation API #4619

Merged
merged 4 commits into from
Nov 10, 2021
Merged
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
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;
Comment on lines +171 to +173
Copy link
Member

Choose a reason for hiding this comment

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

oh interesting, great find


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