open-telemetry
/
opentelemetry-java-instrumentation
mirror of https://github.com/open-telemetry/opentelemetry-java-instrumentation.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Instrument all jetty handlers (#2320)

This commit is contained in:
Lauri Tulmin 2021-02-18 11:51:48 +02:00 committed by GitHub
parent f127a972dc
commit 80f389c778
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 43 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
instrumentation/jetty-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/JettyHandlerAdvice.java
View File

@ -55,8 +55,15 @@ public class JettyHandlerAdvice {
tracer().setPrincipal(context, request);
if (throwable != null) {
tracer().endExceptionally(context, throwable, response);
// throwable is read-only, copy it to a new local that can be modified
Throwable exception = throwable;
if (exception == null) {
// on jetty versions before 9.4 exceptions from servlet don't propagate to this method
// check from request whether a throwable has been stored there
exception = (Throwable) request.getAttribute("javax.servlet.error.exception");
}
if (exception != null) {
tracer().endExceptionally(context, exception, response);
return;
}

15
instrumentation/jetty-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/JettyInstrumentationModule.java
View File

@ -11,7 +11,6 @@ import static java.util.Arrays.asList;
import static java.util.Collections.singletonMap;
import static net.bytebuddy.matcher.ElementMatchers.isPublic;
import static net.bytebuddy.matcher.ElementMatchers.named;
import static net.bytebuddy.matcher.ElementMatchers.not;
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
@ -57,25 +56,13 @@ public class JettyInstrumentationModule extends InstrumentationModule {
@Override
public ElementMatcher<TypeDescription> typeMatcher() {
// skipping built-in handlers, so that for servlets there will be no span started by jetty.
// this is so that the servlet instrumentation will capture contextPath and servletPath
// normally, which the jetty instrumentation does not capture since jetty doesn't populate
// contextPath and servletPath until right before calling the servlet
// (another option is to instrument ServletHolder.handle() to capture those fields)
return not(named("org.eclipse.jetty.server.handler.HandlerWrapper"))
.and(not(named("org.eclipse.jetty.server.handler.ScopedHandler")))
.and(not(named("org.eclipse.jetty.server.handler.ContextHandler")))
.and(not(named("org.eclipse.jetty.servlet.ServletHandler")))
.and(implementsInterface(named("org.eclipse.jetty.server.Handler")));
return implementsInterface(named("org.eclipse.jetty.server.Handler"));
}
@Override
public Map<? extends ElementMatcher<? super MethodDescription>, String> transformers() {
return singletonMap(
named("handle")
// need to capture doHandle() for handlers that extend built-in handlers excluded
// above
.or(named("doHandle"))
.and(takesArgument(0, named("java.lang.String")))
.and(takesArgument(1, named("org.eclipse.jetty.server.Request")))
.and(takesArgument(2, named("javax.servlet.http.HttpServletRequest")))

2
instrumentation/jetty-8.0/javaagent/src/test/groovy/JettyHandlerTest.groovy
View File

@ -127,6 +127,6 @@ class JettyHandlerTest extends HttpServerTest<Server> implements AgentTestTrait
@Override
String expectedServerSpanName(ServerEndpoint endpoint) {
"TestHandler.handle"
"HandlerWrapper.handle"
}
}

14
instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/JettyServlet3Test.groovy
View File

@ -10,6 +10,7 @@ import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEn
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.REDIRECT
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.SUCCESS
import io.opentelemetry.instrumentation.test.asserts.TraceAssert
import javax.servlet.Servlet
import javax.servlet.ServletException
import javax.servlet.http.HttpServletRequest
@ -29,6 +30,19 @@ abstract class JettyServlet3Test extends AbstractServlet3Test<Server, ServletCon
ServletException
}
@Override
boolean hasResponseSpan(ServerEndpoint endpoint) {
return endpoint == EXCEPTION || super.hasResponseSpan(endpoint)
}
@Override
void responseSpan(TraceAssert trace, int index, Object controllerSpan, Object handlerSpan, String method, ServerEndpoint endpoint) {
if (endpoint == EXCEPTION) {
sendErrorSpan(trace, index, handlerSpan)
}
super.responseSpan(trace, index, controllerSpan, handlerSpan, method, endpoint)
}
@Override
Server startServer(int port) {
def jettyServer = new Server(port)

16
instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/JettyServletHandlerTest.groovy
View File

@ -3,6 +3,9 @@
* SPDX-License-Identifier: Apache-2.0
*/
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
import io.opentelemetry.instrumentation.test.asserts.TraceAssert
import javax.servlet.Servlet
import javax.servlet.ServletException
import javax.servlet.http.HttpServletRequest
@ -12,6 +15,19 @@ import org.eclipse.jetty.servlet.ServletHandler
class JettyServletHandlerTest extends AbstractServlet3Test<Server, ServletHandler> {
@Override
boolean hasResponseSpan(ServerEndpoint endpoint) {
return endpoint == EXCEPTION || super.hasResponseSpan(endpoint)
}
@Override
void responseSpan(TraceAssert trace, int index, Object controllerSpan, Object handlerSpan, String method, ServerEndpoint endpoint) {
if (endpoint == EXCEPTION) {
sendErrorSpan(trace, index, handlerSpan)
}
super.responseSpan(trace, index, controllerSpan, handlerSpan, method, endpoint)
}
@Override
Server startServer(int port) {
Server server = new Server(port)

7
instrumentation/struts-2.3/javaagent/src/test/groovy/Struts2ActionSpanTest.groovy
View File

@ -17,7 +17,6 @@ import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
import javax.servlet.DispatcherType
import org.apache.struts2.dispatcher.ng.filter.StrutsPrepareAndExecuteFilter
import org.eclipse.jetty.server.Server
import org.eclipse.jetty.server.handler.HandlerCollection
import org.eclipse.jetty.servlet.DefaultServlet
import org.eclipse.jetty.servlet.ServletContextHandler
import org.eclipse.jetty.util.resource.FileResource
@ -101,11 +100,7 @@ class Struts2ActionSpanTest extends HttpServerTest<Server> implements AgentTestT
context.setContextPath(getContextPath())
def resource = new FileResource(getClass().getResource("/"))
context.setBaseResource(resource)
// jetty integration is disabled for some handler classes, using HandlerCollection here
// enables jetty integration
HandlerCollection handlerCollection = new HandlerCollection()
handlerCollection.addHandler(context)
server.setHandler(handlerCollection)
server.setHandler(context)
context.addServlet(DefaultServlet, "/")
context.addFilter(StrutsPrepareAndExecuteFilter, "/*", EnumSet.of(DispatcherType.REQUEST))

2
smoke-tests/src/test/groovy/io/opentelemetry/smoketest/JettySmokeTest.groovy
View File

@ -16,7 +16,7 @@ class JettySmokeTest extends AppServerTest {
}
def getJettySpanName() {
return serverVersion.startsWith("10.") ? "HandlerList.handle" : "HandlerCollection.handle"
"HandlerWrapper.handle"
}
@Override