Changed instrumentation tracer" TRACER.startSpan() to never return `null`
MODIFY
agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/HttpServerTracer.java
agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/HttpServerTracer.java
@@ -51,10 +51,6 @@ public abstract class HttpServerTracer<REQUEST> {
}
public Span startSpan(REQUEST request, Method origin, String originType) {
- if (getAttachedSpan(request) != null) {
- return null;
- }
-
final Span.Builder builder =
tracer
.spanBuilder(spanNameForMethod(origin))
@@ -132,10 +128,6 @@ public abstract class HttpServerTracer<REQUEST> {
// TODO set resource name from URL.
}
- public boolean sameTrace(Span oneSpan, Span otherSpan) {
- return oneSpan.getContext().getTraceId().equals(otherSpan.getContext().getTraceId());
- }
-
/**
* This method is used to generate an acceptable span (operation) name based on a given method
* reference. Anonymous classes are named based on their parent.

MODIFY
instrumentation/grizzly-2.0/src/main/java/io/opentelemetry/auto/instrumentation/grizzly/GrizzlyHttpHandlerInstrumentation.java
instrumentation/grizzly-2.0/src/main/java/io/opentelemetry/auto/instrumentation/grizzly/GrizzlyHttpHandlerInstrumentation.java
@@ -79,6 +79,10 @@ public class GrizzlyHttpHandlerInstrumentation extends Instrumenter.Default {
@Advice.Argument(0) final Request request,
@Advice.Local("otelSpan") Span span,
@Advice.Local("otelScope") Scope scope) {
+ if (TRACER.getAttachedSpan(request) != null) {
+ return;
+ }
+
request.addAfterServiceListener(SpanClosingListener.LISTENER);
span = TRACER.startSpan(request, method, null);
@@ -96,10 +100,6 @@ public class GrizzlyHttpHandlerInstrumentation extends Instrumenter.Default {
}
scope.close();
- if (span == null) {
- return;
- }
-
if (throwable != null) {
TRACER.endExceptionally(span, throwable, response.getStatus());
}
@@ -108,7 +108,6 @@ public class GrizzlyHttpHandlerInstrumentation extends Instrumenter.Default {
public static class SpanClosingListener implements AfterServiceListener {
public static final SpanClosingListener LISTENER = new SpanClosingListener();
- public static final GrizzlyHttpServerTracer TRACER = new GrizzlyHttpServerTracer();
@Override
public void onAfterService(final Request request) {

MODIFY
instrumentation/grizzly-2.0/src/main/java/io/opentelemetry/auto/instrumentation/grizzly/GrizzlyHttpServerTracer.java
instrumentation/grizzly-2.0/src/main/java/io/opentelemetry/auto/instrumentation/grizzly/GrizzlyHttpServerTracer.java
@@ -75,11 +75,12 @@ public class GrizzlyHttpServerTracer extends HttpServerTracer<Request> {
}
@Override
- protected Span getAttachedSpan(Request request) {
+ public Span getAttachedSpan(Request request) {
Object span = request.getAttribute(SPAN_ATTRIBUTE);
return span instanceof Span ? (Span) span : null;
}
+ @Override
public void onRequest(Span span, Request request) {
request.setAttribute("traceId", span.getContext().getTraceId().toLowerBase16());
request.setAttribute("spanId", span.getContext().getSpanId().toLowerBase16());

MODIFY
instrumentation/servlet/request-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3/Servlet2Advice.java
instrumentation/servlet/request-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3/Servlet2Advice.java
@@ -38,12 +38,16 @@ public class Servlet2Advice {
@Advice.Local("otelSpan") Span span,
@Advice.Local("otelScope") Scope scope) {
- if (!(request instanceof HttpServletRequest)) {
+ if (!(request instanceof HttpServletRequest) || !(response instanceof HttpServletResponse)) {
return;
}
final HttpServletRequest httpServletRequest = (HttpServletRequest) request;
+ if (TRACER.getAttachedSpan(httpServletRequest) != null) {
+ return;
+ }
+
// For use by HttpServletResponseInstrumentation:
InstrumentationContext.get(HttpServletResponse.class, HttpServletRequest.class)
.put((HttpServletResponse) response, httpServletRequest);
@@ -64,13 +68,8 @@ public class Servlet2Advice {
}
scope.close();
- if (request instanceof HttpServletRequest && response instanceof HttpServletResponse) {
TRACER.setPrincipal((HttpServletRequest) request);
- if (span == null) {
- return;
- }
-
Integer responseStatus =
InstrumentationContext.get(ServletResponse.class, Integer.class).get(response);
@@ -81,4 +80,3 @@ public class Servlet2Advice {
}
}
}
-}

MODIFY
instrumentation/servlet/request-3.0/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v3_0/Servlet3Advice.java
instrumentation/servlet/request-3.0/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v3_0/Servlet3Advice.java
@@ -39,30 +39,17 @@ public class Servlet3Advice {
@Advice.Argument(1) final ServletResponse response,
@Advice.Local("otelSpan") Span span,
@Advice.Local("otelScope") Scope scope) {
- if (!(request instanceof HttpServletRequest)) {
+
+ if (!(request instanceof HttpServletRequest) || !(response instanceof HttpServletResponse)) {
return;
}
final HttpServletRequest httpServletRequest = (HttpServletRequest) request;
- final Span existingSpan = TRACER.getAttachedSpan(httpServletRequest);
- if (existingSpan != null) {
- /*
- Given request already has a span associated with it.
- As there should not be nested spans of kind SERVER, we should NOT create a new span here.
- But it may happen that there is no span in current Context or it is from a different trace.
- E.g. in case of async servlet request processing we create span for incoming request in one thread,
- but actual request continues processing happens in another thread.
- Depending on servlet container implementation, this processing may again arrive into this method.
- E.g. Jetty handles async requests in a way that calls HttpServlet.service method twice.
-
- In this case we have to put the span from the request into current context before continuing.
- */
- final boolean spanContextWasLost = !TRACER.sameTrace(TRACER.getCurrentSpan(), existingSpan);
- if (spanContextWasLost) {
- // Put span from request attribute into current context.
- // We did not create a new span here, so return null instead
- scope = currentContextWith(existingSpan);
+ Span attachedSpan = TRACER.getAttachedSpan(httpServletRequest);
+ if (attachedSpan != null) {
+ if (TRACER.needsRescoping(attachedSpan)) {
+ scope = currentContextWith(attachedSpan);
}
// We are inside nested servlet/filter, don't create new span
return;
@@ -88,7 +75,11 @@ public class Servlet3Advice {
}
scope.close();
- if (request instanceof HttpServletRequest && response instanceof HttpServletResponse) {
+ if (span == null) {
+ // an existing span was found
+ return;
+ }
+
TRACER.setPrincipal((HttpServletRequest) request);
if (throwable != null) {
@@ -96,10 +87,6 @@ public class Servlet3Advice {
return;
}
- if (span == null) {
- return;
- }
-
final AtomicBoolean responseHandled = new AtomicBoolean(false);
// In case of async servlets wait for the actual response to be ready
@@ -118,4 +105,3 @@ public class Servlet3Advice {
}
}
}
-}

MODIFY
instrumentation/servlet/request-3.0/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v3_0/Servlet3HttpServerTracer.java
instrumentation/servlet/request-3.0/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v3_0/Servlet3HttpServerTracer.java
@@ -29,6 +29,7 @@ import lombok.extern.slf4j.Slf4j;
public class Servlet3HttpServerTracer extends ServletHttpServerTracer {
public static final Servlet3HttpServerTracer TRACER = new Servlet3HttpServerTracer();
+ @Override
protected String getInstrumentationName() {
return "io.opentelemetry.auto.servlet-3.0";
}
@@ -44,6 +45,22 @@ public class Servlet3HttpServerTracer extends ServletHttpServerTracer {
span.end();
}
+ /*
+ Given request already has a span associated with it.
+ As there should not be nested spans of kind SERVER, we should NOT create a new span here.
+
+ But it may happen that there is no span in current Context or it is from a different trace.
+ E.g. in case of async servlet request processing we create span for incoming request in one thread,
+ but actual request continues processing happens in another thread.
+ Depending on servlet container implementation, this processing may again arrive into this method.
+ E.g. Jetty handles async requests in a way that calls HttpServlet.service method twice.
+
+ In this case we have to put the span from the request into current context before continuing.
+ */
+ public boolean needsRescoping(Span attachedSpan) {
+ return !sameTrace(tracer.getCurrentSpan(), attachedSpan);
+ }
+
@Override
public void onRequest(Span span, HttpServletRequest request) {
super.onRequest(span, request);
@@ -75,4 +92,8 @@ public class Servlet3HttpServerTracer extends ServletHttpServerTracer {
}
}
}
+
+ private static boolean sameTrace(final Span oneSpan, final Span otherSpan) {
+ return oneSpan.getContext().getTraceId().equals(otherSpan.getContext().getTraceId());
+ }
}

MODIFY
instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/auto/instrumentation/servlet/ServletHttpServerTracer.java
instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/auto/instrumentation/servlet/ServletHttpServerTracer.java
@@ -29,6 +29,7 @@ import lombok.extern.slf4j.Slf4j;
@Slf4j
public abstract class ServletHttpServerTracer extends HttpServerTracer<HttpServletRequest> {
+ @Override
protected String getVersion() {
return null;
}
@@ -73,6 +74,7 @@ public abstract class ServletHttpServerTracer extends HttpServerTracer<HttpServl
return request.getMethod();
}
+ @Override
public void onRequest(Span span, HttpServletRequest request) {
// we do this e.g. so that servlet containers can use these values in their access logs
request.setAttribute("traceId", span.getContext().getTraceId().toLowerBase16());
@@ -89,6 +91,7 @@ public abstract class ServletHttpServerTracer extends HttpServerTracer<HttpServl
return HttpServletRequestGetter.GETTER;
}
+ @Override
protected Throwable unwrapThrowable(Throwable throwable) {
Throwable result = throwable;
if (throwable instanceof ServletException && throwable.getCause() != null) {

Changed instrumentation tracer" TRACER.startSpan() to never return `null` (#499)