Jetty server span handling
MODIFY
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/AppServerBridge.java
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/AppServerBridge.java
@@ -18,35 +18,50 @@ public class AppServerBridge {
private static final ContextKey<AppServerBridge> CONTEXT_KEY =
ContextKey.named("opentelemetry-servlet-app-server-bridge");
+ /**
+ * Attach AppServerBridge to context.
+ *
+ * @param ctx server context
+ * @return new context with AppServerBridge attached.
+ */
public static Context init(Context ctx) {
- return ctx.with(AppServerBridge.CONTEXT_KEY, new AppServerBridge());
+ return init(ctx, true);
}
- private final AtomicBoolean servletUpdatedServerSpanName = new AtomicBoolean(false);
-
/**
- * Check whether given context contains AppServerBridge.
+ * Attach AppServerBridge to context.
*
* @param ctx server context
- * @return <code>true</code> if AppServerBridge is present in the context. <code>false</code>
- * otherwise.
+ * @param shouldRecordException whether servlet integration should record exception thrown during
+ * servlet invocation in server span. Use <code>false</code> on servers where exceptions
+ * thrown during servlet invocation are propagated to the method where server span is closed
+ * and can be added to server span there and <code>true</code> otherwise.
+ * @return new context with AppServerBridge attached.
*/
- public static boolean isPresent(Context ctx) {
- return ctx.get(AppServerBridge.CONTEXT_KEY) != null;
+ public static Context init(Context ctx, boolean shouldRecordException) {
+ return ctx.with(AppServerBridge.CONTEXT_KEY, new AppServerBridge(shouldRecordException));
+ }
+
+ private final AtomicBoolean servletUpdatedServerSpanName = new AtomicBoolean(false);
+ private final AtomicBoolean servletShouldRecordException;
+
+ private AppServerBridge(boolean shouldRecordException) {
+ servletShouldRecordException = new AtomicBoolean(shouldRecordException);
}
/**
- * Returns true, if servlet integration has indicated, that it has updated the name for the server
- * span.
+ * Returns true, if servlet integration should update server span name. After server span name has
+ * been updated with <code>setServletUpdatedServerSpanName</code> this method will return <code>
+ * false</code>.
*
* @param ctx server context
- * @return <code>true</code>, if the server span name was updated by servlet integration, or
+ * @return <code>true</code>, if the server span name should be updated by servlet integration, or
* <code>false</code> otherwise.
*/
- public static boolean isServerSpanNameUpdatedFromServlet(Context ctx) {
+ public static boolean shouldUpdateServerSpanName(Context ctx) {
AppServerBridge appServerBridge = ctx.get(AppServerBridge.CONTEXT_KEY);
if (appServerBridge != null) {
- return appServerBridge.servletUpdatedServerSpanName.get();
+ return !appServerBridge.servletUpdatedServerSpanName.get();
}
return false;
}
@@ -63,6 +78,24 @@ public class AppServerBridge {
}
}
+ /**
+ * Returns true, if servlet integration should record exception thrown during servlet invocation
+ * in server span. This method should return <code>false</code> on servers where exceptions thrown
+ * during servlet invocation are propagated to the method where server span is closed and can be
+ * added to server span there and <code>true</code> otherwise.
+ *
+ * @param ctx server context
+ * @return <code>true</code>, if servlet integration should record exception thrown during servlet
+ * invocation in server span, or <code>false</code> otherwise.
+ */
+ public static boolean shouldRecordException(Context ctx) {
+ AppServerBridge appServerBridge = ctx.get(AppServerBridge.CONTEXT_KEY);
+ if (appServerBridge != null) {
+ return appServerBridge.servletShouldRecordException.get();
+ }
+ return true;
+ }
+
/**
* Class used as key in CallDepthThreadLocalMap for counting servlet invocation depth in
* Servlet3Advice and Servlet2Advice. We can not use helper classes like Servlet3Advice and

MODIFY
instrumentation-core/servlet-2.2/src/main/java/io/opentelemetry/instrumentation/servlet/ServletHttpServerTracer.java
instrumentation-core/servlet-2.2/src/main/java/io/opentelemetry/instrumentation/servlet/ServletHttpServerTracer.java
@@ -109,8 +109,10 @@ public abstract class ServletHttpServerTracer<RESPONSE>
return HttpServletRequestGetter.GETTER;
}
- public void addUnwrappedThrowable(Span span, Throwable throwable) {
- addThrowable(span, unwrapThrowable(throwable));
+ public void addUnwrappedThrowable(Context context, Throwable throwable) {
+ if (AppServerBridge.shouldRecordException(context)) {
+ addThrowable(Span.fromContext(context), unwrapThrowable(throwable));
+ }
}
@Override
@@ -158,8 +160,7 @@ public abstract class ServletHttpServerTracer<RESPONSE>
* reflected in the span name.
*/
public void updateServerSpanNameOnce(Context attachedContext, HttpServletRequest request) {
- if (AppServerBridge.isPresent(attachedContext)
- && !AppServerBridge.isServerSpanNameUpdatedFromServlet(attachedContext)) {
+ if (AppServerBridge.shouldUpdateServerSpanName(attachedContext)) {
updateSpanName(Span.fromContext(attachedContext), request);
AppServerBridge.setServletUpdatedServerSpanName(attachedContext, true);
}

MODIFY
instrumentation/jetty-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/JettyHandlerAdvice.java
instrumentation/jetty-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/JettyHandlerAdvice.java
@@ -32,7 +32,7 @@ public class JettyHandlerAdvice {
return;
}
- context = tracer().startSpan(request, request, request, method);
+ context = tracer().startServerSpan(request, method);
scope = context.makeCurrent();
}

MODIFY
instrumentation/jetty-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/JettyHttpServerTracer.java
instrumentation/jetty-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/JettyHttpServerTracer.java
package io.opentelemetry.javaagent.instrumentation.jetty;
+import io.opentelemetry.context.Context;
+import io.opentelemetry.instrumentation.api.servlet.AppServerBridge;
import io.opentelemetry.javaagent.instrumentation.servlet.v3_0.Servlet3HttpServerTracer;
+import java.lang.reflect.Method;
+import javax.servlet.http.HttpServletRequest;
public class JettyHttpServerTracer extends Servlet3HttpServerTracer {
private static final JettyHttpServerTracer TRACER = new JettyHttpServerTracer();
@@ -14,6 +18,16 @@ public class JettyHttpServerTracer extends Servlet3HttpServerTracer {
return TRACER;
}
+ public Context startServerSpan(HttpServletRequest request, Method instrumentedMethod) {
+ Context context =
+ AppServerBridge.init(startSpan(request, request, request, instrumentedMethod), false);
+
+ // context must be reattached, because it has new attributes compared to the one returned from
+ // startSpan().
+ attachServerContext(context, request);
+ return context;
+ }
+
@Override
protected String getInstrumentationName() {
return "io.opentelemetry.javaagent.jetty";

MODIFY
instrumentation/servlet/servlet-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v2_2/Servlet2Advice.java
instrumentation/servlet/servlet-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v2_2/Servlet2Advice.java
@@ -62,12 +62,13 @@ public class Servlet2Advice {
}
if (context == null && callDepth == 0) {
+ Context currentContext = Java8BytecodeBridge.currentContext();
// Something else is managing the context, we're in the outermost level of Servlet
// instrumentation and we have an uncaught throwable. Let's add it to the current span.
if (throwable != null) {
- tracer().addUnwrappedThrowable(Java8BytecodeBridge.currentSpan(), throwable);
+ tracer().addUnwrappedThrowable(currentContext, throwable);
}
- tracer().setPrincipal(Java8BytecodeBridge.currentContext(), (HttpServletRequest) request);
+ tracer().setPrincipal(currentContext, (HttpServletRequest) request);
}
if (scope == null || context == null) {

MODIFY
instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Advice.java
instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Advice.java
@@ -70,12 +70,13 @@ public class Servlet3Advice {
}
if (context == null && callDepth == 0) {
+ Context currentContext = Java8BytecodeBridge.currentContext();
// Something else is managing the context, we're in the outermost level of Servlet
// instrumentation and we have an uncaught throwable. Let's add it to the current span.
if (throwable != null) {
- tracer().addUnwrappedThrowable(Java8BytecodeBridge.currentSpan(), throwable);
+ tracer().addUnwrappedThrowable(currentContext, throwable);
}
- tracer().setPrincipal(Java8BytecodeBridge.currentContext(), (HttpServletRequest) request);
+ tracer().setPrincipal(currentContext, (HttpServletRequest) request);
}
if (scope == null || context == null) {

MODIFY
smoke-tests/src/test/groovy/io/opentelemetry/smoketest/JettySmokeTest.groovy
smoke-tests/src/test/groovy/io/opentelemetry/smoketest/JettySmokeTest.groovy
package io.opentelemetry.smoketest
-import io.opentelemetry.proto.trace.v1.Span
-import java.util.jar.Attributes
-import java.util.jar.JarFile
-import okhttp3.Request
-import spock.lang.Unroll
-
@AppServer(version = "9.4.35", jdk = "8")
@AppServer(version = "9.4.35", jdk = "11")
@AppServer(version = "10.0.0", jdk = "11")
@@ -27,151 +21,11 @@ class JettySmokeTest extends AppServerTest {
@Override
protected String getSpanName(String path) {
+ switch (path) {
+ case "/app/WEB-INF/web.xml":
+ case "/this-is-definitely-not-there-but-there-should-be-a-trace-nevertheless":
return getJettySpanName()
}
-
- @Override
- boolean testSmoke() {
- false
- }
-
- @Override
- boolean testAsyncSmoke() {
- false
- }
-
- @Override
- boolean testException() {
- false
- }
-
- @Unroll
- def "#appServer smoke test on JDK #jdk"(String appServer, String jdk) {
- String url = "http://localhost:${target.getMappedPort(8080)}/app/greeting"
- def request = new Request.Builder().url(url).get().build()
- def currentAgentVersion = new JarFile(agentPath).getManifest().getMainAttributes().get(Attributes.Name.IMPLEMENTATION_VERSION)
-
- when:
- def response = CLIENT.newCall(request).execute()
- TraceInspector traces = new TraceInspector(waitForTraces())
- Set<String> traceIds = traces.traceIds
- String responseBody = response.body().string()
-
- then: "There is one trace"
- traceIds.size() == 1
-
- and: "trace id is present in the HTTP headers as reported by the called endpoint"
- responseBody.contains(traceIds.find())
-
- and: "Server spans in the distributed trace"
- traces.countSpansByKind(Span.SpanKind.SPAN_KIND_SERVER) == 2
-
- and: "Expected span names"
- traces.countSpansByName(getJettySpanName()) == 2
-
- and: "The span for the initial web request"
- traces.countFilteredAttributes("http.url", url) == 1
-
- and: "Client and server spans for the remote call"
- traces.countFilteredAttributes("http.url", "http://localhost:8080/app/headers") == 2
-
- and: "Number of spans tagged with current otel library version"
- traces.countFilteredResourceAttributes("telemetry.auto.version", currentAgentVersion) == 3
-
- and:
- traces.findResourceAttribute("os.name")
- .map { it.stringValue }
- .findAny()
- .isPresent()
-
- where:
- [appServer, jdk] << getTestParams()
- }
-
- @Unroll
- def "#appServer async smoke test on JDK #jdk"(String appServer, String jdk) {
- String url = "http://localhost:${target.getMappedPort(8080)}/app/asyncgreeting"
- def request = new Request.Builder().url(url).get().build()
- def currentAgentVersion = new JarFile(agentPath).getManifest().getMainAttributes().get(Attributes.Name.IMPLEMENTATION_VERSION)
-
- when:
- def response = CLIENT.newCall(request).execute()
- TraceInspector traces = new TraceInspector(waitForTraces())
- Set<String> traceIds = traces.traceIds
- String responseBody = response.body().string()
-
- then: "There is one trace"
- traceIds.size() == 1
-
- and: "trace id is present in the HTTP headers as reported by the called endpoint"
- responseBody.contains(traceIds.find())
-
- and: "Server spans in the distributed trace"
- traces.countSpansByKind(Span.SpanKind.SPAN_KIND_SERVER) == 2
-
- and: "Expected span names"
- traces.countSpansByName(getJettySpanName()) == 2
-
- and: "The span for the initial web request"
- traces.countFilteredAttributes("http.url", url) == 1
-
- and: "Client and server spans for the remote call"
- traces.countFilteredAttributes("http.url", "http://localhost:8080/app/headers") == 2
-
- and: "Number of spans tagged with current otel library version"
- traces.countFilteredResourceAttributes("telemetry.auto.version", currentAgentVersion) == 3
-
- and:
- traces.findResourceAttribute("os.name")
- .map { it.stringValue }
- .findAny()
- .isPresent()
-
- where:
- [appServer, jdk] << getTestParams()
- }
-
- @Unroll
- def "#appServer test request with error JDK #jdk"(String appServer, String jdk) {
- String url = "http://localhost:${target.getMappedPort(8080)}/app/exception"
- def request = new Request.Builder().url(url).get().build()
- def currentAgentVersion = new JarFile(agentPath).getManifest().getMainAttributes().get(Attributes.Name.IMPLEMENTATION_VERSION)
-
- when:
- def response = CLIENT.newCall(request).execute()
- TraceInspector traces = new TraceInspector(waitForTraces())
- Set<String> traceIds = traces.traceIds
-
- then: "There is one trace"
- traceIds.size() == 1
-
- and: "Response code is 500"
- response.code() == 500
-
- and: "There is one server span"
- traces.countSpansByKind(Span.SpanKind.SPAN_KIND_SERVER) == 1
-
- and: "Expected span names"
- traces.countSpansByName(getJettySpanName()) == 1
-
- //FIXME: exception not reported on jetty
-// and: "There is one exception"
-// traces.countFilteredEventAttributes('exception.message', 'This is expected') == 1
-
- and: "The span for the initial web request"
- traces.countFilteredAttributes("http.url", url) == 1
-
- and: "Number of spans tagged with current otel library version"
- traces.countFilteredResourceAttributes("telemetry.auto.version", currentAgentVersion) == 1
-
- and:
- traces.findResourceAttribute("os.name")
- .map { it.stringValue }
- .findAny()
- .isPresent()
-
- where:
- [appServer, jdk] << getTestParams()
+ return path
}
-
}

Jetty server span handling (#2029)