Resource naming and other review comments

This commit is contained in:
Jon Mort 2018-04-05 14:52:47 +01:00
parent 582c0af962
commit 1ba0b69750
No known key found for this signature in database
GPG Key ID: F48EC3DC382704A4
7 changed files with 44 additions and 55 deletions

View File

@ -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"
]
}

View File

@ -1,24 +0,0 @@
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;
}
}

View File

@ -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());
}

View File

@ -1,14 +1,7 @@
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"))

View File

@ -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;
}
}

View File

@ -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()

View File

@ -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