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

move ServletContextPath context creation to servlet-common, make servlet2&3 depend on servlet-common so that it would be used in tests that depend on servlet3

This commit is contained in:
Lauri Tulmin 2021-01-29 19:04:49 +02:00
parent a6a13d1c27
commit 8b4065c250
25 changed files with 565 additions and 126 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/AppServerBridge.java
View File

@ -8,7 +8,6 @@ package io.opentelemetry.instrumentation.api.servlet;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.ContextKey;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
/**
* Helper container for Context attributes for transferring certain information between servlet
@ -40,15 +39,7 @@ public class AppServerBridge {
* @return new context with AppServerBridge attached.
*/
public static Context init(Context ctx, boolean shouldRecordException) {
Context context =
ctx.with(AppServerBridge.CONTEXT_KEY, new AppServerBridge(shouldRecordException));
// Add context for storing servlet context path. Servlet context path is updated from servlet
// integration.
if (context.get(ServletContextPath.CONTEXT_KEY) == null) {
context = context.with(ServletContextPath.CONTEXT_KEY, new AtomicReference<>(null));
}
return context;
return ctx.with(AppServerBridge.CONTEXT_KEY, new AppServerBridge(shouldRecordException));
}
private final AtomicBoolean servletUpdatedServerSpanName = new AtomicBoolean(false);

6
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/ServletContextPath.java
View File

