Skip to content

Commit 145e58d

Browse files
chenlujjjchenlujjj
committed
resolve some comments
1 parent 06f0869 commit 145e58d

File tree

19 files changed

+53
-89
lines changed

19 files changed

+53
-89
lines changed

‎docs/supported-libraries.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ These are the supported libraries and frameworks:
9090
| [Jedis](https://github.com/xetorthio/jedis) | 1.4+ | N/A | [Database Client Spans] |
9191
| [JMS](https://javaee.github.io/javaee-spec/javadocs/javax/jms/package-summary.html) | 1.1+ | N/A | [Messaging Spans] |
9292
| [Jodd Http](https://http.jodd.org/) | 4.2+ | N/A | [HTTP Client Spans], [HTTP Client Metrics] |
93-
| [JSON-RPC](https://github.com/briandilley/jsonrpc4j) | 1.3.3+ | N/A | [RPC Client Spans], [RPC Client Metrics], [RPC Server Spans], [RPC Server Metrics] |
93+
| [JSON-RPC for Java](https://github.com/briandilley/jsonrpc4j) | 1.3.3+ | N/A | [RPC Client Spans], [RPC Client Metrics], [RPC Server Spans], [RPC Server Metrics] |
9494
| [JSP](https://javaee.github.io/javaee-spec/javadocs/javax/servlet/jsp/package-summary.html) | 2.3.x only | N/A | Controller Spans [3] |
9595
| [Kotlin Coroutines](https://kotlinlang.org/docs/coroutines-overview.html) | 1.0+ | N/A | Context propagation |
9696
| [Ktor](https://github.com/ktorio/ktor) | 1.0+ | [opentelemetry-ktor-1.0](../instrumentation/ktor/ktor-1.0/library),<br>[opentelemetry-ktor-2.0](../instrumentation/ktor/ktor-2.0/library),<br>[opentelemetry-ktor-3.0](../instrumentation/ktor/ktor-3.0/library) | [HTTP Client Spans], [HTTP Client Metrics], [HTTP Server Spans], [HTTP Server Metrics] |

‎instrumentation/jsonrpc4j-1.3/javaagent/build.gradle.kts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,9 @@ dependencies {
1717
library("com.github.briandilley.jsonrpc4j:jsonrpc4j:1.3.3")
1818

1919
testImplementation(project(":instrumentation:jsonrpc4j-1.3:testing"))
20-
2120
testImplementation("com.fasterxml.jackson.core:jackson-databind:2.13.3")
22-
2321
testImplementation("org.eclipse.jetty:jetty-server:9.4.49.v20220914")
24-
2522
testImplementation("org.eclipse.jetty:jetty-servlet:9.4.49.v20220914")
26-
2723
testImplementation("javax.portlet:portlet-api:2.0")
2824
}
2925

‎instrumentation/jsonrpc4j-1.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jsonrpc4j/v1_3/JsonRpcClientInstrumentation.java

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import io.opentelemetry.instrumentation.jsonrpc4j.v1_3.JsonRpcClientResponse;
2121
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
2222
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
23+
import java.lang.reflect.Type;
2324
import java.util.Map;
2425
import net.bytebuddy.asm.Advice;
2526
import net.bytebuddy.description.type.TypeDescription;
@@ -46,8 +47,8 @@ public void transform(TypeTransformer transformer) {
4647
.and(takesArguments(4))
4748
.and(takesArgument(0, String.class))
4849
.and(takesArgument(1, Object.class))
49-
.and(takesArgument(2, named("java.lang.reflect.Type")))
50-
.and(takesArgument(3, named("java.util.Map")))
50+
.and(takesArgument(2, Type.class))
51+
.and(takesArgument(3, Map.class))
5152
.and(returns(Object.class)),
5253
this.getClass().getName() + "$InvokeAdvice");
5354
}
@@ -60,10 +61,11 @@ public static void onEnter(
6061
@Advice.Argument(0) String methodName,
6162
@Advice.Argument(1) Object argument,
6263
@Advice.Argument(3) Map<String, String> extraHeaders,
64+
@Advice.Local("otelRequest") JsonRpcClientRequest request,
6365
@Advice.Local("otelContext") Context context,
6466
@Advice.Local("otelScope") Scope scope) {
6567
Context parentContext = Context.current();
66-
JsonRpcClientRequest request = new JsonRpcClientRequest(methodName, argument);
68+
request = new JsonRpcClientRequest(methodName, argument);
6769
if (!CLIENT_INSTRUMENTER.shouldStart(parentContext, request)) {
6870
return;
6971
}
@@ -79,18 +81,15 @@ public static void onExit(
7981
@Advice.Argument(3) Map<String, String> extraHeaders,
8082
@Advice.Return Object result,
8183
@Advice.Thrown Throwable throwable,
84+
@Advice.Local("otelRequest") JsonRpcClientRequest request,
8285
@Advice.Local("otelContext") Context context,
8386
@Advice.Local("otelScope") Scope scope) {
8487
if (scope == null) {
8588
return;
8689
}
8790

8891
scope.close();
89-
CLIENT_INSTRUMENTER.end(
90-
context,
91-
new JsonRpcClientRequest(methodName, argument),
92-
new JsonRpcClientResponse(result),
93-
throwable);
92+
CLIENT_INSTRUMENTER.end(context, request, new JsonRpcClientResponse(result), throwable);
9493
}
9594
}
9695
}

‎instrumentation/jsonrpc4j-1.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jsonrpc4j/v1_3/JsonRpcProxyInstrumentation.java

Lines changed: 15 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
2121
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
2222
import java.lang.reflect.InvocationHandler;
23+
import java.lang.reflect.InvocationTargetException;
2324
import java.lang.reflect.Method;
2425
import java.lang.reflect.Proxy;
2526
import java.util.Map;
@@ -60,21 +61,6 @@ public static <T> void onExit(
6061
proxy = instrumentCreateClientProxy(classLoader, proxyInterface, client, extraHeaders, proxy);
6162
}
6263

63-
private static Object proxyObjectMethods(Method method, Object proxyObject, Object[] args) {
64-
String name = method.getName();
65-
switch (name) {
66-
case "toString":
67-
return proxyObject.getClass().getName() + "@" + System.identityHashCode(proxyObject);
68-
case "hashCode":
69-
return System.identityHashCode(proxyObject);
70-
case "equals":
71-
return proxyObject == args[0];
72-
default:
73-
throw new IllegalArgumentException(
74-
method.getName() + " is not a member of java.lang.Object");
75-
}
76-
}
77-
7864
@SuppressWarnings({"unchecked"})
7965
public static <T> T instrumentCreateClientProxy(
8066
ClassLoader classLoader,
@@ -94,28 +80,28 @@ public Object invoke(Object proxy1, Method method, Object[] args) throws Throwab
9480
Context parentContext = Context.current();
9581
JsonRpcClientRequest request = new JsonRpcClientRequest(method, args);
9682
if (!CLIENT_INSTRUMENTER.shouldStart(parentContext, request)) {
97-
return method.invoke(proxy, args);
83+
try {
84+
return method.invoke(proxy, args);
85+
} catch (InvocationTargetException exception) {
86+
throw exception.getCause();
87+
}
9888
}
9989

10090
Context context = CLIENT_INSTRUMENTER.start(parentContext, request);
101-
Scope scope = context.makeCurrent();
102-
try {
103-
Object result = method.invoke(proxy, args);
104-
// after invoke
105-
scope.close();
106-
CLIENT_INSTRUMENTER.end(
107-
context,
108-
new JsonRpcClientRequest(method, args),
109-
new JsonRpcClientResponse(result),
110-
null);
111-
return result;
112-
91+
Object result;
92+
try (Scope scope = context.makeCurrent()) {
93+
result = method.invoke(proxy, args);
11394
} catch (Throwable t) {
11495
// after invoke
115-
scope.close();
11696
CLIENT_INSTRUMENTER.end(context, request, null, t);
11797
throw t;
11898
}
99+
CLIENT_INSTRUMENTER.end(
100+
context,
101+
new JsonRpcClientRequest(method, args),
102+
new JsonRpcClientResponse(result),
103+
null);
104+
return result;
119105
}
120106
});
121107
}

‎instrumentation/jsonrpc4j-1.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jsonrpc4j/v1_3/JsonRpcServerInstrumentation.java

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

66
package io.opentelemetry.javaagent.instrumentation.jsonrpc4j.v1_3;
77

8-
import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
98
import static io.opentelemetry.javaagent.instrumentation.jsonrpc4j.v1_3.JsonRpcSingletons.SERVER_INVOCATION_LISTENER;
109
import static net.bytebuddy.matcher.ElementMatchers.isConstructor;
1110
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
@@ -19,16 +18,10 @@
1918
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
2019
import net.bytebuddy.asm.Advice;
2120
import net.bytebuddy.description.type.TypeDescription;
22-
import net.bytebuddy.implementation.bytecode.assign.Assigner;
2321
import net.bytebuddy.matcher.ElementMatcher;
2422

2523
public class JsonRpcServerInstrumentation implements TypeInstrumentation {
2624

27-
@Override
28-
public ElementMatcher<ClassLoader> classLoaderOptimization() {
29-
return hasClassesNamed("com.googlecode.jsonrpc4j.JsonRpcBasicServer");
30-
}
31-
3225
@Override
3326
public ElementMatcher<TypeDescription> typeMatcher() {
3427
return named("com.googlecode.jsonrpc4j.JsonRpcBasicServer");
@@ -62,8 +55,7 @@ public static class SetInvocationListenerAdvice {
6255
@Advice.OnMethodEnter(suppress = Throwable.class)
6356
public static void setInvocationListener(
6457
@Advice.This JsonRpcBasicServer jsonRpcServer,
65-
@Advice.Argument(value = 0, readOnly = false, typing = Assigner.Typing.DYNAMIC)
66-
InvocationListener invocationListener) {
58+
@Advice.Argument(value = 0, readOnly = false) InvocationListener invocationListener) {
6759
VirtualField<JsonRpcBasicServer, Boolean> instrumented =
6860
VirtualField.find(JsonRpcBasicServer.class, Boolean.class);
6961
if (!Boolean.TRUE.equals(instrumented.get(jsonRpcServer))) {

‎instrumentation/jsonrpc4j-1.3/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jsonrpc4j/v1_3/JettyServer.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,15 @@
99
import com.googlecode.jsonrpc4j.JsonRpcServer;
1010
import java.io.IOException;
1111
import java.lang.reflect.InvocationTargetException;
12-
import java.util.Random;
1312
import javax.servlet.ServletException;
1413
import javax.servlet.http.HttpServlet;
1514
import javax.servlet.http.HttpServletRequest;
1615
import javax.servlet.http.HttpServletResponse;
1716
import org.eclipse.jetty.server.Server;
17+
import org.eclipse.jetty.server.ServerConnector;
1818
import org.eclipse.jetty.servlet.ServletContextHandler;
1919
import org.eclipse.jetty.servlet.ServletHolder;
2020

21-
@SuppressWarnings("WeakerAccess")
2221
public class JettyServer implements AutoCloseable {
2322

2423
public static final String DEFAULT_LOCAL_HOSTNAME = "127.0.0.1";
@@ -40,14 +39,14 @@ public String getCustomServerUrlString(String servletName) {
4039
}
4140

4241
public void startup() throws Exception {
43-
port = 10000 + new Random().nextInt(30000);
44-
jetty = new Server(port);
42+
jetty = new Server(0);
4543
ServletContextHandler context = new ServletContextHandler(ServletContextHandler.NO_SESSIONS);
4644
context.setContextPath("/");
4745
jetty.setHandler(context);
4846
ServletHolder servlet = context.addServlet(JsonRpcTestServlet.class, "/" + SERVLET);
4947
servlet.setInitParameter("class", service.getCanonicalName());
5048
jetty.start();
49+
port = ((ServerConnector) jetty.getConnectors()[0]).getLocalPort();
5150
}
5251

5352
@Override

‎instrumentation/jsonrpc4j-1.3/library/build.gradle.kts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,9 @@ plugins {
22
id("otel.library-instrumentation")
33
}
44

5-
val jacksonVersion = "2.13.3"
6-
75
dependencies {
86
library("com.github.briandilley.jsonrpc4j:jsonrpc4j:1.3.3")
9-
10-
library("com.fasterxml.jackson.core:jackson-databind:$jacksonVersion")
7+
library("com.fasterxml.jackson.core:jackson-databind")
118

129
testImplementation(project(":instrumentation:jsonrpc4j-1.3:testing"))
1310
}

‎instrumentation/jsonrpc4j-1.3/library/src/main/java/io/opentelemetry/instrumentation/jsonrpc4j/v1_3/JsonRpcClientAttributesExtractor.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
package io.opentelemetry.instrumentation.jsonrpc4j.v1_3;
77

8+
import io.opentelemetry.api.common.AttributeKey;
89
import io.opentelemetry.api.common.AttributesBuilder;
910
import io.opentelemetry.context.Context;
1011
import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
@@ -14,10 +15,14 @@
1415
final class JsonRpcClientAttributesExtractor
1516
implements AttributesExtractor<JsonRpcClientRequest, JsonRpcClientResponse> {
1617

18+
// copied from RpcIncubatingAttributes
19+
private static final AttributeKey<String> RPC_JSONRPC_VERSION =
20+
AttributeKey.stringKey("rpc.jsonrpc.version");
21+
1722
@Override
1823
public void onStart(
1924
AttributesBuilder attributes, Context parentContext, JsonRpcClientRequest jsonRpcRequest) {
20-
attributes.put("rpc.jsonrpc.version", "2.0");
25+
attributes.put(RPC_JSONRPC_VERSION, "2.0");
2126
}
2227

2328
@Override

‎instrumentation/jsonrpc4j-1.3/library/src/main/java/io/opentelemetry/instrumentation/jsonrpc4j/v1_3/JsonRpcClientAttributesGetter.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
// Check
1111
// https://github.com/open-telemetry/semantic-conventions/blob/main/docs/rpc/rpc-metrics.md#attributes
1212
// Check https://opentelemetry.io/docs/specs/semconv/rpc/json-rpc/
13-
public enum JsonRpcClientAttributesGetter implements RpcAttributesGetter<JsonRpcClientRequest> {
13+
enum JsonRpcClientAttributesGetter implements RpcAttributesGetter<JsonRpcClientRequest> {
1414
INSTANCE;
1515

1616
@Override

‎instrumentation/jsonrpc4j-1.3/library/src/main/java/io/opentelemetry/instrumentation/jsonrpc4j/v1_3/JsonRpcClientSpanNameExtractor.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor;
99
import java.lang.reflect.Method;
1010

11-
public class JsonRpcClientSpanNameExtractor implements SpanNameExtractor<JsonRpcClientRequest> {
11+
final class JsonRpcClientSpanNameExtractor implements SpanNameExtractor<JsonRpcClientRequest> {
1212
@Override
1313
public String extract(JsonRpcClientRequest request) {
1414
if (request.getMethod() == null) {

‎instrumentation/jsonrpc4j-1.3/library/src/main/java/io/opentelemetry/instrumentation/jsonrpc4j/v1_3/JsonRpcServerAttributesExtractor.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,18 +19,20 @@
1919
final class JsonRpcServerAttributesExtractor
2020
implements AttributesExtractor<JsonRpcServerRequest, JsonRpcServerResponse> {
2121

22+
// copied from RpcIncubatingAttributes
2223
private static final AttributeKey<Long> RPC_JSONRPC_ERROR_CODE =
2324
AttributeKey.longKey("rpc.jsonrpc.error_code");
24-
2525
private static final AttributeKey<String> RPC_JSONRPC_ERROR_MESSAGE =
2626
AttributeKey.stringKey("rpc.jsonrpc.error_message");
27+
private static final AttributeKey<String> RPC_JSONRPC_VERSION =
28+
AttributeKey.stringKey("rpc.jsonrpc.version");
2729

2830
@Override
2931
public void onStart(
3032
AttributesBuilder attributes,
3133
Context parentContext,
3234
JsonRpcServerRequest jsonRpcServerRequest) {
33-
attributes.put("rpc.jsonrpc.version", "2.0");
35+
attributes.put(RPC_JSONRPC_VERSION, "2.0");
3436
}
3537

3638
@Override
@@ -50,8 +52,6 @@ public void onEnd(
5052
error, jsonRpcServerRequest.getMethod(), jsonRpcServerRequest.getArguments());
5153
attributes.put(RPC_JSONRPC_ERROR_CODE, jsonError.code);
5254
attributes.put(RPC_JSONRPC_ERROR_MESSAGE, jsonError.message);
53-
} else {
54-
attributes.put(RPC_JSONRPC_ERROR_CODE, ErrorResolver.JsonError.OK.code);
5555
}
5656
}
5757
}

‎instrumentation/jsonrpc4j-1.3/library/src/main/java/io/opentelemetry/instrumentation/jsonrpc4j/v1_3/JsonRpcServerAttributesGetter.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
// Check
1111
// https://github.com/open-telemetry/semantic-conventions/blob/main/docs/rpc/rpc-metrics.md#attributes
1212
// Check https://opentelemetry.io/docs/specs/semconv/rpc/json-rpc/
13-
public enum JsonRpcServerAttributesGetter implements RpcAttributesGetter<JsonRpcServerRequest> {
13+
enum JsonRpcServerAttributesGetter implements RpcAttributesGetter<JsonRpcServerRequest> {
1414
INSTANCE;
1515

1616
@Override

‎instrumentation/jsonrpc4j-1.3/library/src/main/java/io/opentelemetry/instrumentation/jsonrpc4j/v1_3/JsonRpcServerRequest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
import java.lang.reflect.Method;
1010
import java.util.List;
1111

12-
public final class JsonRpcServerRequest {
12+
final class JsonRpcServerRequest {
1313

1414
private final Method method;
1515
private final List<JsonNode> arguments;

‎instrumentation/jsonrpc4j-1.3/library/src/main/java/io/opentelemetry/instrumentation/jsonrpc4j/v1_3/JsonRpcServerResponse.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
import java.lang.reflect.Method;
1010
import java.util.List;
1111

12-
public final class JsonRpcServerResponse {
12+
final class JsonRpcServerResponse {
1313
private final Method method;
1414
private final List<JsonNode> params;
1515
private final Object result;

‎instrumentation/jsonrpc4j-1.3/library/src/main/java/io/opentelemetry/instrumentation/jsonrpc4j/v1_3/JsonRpcServerSpanNameExtractor.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor;
99
import java.lang.reflect.Method;
1010

11-
public class JsonRpcServerSpanNameExtractor implements SpanNameExtractor<JsonRpcServerRequest> {
11+
final class JsonRpcServerSpanNameExtractor implements SpanNameExtractor<JsonRpcServerRequest> {
1212
// Follow https://opentelemetry.io/docs/specs/semconv/rpc/rpc-spans/#span-name
1313
@Override
1414
public String extract(JsonRpcServerRequest request) {

0 commit comments

Comments
 (0)