Resource naming and other review comments
MODIFY
dd-java-agent/instrumentation/ratpack-1.4/ratpack-1.4.gradle
dd-java-agent/instrumentation/ratpack-1.4/ratpack-1.4.gradle
@@ -7,9 +7,25 @@ versionScan {
verifyPresent = [
"ratpack.exec.ExecStarter" : null,
"ratpack.exec.Execution" : null,
+ "ratpack.exec.Promise" : null,
+ "ratpack.exec.Result" : null,
"ratpack.func.Action" : null,
+ "ratpack.func.Function" : null,
+ "ratpack.handling.Context" : null,
+ "ratpack.handling.Handler" : null,
+ "ratpack.handling.HandlerDecorator" : null,
+ "ratpack.http.HttpMethod" : null,
+ "ratpack.http.MutableHeaders" : null,
+ "ratpack.http.Request" : null,
+ "ratpack.http.Status" : null,
"ratpack.http.client.HttpClient" : null,
- "ratpack.server.internal.ServerRegistry": "buildBaseRegistry",
+ "ratpack.http.client.ReceivedResponse" : null,
+ "ratpack.http.client.RequestSpec" : null,
+ "ratpack.http.client.StreamedResponse" : null,
+ "ratpack.path.PathBinding" : "getDescription",
+ "ratpack.registry.Registry" : null,
+ "ratpack.registry.RegistrySpec" : null,
+ "ratpack.server.internal.ServerRegistry": "buildBaseRegistry"
]
}

DELETE
dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/RatapckInstrumentationUtils.java
-package datadog.trace.instrumentation.ratpack;
-
-import io.opentracing.tag.Tags;
-import java.io.PrintWriter;
-import java.io.StringWriter;
-import java.util.HashMap;
-import java.util.Map;
-
-public class RatapckInstrumentationUtils {
- public static Map<String, Object> errorLogs(final Throwable throwable) {
- final Map<String, Object> errorLogs = new HashMap<>(4);
- errorLogs.put("event", Tags.ERROR.getKey());
- errorLogs.put("error.kind", throwable.getClass().getName());
- errorLogs.put("error.object", throwable);
-
- errorLogs.put("message", throwable.getMessage());
-
- final StringWriter sw = new StringWriter();
- throwable.printStackTrace(new PrintWriter(sw));
- errorLogs.put("stack", sw.toString());
-
- return errorLogs;
- }
-}

MODIFY
dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/RatpackHttpClientInstrumentation.java
dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/RatpackHttpClientInstrumentation.java
@@ -3,6 +3,7 @@ package datadog.trace.instrumentation.ratpack;
import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses;
import static datadog.trace.instrumentation.ratpack.RatpackInstrumentation.ACTION_TYPE_DESCRIPTION;
import static datadog.trace.instrumentation.ratpack.RatpackInstrumentation.EXEC_NAME;
+import static io.opentracing.log.Fields.ERROR_OBJECT;
import static net.bytebuddy.matcher.ElementMatchers.hasSuperType;
import static net.bytebuddy.matcher.ElementMatchers.isInterface;
import static net.bytebuddy.matcher.ElementMatchers.named;
@@ -17,6 +18,7 @@ import io.opentracing.Span;
import io.opentracing.tag.Tags;
import io.opentracing.util.GlobalTracer;
import java.net.URI;
+import java.util.Collections;
import java.util.concurrent.atomic.AtomicReference;
import net.bytebuddy.agent.builder.AgentBuilder;
import net.bytebuddy.asm.Advice;
@@ -36,8 +38,7 @@ public final class RatpackHttpClientInstrumentation extends Instrumenter.Configu
"datadog.trace.instrumentation.ratpack.RatpackHttpClientInstrumentation$RatpackHttpClientRequestAdvice",
"datadog.trace.instrumentation.ratpack.RatpackHttpClientInstrumentation$RatpackHttpClientRequestStreamAdvice",
"datadog.trace.instrumentation.ratpack.RatpackHttpClientInstrumentation$RatpackHttpGetAdvice",
- "datadog.trace.instrumentation.ratpack.RatpackHttpClientInstrumentation$GetRequestSpecAction",
- "datadog.trace.instrumentation.ratpack.RatapckInstrumentationUtils");
+ "datadog.trace.instrumentation.ratpack.RatpackHttpClientInstrumentation$GetRequestSpecAction");
public static final TypeDescription.ForLoadedType URI_TYPE_DESCRIPTION =
new TypeDescription.ForLoadedType(URI.class);
@@ -126,7 +127,7 @@ public final class RatpackHttpClientInstrumentation extends Instrumenter.Configu
span.finish();
if (result.isError()) {
Tags.ERROR.set(span, true);
- span.log(RatapckInstrumentationUtils.errorLogs(result.getThrowable()));
+ span.log(Collections.singletonMap(ERROR_OBJECT, result.getThrowable()));
} else {
Tags.HTTP_STATUS.set(span, result.getValue().getStatusCode());
}
@@ -145,7 +146,7 @@ public final class RatpackHttpClientInstrumentation extends Instrumenter.Configu
span.finish();
if (result.isError()) {
Tags.ERROR.set(span, true);
- span.log(RatapckInstrumentationUtils.errorLogs(result.getThrowable()));
+ span.log(Collections.singletonMap(ERROR_OBJECT, result.getThrowable()));
} else {
Tags.HTTP_STATUS.set(span, result.getValue().getStatusCode());
}