@ -7,7 +7,6 @@ package io.opentelemetry.instrumentation.api.servlet;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.ContextKey;
import java.util.concurrent.atomic.AtomicReference;
/**
* The context key here is used to propagate the servlet context path throughout the request, so
@ -23,12 +22,11 @@ public class ServletContextPath {
// Keeps track of the servlet context path that needs to be prepended to the route when updating
// the span name
public static final ContextKey<AtomicReference<String>> CONTEXT_KEY =
public static final ContextKey<String> CONTEXT_KEY =
ContextKey.named("opentelemetry-servlet-context-path-key");
public static String prepend(Context context, String spanName) {
AtomicReference<String> valueReference = context.get(CONTEXT_KEY);
String value = valueReference != null ? valueReference.get() : null;
String value = context.get(CONTEXT_KEY);
// checking isEmpty just to avoid unnecessary string concat / allocation
if (value != null && !value.isEmpty()) {
return value + spanName;

37
instrumentation-core/servlet-2.2/src/main/java/io/opentelemetry/instrumentation/servlet/ServletHttpServerTracer.java
View File

@ -9,13 +9,11 @@ import io.opentelemetry.api.trace.Span;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.propagation.TextMapPropagator.Getter;
import io.opentelemetry.instrumentation.api.servlet.AppServerBridge;
import io.opentelemetry.instrumentation.api.servlet.ServletContextPath;
import io.opentelemetry.instrumentation.api.tracer.HttpServerTracer;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import java.net.URI;
import java.net.URISyntaxException;
import java.security.Principal;
import java.util.concurrent.atomic.AtomicReference;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import org.slf4j.Logger;
@ -27,16 +25,7 @@ public abstract class ServletHttpServerTracer<RESPONSE>
private static final Logger log = LoggerFactory.getLogger(ServletHttpServerTracer.class);
public Context startSpan(HttpServletRequest request) {
Context context = startSpan(request, request, request, getSpanName(request));
return addContextPathContext(context, request);
}
protected Context addContextPathContext(Context context, HttpServletRequest request) {
String contextPath = request.getContextPath();
if (contextPath != null && !contextPath.isEmpty() && !contextPath.equals("/")) {
context = context.with(ServletContextPath.CONTEXT_KEY, new AtomicReference<>(contextPath));
}
return context;
return startSpan(request, request, request, getSpanName(request));
}
@Override
@ -158,41 +147,19 @@ public abstract class ServletHttpServerTracer<RESPONSE>
return contextPath + servletPath;
}
/**
* When server spans are managed by app server instrumentation servlet instrumentation should set
* information that was not available from app server instrumentation.
*/
public void updateServerSpan(Context attachedContext, HttpServletRequest request) {
updateServerSpanName(attachedContext, request);
updateContextPath(attachedContext, request);
}
/**
* When server spans are managed by app server instrumentation, servlet must update server span
* name only once and only during the first pass through the servlet stack. There are potential
* forward and other scenarios, where servlet path may change, but we don't want this to be
* reflected in the span name.
*/
private void updateServerSpanName(Context attachedContext, HttpServletRequest request) {
// Update name only when server span wasn't created by servlet integration
// and has not been already updated
public void updateServerSpanNameOnce(Context attachedContext, HttpServletRequest request) {
if (AppServerBridge.shouldUpdateServerSpanName(attachedContext)) {
updateSpanName(Span.fromContext(attachedContext), request);
AppServerBridge.setServletUpdatedServerSpanName(attachedContext, true);
}
}
private void updateContextPath(Context attachedContext, HttpServletRequest request) {
// Update context path if it isn't already set
AtomicReference<String> reference = attachedContext.get(ServletContextPath.CONTEXT_KEY);
if (reference != null && reference.get() == null) {
String contextPath = request.getContextPath();
if (contextPath != null && !contextPath.isEmpty() && !contextPath.equals("/")) {
reference.compareAndSet(null, contextPath);
}
}
}
public void updateSpanName(HttpServletRequest request) {
updateSpanName(getServerSpan(request), request);
}

5
instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/test/groovy/JerseyHttpServerTest.groovy
View File

@ -30,4 +30,9 @@ class JerseyHttpServerTest extends JaxRsHttpServerTest<Server> {
void stopServer(Server httpServer) {
httpServer.stop()
}
@Override
boolean asyncCancelHasSendError() {
true
}
}

14
instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsHttpServerTest.groovy
View File

@ -49,10 +49,18 @@ abstract class JaxRsHttpServerTest<S> extends HttpServerTest<S> {
assert response.code() == statusCode
assert bodyPredicate(response.body().string())
def spanCount = 2
def hasSendError = asyncCancelHasSendError() && action == "cancel"
if (hasSendError) {
spanCount++
}
assertTraces(1) {
trace(0, 2) {
trace(0, spanCount) {
asyncServerSpan(it, 0, url, statusCode)
handlerSpan(it, 1, span(0), "asyncOp", isCancelled, isError, errorMessage)
if (hasSendError) {
sendErrorSpan(it, 2, span(1))
}
}
}
@ -117,6 +125,10 @@ abstract class JaxRsHttpServerTest<S> extends HttpServerTest<S> {
true
}
boolean asyncCancelHasSendError() {
false
}
private static boolean shouldTestCompletableStageAsync() {
Boolean.getBoolean("testLatestDeps")
}

18
instrumentation/jetty-8.0/javaagent/src/test/groovy/JettyHandlerTest.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 io.opentelemetry.instrumentation.test.base.HttpServerTest
import javax.servlet.DispatcherType
import javax.servlet.ServletException
@ -62,6 +63,23 @@ class JettyHandlerTest extends HttpServerTest<Server> {
false
}
@Override
boolean hasResponseSpan(ServerEndpoint endpoint) {
endpoint == REDIRECT || endpoint == ERROR
}
@Override
void responseSpan(TraceAssert trace, int index, Object parent, String method, ServerEndpoint endpoint) {
switch (endpoint) {
case REDIRECT:
redirectSpan(trace, index, parent)
break
case ERROR:
sendErrorSpan(trace, index, parent)
break
}
}
static void handleRequest(Request request, HttpServletResponse response) {
ServerEndpoint endpoint = ServerEndpoint.forPath(request.requestURI)
controller(endpoint) {

29
instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationBasicTests.groovy
View File

@ -332,7 +332,7 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
then:
assertTraces(1) {
trace(0, 3) {
trace(0, 4) {
span(0) {
hasNoParent()
name "/$jspWebappContext/includes/includeHtml.jsp"
@ -366,6 +366,11 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
"jsp.requestURL" reqUrl
}
}
span(3) {
childOf span(2)
name "ApplicationDispatcher.include"
errored false
}
}
}
res.code() == 200
@ -384,7 +389,7 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
then:
assertTraces(1) {
trace(0, 7) {
trace(0, 9) {
span(0) {
hasNoParent()
name "/$jspWebappContext/includes/includeMulti.jsp"
@ -420,6 +425,11 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
}
span(3) {
childOf span(2)
name "ApplicationDispatcher.include"
errored false
}
span(4) {
childOf span(3)
name "Compile /common/javaLoopH2.jsp"
errored false
attributes {
@ -427,16 +437,21 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
"jsp.compiler" "org.apache.jasper.compiler.JDTCompiler"
}
}
span(4) {
childOf span(2)
span(5) {
childOf span(3)
name "Render /common/javaLoopH2.jsp"
errored false
attributes {
"jsp.requestURL" reqUrl
}
}
span(5) {
span(6) {
childOf span(2)
name "ApplicationDispatcher.include"
errored false
}
span(7) {
childOf span(6)
name "Compile /common/javaLoopH2.jsp"
errored false
attributes {
@ -444,8 +459,8 @@ class JspInstrumentationBasicTests extends AgentInstrumentationSpecification {
"jsp.compiler" "org.apache.jasper.compiler.JDTCompiler"
}
}
span(6) {
childOf span(2)
span(8) {
childOf span(6)
name "Render /common/javaLoopH2.jsp"
errored false
attributes {

115
instrumentation/jsp-2.3/javaagent/src/test/groovy/JspInstrumentationForwardTests.groovy
View File

@ -80,7 +80,7 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
then:
assertTraces(1) {
trace(0, 5) {
trace(0, 6) {
span(0) {
hasNoParent()
name "/$jspWebappContext/$forwardFromFileName"
@ -116,6 +116,11 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
}
span(3) {
childOf span(2)
name "ApplicationDispatcher.forward"
errored false
}
span(4) {
childOf span(3)
name "Compile /$forwardDestFileName"
errored false
attributes {
@ -123,8 +128,8 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
"jsp.compiler" "org.apache.jasper.compiler.JDTCompiler"
}
}
span(4) {
childOf span(2)
span(5) {
childOf span(3)
name "Render /$forwardDestFileName"
errored false
attributes {
@ -155,7 +160,7 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
then:
assertTraces(1) {
trace(0, 3) {
trace(0, 4) {
span(0) {
hasNoParent()
name "/$jspWebappContext/forwards/forwardToHtml.jsp"
@ -189,6 +194,11 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
"jsp.requestURL" reqUrl
}
}
span(3) {
childOf span(2)
name "ApplicationDispatcher.forward"
errored false
}
}
}
res.code() == 200
@ -207,7 +217,7 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
then:
assertTraces(1) {
trace(0, 9) {
trace(0, 12) {
span(0) {
hasNoParent()
name "/$jspWebappContext/forwards/forwardToIncludeMulti.jsp"
@ -243,6 +253,11 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
}
span(3) {
childOf span(2)
name "ApplicationDispatcher.forward"
errored false
}
span(4) {
childOf span(3)
name "Compile /includes/includeMulti.jsp"
errored false
attributes {
@ -250,8 +265,8 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
"jsp.compiler" "org.apache.jasper.compiler.JDTCompiler"
}
}
span(4) {
childOf span(2)
span(5) {
childOf span(3)
name "Render /includes/includeMulti.jsp"
errored false
attributes {
@ -259,26 +274,13 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
"jsp.requestURL" baseUrl + "/includes/includeMulti.jsp"
}
}
span(5) {
childOf span(4)
name "Compile /common/javaLoopH2.jsp"
errored false
attributes {
"jsp.classFQCN" "org.apache.jsp.common.javaLoopH2_jsp"
"jsp.compiler" "org.apache.jasper.compiler.JDTCompiler"
}
}
span(6) {
childOf span(4)
name "Render /common/javaLoopH2.jsp"
childOf span(5)
name "ApplicationDispatcher.include"
errored false
attributes {
"jsp.forwardOrigin" "/forwards/forwardToIncludeMulti.jsp"
"jsp.requestURL" baseUrl + "/includes/includeMulti.jsp"
}
}
span(7) {
childOf span(4)
childOf span(6)
name "Compile /common/javaLoopH2.jsp"
errored false
attributes {
@ -287,7 +289,30 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
}
}
span(8) {
childOf span(4)
childOf span(6)
name "Render /common/javaLoopH2.jsp"
errored false
attributes {
"jsp.forwardOrigin" "/forwards/forwardToIncludeMulti.jsp"
"jsp.requestURL" baseUrl + "/includes/includeMulti.jsp"
}
}
span(9) {
childOf span(5)
name "ApplicationDispatcher.include"
errored false
}
span(10) {
childOf span(9)
name "Compile /common/javaLoopH2.jsp"
errored false
attributes {
"jsp.classFQCN" "org.apache.jsp.common.javaLoopH2_jsp"
"jsp.compiler" "org.apache.jasper.compiler.JDTCompiler"
}
}
span(11) {
childOf span(9)
name "Render /common/javaLoopH2.jsp"
errored false
attributes {
@ -313,7 +338,7 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
then:
assertTraces(1) {
trace(0, 7) {
trace(0, 9) {
span(0) {
hasNoParent()
name "/$jspWebappContext/forwards/forwardToJspForward.jsp"
@ -349,6 +374,11 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
}
span(3) {
childOf span(2)
name "ApplicationDispatcher.forward"
errored false
}
span(4) {
childOf span(3)
name "Compile /forwards/forwardToSimpleJava.jsp"
errored false
attributes {
@ -356,8 +386,8 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
"jsp.compiler" "org.apache.jasper.compiler.JDTCompiler"
}
}
span(4) {
childOf span(2)
span(5) {
childOf span(3)
name "Render /forwards/forwardToSimpleJava.jsp"
errored false
attributes {
@ -365,8 +395,13 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
"jsp.requestURL" baseUrl + "/forwards/forwardToSimpleJava.jsp"
}
}
span(5) {
childOf span(4)
span(6) {
childOf span(5)
name "ApplicationDispatcher.forward"
errored false
}
span(7) {
childOf span(6)
name "Compile /common/loop.jsp"
errored false
attributes {
@ -374,8 +409,8 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
"jsp.compiler" "org.apache.jasper.compiler.JDTCompiler"
}
}
span(6) {
childOf span(4)
span(8) {
childOf span(6)
name "Render /common/loop.jsp"
errored false
attributes {
@ -401,7 +436,7 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
then:
assertTraces(1) {
trace(0, 4) {
trace(0, 5) {
span(0) {
hasNoParent()
name "/$jspWebappContext/forwards/forwardToCompileError.jsp"
@ -439,6 +474,12 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
}
span(3) {
childOf span(2)
name "ApplicationDispatcher.forward"
errored true
errorEvent(JasperException, String)
}
span(4) {
childOf span(3)
name "Compile /compileError.jsp"
errored true
errorEvent(JasperException, String)
@ -465,7 +506,7 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
then:
assertTraces(1) {
trace(0, 3) {
trace(0, 5) {
span(0) {
hasNoParent()
name "/$jspWebappContext/forwards/forwardToNonExistent.jsp"
@ -499,6 +540,14 @@ class JspInstrumentationForwardTests extends AgentInstrumentationSpecification {
"jsp.requestURL" reqUrl
}
}
span(3) {
childOf span(2)
name "ApplicationDispatcher.forward"
}
span(4) {
childOf span(3)
name "ResponseFacade.sendError"
}
}
}
res.code() == 404

3
instrumentation/liberty/liberty/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/liberty/LibertyHttpServerTracer.java
View File

@ -21,8 +21,7 @@ public class LibertyHttpServerTracer extends Servlet3HttpServerTracer {
// using request method as span name as server isn't ready for calling request.getServletPath()
// span name will be updated a bit later when calling request.getServletPath() works
// see LibertyUpdateSpanAdvice
Context context = startSpan(request, request, request, "HTTP " + request.getMethod());
return addContextPathContext(context, request);
return startSpan(request, request, request, "HTTP " + request.getMethod());
}
@Override

23
instrumentation/servlet/glassfish-testing/src/test/groovy/GlassFishServerTest.groovy
View File

@ -3,6 +3,11 @@
* SPDX-License-Identifier: Apache-2.0
*/
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.ERROR
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.REDIRECT
import io.opentelemetry.instrumentation.test.asserts.TraceAssert
import io.opentelemetry.instrumentation.test.base.HttpServerTest
import org.glassfish.embeddable.BootstrapProperties
import org.glassfish.embeddable.Deployer
@ -63,4 +68,22 @@ class GlassFishServerTest extends HttpServerTest<GlassFish> {
boolean redirectHasBody() {
true
}
@Override
boolean hasResponseSpan(ServerEndpoint endpoint) {
endpoint == REDIRECT || endpoint == ERROR || endpoint == NOT_FOUND
}
@Override
void responseSpan(TraceAssert trace, int index, Object parent, String method, ServerEndpoint endpoint) {
switch (endpoint) {
case REDIRECT:
redirectSpan(trace, index, parent)
break
case ERROR:
case NOT_FOUND:
sendErrorSpan(trace, index, parent)
break
}
}
}

1
instrumentation/servlet/servlet-2.2/javaagent/servlet-2.2-javaagent.gradle
View File

@ -18,6 +18,7 @@ muzzle {
dependencies {
compileOnly group: 'javax.servlet', name: 'servlet-api', version: '2.2'
api(project(':instrumentation-core:servlet-2.2'))
api(project(':instrumentation:servlet:servlet-common:javaagent'))
testImplementation(project(':testing-common')) {
exclude group: 'org.eclipse.jetty', module: 'jetty-server'

2
instrumentation/servlet/servlet-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v2_2/Servlet2Advice.java
View File

@ -37,7 +37,7 @@ public class Servlet2Advice {
Context serverContext = tracer().getServerContext(httpServletRequest);
if (serverContext != null) {
tracer().updateServerSpan(serverContext, httpServletRequest);
tracer().updateServerSpanNameOnce(serverContext, httpServletRequest);
return;
}

7
instrumentation/servlet/servlet-2.2/javaagent/src/test/groovy/JettyServlet2Test.groovy
View File

@ -70,10 +70,15 @@ class JettyServlet2Test extends HttpServerTest<Server> {
false
}
@Override
boolean hasResponseSpan(ServerEndpoint endpoint) {
endpoint == REDIRECT || endpoint == ERROR
}
@Override
void responseSpan(TraceAssert trace, int index, Object parent, String method = "GET", ServerEndpoint endpoint = SUCCESS) {
trace.span(index) {
name endpoint == REDIRECT ? "HttpServletResponse.sendRedirect" : "HttpServletResponse.sendError"
name endpoint == REDIRECT ? "Response.sendRedirect" : "Response.sendError"
kind INTERNAL
errored false
childOf((SpanData) parent)

1
instrumentation/servlet/servlet-3.0/javaagent/servlet-3.0-javaagent.gradle
View File

@ -17,6 +17,7 @@ muzzle {
dependencies {
compileOnly group: 'javax.servlet', name: 'javax.servlet-api', version: '3.0.1'
api(project(':instrumentation-core:servlet-2.2'))
api(project(':instrumentation:servlet:servlet-common:javaagent'))
testInstrumentation project(':instrumentation:jetty-8.0:javaagent')

4
instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Advice.java
View File

@ -40,14 +40,14 @@ public class Servlet3Advice {
scope = attachedContext.makeCurrent();
}
tracer().updateServerSpan(attachedContext, httpServletRequest);
tracer().updateServerSpanNameOnce(attachedContext, httpServletRequest);
// We are inside nested servlet/filter/app-server span, don't create new span
return;
}
Context parentContext = Java8BytecodeBridge.currentContext();
if (parentContext != null && Java8BytecodeBridge.spanFromContext(parentContext).isRecording()) {
tracer().updateServerSpan(parentContext, httpServletRequest);
tracer().updateServerSpanNameOnce(parentContext, httpServletRequest);
// We are inside nested servlet/filter/app-server span, don't create new span
return;
}

22
instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/AbstractServlet3Test.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 io.opentelemetry.instrumentation.test.base.HttpServerTest
import javax.servlet.Servlet
import okhttp3.Request
@ -49,4 +50,25 @@ abstract class AbstractServlet3Test<SERVER, CONTEXT> extends HttpServerTest<SERV
lastRequest = uri
super.request(uri, method, body)
}
boolean errorEndpointUsesSendError() {
true
}
@Override
boolean hasResponseSpan(ServerEndpoint endpoint) {
endpoint == REDIRECT || (endpoint == ERROR && errorEndpointUsesSendError())
}
@Override
void responseSpan(TraceAssert trace, int index, Object parent, String method, ServerEndpoint endpoint) {
switch (endpoint) {
case REDIRECT:
redirectSpan(trace, index, parent)
break
case ERROR:
sendErrorSpan(trace, index, parent)
break
}
}
}

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

@ -108,6 +108,11 @@ class JettyServlet3TestAsync extends JettyServlet3Test {
TestServlet3.Async
}
@Override
boolean errorEndpointUsesSendError() {
false
}
@Override
boolean testException() {
// https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/807
@ -135,6 +140,11 @@ class JettyServlet3TestForward extends JettyDispatchTest {
TestServlet3.Sync // dispatch to sync servlet
}
@Override
boolean hasForwardSpan() {
true
}
@Override
protected void setupServlets(ServletContextHandler context) {
super.setupServlets(context)
@ -164,6 +174,11 @@ class JettyServlet3TestInclude extends JettyDispatchTest {
false
}
@Override
boolean hasIncludeSpan() {
true
}
@Override
protected void setupServlets(ServletContextHandler context) {
super.setupServlets(context)
@ -223,6 +238,11 @@ class JettyServlet3TestDispatchAsync extends JettyDispatchTest {
addServlet(context, "/dispatch/recursive", TestServlet3.DispatchRecursive)
}
@Override
boolean errorEndpointUsesSendError() {
false
}
@Override
boolean testException() {
// https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/807

78
instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/TomcatServlet3Test.groovy
View File

@ -6,11 +6,13 @@
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.AUTH_REQUIRED
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.ERROR
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.REDIRECT
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.SUCCESS
import static org.junit.Assume.assumeTrue
import io.opentelemetry.instrumentation.test.asserts.TraceAssert
import java.nio.file.Files
import javax.servlet.Servlet
import javax.servlet.ServletException
@ -40,6 +42,21 @@ abstract class TomcatServlet3Test extends AbstractServlet3Test<Tomcat, Context>
ServletException
}
@Override
boolean hasResponseSpan(ServerEndpoint endpoint) {
endpoint == NOT_FOUND || super.hasResponseSpan(endpoint)
}
@Override
void responseSpan(TraceAssert trace, int index, Object parent, String method, ServerEndpoint endpoint) {
switch (endpoint) {
case NOT_FOUND:
sendErrorSpan(trace, index, parent)
break
}
super.responseSpan(trace, index, parent, method, endpoint)
}
@Shared
def accessLogValue = new TestAccessLogValve()
@ -122,10 +139,26 @@ abstract class TomcatServlet3Test extends AbstractServlet3Test<Tomcat, Context>
def loggedTraces = accessLogValue.loggedIds*.first
def loggedSpans = accessLogValue.loggedIds*.second
def expectedCount = 2
if (hasIncludeSpan()) {
expectedCount++
}
if (hasForwardSpan()) {
expectedCount++
}
(0..count - 1).each {
trace(it, 2) {
trace(it, expectedCount) {
serverSpan(it, 0, null, null, "GET", SUCCESS.body.length())
controllerSpan(it, 1, span(0))
def controllerIndex = 1
if (hasIncludeSpan()) {
includeSpan(it, 1, span(0))
controllerIndex++
}
if (hasForwardSpan()) {
forwardSpan(it, 1, span(0))
controllerIndex++
}
controllerSpan(it, controllerIndex, span(controllerIndex - 1))
}
assert loggedTraces.contains(traces[it][0].traceId)
@ -150,10 +183,27 @@ abstract class TomcatServlet3Test extends AbstractServlet3Test<Tomcat, Context>
response.body().string() == ERROR.body
and:
def spanCount = 2
if (errorEndpointUsesSendError()) {
spanCount++
}
if (hasForwardSpan()) {
spanCount++
}
assertTraces(1) {
trace(0, 2) {
trace(0, spanCount) {
serverSpan(it, 0, null, null, method, response.body().contentLength(), ERROR)
controllerSpan(it, 1, span(0))
def spanIndex = 1
if (hasForwardSpan()) {
forwardSpan(it, spanIndex, span(spanIndex - 1))
spanIndex++
}
controllerSpan(it, spanIndex, span(spanIndex - 1))
spanIndex++
if (errorEndpointUsesSendError()) {
sendErrorSpan(it, spanIndex, span(spanIndex - 1))
spanIndex++
}
}
def (String traceId, String spanId) = accessLogValue.loggedIds[0]
@ -256,6 +306,11 @@ class TomcatServlet3TestAsync extends TomcatServlet3Test {
TestServlet3.Async
}
@Override
boolean errorEndpointUsesSendError() {
false
}
@Override
boolean testException() {
// https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/807
@ -288,6 +343,11 @@ class TomcatServlet3TestForward extends TomcatDispatchTest {
false
}
@Override
boolean hasForwardSpan() {
true
}
@Override
protected void setupServlets(Context context) {
super.setupServlets(context)
@ -322,6 +382,11 @@ class TomcatServlet3TestInclude extends TomcatDispatchTest {
false
}
@Override
boolean hasIncludeSpan() {
true
}
@Override
protected void setupServlets(Context context) {
super.setupServlets(context)
@ -379,6 +444,11 @@ class TomcatServlet3TestDispatchAsync extends TomcatDispatchTest {
addServlet(context, "/dispatch/recursive", TestServlet3.DispatchRecursive)
}
@Override
boolean errorEndpointUsesSendError() {
false
}
@Override
boolean testException() {
// https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/807

100
instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/contextpath/ServletContextPathInstrumentationModule.java Normal file
View File

@ -0,0 +1,100 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.javaagent.instrumentation.servlet.contextpath;
import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.AgentElementMatchers.safeHasSuperType;
import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.ClassLoaderMatcher.hasClassesNamed;
import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.NameMatchers.namedOneOf;
import static java.util.Collections.singletonList;
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.takesArgument;
import com.google.auto.service.AutoService;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.instrumentation.api.servlet.ServletContextPath;
import io.opentelemetry.javaagent.instrumentation.api.CallDepthThreadLocalMap;
import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge;
import io.opentelemetry.javaagent.tooling.InstrumentationModule;
import io.opentelemetry.javaagent.tooling.TypeInstrumentation;
import java.util.List;
import java.util.Map;
import javax.servlet.ServletRequest;
import javax.servlet.http.HttpServletRequest;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.method.MethodDescription;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.matcher.ElementMatcher;
@AutoService(InstrumentationModule.class)
public class ServletContextPathInstrumentationModule extends InstrumentationModule {
public ServletContextPathInstrumentationModule() {
super("servlet", "servlet-context-path");
}
@Override
public List<TypeInstrumentation> typeInstrumentations() {
return singletonList(new ServletContextPathInstrumentation());
}
public static class ServletContextPathInstrumentation implements TypeInstrumentation {
@Override
public ElementMatcher<ClassLoader> classLoaderOptimization() {
return hasClassesNamed("javax.servlet.Filter");
}
@Override
public ElementMatcher<TypeDescription> typeMatcher() {
return safeHasSuperType(namedOneOf("javax.servlet.Filter", "javax.servlet.Servlet"));
}
@Override
public Map<? extends ElementMatcher<? super MethodDescription>, String> transformers() {
return singletonMap(
namedOneOf("doFilter", "service")
.and(takesArgument(0, named("javax.servlet.ServletRequest")))
.and(takesArgument(1, named("javax.servlet.ServletResponse")))
.and(isPublic()),
ServletContextPathAdvice.class.getName());
}
}
public static class ServletContextPathAdvice {
@Advice.OnMethodEnter(suppress = Throwable.class)
public static void onEnter(
@Advice.Argument(0) ServletRequest request, @Advice.Local("otelScope") Scope scope) {
if (!(request instanceof HttpServletRequest)) {
return;
}
int callDepth = CallDepthThreadLocalMap.incrementCallDepth(ServletContextPath.class);
if (callDepth > 0) {
return;
}
HttpServletRequest httpServletRequest = (HttpServletRequest) request;
String contextPath = httpServletRequest.getContextPath();
if (contextPath != null && !contextPath.isEmpty() && !contextPath.equals("/")) {
Context context =
Java8BytecodeBridge.currentContext().with(ServletContextPath.CONTEXT_KEY, contextPath);
scope = context.makeCurrent();
}
}
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void stop(@Advice.Local("otelScope") Scope scope) {
if (scope != null) {
CallDepthThreadLocalMap.reset(ServletContextPath.class);
scope.close();
}
}
}
}

2
instrumentation/spring/spring-webmvc-3.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/HandlerAdapterInstrumentation.java
View File

@ -64,11 +64,11 @@ public class HandlerAdapterInstrumentation implements TypeInstrumentation {
if (serverSpan != null) {
// Name the parent span based on the matching pattern
tracer().onRequest(context, serverSpan, request);
}
// Now create a span for handler/controller execution.
span = tracer().startHandlerSpan(handler);
scope = context.with(span).makeCurrent();
}
}
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void stopSpan(

16
instrumentation/spring/spring-webmvc-3.1/javaagent/src/test/groovy/test/boot/SpringBootBasedTest.groovy
View File

@ -58,6 +58,11 @@ class SpringBootBasedTest extends HttpServerTest<ConfigurableApplicationContext>
endpoint == REDIRECT
}
@Override
boolean hasResponseSpan(ServerEndpoint endpoint) {
endpoint == REDIRECT
}
@Override
boolean testNotFound() {
// FIXME: the instrumentation adds an extra controller span which is not consistent.
@ -84,9 +89,11 @@ class SpringBootBasedTest extends HttpServerTest<ConfigurableApplicationContext>
and:
assertTraces(1) {
trace(0, 2) {
trace(0, 4) {
serverSpan(it, 0, null, null, "GET", null, AUTH_ERROR)
errorPageSpans(it, 1, null)
sendErrorSpan(it, 1, span(0))
forwardSpan(it, 2, span(0))
errorPageSpans(it, 3, null)
}
}
}
@ -111,8 +118,9 @@ class SpringBootBasedTest extends HttpServerTest<ConfigurableApplicationContext>
and:
assertTraces(1) {
trace(0, 1) {
trace(0, 2) {
serverSpan(it, 0, null, null, "POST", response.body()?.contentLength(), LOGIN)
redirectSpan(it, 1, span(0))
}
}
@ -134,7 +142,7 @@ class SpringBootBasedTest extends HttpServerTest<ConfigurableApplicationContext>
@Override
void responseSpan(TraceAssert trace, int index, Object parent, String method = "GET", ServerEndpoint endpoint = SUCCESS) {
trace.span(index) {
name "HttpServletResponse.sendRedirect"
name "OnCommittedResponseWrapper.sendRedirect"
kind INTERNAL
errored false
attributes {

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

@ -3,8 +3,10 @@
* SPDX-License-Identifier: Apache-2.0
*/
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.ERROR
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.PATH_PARAM
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.REDIRECT
import io.opentelemetry.api.trace.SpanKind
import io.opentelemetry.instrumentation.test.asserts.TraceAssert
@ -46,6 +48,24 @@ class Struts2ActionSpanTest extends HttpServerTest<Server> {
return true
}
@Override
boolean hasResponseSpan(ServerEndpoint endpoint) {
endpoint == REDIRECT || endpoint == ERROR || endpoint == EXCEPTION
}
@Override
void responseSpan(TraceAssert trace, int index, Object controllerSpan, Object handlerSpan, String method, ServerEndpoint endpoint) {
switch (endpoint) {
case REDIRECT:
redirectSpan(trace, index, handlerSpan)
break
case ERROR:
case EXCEPTION:
sendErrorSpan(trace, index, handlerSpan)
break
}
}
String expectedServerSpanName(ServerEndpoint endpoint) {
return endpoint == PATH_PARAM ? getContextPath() + "/path/{id}/param" : endpoint.resolvePath(address).path
}

11
instrumentation/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat7/TestServlet.java
View File

@ -19,21 +19,22 @@ public class TestServlet extends HttpServlet {
HttpServerTest.ServerEndpoint serverEndpoint = HttpServerTest.ServerEndpoint.forPath(path);
if (serverEndpoint != null) {
if (serverEndpoint == HttpServerTest.ServerEndpoint.EXCEPTION) {
HttpServerTest.controller(
serverEndpoint,
() -> {
if (serverEndpoint == HttpServerTest.ServerEndpoint.EXCEPTION) {
throw new Exception(serverEndpoint.getBody());
});
} else {
resp.getWriter().print(HttpServerTest.controller(serverEndpoint, serverEndpoint::getBody));
}
resp.getWriter().print(serverEndpoint.getBody());
if (serverEndpoint == HttpServerTest.ServerEndpoint.REDIRECT) {
resp.sendRedirect(serverEndpoint.getBody());
} else if (serverEndpoint == HttpServerTest.ServerEndpoint.ERROR) {
resp.sendError(serverEndpoint.getStatus());
} else {
resp.setStatus(serverEndpoint.getStatus());
}
return null;
});
} else if ("/errorPage".equals(path)) {
resp.getWriter().print(HttpServerTest.ServerEndpoint.EXCEPTION.getBody());
resp.setStatus(500);

46
instrumentation/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat7/TomcatHandlerTest.groovy
View File

@ -5,7 +5,15 @@
package io.opentelemetry.javaagent.instrumentation.tomcat7
import static io.opentelemetry.api.trace.Span.Kind.INTERNAL
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.ERROR
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
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 io.opentelemetry.instrumentation.test.base.HttpServerTest
import io.opentelemetry.sdk.trace.data.SpanData
import org.apache.catalina.Context
import org.apache.catalina.startup.Tomcat
import org.apache.tomcat.util.descriptor.web.ErrorPage
@ -53,4 +61,42 @@ class TomcatHandlerTest extends HttpServerTest<Tomcat> {
tomcat.getServer().stop()
}
@Override
boolean testExceptionBody() {
false
}
@Override
boolean hasErrorPageSpans(ServerEndpoint endpoint) {
endpoint == ERROR || endpoint == EXCEPTION
}
@Override
void errorPageSpans(TraceAssert trace, int index, Object parent, String method = "GET", ServerEndpoint endpoint = SUCCESS) {
trace.span(index) {
name "ApplicationDispatcher.forward"
kind INTERNAL
errored false
childOf((SpanData) parent)
attributes {
}
}
}
@Override
boolean hasResponseSpan(ServerEndpoint endpoint) {
endpoint == REDIRECT || endpoint == ERROR
}
@Override
void responseSpan(TraceAssert trace, int index, Object parent, String method, ServerEndpoint endpoint) {
switch (endpoint) {
case REDIRECT:
redirectSpan(trace, index, parent)
break
case ERROR:
sendErrorSpan(trace, index, parent)
break
}
}
}

78
testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy
View File

@ -51,6 +51,14 @@ abstract class HttpServerTest<SERVER> extends AgentInstrumentationSpecification
false
}
boolean hasForwardSpan() {
false
}
boolean hasIncludeSpan() {
false
}
boolean hasErrorPageSpans(ServerEndpoint endpoint) {
false
}
@ -341,12 +349,18 @@ abstract class HttpServerTest<SERVER> extends AgentInstrumentationSpecification
if (hasHandlerSpan()) {
spanCount++
}
if (hasResponseSpan(endpoint)) {
spanCount++
}
if (endpoint != NOT_FOUND) {
spanCount++ // controller span
if (hasRenderSpan(endpoint)) {
spanCount++
}
if (hasResponseSpan(endpoint)) {
if (hasForwardSpan()) {
spanCount++
}
if (hasIncludeSpan()) {
spanCount++
}
if (hasErrorPageSpans(endpoint)) {
@ -362,21 +376,31 @@ abstract class HttpServerTest<SERVER> extends AgentInstrumentationSpecification
handlerSpan(it, spanIndex++, span(0), method, endpoint)
}
if (endpoint != NOT_FOUND) {
def controllerSpanIndex = 0
if (hasHandlerSpan()) {
controllerSpan(it, spanIndex++, span(1), errorMessage, expectedExceptionClass())
} else {
controllerSpan(it, spanIndex++, span(0), errorMessage, expectedExceptionClass())
controllerSpanIndex++
}
if (hasForwardSpan()) {
forwardSpan(it, spanIndex++, span(0), errorMessage, expectedExceptionClass())
controllerSpanIndex++
}
if (hasIncludeSpan()) {
includeSpan(it, spanIndex++, span(0), errorMessage, expectedExceptionClass())
controllerSpanIndex++
}
controllerSpan(it, spanIndex++, span(controllerSpanIndex), errorMessage, expectedExceptionClass())
if (hasRenderSpan(endpoint)) {
renderSpan(it, spanIndex++, span(0), method, endpoint)
}
if (hasResponseSpan(endpoint)) {
responseSpan(it, spanIndex, span(spanIndex - 1), method, endpoint)
responseSpan(it, spanIndex, span(spanIndex - 1), span(0), method, endpoint)
spanIndex++
}
if (hasErrorPageSpans(endpoint)) {
errorPageSpans(it, spanIndex, span(0), method, endpoint)
}
} else if (hasResponseSpan(endpoint)) {
responseSpan(it, 1, span(0), span(0), method, endpoint)
}
}
}
@ -402,6 +426,10 @@ abstract class HttpServerTest<SERVER> extends AgentInstrumentationSpecification
throw new UnsupportedOperationException("renderSpan not implemented in " + getClass().name)
}
void responseSpan(TraceAssert trace, int index, Object controllerSpan, Object handlerSpan, String method = "GET", ServerEndpoint endpoint = SUCCESS) {
responseSpan(trace, index, controllerSpan, method, endpoint)
}
void responseSpan(TraceAssert trace, int index, Object parent, String method = "GET", ServerEndpoint endpoint = SUCCESS) {
throw new UnsupportedOperationException("responseSpan not implemented in " + getClass().name)
}
@ -410,6 +438,46 @@ abstract class HttpServerTest<SERVER> extends AgentInstrumentationSpecification
throw new UnsupportedOperationException("errorPageSpans not implemented in " + getClass().name)
}
void redirectSpan(TraceAssert trace, int index, Object parent) {
trace.span(index) {
name ~/\.sendRedirect$/
kind Span.Kind.INTERNAL
childOf((SpanData) parent)
}
}
void sendErrorSpan(TraceAssert trace, int index, Object parent) {
trace.span(index) {
name ~/\.sendError$/
kind Span.Kind.INTERNAL
childOf((SpanData) parent)
}
}
void forwardSpan(TraceAssert trace, int index, Object parent, String errorMessage = null, Class exceptionClass = Exception) {
trace.span(index) {
name ~/\.forward$/
kind Span.Kind.INTERNAL
errored errorMessage != null
if (errorMessage) {
errorEvent(exceptionClass, errorMessage)
}
childOf((SpanData) parent)
}
}
void includeSpan(TraceAssert trace, int index, Object parent, String errorMessage = null, Class exceptionClass = Exception) {
trace.span(index) {
name ~/\.include$/
kind Span.Kind.INTERNAL
errored errorMessage != null
if (errorMessage) {
errorEvent(exceptionClass, errorMessage)
}
childOf((SpanData) parent)
}
}
// parent span must be cast otherwise it breaks debugging classloading (junit loads it early)
void serverSpan(TraceAssert trace, int index, String traceID = null, String parentID = null, String method = "GET", Long responseContentLength = null, ServerEndpoint endpoint = SUCCESS) {
trace.span(index) {