Skip to content

Commit 8ec21ac

Browse files
SylvainJugeSylvainJuge
authored
make rmi indy-ready (#14845)
1 parent 6ea0731 commit 8ec21ac

File tree

9 files changed

+115
-72
lines changed

9 files changed

+115
-72
lines changed

‎instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/client/RmiClientInstrumentationModule.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,12 @@
1010
import com.google.auto.service.AutoService;
1111
import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
1212
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
13+
import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule;
1314
import java.util.List;
1415

1516
@AutoService(InstrumentationModule.class)
16-
public class RmiClientInstrumentationModule extends InstrumentationModule {
17+
public class RmiClientInstrumentationModule extends InstrumentationModule
18+
implements ExperimentalInstrumentationModule {
1719

1820
public RmiClientInstrumentationModule() {
1921
super("rmi", "rmi-client");
@@ -23,4 +25,9 @@ public RmiClientInstrumentationModule() {
2325
public List<TypeInstrumentation> typeInstrumentations() {
2426
return singletonList(new UnicastRefInstrumentation());
2527
}
28+
29+
@Override
30+
public boolean isIndyReady() {
31+
return true;
32+
}
2633
}

‎instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/client/UnicastRefInstrumentation.java‎

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,10 @@
1212

1313
import io.opentelemetry.context.Context;
1414
import io.opentelemetry.context.Scope;
15-
import io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge;
1615
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
1716
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
1817
import java.lang.reflect.Method;
18+
import javax.annotation.Nullable;
1919
import net.bytebuddy.asm.Advice;
2020
import net.bytebuddy.description.type.TypeDescription;
2121
import net.bytebuddy.matcher.ElementMatcher;
@@ -40,34 +40,45 @@ public void transform(TypeTransformer transformer) {
4040
@SuppressWarnings("unused")
4141
public static class InvokeAdvice {
4242

43-
@Advice.OnMethodEnter(suppress = Throwable.class)
44-
public static void onEnter(
45-
@Advice.Argument(value = 1) Method method,
46-
@Advice.Local("otelContext") Context context,
47-
@Advice.Local("otelScope") Scope scope) {
43+
public static class AdviceScope {
44+
private final Context context;
45+
private final Scope scope;
46+
47+
private AdviceScope(Context context, Scope scope) {
48+
this.context = context;
49+
this.scope = scope;
50+
}
4851

49-
Context parentContext = Java8BytecodeBridge.currentContext();
52+
@Nullable
53+
public static AdviceScope start(Method method) {
54+
Context parentContext = Context.current();
55+
if (!instrumenter().shouldStart(parentContext, method)) {
56+
return null;
57+
}
58+
Context context = instrumenter().start(parentContext, method);
59+
return new AdviceScope(context, context.makeCurrent());
60+
}
5061

51-
if (!instrumenter().shouldStart(parentContext, method)) {
52-
return;
62+
public void end(Method method, @Nullable Throwable throwable) {
63+
scope.close();
64+
instrumenter().end(context, method, null, throwable);
5365
}
66+
}
5467

55-
context = instrumenter().start(parentContext, method);
56-
scope = context.makeCurrent();
68+
@Nullable
69+
@Advice.OnMethodEnter(suppress = Throwable.class)
70+
public static AdviceScope onEnter(@Advice.Argument(value = 1) Method method) {
71+
return AdviceScope.start(method);
5772
}
5873

5974
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
6075
public static void stopSpan(
6176
@Advice.Argument(value = 1) Method method,
62-
@Advice.Thrown Throwable throwable,
63-
@Advice.Local("otelContext") Context context,
64-
@Advice.Local("otelScope") Scope scope) {
65-
66-
if (scope == null) {
67-
return;
77+
@Advice.Thrown @Nullable Throwable throwable,
78+
@Advice.Enter @Nullable AdviceScope adviceScope) {
79+
if (adviceScope != null) {
80+
adviceScope.end(method, throwable);
6881
}
69-
scope.close();
70-
instrumenter().end(context, method, null, throwable);
7182
}
7283
}
7384
}

‎instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/context/ContextPropagator.java‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ public class ContextPropagator {
2222

2323
private static final Logger logger = Logger.getLogger(ContextPropagator.class.getName());
2424

25+
private static final VirtualField<Connection, Boolean> KNOWN_CONNECTION =
26+
VirtualField.find(Connection.class, Boolean.class);
27+
2528
// Internal RMI object ids that we don't want to trace
2629
private static final ObjID ACTIVATOR_ID = new ObjID(ObjID.ACTIVATOR_ID);
2730
private static final ObjID DGC_ID = new ObjID(ObjID.DGC_ID);
@@ -48,24 +51,22 @@ public boolean isOperationWithPayload(int operationId) {
4851
return operationId == CONTEXT_PAYLOAD_OPERATION_ID;
4952
}
5053

51-
public void attemptToPropagateContext(
52-
VirtualField<Connection, Boolean> knownConnections, Connection c, Context context) {
53-
if (checkIfContextCanBePassed(knownConnections, c)) {
54+
public void attemptToPropagateContext(Connection c, Context context) {
55+
if (checkIfContextCanBePassed(c)) {
5456
if (!syntheticCall(c, ContextPayload.from(context), CONTEXT_PAYLOAD_OPERATION_ID)) {
5557
logger.fine("Couldn't send context payload");
5658
}
5759
}
5860
}
5961

60-
private static boolean checkIfContextCanBePassed(
61-
VirtualField<Connection, Boolean> knownConnections, Connection c) {
62-
Boolean storedResult = knownConnections.get(c);
62+
private static boolean checkIfContextCanBePassed(Connection c) {
63+
Boolean storedResult = KNOWN_CONNECTION.get(c);
6364
if (storedResult != null) {
6465
return storedResult;
6566
}
6667

6768
boolean result = syntheticCall(c, null, CONTEXT_CHECK_CALL_OPERATION_ID);
68-
knownConnections.set(c, result);
69+
KNOWN_CONNECTION.set(c, result);
6970
return result;
7071
}
7172

‎instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/context/RmiContextPropagationInstrumentationModule.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,4 +37,9 @@ public Map<JavaModule, List<String>> jpmsModulesToOpen() {
3737
return Collections.singletonMap(
3838
JavaModule.ofType(Remote.class), Arrays.asList("sun.rmi.server", "sun.rmi.transport"));
3939
}
40+
41+
@Override
42+
public boolean isIndyReady() {
43+
return true;
44+
}
4045
}

‎instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/context/client/RmiClientContextInstrumentation.java‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212

1313
import io.opentelemetry.api.trace.Span;
1414
import io.opentelemetry.context.Context;
15-
import io.opentelemetry.instrumentation.api.util.VirtualField;
1615
import io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge;
1716
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
1817
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
@@ -77,10 +76,7 @@ public static void onEnter(@Advice.Argument(0) Connection c, @Advice.Argument(1)
7776
}
7877

7978
// caching if a connection can support enhanced format
80-
VirtualField<Connection, Boolean> knownConnections =
81-
VirtualField.find(Connection.class, Boolean.class);
82-
83-
PROPAGATOR.attemptToPropagateContext(knownConnections, c, currentContext);
79+
PROPAGATOR.attemptToPropagateContext(c, currentContext);
8480
}
8581
}
8682
}

‎instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/context/server/RmiServerContextInstrumentation.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
1515
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
1616
import net.bytebuddy.asm.Advice;
17+
import net.bytebuddy.asm.Advice.AssignReturned;
1718
import net.bytebuddy.description.type.TypeDescription;
1819
import net.bytebuddy.matcher.ElementMatcher;
1920
import sun.rmi.transport.Target;
@@ -38,17 +39,17 @@ public void transform(TypeTransformer transformer) {
3839
@SuppressWarnings("unused")
3940
public static class ObjectTableAdvice {
4041

42+
@AssignReturned.ToReturned
4143
@Advice.OnMethodExit(suppress = Throwable.class)
42-
public static void methodExit(
43-
@Advice.Argument(0) Object oe, @Advice.Return(readOnly = false) Target result) {
44+
public static Target methodExit(@Advice.Argument(0) Object oe, @Advice.Return Target result) {
4445
// comparing toString() output allows us to avoid using reflection to be able to compare
4546
// ObjID and ObjectEndpoint objects
4647
// ObjectEndpoint#toString() only returns this.objId.toString() value which is exactly
4748
// what we're interested in here.
4849
if (!CONTEXT_CALL_ID.toString().equals(oe.toString())) {
49-
return;
50+
return result;
5051
}
51-
result = ContextDispatcher.newDispatcherTarget();
52+
return ContextDispatcher.newDispatcherTarget();
5253
}
5354
}
5455
}

‎instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/server/RemoteServerInstrumentation.java‎

Lines changed: 49 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
2323
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
2424
import java.rmi.Remote;
25+
import javax.annotation.Nullable;
2526
import net.bytebuddy.asm.Advice;
2627
import net.bytebuddy.description.type.TypeDescription;
2728
import net.bytebuddy.matcher.ElementMatcher;
@@ -43,50 +44,64 @@ public void transform(TypeTransformer transformer) {
4344
@SuppressWarnings("unused")
4445
public static class PublicMethodAdvice {
4546

46-
@Advice.OnMethodEnter(suppress = Throwable.class)
47-
public static void onEnter(
48-
@Advice.Origin("#t") Class<?> declaringClass,
49-
@Advice.Origin("#m") String methodName,
50-
@Advice.Local("otelCallDepth") CallDepth callDepth,
51-
@Advice.Local("otelRequest") ClassAndMethod request,
52-
@Advice.Local("otelContext") Context context,
53-
@Advice.Local("otelScope") Scope scope) {
47+
public static class AdviceScope {
48+
private final CallDepth callDepth;
49+
@Nullable private final ClassAndMethod classAndMethod;
50+
@Nullable private final Context context;
51+
@Nullable private final Scope scope;
5452

55-
callDepth = CallDepth.forClass(Remote.class);
56-
if (callDepth.getAndIncrement() > 0) {
57-
return;
53+
private AdviceScope(
54+
CallDepth callDepth,
55+
@Nullable ClassAndMethod classAndMethod,
56+
@Nullable Context context,
57+
@Nullable Scope scope) {
58+
this.callDepth = callDepth;
59+
this.classAndMethod = classAndMethod;
60+
this.context = context;
61+
this.scope = scope;
5862
}
5963

60-
// TODO review and unify with all other SERVER instrumentation
61-
Context parentContext = THREAD_LOCAL_CONTEXT.getAndResetContext();
62-
if (parentContext == null) {
63-
return;
64+
public static AdviceScope start(
65+
CallDepth callDepth, Class<?> declaringClass, String methodName) {
66+
if (callDepth.getAndIncrement() > 0) {
67+
return new AdviceScope(callDepth, null, null, null);
68+
}
69+
70+
// TODO review and unify with all other SERVER instrumentation
71+
Context parentContext = THREAD_LOCAL_CONTEXT.getAndResetContext();
72+
if (parentContext == null) {
73+
return new AdviceScope(callDepth, null, null, null);
74+
}
75+
ClassAndMethod classAndMethod = ClassAndMethod.create(declaringClass, methodName);
76+
if (!instrumenter().shouldStart(parentContext, classAndMethod)) {
77+
return new AdviceScope(callDepth, null, null, null);
78+
}
79+
Context context = instrumenter().start(parentContext, classAndMethod);
80+
return new AdviceScope(callDepth, classAndMethod, context, context.makeCurrent());
6481
}
65-
request = ClassAndMethod.create(declaringClass, methodName);
66-
if (!instrumenter().shouldStart(parentContext, request)) {
67-
return;
82+
83+
public void end(Throwable throwable) {
84+
if (callDepth.decrementAndGet() > 0) {
85+
return;
86+
}
87+
if (scope == null || context == null || classAndMethod == null) {
88+
return;
89+
}
90+
scope.close();
91+
instrumenter().end(context, classAndMethod, null, throwable);
6892
}
93+
}
6994

70-
context = instrumenter().start(parentContext, request);
71-
scope = context.makeCurrent();
95+
@Advice.OnMethodEnter(suppress = Throwable.class)
96+
public static AdviceScope onEnter(
97+
@Advice.Origin("#t") Class<?> declaringClass, @Advice.Origin("#m") String methodName) {
98+
return AdviceScope.start(CallDepth.forClass(Remote.class), declaringClass, methodName);
7299
}
73100

74101
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
75102
public static void stopSpan(
76-
@Advice.Thrown Throwable throwable,
77-
@Advice.Local("otelCallDepth") CallDepth callDepth,
78-
@Advice.Local("otelRequest") ClassAndMethod request,
79-
@Advice.Local("otelContext") Context context,
80-
@Advice.Local("otelScope") Scope scope) {
81-
82-
if (callDepth.decrementAndGet() > 0) {
83-
return;
84-
}
85-
if (scope == null) {
86-
return;
87-
}
88-
scope.close();
89-
instrumenter().end(context, request, null, throwable);
103+
@Advice.Thrown @Nullable Throwable throwable, @Advice.Enter AdviceScope adviceScope) {
104+
adviceScope.end(throwable);
90105
}
91106
}
92107
}

‎instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/server/RmiServerInstrumentationModule.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,12 @@
1010
import com.google.auto.service.AutoService;
1111
import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
1212
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
13+
import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule;
1314
import java.util.List;
1415

1516
@AutoService(InstrumentationModule.class)
16-
public class RmiServerInstrumentationModule extends InstrumentationModule {
17+
public class RmiServerInstrumentationModule extends InstrumentationModule
18+
implements ExperimentalInstrumentationModule {
1719

1820
public RmiServerInstrumentationModule() {
1921
super("rmi", "rmi-server");
@@ -23,4 +25,9 @@ public RmiServerInstrumentationModule() {
2325
public List<TypeInstrumentation> typeInstrumentations() {
2426
return singletonList(new RemoteServerInstrumentation());
2527
}
28+
29+
@Override
30+
public boolean isIndyReady() {
31+
return true;
32+
}
2633
}

‎javaagent-tooling/javaagent-tooling-java9/src/main/java/io/opentelemetry/javaagent/tooling/ModuleOpener.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public static void open(
6060
missingOpens.put(packageName, openToModuleSet);
6161
logger.log(
6262
FINE,
63-
"Exposing package '{0}' in module '{1}' to module '{2}'",
63+
"Exposing package {0} in module {1} to module {2}",
6464
new Object[] {packageName, targetModule, openToModule});
6565
}
6666
}

0 commit comments

Comments
 (0)