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 8, 2025
1 parent 466971e commit a5e1a43
Show file tree
Hide file tree
Showing 11 changed files with 42 additions and 110 deletions.
2 changes: 1 addition & 1 deletion instrumentation/jsonrpc4j-1.6/javaagent/build.gradle.kts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ val jsonrpcVersion = "1.6"

dependencies {
implementation(project(":instrumentation:jsonrpc4j-1.6:library"))
implementation("com.github.briandilley.jsonrpc4j:jsonrpc4j:$jsonrpcVersion")
library("com.github.briandilley.jsonrpc4j:jsonrpc4j:$jsonrpcVersion")
testImplementation(project(":instrumentation:jsonrpc4j-1.6:testing"))
}

Expand Down
14 changes: 5 additions & 9 deletions .../JsonRpcClientBuilderInstrumentation.java → ...4j/v1_6/JsonRpcClientInstrumentation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface;
import static io.opentelemetry.javaagent.instrumentation.jsonrpc4j.v1_6.JsonRpcSingletons.CLIENT_INSTRUMENTER;
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
import static net.bytebuddy.matcher.ElementMatchers.named;
import static net.bytebuddy.matcher.ElementMatchers.returns;
Expand All @@ -15,7 +16,6 @@

import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.instrumentation.jsonrpc4j.v1_6.HeadersSetter;
import io.opentelemetry.instrumentation.jsonrpc4j.v1_6.SimpleJsonRpcRequest;
import io.opentelemetry.instrumentation.jsonrpc4j.v1_6.SimpleJsonRpcResponse;
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
Expand All @@ -25,7 +25,7 @@
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.matcher.ElementMatcher;

