Merge pull request #320 from DataDog/tyler/servlet-context-naming

Name service based on servlet context
This commit is contained in:
Tyler Benson 2018-05-15 10:27:06 +10:00 committed by GitHub
commit c011eb0fbd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
22 changed files with 181 additions and 85 deletions

View File

@ -64,7 +64,7 @@ class ApacheHttpClientTest extends Specification {
final DDSpan localSpan = clientTrace.get(1)
localSpan.getType() == null
localSpan.getTags()[Tags.COMPONENT.getKey()] == "apache-httpclient"
localSpan.getOperationName() == "http.request"
localSpan.getOperationName() == "apache.http"
final DDSpan clientSpan = clientTrace.get(2)
clientSpan.getOperationName() == "http.request"
@ -109,7 +109,7 @@ class ApacheHttpClientTest extends Specification {
// our instrumentation makes 2 spans for apache-httpclient
final DDSpan localSpan = clientTrace.get(1)
localSpan.getTags()[Tags.COMPONENT.getKey()] == "apache-httpclient"
localSpan.getOperationName() == "http.request"
localSpan.getOperationName() == "apache.http"
final DDSpan clientSpan = clientTrace.get(2)
clientSpan.getOperationName() == "http.request"

View File

@ -52,5 +52,13 @@ public interface Instrumenter {
}
protected abstract AgentBuilder apply(AgentBuilder agentBuilder);
protected static String getPropOrEnv(final String name) {
return System.getProperty(name, System.getenv(propToEnvName(name)));
}
private static String propToEnvName(final String name) {
return name.toUpperCase().replace(".", "_");
}
}
}

View File

