Skip to content

Commit 90c9aef

Browse files
ValentinZakharovValentinZakharov
committed
Simplify sofarpc instrumentation: merge modules, add REST/Triple server spans, rpc.method tag
1 parent 4c54caf commit 90c9aef

18 files changed

Lines changed: 165 additions & 415 deletions

File tree

‎dd-java-agent/instrumentation/sofarpc/sofarpc-5.0/build.gradle‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,25 @@ apply from: "$rootDir/gradle/java.gradle"
1111

1212
addTestSuiteForDir('latestDepTest', 'test')
1313

14+
configurations.testRuntimeClasspath {
15+
resolutionStrategy.force "com.google.guava:guava:32.1.3-jre"
16+
}
17+
1418
dependencies {
1519
compileOnly group: "com.alipay.sofa", name: "sofa-rpc-all", version: "5.6.0"
1620

17-
testImplementation group: "com.alipay.sofa", name: "sofa-rpc-all", version: "5.6.0"
21+
testImplementation group: "com.alipay.sofa", name: "sofa-rpc-all", version: "5.14.2"
1822
// JAX-RS annotations required for the REST protocol test interface (@Path, @GET, etc.)
1923
testImplementation group: "javax.ws.rs", name: "javax.ws.rs-api", version: "2.1.1"
24+
// Required so that GrpcServerModule / GrpcClientModule are discovered via ServiceLoader
25+
// in SofaRpcTripleWithGrpcForkedTest.
26+
testImplementation project(':dd-java-agent:instrumentation:grpc-1.5')
27+
testImplementation group: "io.grpc", name: "grpc-netty", version: "1.53.0"
28+
testImplementation group: "io.grpc", name: "grpc-core", version: "1.53.0"
29+
testImplementation group: "io.grpc", name: "grpc-stub", version: "1.53.0"
30+
testImplementation group: "com.google.protobuf", name: "protobuf-java", version: "3.25.3"
31+
testImplementation group: "com.alibaba", name: "fastjson", version: "1.2.83"
32+
testImplementation group: "com.google.guava", name: "guava", version: "32.1.3-jre"
2033

2134
latestDepTestImplementation group: "com.alipay.sofa", name: "sofa-rpc-all", version: "+"
2235
}

‎dd-java-agent/instrumentation/sofarpc/sofarpc-5.0/src/main/java/datadog/trace/instrumentation/sofarpc/H2cServerTaskInstrumentation.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@ public String instrumentedType() {
1818
@Override
1919
public void methodAdvice(MethodTransformer transformer) {
2020
transformer.applyAdvice(
21-
isMethod().and(named("run")).and(takesNoArguments()),
22-
getClass().getName() + "$RunAdvice");
21+
isMethod().and(named("run")).and(takesNoArguments()), getClass().getName() + "$RunAdvice");
2322
}
2423

