Merge pull request #961 from DataDog/tyler/glassfish

Disable Grizzly instrumentation by default
This commit is contained in:
Tyler Benson 2019-08-29 11:37:55 -04:00 committed by GitHub
commit 534df900c3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
17 changed files with 299 additions and 34 deletions

View File

@ -12,6 +12,8 @@ import lombok.extern.slf4j.Slf4j;
@Slf4j
public abstract class HttpServerDecorator<REQUEST, CONNECTION, RESPONSE> extends ServerDecorator {
public static final String DD_SPAN_ATTRIBUTE = "datadog.span";
// Source: https://www.regextester.com/22
private static final Pattern VALID_IPV4_ADDRESS =
Pattern.compile(

View File

@ -1,6 +1,6 @@
apply from: "${rootDir}/gradle/java.gradle"
apply plugin: 'idea'
apply plugin: 'org.unbroken-dome.test-sets'
ext {
maxJavaVersionForTests = JavaVersion.VERSION_1_8
}
muzzle {
pass {
@ -11,6 +11,10 @@ muzzle {
}
}
apply from: "${rootDir}/gradle/java.gradle"
apply plugin: 'org.unbroken-dome.test-sets'
testSets {
latestDepTest {
dirName = 'test'
@ -18,10 +22,9 @@ testSets {
}
dependencies {
testCompile project(':dd-java-agent:instrumentation:servlet-3')
testCompile project(':dd-java-agent:instrumentation:grizzly-2')
compile project(':dd-java-agent:agent-tooling')
testCompile group: 'org.glassfish.main.extras', name: 'glassfish-embedded-all', version: '4.1.2'
latestDepTestCompile sourceSets.test.output
testCompile group: 'org.glassfish.main.extras', name: 'glassfish-embedded-all', version: '4.0'
latestDepTestCompile group: 'org.glassfish.main.extras', name: 'glassfish-embedded-all', version: '+'
}

View File

@ -26,9 +26,9 @@ import net.bytebuddy.matcher.ElementMatchers;
*/
@Slf4j
@AutoService(Instrumenter.class)
public final class GlassfishInstrumentation extends Instrumenter.Default {
public final class GlassFishInstrumentation extends Instrumenter.Default {
public GlassfishInstrumentation() {
public GlassFishInstrumentation() {
super("glassfish");
}
@ -55,7 +55,7 @@ public final class GlassfishInstrumentation extends Instrumenter.Default {
@Advice.OnMethodEnter(suppress = Throwable.class)
public static void preventBlacklistingOfTracerClasses(
@Advice.Argument(value = 0, readOnly = false) String name) {
for (String prefix : Constants.BOOTSTRAP_PACKAGE_PREFIXES) {
for (final String prefix : Constants.BOOTSTRAP_PACKAGE_PREFIXES) {
if (name.startsWith(prefix)) {
name = "__datadog_no_blacklist." + name;
break;

View File

@ -0,0 +1,125 @@
import datadog.trace.agent.test.asserts.TraceAssert
import datadog.trace.agent.test.base.HttpServerTest
import datadog.trace.api.DDSpanTypes
import datadog.trace.instrumentation.servlet3.Servlet3Decorator
import io.opentracing.tag.Tags
import org.apache.catalina.servlets.DefaultServlet
import org.glassfish.embeddable.BootstrapProperties
import org.glassfish.embeddable.Deployer
import org.glassfish.embeddable.GlassFish
import org.glassfish.embeddable.GlassFishProperties
import org.glassfish.embeddable.GlassFishRuntime
import org.glassfish.embeddable.archive.ScatteredArchive
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS
/**
* Unfortunately because we're using an embedded GlassFish instance, we aren't exercising the standard
* OSGi setup that required {@link datadog.trace.instrumentation.glassfish.GlassFishInstrumentation}.
*/
// TODO: Figure out a better way to test with OSGi included.
class GlassFishServerTest extends HttpServerTest<GlassFish, Servlet3Decorator> {
// static {
// System.setProperty("dd.integration.grizzly.enabled", "true")
// }
@Override
URI buildAddress() {
return new URI("http://localhost:$port/$context/")
}
String getContext() {
"test-gf"
}
@Override
GlassFish startServer(int port) {
// Setup the deployment archive
def testDir = new File(TestServlets.protectionDomain.codeSource.location.path)
assert testDir.exists() && testDir.directory
def testResourcesDir = new File(TestServlets.getResource("error.jsp").path).parentFile
assert testResourcesDir.exists() && testResourcesDir.directory
ScatteredArchive archive = new ScatteredArchive(context, ScatteredArchive.Type.WAR, testResourcesDir)
archive.addClassPath(testDir)
// Initialize the server
BootstrapProperties bootstrapProperties = new BootstrapProperties()
GlassFishRuntime glassfishRuntime = GlassFishRuntime.bootstrap(bootstrapProperties)
GlassFishProperties glassfishProperties = new GlassFishProperties()
glassfishProperties.setPort('http-listener', port)
def server = glassfishRuntime.newGlassFish(glassfishProperties)
server.start()
// Deploy war to server
Deployer deployer = server.getDeployer()
println "Deploying $testDir.absolutePath with $testResourcesDir.absolutePath"
deployer.deploy(archive.toURI())
return server
}
@Override
void stopServer(GlassFish server) {
server.stop()
}
@Override
Servlet3Decorator decorator() {
return Servlet3Decorator.DECORATE
}
@Override
String expectedOperationName() {
return "servlet.request"
}
@Override
String expectedServiceName() {
context
}
@Override
boolean redirectHasBody() {
true
}
@Override
void serverSpan(TraceAssert trace, int index, String traceID = null, String parentID = null, String method = "GET", ServerEndpoint endpoint = SUCCESS) {
trace.span(index) {
serviceName expectedServiceName()
operationName expectedOperationName()
resourceName endpoint.status == 404 ? "404" : "$method ${endpoint.resolve(address).path}"
spanType DDSpanTypes.HTTP_SERVER
errored endpoint.errored
if (parentID != null) {
traceId traceID
parentId parentID
} else {
parent()
}
tags {
"servlet.context" "/$context"
"span.origin.type" { it.startsWith("TestServlets\$") || it == DefaultServlet.name }
defaultTags(true)
"$Tags.COMPONENT.key" serverDecorator.component()
if (endpoint.errored) {
"$Tags.ERROR.key" endpoint.errored
"error.msg" { it == null || it == EXCEPTION.body }
"error.type" { it == null || it == Exception.name }
"error.stack" { it == null || it instanceof String }
}
"$Tags.HTTP_STATUS.key" endpoint.status
"$Tags.HTTP_URL.key" "${endpoint.resolve(address)}"
"$Tags.PEER_HOSTNAME.key" { it == "localhost" || it == "127.0.0.1" }
"$Tags.PEER_PORT.key" Integer
"$Tags.PEER_HOST_IPV4.key" { it == null || it == "127.0.0.1" } // Optional
"$Tags.HTTP_METHOD.key" method
"$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER
}
}
}
}

View File

@ -0,0 +1,20 @@
import static net.bytebuddy.matcher.ElementMatchers.named;
import com.google.auto.service.AutoService;
import datadog.trace.agent.test.base.HttpServerTestAdvice;
import datadog.trace.agent.tooling.Instrumenter;
import net.bytebuddy.agent.builder.AgentBuilder;
@AutoService(Instrumenter.class)
public class GrizzlyTestInstrumentation implements Instrumenter {
@Override
public AgentBuilder instrument(final AgentBuilder agentBuilder) {
return agentBuilder
.type(named("org.glassfish.grizzly.http.server.HttpServerFilter"))
.transform(
new AgentBuilder.Transformer.ForAdvice()
.advice(
named("handleRead"), HttpServerTestAdvice.ServerEntryAdvice.class.getName()));
}
}

View File

@ -0,0 +1,79 @@
import datadog.trace.agent.test.base.HttpServerTest;
import groovy.lang.Closure;
import javax.servlet.annotation.WebServlet;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
public class TestServlets {
@WebServlet("/success")
public static class Success extends HttpServlet {
@Override
protected void service(final HttpServletRequest req, final HttpServletResponse resp) {
final HttpServerTest.ServerEndpoint endpoint =
HttpServerTest.ServerEndpoint.forPath(req.getServletPath());
HttpServerTest.controller(
endpoint,
new Closure(null) {
public Object doCall() throws Exception {
resp.setContentType("text/plain");
resp.setStatus(endpoint.getStatus());
resp.getWriter().print(endpoint.getBody());
return null;
}
});
}
}
@WebServlet("/redirect")
public static class Redirect extends HttpServlet {
@Override
protected void service(final HttpServletRequest req, final HttpServletResponse resp) {
final HttpServerTest.ServerEndpoint endpoint =
HttpServerTest.ServerEndpoint.forPath(req.getServletPath());
HttpServerTest.controller(
endpoint,
new Closure(null) {
public Object doCall() throws Exception {
resp.sendRedirect(endpoint.getBody());
return null;
}
});
}
}
@WebServlet("/error")
public static class Error extends HttpServlet {
@Override
protected void service(final HttpServletRequest req, final HttpServletResponse resp) {
final HttpServerTest.ServerEndpoint endpoint =
HttpServerTest.ServerEndpoint.forPath(req.getServletPath());
HttpServerTest.controller(
endpoint,
new Closure(null) {
public Object doCall() throws Exception {
resp.setContentType("text/plain");
resp.sendError(endpoint.getStatus(), endpoint.getBody());
return null;
}
});
}
}
@WebServlet("/exception")
public static class ExceptionServlet extends HttpServlet {
@Override
protected void service(final HttpServletRequest req, final HttpServletResponse resp) {
final HttpServerTest.ServerEndpoint endpoint =
HttpServerTest.ServerEndpoint.forPath(req.getServletPath());
HttpServerTest.controller(
endpoint,
new Closure(null) {
public Object doCall() throws Exception {
throw new Exception(endpoint.getBody());
}
});
}
}
}

View File

@ -0,0 +1,10 @@
<web-app xmlns="http://xmlns.jcp.org/xml/ns/javaee"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://xmlns.jcp.org/xml/ns/javaee
http://xmlns.jcp.org/xml/ns/javaee/web-app_3_1.xsd"
version="3.1">
<error-page>
<location>/error.jsp</location>
</error-page>
</web-app>

View File

@ -0,0 +1,7 @@
<%@ page contentType="text/plain;charset=UTF-8" isErrorPage="true" trimDirectiveWhitespaces="true" %>
<% if (exception != null) {%>
<%= exception.getMessage() %>
<% } else { %>
<%= request.getAttribute("javax.servlet.error.message") %>
<% } %>

View File

@ -1,5 +1,6 @@
package datadog.trace.instrumentation.grizzly;
import static datadog.trace.agent.decorator.HttpServerDecorator.DD_SPAN_ATTRIBUTE;
import static datadog.trace.instrumentation.grizzly.GrizzlyDecorator.DECORATE;
import static io.opentracing.propagation.Format.Builtin.TEXT_MAP;
import static java.util.Collections.singletonMap;
@ -30,6 +31,11 @@ public class GrizzlyHttpHandlerInstrumentation extends Instrumenter.Default {
super("grizzly");
}
@Override
public boolean defaultEnabled() {
return false;
}
@Override
public ElementMatcher<TypeDescription> typeMatcher() {
return named("org.glassfish.grizzly.http.server.HttpHandler");
@ -62,7 +68,7 @@ public class GrizzlyHttpHandlerInstrumentation extends Instrumenter.Default {
@Advice.OnMethodEnter(suppress = Throwable.class)
public static Scope methodEnter(@Advice.Argument(0) final Request request) {
if (request.getAttribute(SpanClosingListener.GRIZZLY_SPAN_SPAN) != null) {
if (request.getAttribute(DD_SPAN_ATTRIBUTE) != null) {
return null;
}
@ -80,7 +86,7 @@ public class GrizzlyHttpHandlerInstrumentation extends Instrumenter.Default {
((TraceScope) scope).setAsyncPropagation(true);
}
request.setAttribute(SpanClosingListener.GRIZZLY_SPAN_SPAN, span);
request.setAttribute(DD_SPAN_ATTRIBUTE, span);
request.addAfterServiceListener(SpanClosingListener.LISTENER);
return scope;
@ -104,14 +110,13 @@ public class GrizzlyHttpHandlerInstrumentation extends Instrumenter.Default {
}
public static class SpanClosingListener implements AfterServiceListener {
public static final String GRIZZLY_SPAN_SPAN = "datadog.grizzly.span";
public static final SpanClosingListener LISTENER = new SpanClosingListener();
@Override
public void onAfterService(final Request request) {
final Object spanAttr = request.getAttribute(GRIZZLY_SPAN_SPAN);
final Object spanAttr = request.getAttribute(DD_SPAN_ATTRIBUTE);
if (spanAttr instanceof Span) {
request.removeAttribute(GRIZZLY_SPAN_SPAN);
request.removeAttribute(DD_SPAN_ATTRIBUTE);
final Span span = (Span) spanAttr;
DECORATE.onResponse(span, request.getResponse());
DECORATE.beforeFinish(span);

View File

@ -16,6 +16,10 @@ import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCES
class GrizzlyTest extends HttpServerTest<HttpServer, GrizzlyDecorator> {
static {
System.setProperty("dd.integration.grizzly.enabled", "true")
}
@Override
HttpServer startServer(int port) {
ResourceConfig rc = new ResourceConfig()

View File

@ -7,6 +7,7 @@ import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
class JerseyTest extends AgentTestRunner {
// FIXME: migrate test.
@Shared
@ClassRule
ResourceTestRule resources = ResourceTestRule.builder().addResource(new TestResource()).build()

View File

@ -1,5 +1,6 @@
package datadog.trace.instrumentation.jaxrs.v1;
import static datadog.trace.agent.decorator.HttpServerDecorator.DD_SPAN_ATTRIBUTE;
import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType;
import static datadog.trace.instrumentation.jaxrs.v1.JaxRsClientV1Decorator.DECORATE;
import static java.util.Collections.singletonMap;
@ -66,7 +67,7 @@ public final class JaxRsClientV1Instrumentation extends Instrumenter.Default {
@Advice.This final ClientHandler thisObj) {
// WARNING: this might be a chain...so we only have to trace the first in the chain.
final boolean isRootClientHandler = null == request.getProperties().get("dd.span");
final boolean isRootClientHandler = null == request.getProperties().get(DD_SPAN_ATTRIBUTE);
if (isRootClientHandler) {
final Tracer tracer = GlobalTracer.get();
final Span span =
@ -76,7 +77,7 @@ public final class JaxRsClientV1Instrumentation extends Instrumenter.Default {
.start();
DECORATE.afterStart(span);
DECORATE.onRequest(span, request);
request.getProperties().put("dd.span", span);
request.getProperties().put(DD_SPAN_ATTRIBUTE, span);
tracer.inject(
span.context(), Format.Builtin.HTTP_HEADERS, new InjectAdapter(request.getHeaders()));

View File

@ -1,5 +1,6 @@
package datadog.trace.instrumentation.jetty8;
import static datadog.trace.agent.decorator.HttpServerDecorator.DD_SPAN_ATTRIBUTE;
import static datadog.trace.instrumentation.jetty8.JettyDecorator.DECORATE;
import datadog.trace.api.DDTags;
@ -16,13 +17,12 @@ import javax.servlet.http.HttpServletResponse;
import net.bytebuddy.asm.Advice;
public class JettyHandlerAdvice {
public static final String SERVLET_SPAN = "datadog.servlet.span";
@Advice.OnMethodEnter(suppress = Throwable.class)
public static Scope startSpan(
@Advice.This final Object source, @Advice.Argument(2) final HttpServletRequest req) {
if (req.getAttribute(SERVLET_SPAN) != null) {
if (req.getAttribute(DD_SPAN_ATTRIBUTE) != null) {
// Request already being traced elsewhere.
return null;
}
@ -48,7 +48,7 @@ public class JettyHandlerAdvice {
if (scope instanceof TraceScope) {
((TraceScope) scope).setAsyncPropagation(true);
}
req.setAttribute(SERVLET_SPAN, span);
req.setAttribute(DD_SPAN_ATTRIBUTE, span);
return scope;
}

View File

@ -1,5 +1,6 @@
package datadog.trace.instrumentation.servlet2;
import static datadog.trace.agent.decorator.HttpServerDecorator.DD_SPAN_ATTRIBUTE;
import static datadog.trace.instrumentation.servlet2.Servlet2Decorator.DECORATE;
import datadog.trace.api.DDTags;
@ -19,7 +20,6 @@ import net.bytebuddy.asm.Advice;
import net.bytebuddy.implementation.bytecode.assign.Assigner;
public class Servlet2Advice {
public static final String SERVLET_SPAN = "datadog.servlet.span";
@Advice.OnMethodEnter(suppress = Throwable.class)
public static Scope startSpan(
@ -27,7 +27,7 @@ public class Servlet2Advice {
@Advice.Argument(0) final ServletRequest req,
@Advice.Argument(value = 1, readOnly = false, typing = Assigner.Typing.DYNAMIC)
ServletResponse resp) {
final Object spanAttr = req.getAttribute(SERVLET_SPAN);
final Object spanAttr = req.getAttribute(DD_SPAN_ATTRIBUTE);
if (!(req instanceof HttpServletRequest) || spanAttr != null) {
// Tracing might already be applied by the FilterChain. If so ignore this.
return null;
@ -61,7 +61,7 @@ public class Servlet2Advice {
((TraceScope) scope).setAsyncPropagation(true);
}
req.setAttribute(SERVLET_SPAN, span);
req.setAttribute(DD_SPAN_ATTRIBUTE, span);
return scope;
}

View File

@ -1,7 +1,7 @@
package datadog.trace.instrumentation.servlet3;
import static datadog.trace.agent.decorator.HttpServerDecorator.DD_SPAN_ATTRIBUTE;
import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType;
import static datadog.trace.instrumentation.servlet3.Servlet3Advice.SERVLET_SPAN;
import static java.util.Collections.singletonMap;
import static net.bytebuddy.matcher.ElementMatchers.isInterface;
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
@ -63,9 +63,9 @@ public final class AsyncContextInstrumentation extends Instrumenter.Default {
}
final ServletRequest request = context.getRequest();
final Object spanAttr = request.getAttribute(SERVLET_SPAN);
final Object spanAttr = request.getAttribute(DD_SPAN_ATTRIBUTE);
if (spanAttr instanceof Span) {
request.removeAttribute(SERVLET_SPAN);
request.removeAttribute(DD_SPAN_ATTRIBUTE);
final Span span = (Span) spanAttr;
// Override propagation headers by injecting attributes from the current span
// into the new request

View File

@ -1,5 +1,6 @@
package datadog.trace.instrumentation.servlet3;
import static datadog.trace.agent.decorator.HttpServerDecorator.DD_SPAN_ATTRIBUTE;
import static datadog.trace.instrumentation.servlet3.Servlet3Decorator.DECORATE;
import datadog.trace.api.DDTags;
@ -19,12 +20,11 @@ import javax.servlet.http.HttpServletResponse;
import net.bytebuddy.asm.Advice;
public class Servlet3Advice {
public static final String SERVLET_SPAN = "datadog.servlet.span";
@Advice.OnMethodEnter(suppress = Throwable.class)
public static Scope startSpan(
@Advice.This final Object servlet, @Advice.Argument(0) final ServletRequest req) {
final Object spanAttr = req.getAttribute(SERVLET_SPAN);
final Object spanAttr = req.getAttribute(DD_SPAN_ATTRIBUTE);
if (!(req instanceof HttpServletRequest) || spanAttr != null) {
// Tracing might already be applied by the FilterChain. If so ignore this.
return null;
@ -54,7 +54,7 @@ public class Servlet3Advice {
((TraceScope) scope).setAsyncPropagation(true);
}
req.setAttribute(SERVLET_SPAN, span);
req.setAttribute(DD_SPAN_ATTRIBUTE, span);
return scope;
}
@ -65,7 +65,7 @@ public class Servlet3Advice {
@Advice.Enter final Scope scope,
@Advice.Thrown final Throwable throwable) {
// Set user.principal regardless of who created this span.
final Object spanAttr = request.getAttribute(SERVLET_SPAN);
final Object spanAttr = request.getAttribute(DD_SPAN_ATTRIBUTE);
if (spanAttr instanceof Span && request instanceof HttpServletRequest) {
final Principal principal = ((HttpServletRequest) request).getUserPrincipal();
if (principal != null) {
@ -87,7 +87,7 @@ public class Servlet3Advice {
}
DECORATE.onError(span, throwable);
DECORATE.beforeFinish(span);
req.removeAttribute(SERVLET_SPAN);
req.removeAttribute(DD_SPAN_ATTRIBUTE);
span.finish(); // Finish the span manually since finishSpanOnClose was false
} else {
final AtomicBoolean activated = new AtomicBoolean(false);
@ -103,7 +103,7 @@ public class Servlet3Advice {
if (!req.isAsyncStarted() && activated.compareAndSet(false, true)) {
DECORATE.onResponse(span, resp);
DECORATE.beforeFinish(span);
req.removeAttribute(SERVLET_SPAN);
req.removeAttribute(DD_SPAN_ATTRIBUTE);
span.finish(); // Finish the span manually since finishSpanOnClose was false
}
}

View File

@ -8,9 +8,11 @@ import datadog.trace.agent.test.asserts.TraceAssert
import datadog.trace.agent.test.utils.OkHttpUtils
import datadog.trace.agent.test.utils.PortUtils
import datadog.trace.api.DDSpanTypes
import datadog.trace.context.TraceScope
import groovy.transform.stc.ClosureParams
import groovy.transform.stc.SimpleType
import io.opentracing.tag.Tags
import io.opentracing.util.GlobalTracer
import okhttp3.HttpUrl
import okhttp3.OkHttpClient
import okhttp3.Request
@ -89,6 +91,10 @@ abstract class HttpServerTest<SERVER, DECORATOR extends HttpServerDecorator> ext
false
}
boolean redirectHasBody() {
false
}
boolean testNotFound() {
true
}
@ -143,6 +149,7 @@ abstract class HttpServerTest<SERVER, DECORATOR extends HttpServerDecorator> ext
}
static <T> T controller(ServerEndpoint endpoint, Closure<T> closure) {
assert ((TraceScope) GlobalTracer.get().scopeManager().active()).asyncPropagating
if (endpoint == NOT_FOUND) {
return closure()
}
@ -230,7 +237,7 @@ abstract class HttpServerTest<SERVER, DECORATOR extends HttpServerDecorator> ext
response.code() == REDIRECT.status
response.header("location") == REDIRECT.body ||
response.header("location") == "${address.resolve(REDIRECT.body)}"
response.body().contentLength() < 1
response.body().contentLength() < 1 || redirectHasBody()
and:
cleanAndAssertTraces(1) {
@ -376,6 +383,7 @@ abstract class HttpServerTest<SERVER, DECORATOR extends HttpServerDecorator> ext
}
}
}
if (reorderControllerSpan() || reorderHandlerSpan()) {
// Some frameworks close the handler span before the controller returns, so we need to manually reorder it.
TEST_WRITER.each {