Skip to content

Commit 70e0c21

Browse files
amarzialiamarziali
authored
Make redisson tracing the full request (#7571)
* Make redisson tracing the full request * refactor and fix test for newest jvm
1 parent 0a583fe commit 70e0c21

20 files changed

Lines changed: 660 additions & 355 deletions

File tree

‎dd-java-agent/instrumentation/redisson-2.0.0/build.gradle‎

Lines changed: 0 additions & 46 deletions
This file was deleted.

‎dd-java-agent/instrumentation/redisson-2.0.0/gradle.lockfile‎

Lines changed: 0 additions & 216 deletions
This file was deleted.

‎dd-java-agent/instrumentation/redisson/build.gradle‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
apply from: "$rootDir/gradle/java.gradle"

‎dd-java-agent/instrumentation/redisson/redisson-2.0.0/build.gradle‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
2+
muzzle {
3+
pass {
4+
group = "org.redisson"
5+
module = "redisson"
6+
versions = "[2.0.0,2.3.0)"
7+
skipVersions += "0.9.0"
8+
assertInverse = true
9+
}
10+
}
11+
12+
apply from: "$rootDir/gradle/java.gradle"
13+
14+
addTestSuiteForDir('latestDepTest', 'test')
15+
addTestSuiteExtendingForDir('latestDepForkedTest', 'latestDepTest', 'test')
16+
17+
dependencies {
18+
compileOnly group: 'org.redisson', name: 'redisson', version: '2.0.0'
19+
20+
testImplementation group: 'com.redis.testcontainers', name: 'testcontainers-redis', version: '1.6.2'
21+
testImplementation libs.testcontainers
22+
testImplementation group: 'org.redisson', name: 'redisson', version: '2.0.0'
23+
latestDepTestImplementation group: 'org.redisson', name: 'redisson', version: '2.2.+'
24+
}
25+
26+
tasks.withType(Test).configureEach {
27+
usesService(testcontainersLimit)
28+
}

dd-java-agent/instrumentation/redisson-2.0.0/src/main/java/datadog/trace/instrumentation/redisson/RedissonClientDecorator.java renamed to dd-java-agent/instrumentation/redisson/redisson-2.0.0/src/main/java/datadog/trace/instrumentation/redisson/RedissonClientDecorator.java

File renamed without changes.

dd-java-agent/instrumentation/redisson-2.0.0/src/main/java/datadog/trace/instrumentation/redisson/RedissonInstrumentation.java renamed to dd-java-agent/instrumentation/redisson/redisson-2.0.0/src/main/java/datadog/trace/instrumentation/redisson/RedissonInstrumentation.java

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import datadog.trace.agent.tooling.InstrumenterModule;
1414
import datadog.trace.bootstrap.instrumentation.api.AgentScope;
1515
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
16+
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
1617
import datadog.trace.util.Strings;
1718
import java.util.ArrayList;
1819
import java.util.List;
@@ -37,7 +38,7 @@ public String instrumentedType() {
3738
@Override
3839
public String[] helperClassNames() {
3940
return new String[] {
40-
packageName + ".RedissonClientDecorator",
41+
packageName + ".RedissonClientDecorator", packageName + ".SpanFinishListener",
4142
};
4243
}
4344

@@ -63,20 +64,22 @@ public static class RedissonCommandAdvice {
6364
@Advice.OnMethodEnter(suppress = Throwable.class)
6465
public static AgentScope onEnter(
6566
@Advice.Argument(0) final CommandData<?, ?> command, @Advice.This RedisConnection thiz) {
67+
if (command.getPromise() == null) {
68+
return null;
69+
}
6670
final AgentSpan span = startSpan(RedissonClientDecorator.OPERATION_NAME);
6771
DECORATE.afterStart(span);
6872
DECORATE.onPeerConnection(span, thiz.getRedisClient().getAddr());
6973
DECORATE.onStatement(span, command.getCommand().getName());
74+
command.getPromise().addListener(new SpanFinishListener(AgentTracer.captureSpan(span)));
7075
return activateSpan(span);
7176
}
7277

7378
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
74-
public static void stopSpan(
75-
@Advice.Enter final AgentScope scope, @Advice.Thrown final Throwable throwable) {
76-
DECORATE.onError(scope.span(), throwable);
77-
DECORATE.beforeFinish(scope.span());
78-
scope.close();
79-
scope.span().finish();
79+
public static void after(@Advice.Enter final AgentScope scope) {
80+
if (scope != null) {
81+
scope.close();
82+
}
8083
}
8184
}
8285

@@ -85,6 +88,10 @@ public static class RedissonCommandsAdvice {
8588
@Advice.OnMethodEnter(suppress = Throwable.class)
8689
public static AgentScope onEnter(
8790
@Advice.Argument(0) final CommandsData command, @Advice.This final RedisConnection thiz) {
91+
if (command.getPromise() == null) {
92+
return null;
93+
}
94+
8895
final AgentSpan span = startSpan(RedissonClientDecorator.OPERATION_NAME);
8996
DECORATE.afterStart(span);
9097
DECORATE.onPeerConnection(span, thiz.getRedisClient().getAddr());
@@ -94,16 +101,15 @@ public static AgentScope onEnter(
94101
commandResourceNames.add(commandData.getCommand().getName());
95102
}
96103
DECORATE.onStatement(span, Strings.join(";", commandResourceNames));
104+
command.getPromise().addListener(new SpanFinishListener(AgentTracer.captureSpan(span)));
97105
return activateSpan(span);
98106
}
99107

100108
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
101-
public static void stopSpan(
102-
@Advice.Enter final AgentScope scope, @Advice.Thrown final Throwable throwable) {
103-
DECORATE.onError(scope.span(), throwable);
104-
DECORATE.beforeFinish(scope.span());
105-
scope.close();
106-
scope.span().finish();
109+
public static void after(@Advice.Enter final AgentScope scope) {
110+
if (scope != null) {
111+
scope.close();
112+
}
107113
}
108114
}
109115
}

‎dd-java-agent/instrumentation/redisson/redisson-2.0.0/src/main/java/datadog/trace/instrumentation/redisson/SpanFinishListener.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package datadog.trace.instrumentation.redisson;
2+
3+
import static datadog.trace.instrumentation.redisson.RedissonClientDecorator.DECORATE;
4+
5+
import datadog.trace.bootstrap.instrumentation.api.AgentScope;
6+
import io.netty.util.concurrent.Future;
7+
import io.netty.util.concurrent.GenericFutureListener;
8+
9+
public class SpanFinishListener implements GenericFutureListener<Future<Object>> {
10+
private final AgentScope.Continuation continuation;
11+
12+
public SpanFinishListener(final AgentScope.Continuation continuation) {
13+
this.continuation = continuation;
14+
}
15+
16+
@Override
17+
public void operationComplete(Future<Object> future) throws Exception {
18+
try (final AgentScope scope = continuation.activate()) {
19+
if (!future.isSuccess()) {
20+
DECORATE.onError(scope, future.cause());
21+
}
22+
DECORATE.beforeFinish(scope);
23+
scope.span().finish();
24+
}
25+
}
26+
}

dd-java-agent/instrumentation/redisson-2.0.0/src/test/redisson20/groovy/RedissonClientTest.groovy renamed to dd-java-agent/instrumentation/redisson/redisson-2.0.0/src/test/groovy/RedissonClientTest.groovy

Lines changed: 9 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,30 @@
1-
import spock.lang.Ignore
2-
31
import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
42
import static datadog.trace.api.config.TraceInstrumentationConfig.DB_CLIENT_HOST_SPLIT_BY_INSTANCE
53
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeSpan
64

5+
import com.redis.testcontainers.RedisContainer
6+
import com.redis.testcontainers.RedisServer
77
import datadog.trace.agent.test.asserts.TraceAssert
88
import datadog.trace.agent.test.naming.VersionedNamingTestBase
9-
import datadog.trace.agent.test.utils.PortUtils
109
import datadog.trace.api.DDSpanTypes
1110
import datadog.trace.bootstrap.instrumentation.api.Tags
1211
import org.redisson.Config
1312
import org.redisson.Redisson
1413
import org.redisson.RedissonClient
15-
import org.redisson.SingleServerConfig
1614
import org.redisson.client.RedisClient
1715
import org.redisson.client.RedisConnection
1816
import org.redisson.client.protocol.RedisCommands
19-
import redis.embedded.RedisServer
17+
import org.testcontainers.containers.wait.strategy.Wait
2018
import spock.lang.Shared
2119

2220
abstract class RedissonClientTest extends VersionedNamingTestBase {
2321

2422
@Shared
25-
int port = PortUtils.randomOpenPort()
26-
27-
@Shared
28-
RedisServer redisServer = RedisServer.builder()
29-
// bind to localhost to avoid firewall popup
30-
.setting("bind 127.0.0.1")
31-
// set max memory to avoid problems in CI
32-
.setting("maxmemory 128M")
33-
.port(port).build()
23+
RedisServer redisServer = new RedisContainer(RedisContainer.DEFAULT_IMAGE_NAME).waitingFor(Wait.forListeningPort())
3424

3525
@Shared
3626
Config config = new Config()
3727

38-
@Shared
39-
SingleServerConfig singleServerConfig = config.useSingleServer().setAddress("localhost:${port}")
40-
4128
@Shared
4229
RedissonClient redissonClient
4330

@@ -53,10 +40,11 @@ abstract class RedissonClientTest extends VersionedNamingTestBase {
5340
}
5441

5542
def setupSpec() {
56-
println "Using redis: $redisServer.args"
5743
redisServer.start()
44+
println "Using redis: $redisServer.redisURI"
45+
config.useSingleServer().setAddress("$redisServer.host:$redisServer.firstMappedPort")
5846
redissonClient = Redisson.create(config)
59-
lowLevelRedisClient = new RedisClient("127.0.0.1", port)
47+
lowLevelRedisClient = new RedisClient(redisServer.host, redisServer.firstMappedPort)
6048
}
6149

6250
def cleanupSpec() {
@@ -132,7 +120,6 @@ abstract class RedissonClientTest extends VersionedNamingTestBase {
132120
assertTraces(2) {
133121
trace(1) {
134122
redisSpan(it, "SET")
135-
136123
}
137124
trace(1) {
138125
span {
@@ -147,7 +134,7 @@ abstract class RedissonClientTest extends VersionedNamingTestBase {
147134
"$Tags.DB_TYPE" "redis"
148135
"$Tags.PEER_HOSTNAME" "localhost"
149136
"$Tags.PEER_HOST_IPV4" "127.0.0.1"
150-
"$Tags.PEER_PORT" port
137+
"$Tags.PEER_PORT" redisServer.firstMappedPort
151138
peerServiceFrom(Tags.PEER_HOSTNAME)
152139
defaultTags()
153140
}
@@ -343,7 +330,7 @@ abstract class RedissonClientTest extends VersionedNamingTestBase {
343330
"$Tags.DB_TYPE" "redis"
344331
"$Tags.PEER_HOSTNAME" "localhost"
345332
"$Tags.PEER_HOST_IPV4" "127.0.0.1"
346-
"$Tags.PEER_PORT" port
333+
"$Tags.PEER_PORT" redisServer.firstMappedPort
347334
peerServiceFrom(Tags.PEER_HOSTNAME)
348335
defaultTags()
349336
}
@@ -369,7 +356,6 @@ class RedissonClientV0Test extends RedissonClientTest {
369356
}
370357
}
371358

372-
@Ignore("https://github.com/DataDog/dd-trace-java/pull/5213")
373359
class RedissonClientV1ForkedTest extends RedissonClientTest {
374360

375361
@Override

‎dd-java-agent/instrumentation/redisson/redisson-2.3.0/build.gradle‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
2+
muzzle {
3+
pass {
4+
group = "org.redisson"
5+
module = "redisson"
6+
versions = "[2.3.0, 3.10.3)"
7+
skipVersions += "0.9.0"
8+
assertInverse = true
9+
}
10+
}
11+
12+
apply from: "$rootDir/gradle/java.gradle"
13+
14+
addTestSuiteForDir('latestDepTest', 'test')
15+
addTestSuiteExtendingForDir('latestDepForkedTest', 'latestDepTest', 'test')
16+
17+
dependencies {
18+
compileOnly group: 'org.redisson', name: 'redisson', version: '2.3.0'
19+
20+
testImplementation group: 'com.redis.testcontainers', name: 'testcontainers-redis', version: '1.6.2'
21+
testImplementation libs.testcontainers
22+
testImplementation group: 'org.redisson', name: 'redisson', version: '2.3.0'
23+
testImplementation(project(':dd-java-agent:instrumentation:redisson:redisson-2.0.0'))
24+
latestDepTestImplementation group: 'org.redisson', name: 'redisson', version: '3.10.2', {
25+
exclude group: 'org.slf4j', module: 'slf4j-api'
26+
}
27+
}
28+
29+
project.afterEvaluate {
30+
tasks.withType(Test).configureEach {
31+
if (javaLauncher.get().metadata.languageVersion.asInt() >= 16) {
32+
jvmArgs += ['--add-opens', 'java.base/java.math=ALL-UNNAMED']
33+
jvmArgs += ['--add-opens', 'java.base/java.util.concurrent=ALL-UNNAMED']
34+
jvmArgs += ['--add-opens', 'java.base/java.net=ALL-UNNAMED']
35+
jvmArgs += ['--add-opens', 'java.base/java.text=ALL-UNNAMED']
36+
}
37+
usesService(testcontainersLimit)
38+
}
39+
}

‎dd-java-agent/instrumentation/redisson/redisson-2.3.0/src/main/java/datadog/trace/instrumentation/redisson23/RedissonClientDecorator.java‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
package datadog.trace.instrumentation.redisson23;
2+
3+
import datadog.trace.api.naming.SpanNaming;
4+
import datadog.trace.bootstrap.instrumentation.api.InternalSpanTypes;
5+
import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString;
6+
import datadog.trace.bootstrap.instrumentation.decorator.DBTypeProcessingDatabaseClientDecorator;
7+
import org.redisson.client.protocol.CommandData;
8+
9+
public class RedissonClientDecorator
10+
extends DBTypeProcessingDatabaseClientDecorator<CommandData<?, ?>> {
11+
public static final RedissonClientDecorator DECORATE = new RedissonClientDecorator();
12+
13+
public static final CharSequence OPERATION_NAME =
14+
UTF8BytesString.create(SpanNaming.instance().namingSchema().cache().operation("redis"));
15+
private static final String SERVICE_NAME =
16+
SpanNaming.instance().namingSchema().cache().service("redis");
17+
18+
private static final CharSequence COMPONENT_NAME = UTF8BytesString.create("redis-command");
19+
20+
@Override
21+
protected String[] instrumentationNames() {
22+
return new String[] {"redisson", "redis"};
23+
}
24+
25+
@Override
26+
protected String service() {
27+
return SERVICE_NAME;
28+
}
29+
30+
@Override
31+
protected CharSequence component() {
32+
return COMPONENT_NAME;
33+
}
34+
35+
@Override
36+
protected CharSequence spanType() {
37+
return InternalSpanTypes.REDIS;
38+
}
39+
40+
@Override
41+
protected String dbType() {
42+
return "redis";
43+
}
44+
45+
@Override
46+
protected String dbUser(CommandData<?, ?> commandData) {
47+
return null;
48+
}
49+
50+
@Override
51+
protected String dbInstance(CommandData<?, ?> commandData) {
52+
return null;
53+
}
54+
55+
@Override
56+
protected CharSequence dbHostname(CommandData<?, ?> commandData) {
57+
return null;
58+
}
59+
}

0 commit comments

Comments
 (0)