2524
public static class RunAdvice {

‎dd-java-agent/instrumentation/sofarpc/sofarpc-5.0/src/main/java/datadog/trace/instrumentation/sofarpc/ProviderProxyInvokerInstrumentation.java‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -46,17 +46,11 @@ public static AgentScope enter(@Advice.Argument(0) SofaRequest request) {
4646
if (protocol == null) {
4747
return null;
4848
}
49-
// Prefer the parent context stored by transport-specific instrumentation (e.g.
50-
// TripleServerInstrumentation reads it from gRPC Metadata). For Bolt and H2C the
51-
// transport instrumentations only set the protocol, so parentContext is null here and
52-
// we fall back to extracting from SofaRequest.requestProps as before.
53-
AgentSpanContext parentContext = SofaRpcProtocolContext.getParentContext();
54-
if (parentContext == null) {
55-
parentContext = extractContextAndGetSpanContext(request, GETTER);
56-
}
57-
// parentContext may be null for Triple+gRPC-enabled: TripleServerInstrumentation skips
58-
// Metadata extraction when a grpc.server span is already active. In that case
59-
// startSpan() without an explicit parent naturally attaches to the active grpc.server span.
49+
// For Bolt and H2C the client injects trace context into SofaRequest.requestProps;
50+
// extract it here. For Triple, parentContext will be null and startSpan() without an
51+
// explicit parent naturally attaches to the active grpc.server span. For REST,
52+
// parentContext will also be null and the active netty.request span becomes the parent.
53+
AgentSpanContext parentContext = extractContextAndGetSpanContext(request, GETTER);
6054
AgentSpan span =
6155
parentContext != null
6256
? startSpan(SOFA_RPC_SERVER, parentContext)

‎dd-java-agent/instrumentation/sofarpc/sofarpc-5.0/src/main/java/datadog/trace/instrumentation/sofarpc/RestServerHandlerInstrumentation.java‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
package datadog.trace.instrumentation.sofarpc;
2+
3+
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
4+
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
5+
import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
6+
7+
import datadog.trace.agent.tooling.Instrumenter;
8+
import net.bytebuddy.asm.Advice;
9+
10+
public class RestServerHandlerInstrumentation
11+
implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice {
12+
13+
@Override
14+
public String instrumentedType() {
15+
return "com.alipay.sofa.rpc.server.rest.SofaRestRequestHandler";
16+
}
17+
18+
@Override
19+
public void methodAdvice(MethodTransformer transformer) {
20+
transformer.applyAdvice(
21+
isMethod().and(named("channelRead0")).and(takesArguments(2)),
22+
getClass().getName() + "$ChannelRead0Advice");
23+
}
24+
25+
public static class ChannelRead0Advice {
26+
@Advice.OnMethodEnter(suppress = Throwable.class)
27+
public static void enter() {
28+
SofaRpcProtocolContext.set("rest");
29+
}
30+
31+
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
32+
public static void exit() {
33+
SofaRpcProtocolContext.clear();
34+
}
35+
}
36+
}

‎dd-java-agent/instrumentation/sofarpc/sofarpc-5.0/src/main/java/datadog/trace/instrumentation/sofarpc/SofaRpcClientDecorator.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ public AgentSpan onRequest(AgentSpan span, SofaRequest request) {
4747
String serviceName = request.getTargetServiceUniqueName();
4848
String methodName = request.getMethodName();
4949
span.setTag(Tags.RPC_SERVICE, serviceName);
50+
span.setTag("rpc.method", methodName);
5051
// peer.service is derived automatically by PeerServiceCalculator from rpc.service.
5152
if (serviceName != null && methodName != null) {
5253
span.setResourceName(serviceName + "/" + methodName);

‎dd-java-agent/instrumentation/sofarpc/sofarpc-5.0/src/main/java/datadog/trace/instrumentation/sofarpc/SofaRpcModule.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ public List<Instrumenter> typeInstrumentations() {
3131
new AbstractClusterInstrumentation(),
3232
new BoltServerProcessorInstrumentation(),
3333
new H2cServerTaskInstrumentation(),
34+
new RestServerHandlerInstrumentation(),
35+
new TripleServerInstrumentation(),
3436
new ProviderProxyInvokerInstrumentation());
3537
}
3638
}

‎dd-java-agent/instrumentation/sofarpc/sofarpc-5.0/src/main/java/datadog/trace/instrumentation/sofarpc/SofaRpcProtocolContext.java‎

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,21 @@
11
package datadog.trace.instrumentation.sofarpc;
22

3-
import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext;
4-
5-
/**
6-
* Thread-local carrier for the SOFA RPC transport protocol name and the propagated parent span
7-
* context. The parent context is extracted by transport-specific instrumentation (e.g.
8-
* TripleServerInstrumentation reads it from gRPC Metadata) so that ProviderProxyInvokerInstrumentation
9-
* can start the server span with an explicit parent regardless of whether other instrumentation
10-
* (e.g. gRPC) is enabled.
11-
*/
3+
/** Thread-local carrier for the SOFA RPC transport protocol name. */
124
public final class SofaRpcProtocolContext {
135

146
private static final ThreadLocal<String> PROTOCOL = new ThreadLocal<>();
15-
private static final ThreadLocal<AgentSpanContext> PARENT_CONTEXT = new ThreadLocal<>();
167

178
private SofaRpcProtocolContext() {}
189

1910
public static void set(String protocol) {
2011
PROTOCOL.set(protocol);
2112
}
2213

23-
public static void setParentContext(AgentSpanContext parentContext) {
24-
PARENT_CONTEXT.set(parentContext);
25-
}
26-
2714
public static String get() {
2815
return PROTOCOL.get();
2916
}
3017

31-
public static AgentSpanContext getParentContext() {
32-
return PARENT_CONTEXT.get();
33-
}
34-
3518
public static void clear() {
3619
PROTOCOL.remove();
37-
PARENT_CONTEXT.remove();
3820
}
3921
}

‎dd-java-agent/instrumentation/sofarpc/sofarpc-5.0/src/main/java/datadog/trace/instrumentation/sofarpc/SofaRpcServerDecorator.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ public AgentSpan onRequest(AgentSpan span, SofaRequest request) {
4848
String serviceName = request.getTargetServiceUniqueName();
4949
String methodName = request.getMethodName();
5050
span.setTag(Tags.RPC_SERVICE, serviceName);
51+
span.setTag("rpc.method", methodName);
5152
if (serviceName != null && methodName != null) {
5253
span.setResourceName(serviceName + "/" + methodName);
5354
} else if (methodName != null) {

‎dd-java-agent/instrumentation/sofarpc/sofarpc-5.0/src/main/java/datadog/trace/instrumentation/sofarpc/TripleServerInstrumentation.java‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
package datadog.trace.instrumentation.sofarpc;
2+
3+
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
4+
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
5+
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
6+
import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
7+
8+
import datadog.trace.agent.tooling.Instrumenter;
9+
import net.bytebuddy.asm.Advice;
10+
11+
public class TripleServerInstrumentation
12+
implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice {
13+
14+
@Override
15+
public String instrumentedType() {
16+
return "com.alipay.sofa.rpc.server.triple.UniqueIdInvoker";
17+
}
18+
19+
@Override
20+
public void methodAdvice(MethodTransformer transformer) {
21+
transformer.applyAdvice(
22+
isMethod()
23+
.and(named("invoke"))
24+
.and(takesArguments(1))
25+
.and(takesArgument(0, named("com.alipay.sofa.rpc.core.request.SofaRequest"))),
26+
getClass().getName() + "$InvokeAdvice");
27+
}
28+
29+
public static class InvokeAdvice {
30+
@Advice.OnMethodEnter(suppress = Throwable.class)
31+
public static void enter() {
32+
SofaRpcProtocolContext.set("tri");
33+
}
34+
35+
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
36+
public static void exit() {
37+
SofaRpcProtocolContext.clear();
38+
}
39+
}
40+
}

‎dd-java-agent/instrumentation/sofarpc/sofarpc-5.0/src/test/groovy/datadog/trace/instrumentation/sofarpc/SofaRpcRestTest.groovy‎

Lines changed: 45 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,11 @@ import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
2121
/**
2222
* Tests SOFA RPC REST protocol instrumentation.
2323
*
24-
* REST works out-of-the-box via dd-trace-java's Apache HttpClient and HTTP server
25-
* instrumentation. Our SOFA RPC instrumentation contributes the sofarpc.request[client]
26-
* span (from AbstractClusterInstrumentation). There is no sofarpc.request[server] span
27-
* because no transport-level instrumentation sets SofaRpcProtocolContext for REST — the
28-
* server side is covered by JAX-RS / HTTP server instrumentation instead.
24+
* Our instrumentation contributes sofarpc.request[client] (AbstractClusterInstrumentation)
25+
* and sofarpc.request[server] (RestServerHandlerInstrumentation + ProviderProxyInvokerInstrumentation).
26+
* Distributed trace propagation is delegated to dd-trace-java's HTTP instrumentation
27+
* (Apache HttpClient on the client side, Netty on the server side), which is not active in
28+
* this unit test — so the server span appears as a separate trace root here.
2929
*/
3030
class SofaRpcRestTest extends InstrumentationSpecification {
3131

@@ -41,36 +41,36 @@ class SofaRpcRestTest extends InstrumentationSpecification {
4141
def setupSpec() {
4242
ServerConfig serverConfig =
4343
new ServerConfig()
44-
.setProtocol("rest")
45-
.setHost("127.0.0.1")
46-
.setPort(port)
44+
.setProtocol("rest")
45+
.setHost("127.0.0.1")
46+
.setPort(port)
4747

4848
ProviderConfig<GreeterService> providerConfig =
4949
new ProviderConfig<GreeterService>()
50-
.setApplication(new ApplicationConfig().setAppName("test-server"))
51-
.setInterfaceId(GreeterService.name)
52-
.setRef(new GreeterServiceImpl())
53-
.setServer(serverConfig)
54-
.setRegister(false)
50+
.setApplication(new ApplicationConfig().setAppName("test-server"))
51+
.setInterfaceId(GreeterService.name)
52+
.setRef(new GreeterServiceImpl())
53+
.setServer(serverConfig)
54+
.setRegister(false)
5555

5656
restProviderBootstrap = providerConfig.export()
5757

5858
greeterService =
5959
new ConsumerConfig<GreeterService>()
60-
.setApplication(new ApplicationConfig().setAppName("test-client"))
61-
.setInterfaceId(GreeterService.name)
62-
.setDirectUrl("rest://127.0.0.1:${port}")
63-
.setProtocol("rest")
64-
.setRegister(false)
65-
.setSubscribe(false)
66-
.refer()
60+
.setApplication(new ApplicationConfig().setAppName("test-client"))
61+
.setInterfaceId(GreeterService.name)
62+
.setDirectUrl("rest://127.0.0.1:${port}")
63+
.setProtocol("rest")
64+
.setRegister(false)
65+
.setSubscribe(false)
66+
.refer()
6767
}
6868

6969
def cleanupSpec() {
7070
restProviderBootstrap?.unExport()
7171
}
7272

73-
def "client span created for REST call"() {
73+
def "client and server spans created for REST call"() {
7474
setup:
7575
String serviceUniqueName = GreeterService.name + ":1.0"
7676

@@ -81,11 +81,8 @@ class SofaRpcRestTest extends InstrumentationSpecification {
8181
reply == "Hello, World"
8282

8383
and:
84-
// REST is instrumented by Apache HttpClient (client) + HTTP server (server) integrations.
85-
// Our SOFA RPC instrumentation adds sofarpc.request[client] on top. No server-side
86-
// SofaRpcProtocolContext is set for REST, so ProviderProxyInvoker produces no sofarpc span.
87-
// Only the client-side trace is collected here.
88-
assertTraces(1) {
84+
assertTraces(2) {
85+
// trace(0): client side — caller + sofarpc.request[client]
8986
trace(2) {
9087
basicSpan(it, "caller")
9188
span {
@@ -96,6 +93,7 @@ class SofaRpcRestTest extends InstrumentationSpecification {
9693
childOf span(0)
9794
tags {
9895
"$Tags.RPC_SERVICE" serviceUniqueName
96+
"rpc.method" "sayHello"
9997
"rpc.system" "sofarpc"
10098
"sofarpc.protocol" "rest"
10199
"component" "sofarpc-client"
@@ -105,6 +103,27 @@ class SofaRpcRestTest extends InstrumentationSpecification {
105103
}
106104
}
107105
}
106+
// trace(1): server side — sofarpc.request[server].
107+
// SofaRequest.getTargetServiceUniqueName() is null on the server side for REST
108+
// (not propagated through the JAX-RS layer), so resourceName is the method name only
109+
// and rpc.service tag is absent. Parent link to the client trace is provided by
110+
// HTTP instrumentation (not active in this test), so this span is a trace root here.
111+
trace(1) {
112+
span {
113+
operationName "sofarpc.request"
114+
resourceName "sayHello"
115+
spanType "rpc"
116+
errored false
117+
tags {
118+
"rpc.method" "sayHello"
119+
"rpc.system" "sofarpc"
120+
"sofarpc.protocol" "rest"
121+
"component" "sofarpc-server"
122+
"span.kind" "server"
123+
defaultTags(true)
124+
}
125+
}
126+
}
108127
}
109128
}
110129

0 commit comments

Comments
 (0)