Skip to content

Commit

Permalink
Add span for jax-rs representing controller execution
Browse files Browse the repository at this point in the history 
Also add additional `span.origin.type` tags for better visibility.
  • Loading branch information
@tylerbenson
tylerbenson committed Aug 10, 2018
1 parent a3875af commit d5cc84e
Show file tree
Hide file tree
Showing 7 changed files with 144 additions and 39 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 @@ -54,11 +57,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 @@ -92,6 +95,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
}
}
7 changes: 4 additions & 3 deletions ...tation/servlet-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@
public class Servlet2Advice {

@Advice.OnMethodEnter(suppress = Throwable.class)
public static Scope startSpan(@Advice.Argument(0) final ServletRequest req) {
if (GlobalTracer.get().scopeManager().active() != null
|| !(req instanceof HttpServletRequest)) {
public static Scope startSpan(
@Advice.This final Object servlet, @Advice.Argument(0) final ServletRequest req) {
if (GlobalTracer.get().activeSpan() != null || !(req instanceof HttpServletRequest)) {
// Tracing might already be applied by the FilterChain. If so ignore this.
return null;
}
Expand All @@ -44,6 +44,7 @@ public static Scope startSpan(@Advice.Argument(0) final ServletRequest req) {
.withTag(Tags.HTTP_URL.getKey(), httpServletRequest.getRequestURL().toString())
.withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_SERVER)
.withTag(DDTags.SPAN_TYPE, DDSpanTypes.WEB_SERVLET)
.withTag("span.origin.type", servlet.getClass().getName())
.withTag("servlet.context", httpServletRequest.getContextPath())
.startActive(true);

Expand Down
3 changes: 3 additions & 0 deletions dd-java-agent/instrumentation/servlet-2/src/test/groovy/JettyServlet2Test.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ class JettyServlet2Test extends AgentTestRunner {
"http.method" "GET"
"span.kind" "server"
"component" "java-web-servlet"
"span.origin.type" "TestServlet2\$Sync"
"span.type" DDSpanTypes.WEB_SERVLET
"servlet.context" "/ctx"
if (auth) {
Expand Down Expand Up @@ -129,6 +130,7 @@ class JettyServlet2Test extends AgentTestRunner {
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"span.origin.type" "TestServlet2\$Sync"
"servlet.context" "/ctx"
errorTags(RuntimeException, "some $path error")
defaultTags()
Expand Down Expand Up @@ -169,6 +171,7 @@ class JettyServlet2Test extends AgentTestRunner {
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"span.origin.type" "TestServlet2\$Sync"
"servlet.context" "/ctx"
defaultTags()
}
Expand Down
4 changes: 3 additions & 1 deletion ...tation/servlet-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Advice.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
public class Servlet3Advice {

@Advice.OnMethodEnter(suppress = Throwable.class)
public static Scope startSpan(@Advice.Argument(0) final ServletRequest req) {
public static Scope startSpan(
@Advice.This final Object servlet, @Advice.Argument(0) final ServletRequest req) {
if (GlobalTracer.get().activeSpan() != null || !(req instanceof HttpServletRequest)) {
// Tracing might already be applied by the FilterChain. If so ignore this.
return null;
Expand All @@ -44,6 +45,7 @@ public static Scope startSpan(@Advice.Argument(0) final ServletRequest req) {
.withTag(Tags.HTTP_URL.getKey(), httpServletRequest.getRequestURL().toString())
.withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_SERVER)
.withTag(DDTags.SPAN_TYPE, DDSpanTypes.WEB_SERVLET)
.withTag("span.origin.type", servlet.getClass().getName())
.withTag("servlet.context", httpServletRequest.getContextPath())
.startActive(false);

Expand Down
13 changes: 8 additions & 5 deletions dd-java-agent/instrumentation/servlet-3/src/test/groovy/JettyServlet3Test.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ class JettyServlet3Test extends AgentTestRunner {
"http.method" "GET"
"span.kind" "server"
"component" "java-web-servlet"
"span.origin.type" "TestServlet3\$$origin"
"span.type" DDSpanTypes.WEB_SERVLET
"http.status_code" 200
if (auth) {
Expand All @@ -99,11 +100,11 @@ class JettyServlet3Test extends AgentTestRunner {
}

where:
path | expectedResponse | auth
"async" | "Hello Async" | false
"sync" | "Hello Sync" | false
"auth/async" | "Hello Async" | true
"auth/sync" | "Hello Sync" | true
path | expectedResponse | auth | origin
"async" | "Hello Async" | false | "Async"
"sync" | "Hello Sync" | false | "Sync"
"auth/async" | "Hello Async" | true | "Async"
"auth/sync" | "Hello Sync" | true | "Sync"
}

def "servlet instrumentation clears state after async request"() {
Expand Down Expand Up @@ -157,6 +158,7 @@ class JettyServlet3Test extends AgentTestRunner {
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"span.origin.type" "TestServlet3\$Sync"
"http.status_code" 500
errorTags(RuntimeException, "some $path error")
defaultTags()
Expand Down Expand Up @@ -197,6 +199,7 @@ class JettyServlet3Test extends AgentTestRunner {
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"span.origin.type" "TestServlet3\$Sync"
"http.status_code" 500
"error" true
defaultTags()
Expand Down
4 changes: 4 additions & 0 deletions dd-java-agent/instrumentation/servlet-3/src/test/groovy/TomcatServlet3Test.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import datadog.trace.api.DDSpanTypes
import okhttp3.OkHttpClient
import okhttp3.Request
import org.apache.catalina.Context
import org.apache.catalina.core.ApplicationFilterChain
import org.apache.catalina.startup.Tomcat
import org.apache.tomcat.JarScanFilter
import org.apache.tomcat.JarScanType
Expand Down Expand Up @@ -84,6 +85,7 @@ class TomcatServlet3Test extends AgentTestRunner {
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"span.origin.type" ApplicationFilterChain.name
"servlet.context" "/my-context"
"http.status_code" 200
defaultTags()
Expand Down Expand Up @@ -124,6 +126,7 @@ class TomcatServlet3Test extends AgentTestRunner {
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"span.origin.type" ApplicationFilterChain.name
"servlet.context" "/my-context"
"http.status_code" 500
errorTags(RuntimeException, "some $path error")
Expand Down Expand Up @@ -165,6 +168,7 @@ class TomcatServlet3Test extends AgentTestRunner {
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"span.origin.type" ApplicationFilterChain.name
"servlet.context" "/my-context"
"http.status_code" 500
"error" true
Expand Down

0 comments on commit d5cc84e

Please sign in to comment.