Skip to content

Commit 3db2d65

Browse files
tylerbensontylerbenson
committed
Fix spark instrumentation
and rebase off master.
1 parent de350c7 commit 3db2d65

14 files changed

Lines changed: 267 additions & 133 deletions

File tree

‎buildSrc/src/main/groovy/VersionScanPlugin.groovy‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,9 @@ class VersionScanPlugin implements Plugin<Project> {
181181
}
182182
}
183183

184-
// println "Scanning ${includeVersionSet.size()} included and ${excludeVersionSet.size()} excluded versions. Included: ${includeVersionSet.collect { it.version }}}"
184+
// println "Scanning ${includeVersionSet.size()} included and ${excludeVersionSet.size()} excluded versions."
185+
// println "Included: ${includeVersionSet.collect { it.version }}}"
186+
// println "Excluded: ${excludeVersionSet.collect { it.version }}}"
185187

186188
includeVersionSet.each { version ->
187189
addScanTask("Include", new DefaultArtifact(version.groupId, version.artifactId, "jar", version.version), keyPresent, allInclude, project)

‎dd-java-agent-ittests/src/test/groovy/datadog/trace/agent/integration/classloading/ShadowPackageRenamingTest.groovy‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import spock.lang.Timeout
99

1010
import java.lang.reflect.Field
1111

12-
@Timeout(1)
12+
@Timeout(10)
1313
class ShadowPackageRenamingTest extends Specification {
1414
def "agent dependencies renamed"() {
1515
setup:

‎dd-java-agent/instrumentation/jetty-8/jetty-8.gradle‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
11
apply plugin: 'version-scan'
22

33
versionScan {
4-
group = "org.eclipse.jetty.server"
5-
module = 'org.eclipse.jetty.server-Handler'
4+
group = "org.eclipse.jetty"
5+
module = 'jetty-server'
66
versions = "[8.0.0.v20110901,)"
7-
verifyPresent = [
8-
"javax.servlet.AsyncEvent" : null,
9-
"javax.servlet.AsyncListener": null,
7+
verifyMissing = [
8+
"org.eclipse.jetty.server.AsyncContext",
109
]
1110
}
1211

@@ -29,6 +28,5 @@ dependencies {
2928
testCompile group: 'org.eclipse.jetty', name: 'jetty-server', version: '8.0.0.v20110901'
3029
testCompile group: 'org.eclipse.jetty', name: 'jetty-servlet', version: '8.0.0.v20110901'
3130

32-
testCompile project(':dd-java-agent:instrumentation:okhttp-3') // used in the tests
3331
testCompile group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.6.0'
3432
}

‎dd-java-agent/instrumentation/jetty-8/src/main/java/datadog/trace/instrumentation/jetty8/HandlerInstrumentation.java‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,12 @@
22

33
import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses;
44
import static io.opentracing.log.Fields.ERROR_OBJECT;
5-
import static net.bytebuddy.matcher.ElementMatchers.*;
5+
import static net.bytebuddy.matcher.ElementMatchers.hasSuperType;
6+
import static net.bytebuddy.matcher.ElementMatchers.isInterface;
7+
import static net.bytebuddy.matcher.ElementMatchers.isPublic;
8+
import static net.bytebuddy.matcher.ElementMatchers.named;
9+
import static net.bytebuddy.matcher.ElementMatchers.not;
10+
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
611

712
import com.google.auto.service.AutoService;
813
import datadog.trace.agent.tooling.DDAdvice;
@@ -45,8 +50,10 @@ public boolean defaultEnabled() {
4550
public AgentBuilder apply(final AgentBuilder agentBuilder) {
4651
return agentBuilder
4752
.type(
48-
not(isInterface()).and(hasSuperType(named("org.eclipse.jetty.server.Handler"))),
49-
classLoaderHasClasses("javax.servlet.AsyncEvent", "javax.servlet.AsyncListener"))
53+
not(isInterface())
54+
.and(hasSuperType(named("org.eclipse.jetty.server.Handler")))
55+
.and(not(named("org.eclipse.jetty.server.handler.HandlerWrapper"))),
56+
not(classLoaderHasClasses("org.eclipse.jetty.server.AsyncContext")))
5057
.transform(
5158
new HelperInjector(
5259
"io.opentracing.contrib.web.servlet.filter.HttpServletRequestExtractAdapter",
@@ -60,10 +67,10 @@ public AgentBuilder apply(final AgentBuilder agentBuilder) {
6067
DDAdvice.create()
6168
.advice(
6269
named("handle")
63-
.and(takesArgument(0, named("String")))
70+
.and(takesArgument(0, named("java.lang.String")))
6471
.and(takesArgument(1, named("org.eclipse.jetty.server.Request")))
65-
.and(takesArgument(2, named("javax.servlet.HttpServletRequest")))
66-
.and(takesArgument(3, named("javax.servlet.HttpServletResponse")))
72+
.and(takesArgument(2, named("javax.servlet.http.HttpServletRequest")))
73+
.and(takesArgument(3, named("javax.servlet.http.HttpServletResponse")))
6774
.and(isPublic()),
6875
HandlerInstrumentationAdvice.class.getName()))
6976
.asDecorator();
@@ -73,7 +80,8 @@ public static class HandlerInstrumentationAdvice {
7380

7481
@Advice.OnMethodEnter(suppress = Throwable.class)
7582
public static Scope startSpan(
76-
@Advice.Argument(0) final String target, @Advice.Argument(2) final HttpServletRequest req) {
83+
@Advice.This final Object source, @Advice.Argument(2) final HttpServletRequest req) {
84+
7785
if (GlobalTracer.get().activeSpan() != null) {
7886
// Tracing might already be applied. If so ignore this.
7987
return null;
@@ -82,18 +90,19 @@ public static Scope startSpan(
8290
final SpanContext extractedContext =
8391
GlobalTracer.get()
8492
.extract(Format.Builtin.HTTP_HEADERS, new HttpServletRequestExtractAdapter(req));
85-
final String resourceName = req.getMethod() + target;
93+
final String resourceName = req.getMethod() + " " + source.getClass().getName();
8694
final Scope scope =
8795
GlobalTracer.get()
8896
.buildSpan(SERVLET_OPERATION_NAME)
8997
.asChildOf(extractedContext)
9098
.withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_SERVER)
9199
.withTag(DDTags.SPAN_TYPE, DDSpanTypes.WEB_SERVLET)
92-
.withTag("span.origin.type", HandlerInstrumentationAdvice.class.getName())
93-
.withTag(DDTags.RESOURCE_NAME, resourceName)
100+
.withTag("span.origin.type", source.getClass().getName())
94101
.startActive(false);
95102

96103
ServletFilterSpanDecorator.STANDARD_TAGS.onRequest(req, scope.span());
104+
Tags.COMPONENT.set(scope.span(), "jetty-handler");
105+
scope.span().setTag(DDTags.RESOURCE_NAME, resourceName);
97106
return scope;
98107
}
99108

@@ -103,6 +112,7 @@ public static void stopSpan(
103112
@Advice.Argument(3) final HttpServletResponse resp,
104113
@Advice.Enter final Scope scope,
105114
@Advice.Thrown final Throwable throwable) {
115+
106116
if (scope != null) {
107117
final Span span = scope.span();
108118
if (throwable != null) {

‎dd-java-agent/instrumentation/jetty-8/src/test/groovy/JettyHandlerTest.groovy‎

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
import datadog.trace.agent.test.AgentTestRunner
2+
import datadog.trace.agent.test.TestUtils
3+
import datadog.trace.api.DDSpanTypes
4+
import okhttp3.OkHttpClient
5+
import org.eclipse.jetty.server.Handler
6+
import org.eclipse.jetty.server.Request
7+
import org.eclipse.jetty.server.Server
8+
import org.eclipse.jetty.server.handler.AbstractHandler
9+
10+
import javax.servlet.ServletException
11+
import javax.servlet.http.HttpServletRequest
12+
import javax.servlet.http.HttpServletResponse
13+
14+
class JettyHandlerTest extends AgentTestRunner {
15+
16+
static {
17+
System.setProperty("dd.integration.jetty.enabled", "true")
18+
}
19+
20+
int port = TestUtils.randomOpenPort()
21+
Server server = new Server(port)
22+
23+
OkHttpClient client = new OkHttpClient.Builder()
24+
// Uncomment when debugging:
25+
// .connectTimeout(1, TimeUnit.HOURS)
26+
// .writeTimeout(1, TimeUnit.HOURS)
27+
// .readTimeout(1, TimeUnit.HOURS)
28+
.build()
29+
30+
def cleanup() {
31+
server.stop()
32+
}
33+
34+
@Override
35+
void afterTest() {
36+
}
37+
38+
def "call to jetty creates a trace"() {
39+
setup:
40+
Handler handler = new AbstractHandler() {
41+
@Override
42+
void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {
43+
response.setContentType("text/plain;charset=utf-8")
44+
response.setStatus(HttpServletResponse.SC_OK)
45+
baseRequest.setHandled(true)
46+
response.getWriter().println("Hello World")
47+
}
48+
}
49+
server.setHandler(handler)
50+
server.start()
51+
def request = new okhttp3.Request.Builder()
52+
.url("http://localhost:$port/")
53+
.get()
54+
.build()
55+
def response = client.newCall(request).execute()
56+
57+
expect:
58+
response.body().string().trim() == "Hello World"
59+
TEST_WRITER.waitForTraces(1)
60+
TEST_WRITER.size() == 1
61+
def trace = TEST_WRITER.firstTrace()
62+
trace.size() == 1
63+
def context = trace[0].context()
64+
context.serviceName == "unnamed-java-app"
65+
context.operationName == "jetty.request"
66+
context.resourceName == "GET ${handler.class.name}"
67+
context.spanType == DDSpanTypes.WEB_SERVLET
68+
!context.getErrorFlag()
69+
context.parentId == 0
70+
def tags = context.tags
71+
tags["http.url"] == "http://localhost:$port/"
72+
tags["http.method"] == "GET"
73+
tags["span.kind"] == "server"
74+
tags["span.type"] == "web"
75+
tags["component"] == "jetty-handler"
76+
tags["http.status_code"] == 200
77+
tags["thread.name"] != null
78+
tags["thread.id"] != null
79+
tags["span.origin.type"] == handler.class.name
80+
tags.size() == 9
81+
}
82+
83+
def "call to jetty with error creates a trace"() {
84+
setup:
85+
Handler handler = new AbstractHandler() {
86+
@Override
87+
void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {
88+
throw new RuntimeException()
89+
}
90+
}
91+
server.setHandler(handler)
92+
server.start()
93+
def request = new okhttp3.Request.Builder()
94+
.url("http://localhost:$port/")
95+
.get()
96+
.build()
97+
def response = client.newCall(request).execute()
98+
99+
expect:
100+
response.body().string().trim() == ""
101+
TEST_WRITER.waitForTraces(1)
102+
TEST_WRITER.size() == 1
103+
def trace = TEST_WRITER.firstTrace()
104+
trace.size() == 1
105+
def context = trace[0].context()
106+
context.serviceName == "unnamed-java-app"
107+
context.operationName == "jetty.request"
108+
context.resourceName == "GET ${handler.class.name}"
109+
context.spanType == DDSpanTypes.WEB_SERVLET
110+
context.getErrorFlag()
111+
context.parentId == 0
112+
def tags = context.tags
113+
tags["http.url"] == "http://localhost:$port/"
114+
tags["http.method"] == "GET"
115+
tags["span.kind"] == "server"
116+
tags["span.type"] == "web"
117+
tags["component"] == "jetty-handler"
118+
tags["http.status_code"] == 500
119+
tags["thread.name"] != null
120+
tags["thread.id"] != null
121+
tags["span.origin.type"] == handler.class.name
122+
tags["error"] == true
123+
tags["error.type"] == RuntimeException.name
124+
tags["error.stack"] != null
125+
tags.size() == 12
126+
}
127+
}

‎dd-java-agent/instrumentation/servlet-3/servlet-3.gradle‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ dependencies {
2727
compile deps.autoservice
2828

2929
testCompile project(':dd-java-agent:testing')
30+
testCompile project(':dd-java-agent:instrumentation:jetty-8') // See if there's any conflicts.
3031
testCompile group: 'org.eclipse.jetty', name: 'jetty-server', version: '8.2.0.v20160908'
3132
testCompile group: 'org.eclipse.jetty', name: 'jetty-servlet', version: '8.2.0.v20160908'
3233
testCompile group: 'org.apache.tomcat.embed', name: 'tomcat-embed-core', version: '8.0.41'

‎dd-java-agent/instrumentation/servlet-3/src/test/groovy/TomcatServletTest.groovy‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import spock.lang.Unroll
1616

1717
import java.lang.reflect.Field
1818

19-
@Timeout(5)
19+
@Timeout(15)
2020
class TomcatServletTest extends AgentTestRunner {
2121

2222
static final int PORT = TestUtils.randomOpenPort()

‎dd-java-agent/instrumentation/sparkjava-2.3/src/main/java/datadog/trace/instrumentation/sparkjava/RoutesInstrumentation.java‎

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

dd-java-agent/instrumentation/sparkjava-2.3/sparkjava-2.3.gradle renamed to dd-java-agent/instrumentation/sparkjava-2.4/sparkjava-2.4.gradle

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,34 @@
11
apply plugin: 'version-scan'
22

33
versionScan {
4-
group = "spark"
5-
module = 'spark'
6-
versions = "[2.3,)"
4+
group = "com.sparkjava"
5+
module = 'spark-core'
6+
versions = "[2.4,)"
77
verifyPresent = [
8-
"spark.embeddedserver.jetty.EmbeddedJettyServer" : null
8+
"spark.route.Routes": null
99
]
1010
}
1111

1212
apply from: "${rootDir}/gradle/java.gradle"
1313

14-
if (!JavaVersion.current().isJava8Compatible()) {
15-
sourceSets {
16-
test {
17-
groovy {
18-
// Sparkjava is not compatible with < Java 8
19-
exclude '**/SparkJavaBasedTest.groovy'
20-
}
21-
java {
22-
exclude '**/TestSparkJavaApplication.java'
23-
}
24-
}
25-
}
26-
}
14+
testJava8Minimum += '**/SparkJavaBasedTest.class'
2715

28-
dependencies {
16+
compileTestJava {
2917
sourceCompatibility = "1.8"
3018
targetCompatibility = "1.8"
31-
compile project(':dd-java-agent:agent-tooling')
19+
}
3220

21+
dependencies {
22+
compileOnly group: 'com.sparkjava', name: 'spark-core', version: '2.4'
23+
24+
compile project(':dd-java-agent:agent-tooling')
3325
compile deps.bytebuddy
3426
compile deps.opentracing
3527
compile deps.autoservice
36-
compile group: 'com.sparkjava', name: 'spark-core', version: '2.3'
3728

3829
testCompile project(':dd-java-agent:instrumentation:jetty-8')
3930
testCompile project(':dd-java-agent:testing')
4031

32+
testCompile group: 'com.sparkjava', name: 'spark-core', version: '2.4'
4133
testCompile group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.6.0'
42-
4334
}

0 commit comments

Comments
 (0)