MODIFY
dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/RatpackInstrumentation.java
dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/RatpackInstrumentation.java
package datadog.trace.instrumentation.ratpack;
import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses;
-import static net.bytebuddy.matcher.ElementMatchers.hasSuperType;
-import static net.bytebuddy.matcher.ElementMatchers.isInterface;
-import static net.bytebuddy.matcher.ElementMatchers.isMethod;
-import static net.bytebuddy.matcher.ElementMatchers.isStatic;
-import static net.bytebuddy.matcher.ElementMatchers.named;
-import static net.bytebuddy.matcher.ElementMatchers.not;
-import static net.bytebuddy.matcher.ElementMatchers.returns;
-import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
+import static net.bytebuddy.matcher.ElementMatchers.*;
import com.google.auto.service.AutoService;
import datadog.opentracing.scopemanager.ContextualScopeManager;
@@ -25,7 +18,6 @@ import net.bytebuddy.agent.builder.AgentBuilder;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.type.TypeDescription;
import ratpack.exec.ExecStarter;
-import ratpack.exec.Execution;
import ratpack.func.Action;
import ratpack.handling.HandlerDecorator;
import ratpack.registry.Registry;
@@ -92,7 +84,7 @@ public final class RatpackInstrumentation extends Instrumenter.Configurable {
ExecStarterAdvice.class.getName()))
.asDecorator()
.type(
- named(Execution.class.getName())
+ named("ratpack.exec.Execution")
.or(not(isInterface()).and(hasSuperType(named("ratpack.exec.Execution")))),
classLoaderHasClasses(
"ratpack.exec.ExecStarter", "ratpack.registry.RegistrySpec", "ratpack.func.Action"))