public class JsonRpcClientBuilderInstrumentation implements TypeInstrumentation {
public class JsonRpcClientInstrumentation implements TypeInstrumentation {

@Override
public ElementMatcher<ClassLoader> classLoaderOptimization() {
Expand Down Expand Up @@ -64,15 +64,11 @@ public static void onEnter(
@Advice.Local("otelScope") Scope scope) {
Context parentContext = Context.current();
SimpleJsonRpcRequest request = new SimpleJsonRpcRequest(methodName, argument);
if (!JsonRpcSingletons.CLIENT_INSTRUMENTER.shouldStart(parentContext, request)) {
if (!CLIENT_INSTRUMENTER.shouldStart(parentContext, request)) {
return;
}

context = JsonRpcSingletons.CLIENT_INSTRUMENTER.start(parentContext, request);
JsonRpcSingletons.PROPAGATORS
.getTextMapPropagator()
.inject(context, extraHeaders, HeadersSetter.INSTANCE);

context = CLIENT_INSTRUMENTER.start(parentContext, request);
scope = context.makeCurrent();
}

Expand All @@ -90,7 +86,7 @@ public static void onExit(
}

scope.close();
JsonRpcSingletons.CLIENT_INSTRUMENTER.end(
CLIENT_INSTRUMENTER.end(
context,
new SimpleJsonRpcRequest(methodName, argument),
new SimpleJsonRpcResponse(result),
Expand Down
5 changes: 1 addition & 4 deletions .../opentelemetry/javaagent/instrumentation/jsonrpc4j/v1_6/JsonRpcInstrumentationModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@ public JsonRpcInstrumentationModule() {

@Override
public List<TypeInstrumentation> typeInstrumentations() {
return asList(
new JsonRpcServerBuilderInstrumentation(),
new JsonServiceExporterBuilderInstrumentation(),
new JsonRpcClientBuilderInstrumentation());
return asList(new JsonRpcServerInstrumentation(), new JsonRpcClientInstrumentation());
}
}
36 changes: 33 additions & 3 deletions .../JsonRpcServerBuilderInstrumentation.java → ...4j/v1_6/JsonRpcServerInstrumentation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,21 @@

import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
import static net.bytebuddy.matcher.ElementMatchers.isConstructor;
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
import static net.bytebuddy.matcher.ElementMatchers.named;

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 JsonRpcServerBuilderInstrumentation implements TypeInstrumentation {
public class JsonRpcServerInstrumentation implements TypeInstrumentation {

@Override
public ElementMatcher<ClassLoader> classLoaderOptimization() {
Expand All @@ -34,6 +37,10 @@ public ElementMatcher<TypeDescription> typeMatcher() {
public void transform(TypeTransformer transformer) {
transformer.applyAdviceToMethod(
isConstructor(), this.getClass().getName() + "$ConstructorAdvice");

transformer.applyAdviceToMethod(
isMethod().and(named("setInvocationListener")),
this.getClass().getName() + "$SetInvocationListenerAdvice");
}

@SuppressWarnings("unused")
Expand All @@ -42,11 +49,34 @@ public static class ConstructorAdvice {
@Advice.OnMethodExit(suppress = Throwable.class)
public static void setInvocationListener(
@Advice.This JsonRpcBasicServer jsonRpcServer,
@Advice.FieldValue("invocationListener") InvocationListener invocationListener) {
@Advice.FieldValue(value = "invocationListener", readOnly = false)
InvocationListener invocationListener) {
invocationListener = JsonRpcSingletons.SERVER_INVOCATION_LISTENER;
}
}

@SuppressWarnings("unused")
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))) {
jsonRpcServer.setInvocationListener(JsonRpcSingletons.SERVER_INVOCATION_LISTENER);
if (invocationListener == null) {
invocationListener = JsonRpcSingletons.SERVER_INVOCATION_LISTENER;
} else if (invocationListener instanceof MultipleInvocationListener) {
((MultipleInvocationListener) invocationListener)
.addInvocationListener(JsonRpcSingletons.SERVER_INVOCATION_LISTENER);
} else {
invocationListener =
new MultipleInvocationListener(
invocationListener, JsonRpcSingletons.SERVER_INVOCATION_LISTENER);
}

instrumented.set(jsonRpcServer, true);
}
}
Expand Down
4 changes: 0 additions & 4 deletions ...ain/java/io/opentelemetry/javaagent/instrumentation/jsonrpc4j/v1_6/JsonRpcSingletons.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

import com.googlecode.jsonrpc4j.InvocationListener;
import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.context.propagation.ContextPropagators;
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import io.opentelemetry.instrumentation.jsonrpc4j.v1_6.JsonRpcTelemetry;
import io.opentelemetry.instrumentation.jsonrpc4j.v1_6.SimpleJsonRpcRequest;
Expand All @@ -19,14 +18,11 @@ public final class JsonRpcSingletons {

public static final Instrumenter<SimpleJsonRpcRequest, SimpleJsonRpcResponse> CLIENT_INSTRUMENTER;

public static final ContextPropagators PROPAGATORS;

static {
JsonRpcTelemetry telemetry = JsonRpcTelemetry.builder(GlobalOpenTelemetry.get()).build();

SERVER_INVOCATION_LISTENER = telemetry.newServerInvocationListener();
CLIENT_INSTRUMENTER = telemetry.getClientInstrumenter();
PROPAGATORS = telemetry.getPropagators();
}

private JsonRpcSingletons() {}
Expand Down
53 changes: 0 additions & 53 deletions ...y/javaagent/instrumentation/jsonrpc4j/v1_6/JsonServiceExporterBuilderInstrumentation.java
View file

This file was deleted.

2 changes: 1 addition & 1 deletion instrumentation/jsonrpc4j-1.6/library/build.gradle.kts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ val jsonrpcVersion = "1.6"
val jacksonVersion = "2.13.3"

dependencies {
implementation("com.github.briandilley.jsonrpc4j:jsonrpc4j:$jsonrpcVersion")
library("com.github.briandilley.jsonrpc4j:jsonrpc4j:$jsonrpcVersion")

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

Expand Down
19 changes: 0 additions & 19 deletions .../library/src/main/java/io/opentelemetry/instrumentation/jsonrpc4j/v1_6/HeadersSetter.java
View file

This file was deleted.

7 changes: 0 additions & 7 deletions ...ava/io/opentelemetry/instrumentation/jsonrpc4j/v1_6/JsonRpcClientAttributesExtractor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,6 @@
final class JsonRpcClientAttributesExtractor
implements AttributesExtractor<SimpleJsonRpcRequest, SimpleJsonRpcResponse> {

// private final JsonRpcClientAttributesGetter getter;
//
//
// JsonRpcClientAttributesExtractor(JsonRpcClientAttributesGetter getter) {
// this.getter = getter;
// }

@Override
public void onStart(
AttributesBuilder attributes, Context parentContext, SimpleJsonRpcRequest jsonRpcRequest) {
Expand Down
7 changes: 0 additions & 7 deletions ...ava/io/opentelemetry/instrumentation/jsonrpc4j/v1_6/JsonRpcServerAttributesExtractor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,6 @@ final class JsonRpcServerAttributesExtractor
private static final AttributeKey<String> RPC_JSONRPC_ERROR_MESSAGE =
AttributeKey.stringKey("rpc.jsonrpc.error_message");

// private final JsonRpcServerAttributesGetter getter;
//
//
// JsonRpcServerAttributesExtractor(JsonRpcServerAttributesGetter getter) {
// this.getter = getter;
// }

@Override
public void onStart(
AttributesBuilder attributes, Context parentContext, JsonRpcRequest jsonRpcRequest) {
Expand Down
3 changes: 1 addition & 2 deletions ...n/java/io/opentelemetry/instrumentation/jsonrpc4j/v1_6/JsonRpcServerAttributesGetter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

package io.opentelemetry.instrumentation.jsonrpc4j.v1_6;

import com.googlecode.jsonrpc4j.JsonRpcService;
import io.opentelemetry.instrumentation.api.incubator.semconv.rpc.RpcAttributesGetter;

// Check
Expand All @@ -21,7 +20,7 @@ public String getSystem(JsonRpcRequest request) {

@Override
public String getService(JsonRpcRequest request) {
return request.getMethod().getDeclaringClass().getAnnotation(JsonRpcService.class).value();
return request.getMethod().getDeclaringClass().getName();
}

@Override
Expand Down

0 comments on commit a5e1a43

Please sign in to comment.