@ -100,9 +100,9 @@ class AWSClientTest extends AgentTestRunner {
and: // span 0 - from apache-httpclient instrumentation
def span1 = trace[0]
span1.context().operationName == "http.request"
span1.context().operationName == "apache.http"
span1.serviceName == "unnamed-java-app"
span1.resourceName == "http.request"
span1.resourceName == "apache.http"
span1.type == null
!span1.context().getErrorFlag()
span1.context().parentId == 0

View File

@ -58,7 +58,7 @@ class JaxRsClientTest extends AgentTestRunner {
span.context().operationName == "jax-rs.client.call"
span.serviceName == "unnamed-java-app"
span.resourceName == "GET jax-rs.client.call"
span.resourceName == "GET /ping"
span.type == "http"
!span.context().getErrorFlag()
span.context().parentId == 0

View File

@ -1,4 +1,5 @@
import datadog.trace.agent.test.AgentTestRunner
import datadog.trace.api.DDSpanTypes
import datadog.trace.api.DDTags
import io.opentracing.tag.Tags
import okhttp3.OkHttpClient
@ -39,19 +40,20 @@ class OkHttp3Test extends AgentTestRunner {
and: // span 0
def span1 = trace[0]
span1.context().operationName == "http.request"
span1.serviceName == "unnamed-java-app"
span1.resourceName == "http.request"
span1.type == null
span1.context().operationName == "okhttp.http"
span1.serviceName == "okhttp"
span1.resourceName == "okhttp.http"
span1.type == DDSpanTypes.WEB_SERVLET
!span1.context().getErrorFlag()
span1.context().parentId == 0
def tags1 = span1.context().tags
tags1["component"] == "okhttp"
tags1["span.type"] == DDSpanTypes.WEB_SERVLET
tags1["thread.name"] != null
tags1["thread.id"] != null
tags1.size() == 3
tags1.size() == 4
and: // span 1
def span2 = trace[1]

View File

@ -83,6 +83,7 @@ public final class FilterChain2Instrumentation extends Instrumenter.Configurable
.asChildOf(extractedContext)
.withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_SERVER)
.withTag(DDTags.SPAN_TYPE, DDSpanTypes.WEB_SERVLET)
.withTag("servlet.context", ((HttpServletRequest) req).getContextPath())
.startActive(true);
if (scope instanceof TraceScope) {

View File

@ -85,6 +85,7 @@ public final class HttpServlet2Instrumentation extends Instrumenter.Configurable
.asChildOf(extractedContext)
.withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_SERVER)
.withTag(DDTags.SPAN_TYPE, DDSpanTypes.WEB_SERVLET)
.withTag("servlet.context", req.getContextPath())
.startActive(true);
if (scope instanceof TraceScope) {

View File

@ -106,6 +106,7 @@ class JettyServlet2Test extends AgentTestRunner {
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"servlet.context" ""
defaultTags()
}
}
@ -143,6 +144,7 @@ class JettyServlet2Test extends AgentTestRunner {
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"servlet.context" ""
errorTags(RuntimeException, "some $path error")
defaultTags()
}
@ -182,6 +184,7 @@ class JettyServlet2Test extends AgentTestRunner {
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"servlet.context" ""
defaultTags()
}
}

View File

@ -88,6 +88,7 @@ public final class FilterChain3Instrumentation extends Instrumenter.Configurable
.asChildOf(extractedContext)
.withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_SERVER)
.withTag(DDTags.SPAN_TYPE, DDSpanTypes.WEB_SERVLET)
.withTag("servlet.context", ((HttpServletRequest) req).getContextPath())
.startActive(false);
if (scope instanceof TraceScope) {

View File

@ -84,6 +84,7 @@ public final class HttpServlet3Instrumentation extends Instrumenter.Configurable
.asChildOf(extractedContext)
.withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_SERVER)
.withTag(DDTags.SPAN_TYPE, DDSpanTypes.WEB_SERVLET)
.withTag("servlet.context", req.getContextPath())
.startActive(false);
if (scope instanceof TraceScope) {

View File

@ -108,6 +108,7 @@ class JettyServlet3Test extends AgentTestRunner {
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"servlet.context" ""
"http.status_code" 200
defaultTags()
}
@ -147,6 +148,7 @@ class JettyServlet3Test extends AgentTestRunner {
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"servlet.context" ""
"http.status_code" 500
errorTags(RuntimeException, "some $path error")
defaultTags()
@ -187,6 +189,7 @@ class JettyServlet3Test extends AgentTestRunner {
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"servlet.context" ""
"http.status_code" 500
"error" true
defaultTags()

View File

@ -44,7 +44,7 @@ class TomcatServlet3Test extends AgentTestRunner {
if (!applicationDir.exists()) {
applicationDir.mkdirs()
}
appContext = tomcatServer.addWebapp("", applicationDir.getAbsolutePath())
appContext = tomcatServer.addWebapp("/my-context", applicationDir.getAbsolutePath())
// Speed up startup by disabling jar scanning:
appContext.getJarScanner().setJarScanFilter(new JarScanFilter() {
@Override
@ -84,7 +84,7 @@ class TomcatServlet3Test extends AgentTestRunner {
def "test #path servlet call"() {
setup:
def request = new Request.Builder()
.url("http://localhost:$PORT/$path")
.url("http://localhost:$PORT/my-context/$path")
.get()
.build()
def response = client.newCall(request).execute()
@ -95,18 +95,19 @@ class TomcatServlet3Test extends AgentTestRunner {
assertTraces(writer, 1) {
trace(0, 1) {
span(0) {
serviceName "unnamed-java-app"
serviceName "my-context"
operationName "servlet.request"
resourceName "GET /$path"
resourceName "GET /my-context/$path"
spanType DDSpanTypes.WEB_SERVLET
errored false
parent()
tags {
"http.url" "http://localhost:$PORT/$path"
"http.url" "http://localhost:$PORT/my-context/$path"
"http.method" "GET"
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"servlet.context" "/my-context"
"http.status_code" 200
defaultTags()
}
@ -123,7 +124,7 @@ class TomcatServlet3Test extends AgentTestRunner {
def "test #path error servlet call"() {
setup:
def request = new Request.Builder()
.url("http://localhost:$PORT/$path?error=true")
.url("http://localhost:$PORT/my-context/$path?error=true")
.get()
.build()
def response = client.newCall(request).execute()
@ -134,18 +135,19 @@ class TomcatServlet3Test extends AgentTestRunner {
assertTraces(writer, 1) {
trace(0, 1) {
span(0) {
serviceName "unnamed-java-app"
serviceName "my-context"
operationName "servlet.request"
resourceName "GET /$path"
resourceName "GET /my-context/$path"
spanType DDSpanTypes.WEB_SERVLET
errored true
parent()
tags {
"http.url" "http://localhost:$PORT/$path"
"http.url" "http://localhost:$PORT/my-context/$path"
"http.method" "GET"
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"servlet.context" "/my-context"
"http.status_code" 500
errorTags(RuntimeException, "some $path error")
defaultTags()
@ -163,7 +165,7 @@ class TomcatServlet3Test extends AgentTestRunner {
def "test #path error servlet call for non-throwing error"() {
setup:
def request = new Request.Builder()
.url("http://localhost:$PORT/$path?non-throwing-error=true")
.url("http://localhost:$PORT/my-context/$path?non-throwing-error=true")
.get()
.build()
def response = client.newCall(request).execute()
@ -174,18 +176,19 @@ class TomcatServlet3Test extends AgentTestRunner {
assertTraces(writer, 1) {
trace(0, 1) {
span(0) {
serviceName "unnamed-java-app"
serviceName "my-context"
operationName "servlet.request"
resourceName "GET /$path"
resourceName "GET /my-context/$path"
spanType DDSpanTypes.WEB_SERVLET
errored true
parent()
tags {
"http.url" "http://localhost:$PORT/$path"
"http.url" "http://localhost:$PORT/my-context/$path"
"http.method" "GET"
"span.kind" "server"
"component" "java-web-servlet"
"span.type" DDSpanTypes.WEB_SERVLET
"servlet.context" "/my-context"
"http.status_code" 500
"error" true
defaultTags()

View File

@ -49,10 +49,11 @@ class SpringBootBasedTest extends AgentTestRunner {
span.context().tags["span.kind"] == "server"
span.context().tags["span.type"] == "web"
span.context().tags["component"] == "java-web-servlet"
span.context().tags["servlet.context"] == ""
span.context().tags["http.status_code"] == 200
span.context().tags["thread.name"] != null
span.context().tags["thread.id"] != null
span.context().tags.size() == 8
span.context().tags.size() == 9
}
def "generates 404 spans"() {
@ -78,10 +79,11 @@ class SpringBootBasedTest extends AgentTestRunner {
span0.context().tags["span.kind"] == "server"
span0.context().tags["span.type"] == "web"
span0.context().tags["component"] == "java-web-servlet"
span0.context().tags["servlet.context"] == ""
span0.context().tags["http.status_code"] == 404
span0.context().tags["thread.name"] != null
span0.context().tags["thread.id"] != null
span0.context().tags.size() == 8
span0.context().tags.size() == 9
and: // trace 1
def trace1 = TEST_WRITER.get(1)
@ -98,10 +100,11 @@ class SpringBootBasedTest extends AgentTestRunner {
span1.context().tags["span.kind"] == "server"
span1.context().tags["span.type"] == "web"
span1.context().tags["component"] == "java-web-servlet"
span1.context().tags["servlet.context"] == ""
span1.context().tags["http.status_code"] == 404
span1.context().tags["thread.name"] != null
span1.context().tags["thread.id"] != null
span1.context().tags.size() == 8
span1.context().tags.size() == 9
}
def "generates error spans"() {
@ -129,6 +132,7 @@ class SpringBootBasedTest extends AgentTestRunner {
span0.context().tags["span.kind"] == "server"
span0.context().tags["span.type"] == "web"
span0.context().tags["component"] == "java-web-servlet"
span0.context().tags["servlet.context"] == ""
span0.context().tags["http.status_code"] == 500
span0.context().tags["thread.name"] != null
span0.context().tags["thread.id"] != null
@ -136,7 +140,7 @@ class SpringBootBasedTest extends AgentTestRunner {
span0.context().tags["error.msg"] == "Request processing failed; nested exception is java.lang.RuntimeException: qwerty"
span0.context().tags["error.type"] == NestedServletException.getName()
span0.context().tags["error.stack"] != null
span0.context().tags.size() == 12
span0.context().tags.size() == 13
and: // trace 1
def trace1 = TEST_WRITER.get(1)
@ -152,11 +156,12 @@ class SpringBootBasedTest extends AgentTestRunner {
span1.context().tags["span.kind"] == "server"
span1.context().tags["span.type"] == "web"
span1.context().tags["component"] == "java-web-servlet"
span1.context().tags["servlet.context"] == ""
span1.context().tags["http.status_code"] == 500
span1.context().getErrorFlag()
span1.context().tags["thread.name"] != null
span1.context().tags["thread.id"] != null
span1.context().tags.size() == 9
span1.context().tags.size() == 10
}
def "validated form"() {
@ -179,10 +184,11 @@ class SpringBootBasedTest extends AgentTestRunner {
span.context().tags["span.kind"] == "server"
span.context().tags["span.type"] == "web"
span.context().tags["component"] == "java-web-servlet"
span.context().tags["servlet.context"] == ""
span.context().tags["http.status_code"] == 200
span.context().tags["thread.name"] != null
span.context().tags["thread.id"] != null
span.context().tags.size() == 8
span.context().tags.size() == 9
}
def "invalid form"() {
@ -210,6 +216,7 @@ class SpringBootBasedTest extends AgentTestRunner {
span0.context().tags["span.kind"] == "server"
span0.context().tags["span.type"] == "web"
span0.context().tags["component"] == "java-web-servlet"
span0.context().tags["servlet.context"] == ""
span0.context().tags["http.status_code"] == 400
span0.context().tags["thread.name"] != null
span0.context().tags["thread.id"] != null
@ -217,7 +224,7 @@ class SpringBootBasedTest extends AgentTestRunner {
span0.context().tags["error.msg"].toString().startsWith("Validation failed")
span0.context().tags["error.type"] == MethodArgumentNotValidException.getName()
span0.context().tags["error.stack"] != null
span0.context().tags.size() == 12
span0.context().tags.size() == 13
and: // trace 1
def trace1 = TEST_WRITER.get(1)
@ -234,9 +241,10 @@ class SpringBootBasedTest extends AgentTestRunner {
span1.context().tags["span.kind"] == "server"
span1.context().tags["span.type"] == "web"
span1.context().tags["component"] == "java-web-servlet"
span1.context().tags["servlet.context"] == ""
span1.context().tags["http.status_code"] == 400
span1.context().tags["thread.name"] != null
span1.context().tags["thread.id"] != null
span1.context().tags.size() == 8
span1.context().tags.size() == 9
}
}

View File

@ -86,12 +86,4 @@ public final class TraceAnnotationsInstrumentation extends Instrumenter.Configur
.advice(isAnnotatedWith(methodTraceMatcher), TraceAdvice.class.getName()))
.asDecorator();
}
private String getPropOrEnv(final String name) {
return System.getProperty(name, System.getenv(propToEnvName(name)));
}
static String propToEnvName(final String name) {
return name.toUpperCase().replace(".", "_");
}
}

View File

@ -102,12 +102,4 @@ public class TraceConfigInstrumentation extends Instrumenter.Configurable {
}
return builder;
}
private String getPropOrEnv(final String name) {
return System.getProperty(name, System.getenv(propToEnvName(name)));
}
static String propToEnvName(final String name) {
return name.toUpperCase().replace(".", "_");
}
}

View File

@ -4,10 +4,10 @@ import datadog.opentracing.DDSpan
class TagsAssert {
private final Map<String, Object> tags
private final Set<String> assertedTags = new HashSet<>()
private final Set<String> assertedTags = new TreeSet<>()
private TagsAssert(DDSpan span) {
this.tags = span.tags
this.tags = new TreeMap(span.tags)
}
static TagsAssert assertTags(DDSpan span,

View File

@ -241,7 +241,7 @@ public class DDSpanContext implements io.opentracing.SpanContext {
// Call decorators
final List<AbstractDecorator> decorators = tracer.getSpanContextDecorators(tag);
if (decorators != null && value != null) {
if (decorators != null) {
for (final AbstractDecorator decorator : decorators) {
try {
addTag &= decorator.shouldSetTag(this, tag, value);

View File

@ -7,7 +7,6 @@ import datadog.opentracing.propagation.ExtractedContext;
import datadog.opentracing.propagation.HTTPCodec;
import datadog.opentracing.scopemanager.ContextualScopeManager;
import datadog.opentracing.scopemanager.ScopeContext;
import datadog.trace.api.DDTags;
import datadog.trace.api.interceptor.MutableSpan;
import datadog.trace.api.interceptor.TraceInterceptor;
import datadog.trace.api.sampling.PrioritySampling;
@ -379,16 +378,8 @@ public class DDTracer implements io.opentracing.Tracer {
@Override
public DDSpanBuilder withTag(final String tag, final String string) {
if (tag.equals(DDTags.SERVICE_NAME)) {
return withServiceName(string);
} else if (tag.equals(DDTags.RESOURCE_NAME)) {
return withResourceName(string);
} else if (tag.equals(DDTags.SPAN_TYPE)) {
return withSpanType(string);
} else {
return withTag(tag, (Object) string);
}
}
@Override
public DDSpanBuilder withTag(final String tag, final boolean bool) {
@ -534,6 +525,36 @@ public class DDTracer implements io.opentracing.Tracer {
parentTrace,
DDTracer.this);
// Apply Decorators to handle any tags that may have been set via the builder.
for (final Map.Entry<String, Object> tag : this.tags.entrySet()) {
if (tag.getValue() == null) {
context.setTag(tag.getKey(), null);
continue;
}
boolean addTag = true;
// Call decorators
final List<AbstractDecorator> decorators =
DDTracer.this.getSpanContextDecorators(tag.getKey());
if (decorators != null) {
for (final AbstractDecorator decorator : decorators) {
try {
addTag &= decorator.shouldSetTag(context, tag.getKey(), tag.getValue());
} catch (final Throwable ex) {
log.debug(
"Could not decorate the span decorator={}: {}",
decorator.getClass().getSimpleName(),
ex.getMessage());
}
}
}
if (!addTag) {
context.setTag(tag.getKey(), null);
}
}
return context;
}
}

View File

@ -25,6 +25,7 @@ public class DDDecoratorsFactory {
new OperationDecorator(),
new ResourceNameDecorator(),
new ServiceNameDecorator(mappings),
new ServletContextDecorator(),
new SpanTypeDecorator(),
new Status5XXDecorator(),
new Status404Decorator(),

View File

@ -0,0 +1,31 @@
package datadog.opentracing.decorators;
import datadog.opentracing.DDSpanContext;
import datadog.opentracing.DDTracer;
public class ServletContextDecorator extends AbstractDecorator {
public ServletContextDecorator() {
super();
this.setMatchingTag("servlet.context");
}
@Override
public boolean shouldSetTag(final DDSpanContext context, final String tag, final Object value) {
String contextName = String.valueOf(value).trim();
if (contextName.equals("/")
|| (!context.getServiceName().equals(DDTracer.UNASSIGNED_DEFAULT_SERVICE_NAME)
&& !context.getServiceName().isEmpty())) {
return true;
}
if (contextName.startsWith("/")) {
if (contextName.length() > 1) {
contextName = contextName.substring(1);
}
}
if (!contextName.isEmpty()) {
context.setServiceName(contextName);
}
return true;
}
}

View File

@ -53,9 +53,6 @@ class SpanDecoratorTest extends Specification {
}
def "set operation name"() {
setup:
tracer.addDecorator(new OperationDecorator())
when:
Tags.COMPONENT.set(span, component)
@ -68,9 +65,6 @@ class SpanDecoratorTest extends Specification {
}
def "set resource name"() {
setup:
tracer.addDecorator(new ResourceNameDecorator())
when:
span.setTag(DDTags.RESOURCE_NAME, name)
@ -82,9 +76,6 @@ class SpanDecoratorTest extends Specification {
}
def "set span type"() {
setup:
tracer.addDecorator(new SpanTypeDecorator())
when:
span.setTag(DDTags.SPAN_TYPE, type)
@ -96,9 +87,6 @@ class SpanDecoratorTest extends Specification {
}
def "override operation with DBTypeDecorator"() {
setup:
tracer.addDecorator(new DBTypeDecorator())
when:
Tags.DB_TYPE.set(span, type)
@ -119,9 +107,6 @@ class SpanDecoratorTest extends Specification {
}
def "DBStatementAsResource should not interact on Mongo queries"() {
setup:
tracer.addDecorator(new DBStatementAsResourceName())
when:
span.setResourceName("not-change-me")
Tags.COMPONENT.set(span, "java-mongo")
@ -144,9 +129,6 @@ class SpanDecoratorTest extends Specification {
}
def "set 404 as a resource on a 404 issue"() {
setup:
tracer.addDecorator(new Status404Decorator())
when:
Tags.HTTP_STATUS.set(span, 404)
@ -155,9 +137,6 @@ class SpanDecoratorTest extends Specification {
}
def "set 5XX status code as an error"() {
setup:
tracer.addDecorator(new Status5XXDecorator())
when:
Tags.HTTP_STATUS.set(span, status)
@ -176,9 +155,6 @@ class SpanDecoratorTest extends Specification {
}
def "set error flag when error tag reported"() {
setup:
tracer.addDecorator(new ErrorFlag())
when:
Tags.ERROR.set(span, error)
@ -190,4 +166,56 @@ class SpanDecoratorTest extends Specification {
true | _
false | _
}
def "#attribute decorators apply to builder too"() {
setup:
def span = tracer.buildSpan("decorator.test").withTag(name, value).start()
expect:
span.context()."$attribute" == value
where:
attribute | name | value
"serviceName" | DDTags.SERVICE_NAME | "my-service"
"resourceName" | DDTags.RESOURCE_NAME | "my-resource"
"spanType" | DDTags.SPAN_TYPE | "my-span-type"
}
def "decorators apply to builder too"() {
when:
def span = tracer.buildSpan("decorator.test").withTag("servlet.context", "/my-servlet").start()
then:
span.serviceName == "my-servlet"
when:
span = tracer.buildSpan("decorator.test").withTag(Tags.HTTP_STATUS.key, 404).start()
then:
span.resourceName == "404"
when:
span = tracer.buildSpan("decorator.test").withTag("error", "true").start()
then:
span.error
when:
span = tracer.buildSpan("decorator.test").withTag(Tags.HTTP_STATUS.key, 500).start()
then:
span.error
when:
span = tracer.buildSpan("decorator.test").withTag(Tags.HTTP_URL.key, "http://example.com/path/number123/?param=true").start()
then:
span.resourceName == "/path/?/"
when:
span = tracer.buildSpan("decorator.test").withTag(Tags.DB_STATEMENT.key, "some-statement").start()
then:
span.resourceName == "some-statement"
}
}

View File

@ -107,7 +107,7 @@ class DDTraceConfigTest extends Specification {
tracer.sampler instanceof AllSampler
tracer.writer.toString() == "DDAgentWriter { api=DDApi { tracesEndpoint=http://localhost:8126/v0.3/traces } }"
tracer.spanContextDecorators.size() == 9
tracer.spanContextDecorators.size() == 10
}
def "verify mapping configs on tracer"() {