MODIFY
dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/TracingHandler.java
dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/TracingHandler.java
@@ -15,7 +15,7 @@ import ratpack.http.Status;
/**
* This Ratpack handler reads tracing headers from the incoming request, starts a scope and ensures
- * that the scope is closed when the request is sent
+ * that the scope is closed when the response is sent
*/
public final class TracingHandler implements Handler {
@Override
@@ -28,7 +28,7 @@ public final class TracingHandler implements Handler {
final Scope scope =
GlobalTracer.get()
- .buildSpan("ratpack")
+ .buildSpan("ratpack.handler")
.asChildOf(extractedContext)
.withTag(Tags.COMPONENT.getKey(), "handler")
.withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_SERVER)
@@ -58,8 +58,11 @@ public final class TracingHandler implements Handler {
private static String getResourceName(Context ctx) {
String description = ctx.getPathBinding().getDescription();
if (description == null || description.isEmpty()) {
- return ctx.getRequest().getUri();
+ description = ctx.getRequest().getUri();
}
- return description;
+ if (!description.startsWith("/")) {
+ description = "/" + description;
+ }
+ return ctx.getRequest().getMethod().getName() + " " + description;
}
}

MODIFY
dd-java-agent/instrumentation/ratpack-1.4/src/test/groovy/RatpackTest.groovy
dd-java-agent/instrumentation/ratpack-1.4/src/test/groovy/RatpackTest.groovy
@@ -52,8 +52,8 @@ class RatpackTest extends AgentTestRunner {
def span = trace[0]
span.context().serviceName == "unnamed-java-app"
- span.context().operationName == "ratpack"
- span.context().resourceName == "/"
+ span.context().operationName == "ratpack.handler"
+ span.context().resourceName == "GET /"
span.context().tags["component"] == "handler"
span.context().spanType == DDSpanTypes.WEB_SERVLET
!span.context().getErrorFlag()
@@ -92,8 +92,8 @@ class RatpackTest extends AgentTestRunner {
def span = trace[0]
span.context().serviceName == "unnamed-java-app"
- span.context().operationName == "ratpack"
- span.context().resourceName == ":foo/:bar?/baz"
+ span.context().operationName == "ratpack.handler"
+ span.context().resourceName == "GET /:foo/:bar?/baz"
span.context().tags["component"] == "handler"
span.context().spanType == DDSpanTypes.WEB_SERVLET
!span.context().getErrorFlag()
@@ -130,7 +130,8 @@ class RatpackTest extends AgentTestRunner {
span.context().getErrorFlag()
span.context().serviceName == "unnamed-java-app"
- span.context().operationName == "ratpack"
+ span.context().operationName == "ratpack.handler"
+ span.context().resourceName == "GET /"
span.context().tags["component"] == "handler"
span.context().spanType == DDSpanTypes.WEB_SERVLET
span.context().tags["http.url"] == "/"
@@ -194,8 +195,8 @@ class RatpackTest extends AgentTestRunner {
def span = trace[0]
span.context().serviceName == "unnamed-java-app"
- span.context().operationName == "ratpack"
- span.context().resourceName == "/"
+ span.context().operationName == "ratpack.handler"
+ span.context().resourceName == "GET /"
span.context().tags["component"] == "handler"
span.context().spanType == DDSpanTypes.WEB_SERVLET
!span.context().getErrorFlag()
@@ -239,8 +240,8 @@ class RatpackTest extends AgentTestRunner {
def nestedSpan = nestedTrace[0]
nestedSpan.context().serviceName == "unnamed-java-app"
- nestedSpan.context().operationName == "ratpack"
- nestedSpan.context().resourceName == "nested2"
+ nestedSpan.context().operationName == "ratpack.handler"
+ nestedSpan.context().resourceName == "GET /nested2"
nestedSpan.context().tags["component"] == "handler"
nestedSpan.context().spanType == DDSpanTypes.WEB_SERVLET
!nestedSpan.context().getErrorFlag()
@@ -256,8 +257,8 @@ class RatpackTest extends AgentTestRunner {
def nestedSpan2 = nestedTrace2[0]
nestedSpan2.context().serviceName == "unnamed-java-app"
- nestedSpan2.context().operationName == "ratpack"
- nestedSpan2.context().resourceName == "nested"
+ nestedSpan2.context().operationName == "ratpack.handler"
+ nestedSpan2.context().resourceName == "GET /nested"
nestedSpan2.context().tags["component"] == "handler"
nestedSpan2.context().spanType == DDSpanTypes.WEB_SERVLET
!nestedSpan2.context().getErrorFlag()

MODIFY
settings.gradle
settings.gradle
@@ -31,10 +31,10 @@ include ':dd-java-agent:instrumentation:okhttp-3'
include ':dd-java-agent:instrumentation:osgi-classloading'
include ':dd-java-agent:instrumentation:play-2.4'
include ':dd-java-agent:instrumentation:play-2.4:play-2.6-testing'
+include ':dd-java-agent:instrumentation:ratpack-1.4'
include ':dd-java-agent:instrumentation:servlet-2'
include ':dd-java-agent:instrumentation:servlet-3'
include ':dd-java-agent:instrumentation:spring-web'
-include ':dd-java-agent:instrumentation:ratpack-1.4'
include ':dd-java-agent:instrumentation:trace-annotation'
// benchmark

Resource naming and other review comments