Skip to content

Commit

Permalink
resolve some comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@chenlujjj
chenlujjj committed Jan 21, 2025
1 parent 06f0869 commit c92c0ef
Show file tree
Hide file tree
Showing 20 changed files with 61 additions and 108 deletions.
2 changes: 1 addition & 1 deletion docs/supported-libraries.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ These are the supported libraries and frameworks:
| [Jedis](https://github.com/xetorthio/jedis) | 1.4+ | N/A | [Database Client Spans] |
| [JMS](https://javaee.github.io/javaee-spec/javadocs/javax/jms/package-summary.html) | 1.1+ | N/A | [Messaging Spans] |
| [Jodd Http](https://http.jodd.org/) | 4.2+ | N/A | [HTTP Client Spans], [HTTP Client Metrics] |
| [JSON-RPC](https://github.com/briandilley/jsonrpc4j) | 1.3.3+ | N/A | [RPC Client Spans], [RPC Client Metrics], [RPC Server Spans], [RPC Server Metrics] |
| [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] |
| [JSP](https://javaee.github.io/javaee-spec/javadocs/javax/servlet/jsp/package-summary.html) | 2.3.x only | N/A | Controller Spans [3] |
| [Kotlin Coroutines](https://kotlinlang.org/docs/coroutines-overview.html) | 1.0+ | N/A | Context propagation |
| [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] |
Expand Down
4 changes: 0 additions & 4 deletions instrumentation/jsonrpc4j-1.3/javaagent/build.gradle.kts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,9 @@ dependencies {
library("com.github.briandilley.jsonrpc4j:jsonrpc4j:1.3.3")

testImplementation(project(":instrumentation:jsonrpc4j-1.3:testing"))

testImplementation("com.fasterxml.jackson.core:jackson-databind:2.13.3")

testImplementation("org.eclipse.jetty:jetty-server:9.4.49.v20220914")

testImplementation("org.eclipse.jetty:jetty-servlet:9.4.49.v20220914")

testImplementation("javax.portlet:portlet-api:2.0")
}

Expand Down
15 changes: 7 additions & 8 deletions .../opentelemetry/javaagent/instrumentation/jsonrpc4j/v1_3/JsonRpcClientInstrumentation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import io.opentelemetry.instrumentation.jsonrpc4j.v1_3.JsonRpcClientResponse;
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import java.lang.reflect.Type;
import java.util.Map;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.type.TypeDescription;
Expand All @@ -46,8 +47,8 @@ public void transform(TypeTransformer transformer) {
.and(takesArguments(4))
.and(takesArgument(0, String.class))
.and(takesArgument(1, Object.class))
.and(takesArgument(2, named("java.lang.reflect.Type")))
.and(takesArgument(3, named("java.util.Map")))
.and(takesArgument(2, Type.class))
.and(takesArgument(3, Map.class))
.and(returns(Object.class)),
this.getClass().getName() + "$InvokeAdvice");
}
Expand All @@ -60,10 +61,11 @@ public static void onEnter(
@Advice.Argument(0) String methodName,
@Advice.Argument(1) Object argument,
@Advice.Argument(3) Map<String, String> extraHeaders,
@Advice.Local("otelRequest") JsonRpcClientRequest request,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) {
Context parentContext = Context.current();
JsonRpcClientRequest request = new JsonRpcClientRequest(methodName, argument);
request = new JsonRpcClientRequest(methodName, argument);
if (!CLIENT_INSTRUMENTER.shouldStart(parentContext, request)) {
return;
}
Expand All @@ -79,18 +81,15 @@ public static void onExit(
@Advice.Argument(3) Map<String, String> extraHeaders,
@Advice.Return Object result,
@Advice.Thrown Throwable throwable,
@Advice.Local("otelRequest") JsonRpcClientRequest request,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) {
if (scope == null) {
return;
}

scope.close();
CLIENT_INSTRUMENTER.end(
context,
new JsonRpcClientRequest(methodName, argument),
new JsonRpcClientResponse(result),
throwable);
CLIENT_INSTRUMENTER.end(context, request, new JsonRpcClientResponse(result), throwable);
}
}
}
44 changes: 15 additions & 29 deletions ...o/opentelemetry/javaagent/instrumentation/jsonrpc4j/v1_3/JsonRpcProxyInstrumentation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import java.lang.reflect.InvocationHandler;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.lang.reflect.Proxy;
import java.util.Map;
Expand Down Expand Up @@ -60,21 +61,6 @@ public static <T> void onExit(
proxy = instrumentCreateClientProxy(classLoader, proxyInterface, client, extraHeaders, proxy);
}

private static Object proxyObjectMethods(Method method, Object proxyObject, Object[] args) {
String name = method.getName();
switch (name) {
case "toString":
return proxyObject.getClass().getName() + "@" + System.identityHashCode(proxyObject);
case "hashCode":
return System.identityHashCode(proxyObject);
case "equals":
return proxyObject == args[0];
default:
throw new IllegalArgumentException(
method.getName() + " is not a member of java.lang.Object");
}
}

@SuppressWarnings({"unchecked"})
public static <T> T instrumentCreateClientProxy(
ClassLoader classLoader,
Expand All @@ -94,28 +80,28 @@ public Object invoke(Object proxy1, Method method, Object[] args) throws Throwab
Context parentContext = Context.current();
JsonRpcClientRequest request = new JsonRpcClientRequest(method, args);
if (!CLIENT_INSTRUMENTER.shouldStart(parentContext, request)) {
return method.invoke(proxy, args);
try {
return method.invoke(proxy, args);
} catch (InvocationTargetException exception) {
throw exception.getCause();
}
}

Context context = CLIENT_INSTRUMENTER.start(parentContext, request);
Scope scope = context.makeCurrent();
try {
Object result = method.invoke(proxy, args);
// after invoke
scope.close();
CLIENT_INSTRUMENTER.end(
context,
new JsonRpcClientRequest(method, args),
new JsonRpcClientResponse(result),
null);
return result;

Object result;
try (Scope scope = context.makeCurrent()) {
result = method.invoke(proxy, args);
} catch (Throwable t) {
// after invoke
scope.close();
CLIENT_INSTRUMENTER.end(context, request, null, t);
throw t;
}
CLIENT_INSTRUMENTER.end(
context,
new JsonRpcClientRequest(method, args),
new JsonRpcClientResponse(result),
null);
return result;
}
});
}
Expand Down
33 changes: 9 additions & 24 deletions .../opentelemetry/javaagent/instrumentation/jsonrpc4j/v1_3/JsonRpcServerInstrumentation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

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

import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
import static io.opentelemetry.javaagent.instrumentation.jsonrpc4j.v1_3.JsonRpcSingletons.SERVER_INVOCATION_LISTENER;
import static net.bytebuddy.matcher.ElementMatchers.isConstructor;
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
Expand All @@ -14,21 +13,14 @@
import com.googlecode.jsonrpc4j.InvocationListener;
import com.googlecode.jsonrpc4j.JsonRpcBasicServer;
import com.googlecode.jsonrpc4j.MultipleInvocationListener;
import io.opentelemetry.instrumentation.api.util.VirtualField;
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.implementation.bytecode.assign.Assigner;
import net.bytebuddy.matcher.ElementMatcher;

public class JsonRpcServerInstrumentation implements TypeInstrumentation {

@Override
public ElementMatcher<ClassLoader> classLoaderOptimization() {
return hasClassesNamed("com.googlecode.jsonrpc4j.JsonRpcBasicServer");
}

@Override
public ElementMatcher<TypeDescription> typeMatcher() {
return named("com.googlecode.jsonrpc4j.JsonRpcBasicServer");
Expand Down Expand Up @@ -62,22 +54,15 @@ public static class SetInvocationListenerAdvice {
@Advice.OnMethodEnter(suppress = Throwable.class)
public static void setInvocationListener(
@Advice.This JsonRpcBasicServer jsonRpcServer,
@Advice.Argument(value = 0, readOnly = false, typing = Assigner.Typing.DYNAMIC)
InvocationListener invocationListener) {
VirtualField<JsonRpcBasicServer, Boolean> instrumented =
VirtualField.find(JsonRpcBasicServer.class, Boolean.class);
if (!Boolean.TRUE.equals(instrumented.get(jsonRpcServer))) {
if (invocationListener == null) {
invocationListener = SERVER_INVOCATION_LISTENER;
} else if (invocationListener instanceof MultipleInvocationListener) {
((MultipleInvocationListener) invocationListener)
.addInvocationListener(SERVER_INVOCATION_LISTENER);
} else {
invocationListener =
new MultipleInvocationListener(invocationListener, SERVER_INVOCATION_LISTENER);
}

instrumented.set(jsonRpcServer, true);
@Advice.Argument(value = 0, readOnly = false) InvocationListener invocationListener) {
if (invocationListener == null) {
invocationListener = SERVER_INVOCATION_LISTENER;
} else if (invocationListener instanceof MultipleInvocationListener) {
((MultipleInvocationListener) invocationListener)
.addInvocationListener(SERVER_INVOCATION_LISTENER);
} else if (invocationListener != SERVER_INVOCATION_LISTENER) {
invocationListener =
new MultipleInvocationListener(invocationListener, SERVER_INVOCATION_LISTENER);
}
}
}
Expand Down
7 changes: 3 additions & 4 deletions .../src/test/java/io/opentelemetry/javaagent/instrumentation/jsonrpc4j/v1_3/JettyServer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,15 @@
import com.googlecode.jsonrpc4j.JsonRpcServer;
import java.io.IOException;
import java.lang.reflect.InvocationTargetException;
import java.util.Random;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.servlet.ServletContextHandler;
import org.eclipse.jetty.servlet.ServletHolder;

@SuppressWarnings("WeakerAccess")
public class JettyServer implements AutoCloseable {

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

public void startup() throws Exception {
port = 10000 + new Random().nextInt(30000);
jetty = new Server(port);
jetty = new Server(0);
ServletContextHandler context = new ServletContextHandler(ServletContextHandler.NO_SESSIONS);
context.setContextPath("/");
jetty.setHandler(context);
ServletHolder servlet = context.addServlet(JsonRpcTestServlet.class, "/" + SERVLET);
servlet.setInitParameter("class", service.getCanonicalName());
jetty.start();
port = ((ServerConnector) jetty.getConnectors()[0]).getLocalPort();
}

@Override
Expand Down
5 changes: 1 addition & 4 deletions instrumentation/jsonrpc4j-1.3/library/build.gradle.kts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,9 @@ plugins {
id("otel.library-instrumentation")
}

val jacksonVersion = "2.13.3"

dependencies {
library("com.github.briandilley.jsonrpc4j:jsonrpc4j:1.3.3")

library("com.fasterxml.jackson.core:jackson-databind:$jacksonVersion")
library("com.fasterxml.jackson.core:jackson-databind")

testImplementation(project(":instrumentation:jsonrpc4j-1.3:testing"))
}
7 changes: 6 additions & 1 deletion ...ava/io/opentelemetry/instrumentation/jsonrpc4j/v1_3/JsonRpcClientAttributesExtractor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

package io.opentelemetry.instrumentation.jsonrpc4j.v1_3;

import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.api.common.AttributesBuilder;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
Expand All @@ -14,10 +15,14 @@
final class JsonRpcClientAttributesExtractor
implements AttributesExtractor<JsonRpcClientRequest, JsonRpcClientResponse> {

// copied from RpcIncubatingAttributes
private static final AttributeKey<String> RPC_JSONRPC_VERSION =
AttributeKey.stringKey("rpc.jsonrpc.version");

@Override
public void onStart(
AttributesBuilder attributes, Context parentContext, JsonRpcClientRequest jsonRpcRequest) {
attributes.put("rpc.jsonrpc.version", "2.0");
attributes.put(RPC_JSONRPC_VERSION, "2.0");
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion ...n/java/io/opentelemetry/instrumentation/jsonrpc4j/v1_3/JsonRpcClientAttributesGetter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
// Check
// https://github.com/open-telemetry/semantic-conventions/blob/main/docs/rpc/rpc-metrics.md#attributes
// Check https://opentelemetry.io/docs/specs/semconv/rpc/json-rpc/
public enum JsonRpcClientAttributesGetter implements RpcAttributesGetter<JsonRpcClientRequest> {
enum JsonRpcClientAttributesGetter implements RpcAttributesGetter<JsonRpcClientRequest> {
INSTANCE;

@Override
Expand Down
2 changes: 1 addition & 1 deletion .../java/io/opentelemetry/instrumentation/jsonrpc4j/v1_3/JsonRpcClientSpanNameExtractor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor;
import java.lang.reflect.Method;

public class JsonRpcClientSpanNameExtractor implements SpanNameExtractor<JsonRpcClientRequest> {
final class JsonRpcClientSpanNameExtractor implements SpanNameExtractor<JsonRpcClientRequest> {
@Override
public String extract(JsonRpcClientRequest request) {
if (request.getMethod() == null) {
Expand Down
8 changes: 4 additions & 4 deletions ...ava/io/opentelemetry/instrumentation/jsonrpc4j/v1_3/JsonRpcServerAttributesExtractor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,20 @@
final class JsonRpcServerAttributesExtractor
implements AttributesExtractor<JsonRpcServerRequest, JsonRpcServerResponse> {

// copied from RpcIncubatingAttributes
private static final AttributeKey<Long> RPC_JSONRPC_ERROR_CODE =
AttributeKey.longKey("rpc.jsonrpc.error_code");

private static final AttributeKey<String> RPC_JSONRPC_ERROR_MESSAGE =
AttributeKey.stringKey("rpc.jsonrpc.error_message");
private static final AttributeKey<String> RPC_JSONRPC_VERSION =
AttributeKey.stringKey("rpc.jsonrpc.version");

@Override
public void onStart(
AttributesBuilder attributes,
Context parentContext,
JsonRpcServerRequest jsonRpcServerRequest) {
attributes.put("rpc.jsonrpc.version", "2.0");
attributes.put(RPC_JSONRPC_VERSION, "2.0");
}

@Override
Expand All @@ -50,8 +52,6 @@ public void onEnd(
error, jsonRpcServerRequest.getMethod(), jsonRpcServerRequest.getArguments());
attributes.put(RPC_JSONRPC_ERROR_CODE, jsonError.code);
attributes.put(RPC_JSONRPC_ERROR_MESSAGE, jsonError.message);
} else {
attributes.put(RPC_JSONRPC_ERROR_CODE, ErrorResolver.JsonError.OK.code);
}
}
}
2 changes: 1 addition & 1 deletion ...n/java/io/opentelemetry/instrumentation/jsonrpc4j/v1_3/JsonRpcServerAttributesGetter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
// Check
// https://github.com/open-telemetry/semantic-conventions/blob/main/docs/rpc/rpc-metrics.md#attributes
// Check https://opentelemetry.io/docs/specs/semconv/rpc/json-rpc/
public enum JsonRpcServerAttributesGetter implements RpcAttributesGetter<JsonRpcServerRequest> {
enum JsonRpcServerAttributesGetter implements RpcAttributesGetter<JsonRpcServerRequest> {
INSTANCE;

@Override
Expand Down
2 changes: 1 addition & 1 deletion ...y/src/main/java/io/opentelemetry/instrumentation/jsonrpc4j/v1_3/JsonRpcServerRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import java.lang.reflect.Method;
import java.util.List;

public final class JsonRpcServerRequest {
final class JsonRpcServerRequest {

private final Method method;
private final List<JsonNode> arguments;
Expand Down
2 changes: 1 addition & 1 deletion .../src/main/java/io/opentelemetry/instrumentation/jsonrpc4j/v1_3/JsonRpcServerResponse.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import java.lang.reflect.Method;
import java.util.List;

public final class JsonRpcServerResponse {
final class JsonRpcServerResponse {
private final Method method;
private final List<JsonNode> params;
private final Object result;
Expand Down
Loading

0 comments on commit c92c0ef

Please sign in to comment.