diff --git a/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/client/RmiClientInstrumentationModule.java b/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/client/RmiClientInstrumentationModule.java index 11da208fa849..a60b3637ffd2 100644 --- a/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/client/RmiClientInstrumentationModule.java +++ b/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/client/RmiClientInstrumentationModule.java @@ -10,10 +10,12 @@ import com.google.auto.service.AutoService; import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule; import java.util.List; @AutoService(InstrumentationModule.class) -public class RmiClientInstrumentationModule extends InstrumentationModule { +public class RmiClientInstrumentationModule extends InstrumentationModule + implements ExperimentalInstrumentationModule { public RmiClientInstrumentationModule() { super("rmi", "rmi-client"); @@ -23,4 +25,9 @@ public RmiClientInstrumentationModule() { public List typeInstrumentations() { return singletonList(new UnicastRefInstrumentation()); } + + @Override + public boolean isIndyReady() { + return true; + } } diff --git a/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/client/UnicastRefInstrumentation.java b/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/client/UnicastRefInstrumentation.java index 1d32fa89efb1..de0ffc61acfc 100644 --- a/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/client/UnicastRefInstrumentation.java +++ b/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/client/UnicastRefInstrumentation.java @@ -12,10 +12,10 @@ import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; -import io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import java.lang.reflect.Method; +import javax.annotation.Nullable; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @@ -40,34 +40,45 @@ public void transform(TypeTransformer transformer) { @SuppressWarnings("unused") public static class InvokeAdvice { - @Advice.OnMethodEnter(suppress = Throwable.class) - public static void onEnter( - @Advice.Argument(value = 1) Method method, - @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { + public static class AdviceScope { + private final Context context; + private final Scope scope; + + private AdviceScope(Context context, Scope scope) { + this.context = context; + this.scope = scope; + } - Context parentContext = Java8BytecodeBridge.currentContext(); + @Nullable + public static AdviceScope start(Method method) { + Context parentContext = Context.current(); + if (!instrumenter().shouldStart(parentContext, method)) { + return null; + } + Context context = instrumenter().start(parentContext, method); + return new AdviceScope(context, context.makeCurrent()); + } - if (!instrumenter().shouldStart(parentContext, method)) { - return; + public void end(Method method, @Nullable Throwable throwable) { + scope.close(); + instrumenter().end(context, method, null, throwable); } + } - context = instrumenter().start(parentContext, method); - scope = context.makeCurrent(); + @Nullable + @Advice.OnMethodEnter(suppress = Throwable.class) + public static AdviceScope onEnter(@Advice.Argument(value = 1) Method method) { + return AdviceScope.start(method); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void stopSpan( @Advice.Argument(value = 1) Method method, - @Advice.Thrown Throwable throwable, - @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { - - if (scope == null) { - return; + @Advice.Thrown @Nullable Throwable throwable, + @Advice.Enter @Nullable AdviceScope adviceScope) { + if (adviceScope != null) { + adviceScope.end(method, throwable); } - scope.close(); - instrumenter().end(context, method, null, throwable); } } } diff --git a/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/context/ContextPropagator.java b/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/context/ContextPropagator.java index 722f1006b436..070fb73e82a2 100644 --- a/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/context/ContextPropagator.java +++ b/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/context/ContextPropagator.java @@ -22,6 +22,9 @@ public class ContextPropagator { private static final Logger logger = Logger.getLogger(ContextPropagator.class.getName()); + private static final VirtualField KNOWN_CONNECTION = + VirtualField.find(Connection.class, Boolean.class); + // Internal RMI object ids that we don't want to trace private static final ObjID ACTIVATOR_ID = new ObjID(ObjID.ACTIVATOR_ID); private static final ObjID DGC_ID = new ObjID(ObjID.DGC_ID); @@ -48,24 +51,22 @@ public boolean isOperationWithPayload(int operationId) { return operationId == CONTEXT_PAYLOAD_OPERATION_ID; } - public void attemptToPropagateContext( - VirtualField knownConnections, Connection c, Context context) { - if (checkIfContextCanBePassed(knownConnections, c)) { + public void attemptToPropagateContext(Connection c, Context context) { + if (checkIfContextCanBePassed(c)) { if (!syntheticCall(c, ContextPayload.from(context), CONTEXT_PAYLOAD_OPERATION_ID)) { logger.fine("Couldn't send context payload"); } } } - private static boolean checkIfContextCanBePassed( - VirtualField knownConnections, Connection c) { - Boolean storedResult = knownConnections.get(c); + private static boolean checkIfContextCanBePassed(Connection c) { + Boolean storedResult = KNOWN_CONNECTION.get(c); if (storedResult != null) { return storedResult; } boolean result = syntheticCall(c, null, CONTEXT_CHECK_CALL_OPERATION_ID); - knownConnections.set(c, result); + KNOWN_CONNECTION.set(c, result); return result; } diff --git a/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/context/RmiContextPropagationInstrumentationModule.java b/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/context/RmiContextPropagationInstrumentationModule.java index 03043bb0a426..89b57b22cd3d 100644 --- a/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/context/RmiContextPropagationInstrumentationModule.java +++ b/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/context/RmiContextPropagationInstrumentationModule.java @@ -37,4 +37,9 @@ public Map> jpmsModulesToOpen() { return Collections.singletonMap( JavaModule.ofType(Remote.class), Arrays.asList("sun.rmi.server", "sun.rmi.transport")); } + + @Override + public boolean isIndyReady() { + return true; + } } diff --git a/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/context/client/RmiClientContextInstrumentation.java b/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/context/client/RmiClientContextInstrumentation.java index 7436e2afee8b..ede8c549cc10 100644 --- a/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/context/client/RmiClientContextInstrumentation.java +++ b/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/context/client/RmiClientContextInstrumentation.java @@ -12,7 +12,6 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.api.util.VirtualField; import io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; @@ -77,10 +76,7 @@ public static void onEnter(@Advice.Argument(0) Connection c, @Advice.Argument(1) } // caching if a connection can support enhanced format - VirtualField knownConnections = - VirtualField.find(Connection.class, Boolean.class); - - PROPAGATOR.attemptToPropagateContext(knownConnections, c, currentContext); + PROPAGATOR.attemptToPropagateContext(c, currentContext); } } } diff --git a/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/context/server/RmiServerContextInstrumentation.java b/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/context/server/RmiServerContextInstrumentation.java index f56048f5770a..a60498f2b1f1 100644 --- a/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/context/server/RmiServerContextInstrumentation.java +++ b/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/context/server/RmiServerContextInstrumentation.java @@ -14,6 +14,7 @@ import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import net.bytebuddy.asm.Advice; +import net.bytebuddy.asm.Advice.AssignReturned; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; import sun.rmi.transport.Target; @@ -38,17 +39,17 @@ public void transform(TypeTransformer transformer) { @SuppressWarnings("unused") public static class ObjectTableAdvice { + @AssignReturned.ToReturned @Advice.OnMethodExit(suppress = Throwable.class) - public static void methodExit( - @Advice.Argument(0) Object oe, @Advice.Return(readOnly = false) Target result) { + public static Target methodExit(@Advice.Argument(0) Object oe, @Advice.Return Target result) { // comparing toString() output allows us to avoid using reflection to be able to compare // ObjID and ObjectEndpoint objects // ObjectEndpoint#toString() only returns this.objId.toString() value which is exactly // what we're interested in here. if (!CONTEXT_CALL_ID.toString().equals(oe.toString())) { - return; + return result; } - result = ContextDispatcher.newDispatcherTarget(); + return ContextDispatcher.newDispatcherTarget(); } } } diff --git a/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/server/RemoteServerInstrumentation.java b/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/server/RemoteServerInstrumentation.java index 23024bacb3a3..26b8ded0b8b8 100644 --- a/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/server/RemoteServerInstrumentation.java +++ b/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/server/RemoteServerInstrumentation.java @@ -22,6 +22,7 @@ import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import java.rmi.Remote; +import javax.annotation.Nullable; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @@ -43,50 +44,64 @@ public void transform(TypeTransformer transformer) { @SuppressWarnings("unused") public static class PublicMethodAdvice { - @Advice.OnMethodEnter(suppress = Throwable.class) - public static void onEnter( - @Advice.Origin("#t") Class declaringClass, - @Advice.Origin("#m") String methodName, - @Advice.Local("otelCallDepth") CallDepth callDepth, - @Advice.Local("otelRequest") ClassAndMethod request, - @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { + public static class AdviceScope { + private final CallDepth callDepth; + @Nullable private final ClassAndMethod classAndMethod; + @Nullable private final Context context; + @Nullable private final Scope scope; - callDepth = CallDepth.forClass(Remote.class); - if (callDepth.getAndIncrement() > 0) { - return; + private AdviceScope( + CallDepth callDepth, + @Nullable ClassAndMethod classAndMethod, + @Nullable Context context, + @Nullable Scope scope) { + this.callDepth = callDepth; + this.classAndMethod = classAndMethod; + this.context = context; + this.scope = scope; } - // TODO review and unify with all other SERVER instrumentation - Context parentContext = THREAD_LOCAL_CONTEXT.getAndResetContext(); - if (parentContext == null) { - return; + public static AdviceScope start( + CallDepth callDepth, Class declaringClass, String methodName) { + if (callDepth.getAndIncrement() > 0) { + return new AdviceScope(callDepth, null, null, null); + } + + // TODO review and unify with all other SERVER instrumentation + Context parentContext = THREAD_LOCAL_CONTEXT.getAndResetContext(); + if (parentContext == null) { + return new AdviceScope(callDepth, null, null, null); + } + ClassAndMethod classAndMethod = ClassAndMethod.create(declaringClass, methodName); + if (!instrumenter().shouldStart(parentContext, classAndMethod)) { + return new AdviceScope(callDepth, null, null, null); + } + Context context = instrumenter().start(parentContext, classAndMethod); + return new AdviceScope(callDepth, classAndMethod, context, context.makeCurrent()); } - request = ClassAndMethod.create(declaringClass, methodName); - if (!instrumenter().shouldStart(parentContext, request)) { - return; + + public void end(Throwable throwable) { + if (callDepth.decrementAndGet() > 0) { + return; + } + if (scope == null || context == null || classAndMethod == null) { + return; + } + scope.close(); + instrumenter().end(context, classAndMethod, null, throwable); } + } - context = instrumenter().start(parentContext, request); - scope = context.makeCurrent(); + @Advice.OnMethodEnter(suppress = Throwable.class) + public static AdviceScope onEnter( + @Advice.Origin("#t") Class declaringClass, @Advice.Origin("#m") String methodName) { + return AdviceScope.start(CallDepth.forClass(Remote.class), declaringClass, methodName); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void stopSpan( - @Advice.Thrown Throwable throwable, - @Advice.Local("otelCallDepth") CallDepth callDepth, - @Advice.Local("otelRequest") ClassAndMethod request, - @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { - - if (callDepth.decrementAndGet() > 0) { - return; - } - if (scope == null) { - return; - } - scope.close(); - instrumenter().end(context, request, null, throwable); + @Advice.Thrown @Nullable Throwable throwable, @Advice.Enter AdviceScope adviceScope) { + adviceScope.end(throwable); } } } diff --git a/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/server/RmiServerInstrumentationModule.java b/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/server/RmiServerInstrumentationModule.java index cf6731111e0f..8bdaba4be2ef 100644 --- a/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/server/RmiServerInstrumentationModule.java +++ b/instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/server/RmiServerInstrumentationModule.java @@ -10,10 +10,12 @@ import com.google.auto.service.AutoService; import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule; import java.util.List; @AutoService(InstrumentationModule.class) -public class RmiServerInstrumentationModule extends InstrumentationModule { +public class RmiServerInstrumentationModule extends InstrumentationModule + implements ExperimentalInstrumentationModule { public RmiServerInstrumentationModule() { super("rmi", "rmi-server"); @@ -23,4 +25,9 @@ public RmiServerInstrumentationModule() { public List typeInstrumentations() { return singletonList(new RemoteServerInstrumentation()); } + + @Override + public boolean isIndyReady() { + return true; + } } diff --git a/javaagent-tooling/javaagent-tooling-java9/src/main/java/io/opentelemetry/javaagent/tooling/ModuleOpener.java b/javaagent-tooling/javaagent-tooling-java9/src/main/java/io/opentelemetry/javaagent/tooling/ModuleOpener.java index 5f5327f56ba7..b13e6e287859 100644 --- a/javaagent-tooling/javaagent-tooling-java9/src/main/java/io/opentelemetry/javaagent/tooling/ModuleOpener.java +++ b/javaagent-tooling/javaagent-tooling-java9/src/main/java/io/opentelemetry/javaagent/tooling/ModuleOpener.java @@ -60,7 +60,7 @@ public static void open( missingOpens.put(packageName, openToModuleSet); logger.log( FINE, - "Exposing package '{0}' in module '{1}' to module '{2}'", + "Exposing package {0} in module {1} to module {2}", new Object[] {packageName, targetModule, openToModule}); } }