Skip to content

Commit

Permalink
Merge pull request #435 from DataDog/tyler/jax-rs-improvements
Browse files Browse the repository at this point in the history 
 Add span for jax-rs representing controller execution
  • Loading branch information
@tylerbenson
tylerbenson committed Aug 15, 2018
2 parents 67c0f2f + 984bc75 commit 2893eb6
Show file tree
Hide file tree
Showing 11 changed files with 189 additions and 40 deletions.
43 changes: 40 additions & 3 deletions ...ns/src/main/java/datadog/trace/instrumentation/jaxrs/JaxRsAnnotationsInstrumentation.java
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package datadog.trace.instrumentation.jaxrs;

import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType;
import static io.opentracing.log.Fields.ERROR_OBJECT;
import static net.bytebuddy.matcher.ElementMatchers.declaresMethod;
import static net.bytebuddy.matcher.ElementMatchers.isAnnotatedWith;
import static net.bytebuddy.matcher.ElementMatchers.named;
Expand All @@ -9,10 +10,12 @@
import datadog.trace.agent.tooling.Instrumenter;
import datadog.trace.api.DDTags;
import io.opentracing.Scope;
import io.opentracing.Span;
import io.opentracing.tag.Tags;
import io.opentracing.util.GlobalTracer;
import java.lang.annotation.Annotation;
import java.lang.reflect.Method;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.Map;
Expand Down Expand Up @@ -55,11 +58,11 @@ public Map<ElementMatcher, String> transformers() {
public static class JaxRsAnnotationsAdvice {

@Advice.OnMethodEnter(suppress = Throwable.class)
public static void nameSpan(@Advice.This final Object obj, @Advice.Origin final Method method) {
// TODO: do we need caching for this?
public static Scope nameSpan(@Advice.Origin final Method method) {

// TODO: do we need caching for this?
final LinkedList<Path> classPaths = new LinkedList<>();
Class<?> target = obj.getClass();
Class<?> target = method.getDeclaringClass();
while (target != Object.class) {
final Path annotation = target.getAnnotation(Path.class);
if (annotation != null) {
Expand Down Expand Up @@ -93,6 +96,40 @@ public static void nameSpan(@Advice.This final Object obj, @Advice.Origin final
scope.span().setTag(DDTags.RESOURCE_NAME, resourceName);
Tags.COMPONENT.set(scope.span(), "jax-rs");
}

// Now create a span representing the method execution.

final Class<?> clazz = method.getDeclaringClass();
final String methodName = method.getName();

String className = clazz.getSimpleName();
if (className.isEmpty()) {
className = clazz.getName();
if (clazz.getPackage() != null) {
final String pkgName = clazz.getPackage().getName();
if (!pkgName.isEmpty()) {
className = clazz.getName().replace(pkgName, "").substring(1);
}
}
}

final String operationName = className + "." + methodName;

return GlobalTracer.get()
.buildSpan(operationName)
.withTag(Tags.COMPONENT.getKey(), "jax-rs-controller")
.startActive(true);
}

@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void stopSpan(
@Advice.Enter final Scope scope, @Advice.Thrown final Throwable throwable) {
if (throwable != null) {
final Span span = scope.span();
Tags.ERROR.set(span, true);
span.log(Collections.singletonMap(ERROR_OBJECT, throwable));
}
scope.close();
}
}
}
109 changes: 82 additions & 27 deletions ...rumentation/jax-rs-annotations/src/test/groovy/JaxRsAnnotationsInstrumentationTest.groovy
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import datadog.opentracing.DDSpanContext
import datadog.trace.agent.test.AgentTestRunner
import io.opentracing.util.GlobalTracer
import io.opentracing.tag.Tags

import javax.ws.rs.DELETE
import javax.ws.rs.GET
Expand All @@ -10,26 +9,43 @@ import javax.ws.rs.POST
import javax.ws.rs.PUT
import javax.ws.rs.Path

import static datadog.trace.agent.test.ListWriterAssert.assertTraces
import static datadog.trace.agent.test.TestUtils.runUnderTrace

class JaxRsAnnotationsInstrumentationTest extends AgentTestRunner {

def "span named '#resourceName' from annotations on class"() {
def "span named '#name' from annotations on class"() {
setup:
def scope = GlobalTracer.get().buildSpan("test").startActive(false)
DDSpanContext spanContext = scope.span().context()
obj.call()
runUnderTrace("test") {
obj.call()
}

expect:
spanContext.resourceName == resourceName

cleanup:
scope.close()
assertTraces(TEST_WRITER, 1) {
trace(0, 2) {
span(0) {
operationName "test"
resourceName name
parent()
tags {
"$Tags.COMPONENT.key" "jax-rs"
defaultTags()
}
}
span(1) {
operationName "${className}.call"
resourceName "${className}.call"
childOf span(0)
tags {
"$Tags.COMPONENT.key" "jax-rs-controller"
defaultTags()
}
}
}
}

where:
resourceName | obj
"test" | new Jax() {
// invalid because no annotations
void call() {}
}
name | obj
"/a" | new Jax() {
@Path("/a")
void call() {}
Expand All @@ -39,10 +55,6 @@ class JaxRsAnnotationsInstrumentationTest extends AgentTestRunner {
@Path("/b")
void call() {}
}
"test" | new InterfaceWithPath() {
// invalid because no annotations
void call() {}
}
"POST /c" | new InterfaceWithPath() {
@POST
@Path("/c")
Expand All @@ -52,10 +64,6 @@ class JaxRsAnnotationsInstrumentationTest extends AgentTestRunner {
@HEAD
void call() {}
}
"test" | new AbstractClassWithPath() {
// invalid because no annotations
void call() {}
}
"POST /abstract/d" | new AbstractClassWithPath() {
@POST
@Path("/d")
Expand All @@ -65,10 +73,6 @@ class JaxRsAnnotationsInstrumentationTest extends AgentTestRunner {
@PUT
void call() {}
}
"test" | new ChildClassWithPath() {
// invalid because no annotations
void call() {}
}
"OPTIONS /abstract/child/e" | new ChildClassWithPath() {
@OPTIONS
@Path("/e")
Expand All @@ -79,6 +83,43 @@ class JaxRsAnnotationsInstrumentationTest extends AgentTestRunner {
void call() {}
}
"POST /abstract/child" | new ChildClassWithPath()

className = getName(obj.class)
}

def "no annotations has no effect"() {
setup:
runUnderTrace("test") {
obj.call()
}

expect:
assertTraces(TEST_WRITER, 1) {
trace(0, 1) {
span(0) {
operationName "test"
resourceName "test"
tags {
defaultTags()
}
}
}
}

where:
obj | _
new Jax() {
void call() {}
} | _
new InterfaceWithPath() {
void call() {}
} | _
new AbstractClassWithPath() {
void call() {}
} | _
new ChildClassWithPath() {
void call() {}
} | _
}

interface Jax {
Expand All @@ -102,4 +143,18 @@ class JaxRsAnnotationsInstrumentationTest extends AgentTestRunner {
@POST
void call() {}
}

def getName(Class clazz) {
String className = clazz.getSimpleName()
if (className.isEmpty()) {
className = clazz.getName()
if (clazz.getPackage() != null) {
final String pkgName = clazz.getPackage().getName()
if (!pkgName.isEmpty()) {
className = clazz.getName().replace(pkgName, "").substring(1)
}
}
}
return className
}
}
4 changes: 3 additions & 1 deletion ...mentation/jsp-2.3/src/main/java/datadog/trace/instrumentation/jsp/JSPInstrumentation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,14 @@ public Map<ElementMatcher, String> transformers() {
public static class HttpJspPageAdvice {

@Advice.OnMethodEnter(suppress = Throwable.class)
public static Scope startSpan(@Advice.Argument(0) final HttpServletRequest req) {
public static Scope startSpan(
@Advice.This final Object obj, @Advice.Argument(0) final HttpServletRequest req) {
final Scope scope =
GlobalTracer.get()
.buildSpan("jsp.render")
.withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_SERVER)
.withTag(DDTags.SPAN_TYPE, DDSpanTypes.WEB_SERVLET)
.withTag("span.origin.type", obj.getClass().getSimpleName())
.withTag("servlet.context", req.getContextPath())
.startActive(true);

Expand Down
15 changes: 15 additions & 0 deletions dd-java-agent/instrumentation/jsp-2.3/src/test/groovy/JSPInstrumentationBasicTests.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"span.origin.type" "org.apache.catalina.core.ApplicationFilterChain"
"servlet.context" "/$jspWebappContext"
"http.status_code" 200
defaultTags()
Expand All @@ -114,6 +115,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
"span.kind" "server"
"component" "jsp-http-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"span.origin.type" jspClassName
"servlet.context" "/$jspWebappContext"
"jsp.requestURL" reqUrl
defaultTags()
Expand Down Expand Up @@ -175,6 +177,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"span.origin.type" "org.apache.catalina.core.ApplicationFilterChain"
"servlet.context" "/$jspWebappContext"
"http.status_code" 200
defaultTags()
Expand All @@ -191,6 +194,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
"span.kind" "server"
"component" "jsp-http-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"span.origin.type" "getQuery_jsp"
"servlet.context" "/$jspWebappContext"
"jsp.requestURL" reqUrl
defaultTags()
Expand Down Expand Up @@ -249,6 +253,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"span.origin.type" "org.apache.catalina.core.ApplicationFilterChain"
"servlet.context" "/$jspWebappContext"
"http.status_code" 200
defaultTags()
Expand All @@ -265,6 +270,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
"span.kind" "server"
"component" "jsp-http-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"span.origin.type" "post_jsp"
"servlet.context" "/$jspWebappContext"
"jsp.requestURL" reqUrl
defaultTags()
Expand Down Expand Up @@ -320,6 +326,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"span.origin.type" "org.apache.catalina.core.ApplicationFilterChain"
"servlet.context" "/$jspWebappContext"
"http.status_code" 500
errorTags(JasperException, String)
Expand All @@ -337,6 +344,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
"span.kind" "server"
"component" "jsp-http-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"span.origin.type" jspClassName
"servlet.context" "/$jspWebappContext"
"jsp.requestURL" reqUrl
errorTags(exceptionClass, errorMessage)
Expand Down Expand Up @@ -398,6 +406,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"span.origin.type" "org.apache.catalina.core.ApplicationFilterChain"
"servlet.context" "/$jspWebappContext"
"http.status_code" 200
defaultTags()
Expand All @@ -414,6 +423,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
"span.kind" "server"
"component" "jsp-http-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"span.origin.type" "includeHtml_jsp"
"servlet.context" "/$jspWebappContext"
"jsp.requestURL" reqUrl
defaultTags()
Expand Down Expand Up @@ -468,6 +478,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"span.origin.type" "org.apache.catalina.core.ApplicationFilterChain"
"servlet.context" "/$jspWebappContext"
"http.status_code" 200
defaultTags()
Expand All @@ -484,6 +495,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
"span.kind" "server"
"component" "jsp-http-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"span.origin.type" "includeMulti_jsp"
"servlet.context" "/$jspWebappContext"
"jsp.requestURL" reqUrl
defaultTags()
Expand All @@ -500,6 +512,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
"span.kind" "server"
"component" "jsp-http-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"span.origin.type" "javaLoopH2_jsp"
"servlet.context" "/$jspWebappContext"
"jsp.requestURL" reqUrl
defaultTags()
Expand Down Expand Up @@ -533,6 +546,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
"span.kind" "server"
"component" "jsp-http-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"span.origin.type" "javaLoopH2_jsp"
"servlet.context" "/$jspWebappContext"
"jsp.requestURL" reqUrl
defaultTags()
Expand Down Expand Up @@ -604,6 +618,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"span.origin.type" "org.apache.catalina.core.ApplicationFilterChain"
"servlet.context" "/$jspWebappContext"
"http.status_code" 500
errorTags(JasperException, String)
Expand Down

0 comments on commit 2893eb6

Please sign in to comment.