Skip to content

Commit e6bbf25

Browse files
cataphractcataphract
committed
Merge remote-tracking branch 'origin/master' into project/appsec
2 parents c0b4a8b + 7ea85f8 commit e6bbf25

23 files changed

Lines changed: 175 additions & 51 deletions

File tree

‎dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/java/concurrent/AdviceUtils.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,10 @@ public static <T> TraceScope startTaskScope(
3333

3434
public static void endTaskScope(final TraceScope scope) {
3535
if (scope instanceof AgentScope) {
36-
((AgentScope) scope).span().finishWork();
36+
AgentScope agentScope = (AgentScope) scope;
37+
if (agentScope.checkpointed()) {
38+
agentScope.span().finishWork();
39+
}
3740
}
3841
if (scope != null) {
3942
scope.close();
@@ -50,7 +53,7 @@ public static <T> void cancelTask(ContextStore<T, State> contextStore, final T t
5053
public static <T> void capture(
5154
ContextStore<T, State> contextStore, T task, boolean startThreadMigration) {
5255
TraceScope activeScope = activeScope();
53-
if (null != activeScope) {
56+
if (null != activeScope && activeScope.isAsyncPropagating()) {
5457
State state = contextStore.get(task);
5558
if (null == state) {
5659
state = State.FACTORY.create();

‎dd-java-agent/instrumentation/akka-http-10.0/src/baseTest/groovy/AkkaHttpServerInstrumentationTest.groovy‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@ import spock.lang.Shared
77

88
import java.util.concurrent.atomic.AtomicInteger
99

10+
import static datadog.trace.api.Checkpointer.CPU
11+
import static datadog.trace.api.Checkpointer.END
12+
import static datadog.trace.api.Checkpointer.SPAN
13+
import static datadog.trace.api.Checkpointer.THREAD_MIGRATION
14+
1015
abstract class AkkaHttpServerInstrumentationTest extends HttpServerTest<AkkaHttpTestWebServer> {
1116

1217
@Override
@@ -66,6 +71,35 @@ abstract class AkkaHttpServerInstrumentationTest extends HttpServerTest<AkkaHttp
6671
and:
6772
TEST_WRITER.waitForTraces(totalInvocations)
6873
}
74+
75+
def "checkpoints balance"() {
76+
setup:
77+
AtomicInteger suspends = new AtomicInteger(0)
78+
AtomicInteger resumes = new AtomicInteger(0)
79+
AtomicInteger completions = new AtomicInteger(0)
80+
when:
81+
ThreadUtils.runConcurrently(10, totalInvocations, {
82+
def id = counter.incrementAndGet()
83+
doAndValidateRequest(id)
84+
})
85+
TEST_WRITER.waitForTraces(totalInvocations)
86+
then:
87+
totalInvocations * TEST_CHECKPOINTER.checkpoint(_, _, SPAN)
88+
totalInvocations * TEST_CHECKPOINTER.checkpoint(_, _, SPAN | END)
89+
_ * TEST_CHECKPOINTER.checkpoint(_, _, THREAD_MIGRATION) >> {
90+
suspends.getAndIncrement()
91+
}
92+
_ * TEST_CHECKPOINTER.checkpoint(_, _, THREAD_MIGRATION | END) >> {
93+
resumes.getAndIncrement()
94+
}
95+
_ * TEST_CHECKPOINTER.checkpoint(_, _, CPU | END) >> {
96+
completions.getAndIncrement()
97+
}
98+
_ * TEST_CHECKPOINTER.onRootSpanPublished(_, _)
99+
0 * TEST_CHECKPOINTER._
100+
suspends.get() == resumes.get()
101+
resumes.get() == completions.get()
102+
}
69103
}
70104

71105
class AkkaHttpServerInstrumentationSyncTest extends AkkaHttpServerInstrumentationTest {

‎dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/GrpcServerBuilderInstrumentation.java‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,19 @@
33
import static datadog.trace.agent.tooling.bytebuddy.matcher.DDElementMatchers.extendsClass;
44
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
55
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.namedOneOf;
6+
import static datadog.trace.bootstrap.CallDepthThreadLocalMap.incrementCallDepth;
7+
import static java.util.Collections.singletonMap;
68
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
79
import static net.bytebuddy.matcher.ElementMatchers.isPublic;
810
import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
911

1012
import com.google.auto.service.AutoService;
1113
import datadog.trace.agent.tooling.Instrumenter;
1214
import datadog.trace.bootstrap.CallDepthThreadLocalMap;
15+
import datadog.trace.bootstrap.ContextStore;
16+
import datadog.trace.bootstrap.InstrumentationContext;
1317
import io.grpc.ServerBuilder;
18+
import java.util.Map;
1419
import net.bytebuddy.asm.Advice;
1520
import net.bytebuddy.description.type.TypeDescription;
1621
import net.bytebuddy.matcher.ElementMatcher;
@@ -39,6 +44,11 @@ public ElementMatcher<? super TypeDescription> shortCutMatcher() {
3944
"io.grpc.internal.ServerImplBuilder");
4045
}
4146

47+
@Override
48+
public Map<String, String> contextStore() {
49+
return singletonMap("io.grpc.ServerBuilder", Boolean.class.getName());
50+
}
51+
4252
@Override
4353
public String[] helperClassNames() {
4454
return new String[] {
@@ -62,9 +72,14 @@ public static class BuildAdvice {
6272

6373
@Advice.OnMethodEnter(suppress = Throwable.class)
6474
public static void onEnter(@Advice.This ServerBuilder<?> serverBuilder) {
65-
int callDepth = CallDepthThreadLocalMap.incrementCallDepth(ServerBuilder.class);
75+
int callDepth = incrementCallDepth(ServerBuilder.class);
6676
if (callDepth == 0) {
67-
serverBuilder.intercept(TracingServerInterceptor.INSTANCE);
77+
ContextStore<ServerBuilder, Boolean> interceptedStore =
78+
InstrumentationContext.get(ServerBuilder.class, Boolean.class);
79+
if (interceptedStore.get(serverBuilder) == null) {
80+
interceptedStore.put(serverBuilder, Boolean.TRUE);
81+
serverBuilder.intercept(TracingServerInterceptor.INSTANCE);
82+
}
6883
}
6984
}
7085

‎dd-java-agent/instrumentation/grpc-1.5/src/test/groovy/GrpcTest.groovy‎

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,9 @@ class GrpcTest extends AgentTestRunner {
5555
}
5656
}
5757
}
58-
Server server = InProcessServerBuilder.forName(getClass().name).addService(greeter).executor(executor).build().start()
58+
def builder = InProcessServerBuilder.forName(getClass().name).addService(greeter).executor(executor)
59+
(0..extraBuildCalls).each {builder.build()}
60+
Server server = builder.build().start()
5961

6062
ManagedChannel channel = InProcessChannelBuilder.forName(getClass().name).build()
6163
GreeterGrpc.GreeterBlockingStub client = GreeterGrpc.newBlockingStub(channel)
@@ -131,9 +133,9 @@ class GrpcTest extends AgentTestRunner {
131133
}
132134
5 * TEST_CHECKPOINTER.checkpoint(_, _, SPAN)
133135
5 * TEST_CHECKPOINTER.checkpoint(_, _, SPAN | END)
134-
contexts * TEST_CHECKPOINTER.checkpoint(_, _, THREAD_MIGRATION)
135-
contexts * TEST_CHECKPOINTER.checkpoint(_, _, THREAD_MIGRATION | END)
136-
contexts * TEST_CHECKPOINTER.checkpoint(_, _, CPU | END)
136+
(minContexts..contexts) * TEST_CHECKPOINTER.checkpoint(_, _, THREAD_MIGRATION)
137+
(minContexts..contexts) * TEST_CHECKPOINTER.checkpoint(_, _, THREAD_MIGRATION | END)
138+
(minContexts..contexts) * TEST_CHECKPOINTER.checkpoint(_, _, CPU | END)
137139
_ * TEST_CHECKPOINTER.onRootSpanPublished(_, _)
138140
0 * TEST_CHECKPOINTER._
139141

@@ -145,13 +147,19 @@ class GrpcTest extends AgentTestRunner {
145147
}
146148

147149
where:
148-
name | executor | contexts
149-
"some name" | MoreExecutors.directExecutor() | 1
150-
"some other name" | MoreExecutors.directExecutor() | 1
151-
"some name" | newWorkStealingPool() | 3
152-
"some other name" | newWorkStealingPool() | 3
153-
"some name" | Executors.newSingleThreadExecutor() | 3
154-
"some other name" | Executors.newSingleThreadExecutor() | 3
150+
name | executor | contexts | extraBuildCalls | minContexts
151+
"some name" | MoreExecutors.directExecutor() | 1 | 0 | 1
152+
"some other name" | MoreExecutors.directExecutor() | 1 | 0 | 1
153+
"some name" | newWorkStealingPool() | 3 | 0 | 1
154+
"some other name" | newWorkStealingPool() | 3 | 0 | 1
155+
"some name" | Executors.newSingleThreadExecutor() | 3 | 0 | 3
156+
"some other name" | Executors.newSingleThreadExecutor() | 3 | 0 | 3
157+
"some name" | MoreExecutors.directExecutor() | 1 | 1 | 1
158+
"some other name" | MoreExecutors.directExecutor() | 1 | 1 | 1
159+
"some name" | newWorkStealingPool() | 3 | 1 | 1
160+
"some other name" | newWorkStealingPool() | 3 | 1 | 1
161+
"some name" | Executors.newSingleThreadExecutor() | 3 | 1 | 3
162+
"some other name" | Executors.newSingleThreadExecutor() | 3 | 1 | 3
155163
}
156164

157165
def "test error - #name"() {

‎dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/RunnableInstrumentation.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
public final class RunnableInstrumentation extends Instrumenter.Tracing {
2828

2929
public RunnableInstrumentation() {
30-
super(AbstractExecutorInstrumentation.EXEC_NAME);
30+
super(AbstractExecutorInstrumentation.EXEC_NAME, "runnable");
3131
}
3232

3333
@Override

‎dd-java-agent/instrumentation/java-concurrent/src/test/groovy/CheckpointThreadMigrationTest.groovy‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,12 @@ class CheckpointThreadMigrationTest extends AgentTestRunner {
1818
runUnderTrace("parent") {
1919
executor.submit(new CheckpointTask(traceChildTasks)).get()
2020
}
21-
then:
2221
TEST_WRITER.waitForTraces(1)
22+
then:
2323
(traceChildTasks ? 2 : 1) * TEST_CHECKPOINTER.checkpoint(_, _, SPAN)
2424
1 * TEST_CHECKPOINTER.checkpoint(_, _, THREAD_MIGRATION)
2525
1 * TEST_CHECKPOINTER.checkpoint(_, _, THREAD_MIGRATION | END)
26-
_ * TEST_CHECKPOINTER.checkpoint(_, _, CPU | END)
26+
1 * TEST_CHECKPOINTER.checkpoint(_, _, CPU | END)
2727
(traceChildTasks ? 2 : 1) * TEST_CHECKPOINTER.checkpoint(_, _, SPAN | END)
2828

2929
cleanup:
@@ -48,12 +48,12 @@ class CheckpointThreadMigrationTest extends AgentTestRunner {
4848
executor.execute(new CheckpointTask(traceChildTasks, latch))
4949
latch.await(30, SECONDS)
5050
}
51-
then:
5251
TEST_WRITER.waitForTraces(1)
52+
then:
5353
(traceChildTasks ? 2 : 1) * TEST_CHECKPOINTER.checkpoint(_, _, SPAN)
5454
1 * TEST_CHECKPOINTER.checkpoint(_, _, THREAD_MIGRATION)
5555
1 * TEST_CHECKPOINTER.checkpoint(_, _, THREAD_MIGRATION | END)
56-
_ * TEST_CHECKPOINTER.checkpoint(_, _, CPU | END)
56+
1 * TEST_CHECKPOINTER.checkpoint(_, _, CPU | END)
5757
(traceChildTasks ? 2 : 1) * TEST_CHECKPOINTER.checkpoint(_, _, SPAN | END)
5858

5959
cleanup:

‎dd-java-agent/instrumentation/jdbc/jdbc.gradle‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@ testSets {
2121
oldPostgresTest {
2222
dirName = 'test'
2323
}
24+
25+
latestDepJava11Test {
26+
extendsFrom latestDepTest
27+
dirName = 'test'
28+
}
2429
}
2530

2631
dependencies {
@@ -60,11 +65,22 @@ dependencies {
6065

6166
latestDepTestImplementation group: 'org.apache.tomcat', name: 'tomcat-jdbc', version: '+'
6267
latestDepTestImplementation group: 'org.apache.tomcat', name: 'tomcat-juli', version: '+'
63-
latestDepTestImplementation group: 'com.zaxxer', name: 'HikariCP', version: ',5.0.0)' // 5 requires JDK11
68+
latestDepTestImplementation group: 'com.zaxxer', name: 'HikariCP', version: '4.+' // 5+ requires Java 11
6469
latestDepTestImplementation group: 'com.mchange', name: 'c3p0', version: '+'
70+
71+
latestDepJava11TestImplementation group: 'com.zaxxer', name: 'HikariCP', version: '+'
6572
}
6673

6774
tasks.named("test").configure {
6875
dependsOn "oldH2Test"
6976
dependsOn "oldPostgresTest"
7077
}
78+
79+
tasks.named("latestDepJava11Test").configure {
80+
doFirst {
81+
if (!System.env.JAVA_11_HOME) {
82+
throw new GradleException('JAVA_11_HOME must be set for latestDepJava11Test')
83+
}
84+
executable = file("${System.env.JAVA_11_HOME}/bin/java")
85+
}
86+
}

‎dd-java-agent/instrumentation/jdbc/scalikejdbc/scalikejdbc.gradle‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,6 @@ dependencies {
5050
latestDepTestImplementation group: 'org.hsqldb', name: 'hsqldb', version: '2.5+'
5151
latestDepTestImplementation group: 'org.apache.tomcat', name: 'tomcat-jdbc', version: '+'
5252
latestDepTestImplementation group: 'org.apache.tomcat', name: 'tomcat-juli', version: '+'
53-
latestDepTestImplementation group: 'com.zaxxer', name: 'HikariCP', version: ',5.0.0)' // 5 requires JDK11
53+
latestDepTestImplementation group: 'com.zaxxer', name: 'HikariCP', version: '4.+' // 5+ requires Java 11
5454
latestDepTestImplementation group: 'com.mchange', name: 'c3p0', version: '+'
5555
}

‎dd-java-agent/instrumentation/jedis-3.0/jedis-3.0.gradle‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ muzzle {
33
group = "redis.clients"
44
module = "jedis"
55
versions = "[,3.0.0)"
6+
skipVersions += "jedis-3.6.2" // bad release version ("jedis-" prefix)
67
}
78

89
pass {

‎dd-java-agent/instrumentation/rediscala-1.8.0/rediscala-1.8.0.gradle‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,9 @@ testSets {
5959
dependencies {
6060
compileOnly group: 'com.github.etaty', name: 'rediscala_2.11', version: '1.8.0'
6161

62+
testImplementation project(':dd-java-agent:instrumentation:scala-promise:scala-promise-2.10')
63+
testImplementation project(':dd-java-agent:instrumentation:scala-concurrent')
64+
6265
testImplementation group: 'com.github.etaty', name: 'rediscala_2.11', version: '1.8.0'
6366
testImplementation group: 'com.github.kstyrc', name: 'embedded-redis', version: '0.6'
6467

0 commit comments

Comments
 (0)