Add response generic parameter to HttpServerTracer (#870)

agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/HttpServerTracer.java

@@ -39,7 +39,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 // TODO In search for a better home package
-public abstract class HttpServerTracer<REQUEST, CONNECTION, STORAGE> extends BaseTracer {
+public abstract class HttpServerTracer<REQUEST, RESPONSE, CONNECTION, STORAGE> extends BaseTracer {
 
   private static final Logger log = LoggerFactory.getLogger(HttpServerTracer.class);
 
@@ -93,39 +93,49 @@ public abstract class HttpServerTracer<REQUEST, CONNECTION, STORAGE> extends Bas
     return withScopedContext(newContext);
   }
 
+  /**
+   * Convenience method. Delegates to {@link #end(Span, Object, long)}, passing {@code timestamp}
+   * value of {@code -1}.
+   */
   // TODO should end methods remove SPAN attribute from request as well?
-  public void end(Span span, int responseStatus) {
+  public void end(Span span, RESPONSE response) {
-    end(span, responseStatus, -1);
+    end(span, response, -1);
   }
 
   // TODO should end methods remove SPAN attribute from request as well?
-  public void end(Span span, int responseStatus, long timestamp) {
+  public void end(Span span, RESPONSE response, long timestamp) {
-    setStatus(span, responseStatus);
+    setStatus(span, responseStatus(response));
-    if (timestamp >= 0) {
+    endSpan(span, timestamp);
-      span.end(EndSpanOptions.builder().setEndTimestamp(timestamp).build());
-    } else {
-      span.end();
-    }
   }
 
-  /** Ends given span exceptionally with default response status code 500. */
+  /**
+   * Convenience method. Delegates to {@link #endExceptionally(Span, Throwable, RESPONSE)}, passing
+   * {@code response} value of {@code null}.
+   */
   public void endExceptionally(Span span, Throwable throwable) {
-    endExceptionally(span, throwable, 500);
+    endExceptionally(span, throwable, null);
   }
 
-  public void endExceptionally(Span span, Throwable throwable, int responseStatus) {
+  /**
-    endExceptionally(span, throwable, responseStatus, -1);
+   * Convenience method. Delegates to {@link #endExceptionally(Span, Throwable, RESPONSE, long)},
+   * passing {@code timestamp} value of {@code -1}.
+   */
+  public void endExceptionally(Span span, Throwable throwable, RESPONSE response) {
+    endExceptionally(span, throwable, response, -1);
   }
 
-  public void endExceptionally(Span span, Throwable throwable, int responseStatus, long timestamp) {
+  /**
-    if (responseStatus == 200) {
+   * If {@code response} is {@code null}, the {@code http.status_code} will be set to {@code 500}
-      // TODO I think this is wrong.
+   * and the {@link Span} status will be set to {@link io.opentelemetry.trace.Status#INTERNAL}.
-      // We must report that response status that was actually sent to end user
+   */
-      // We may change span status, but not http_status attribute
+  public void endExceptionally(Span span, Throwable throwable, RESPONSE response, long timestamp) {
-      responseStatus = 500;
-    }
     onError(span, unwrapThrowable(throwable));
-    end(span, responseStatus, timestamp);
+    if (response == null) {
+      setStatus(span, 500);
+    } else {
+      setStatus(span, responseStatus(response));
+    }
+    endSpan(span, timestamp);
   }
 
   public Span getServerSpan(STORAGE storage) {
@@ -267,12 +277,20 @@ public abstract class HttpServerTracer<REQUEST, CONNECTION, STORAGE> extends Bas
     return span.getContext();
   }
 
-  private void setStatus(Span span, int status) {
+  private static void setStatus(Span span, int status) {
     SemanticAttributes.HTTP_STATUS_CODE.set(span, status);
     // TODO status_message
     span.setStatus(HttpStatusConverter.statusFromHttpStatus(status));
   }
 
+  private static void endSpan(Span span, long timestamp) {
+    if (timestamp >= 0) {
+      span.end(EndSpanOptions.builder().setEndTimestamp(timestamp).build());
+    } else {
+      span.end();
+    }
+  }
+
   protected abstract Integer peerPort(CONNECTION connection);
 
   protected abstract String peerHostIP(CONNECTION connection);
@@ -287,6 +305,8 @@ public abstract class HttpServerTracer<REQUEST, CONNECTION, STORAGE> extends Bas
 
   protected abstract String requestHeader(REQUEST request, String name);
 
+  protected abstract int responseStatus(RESPONSE response);
+
   /**
    * Stores given context in the given request-response-loop storage in implementation specific way.
    */

instrumentation-core/servlet/src/main/java/io/opentelemetry/instrumentation/servlet/ServletHttpServerTracer.java

@@ -21,15 +21,14 @@ import io.opentelemetry.auto.bootstrap.instrumentation.decorator.HttpServerTrace
 import io.opentelemetry.auto.instrumentation.api.MoreAttributes;
 import io.opentelemetry.context.propagation.HttpTextFormat.Getter;
 import io.opentelemetry.trace.Span;
-import io.opentelemetry.trace.attributes.SemanticAttributes;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.security.Principal;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 
-public abstract class ServletHttpServerTracer
+public abstract class ServletHttpServerTracer<RESPONSE>
-    extends HttpServerTracer<HttpServletRequest, HttpServletRequest, HttpServletRequest> {
+    extends HttpServerTracer<HttpServletRequest, RESPONSE, HttpServletRequest, HttpServletRequest> {
 
   @Override
   protected String getVersion() {
@@ -109,10 +108,6 @@ public abstract class ServletHttpServerTracer
     }
   }
 
-  public void setContentLength(Span span, int length) {
-    SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.set(span, length);
-  }
-
   @Override
   protected String flavor(HttpServletRequest connection, HttpServletRequest request) {
     return connection.getProtocol();

instrumentation-core/spring/spring-webmvc-3.1/src/main/java/io/opentelemetry/instrumentation/springwebmvc/SpringWebMvcServerTracer.java

@@ -24,9 +24,11 @@ import io.opentelemetry.trace.Tracer;
 import java.net.URI;
 import java.net.URISyntaxException;
 import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 
 class SpringWebMvcServerTracer
-    extends HttpServerTracer<HttpServletRequest, HttpServletRequest, HttpServletRequest> {
+    extends HttpServerTracer<
+        HttpServletRequest, HttpServletResponse, HttpServletRequest, HttpServletRequest> {
 
   public SpringWebMvcServerTracer(Tracer tracer) {
     super(tracer);
@@ -62,6 +64,11 @@ class SpringWebMvcServerTracer
     return httpServletRequest.getHeader(name);
   }
 
+  @Override
+  protected int responseStatus(HttpServletResponse httpServletResponse) {
+    return httpServletResponse.getStatus();
+  }
+
   @Override
   protected void attachServerContext(Context context, HttpServletRequest request) {
     request.setAttribute(CONTEXT_ATTRIBUTE, context);

instrumentation-core/spring/spring-webmvc-3.1/src/main/java/io/opentelemetry/instrumentation/springwebmvc/WebMVCTracingFilter.java

@@ -45,9 +45,9 @@ public class WebMVCTracingFilter extends OncePerRequestFilter implements Ordered
 
     try (Scope ignored = tracer.startScope(serverSpan, request)) {
       filterChain.doFilter(request, response);
-      tracer.end(serverSpan, response.getStatus());
+      tracer.end(serverSpan, response);
     } catch (Throwable t) {
-      tracer.endExceptionally(serverSpan, t, response.getStatus());
+      tracer.endExceptionally(serverSpan, t, response);
       throw t;
     }
   }

instrumentation/akka-http-10.0/src/main/java/io/opentelemetry/auto/instrumentation/akkahttp/AkkaHttpServerInstrumentation.java

@@ -110,7 +110,7 @@ public final class AkkaHttpServerInstrumentation extends Instrumenter.Default {
       Span span = TRACER.startSpan(request, request, "akka.request");
       try (Scope ignored = TRACER.startScope(span, null)) {
         HttpResponse response = userHandler.apply(request);
-        TRACER.end(span, response.status().intValue());
+        TRACER.end(span, response);
         return response;
       } catch (final Throwable t) {
         TRACER.endExceptionally(span, t);
@@ -140,7 +140,7 @@ public final class AkkaHttpServerInstrumentation extends Instrumenter.Default {
                 new AbstractFunction1<HttpResponse, HttpResponse>() {
                   @Override
                   public HttpResponse apply(final HttpResponse response) {
-                    TRACER.end(span, response.status().intValue());
+                    TRACER.end(span, response);
                     return response;
                   }
                 },

instrumentation/akka-http-10.0/src/main/java/io/opentelemetry/auto/instrumentation/akkahttp/AkkaHttpServerTracer.java

@@ -18,13 +18,15 @@ package io.opentelemetry.auto.instrumentation.akkahttp;
 
 import akka.http.javadsl.model.HttpHeader;
 import akka.http.scaladsl.model.HttpRequest;
+import akka.http.scaladsl.model.HttpResponse;
 import io.grpc.Context;
 import io.opentelemetry.auto.bootstrap.instrumentation.decorator.HttpServerTracer;
 import io.opentelemetry.context.propagation.HttpTextFormat.Getter;
 import java.net.URI;
 import java.net.URISyntaxException;
 
-public class AkkaHttpServerTracer extends HttpServerTracer<HttpRequest, HttpRequest, Void> {
+public class AkkaHttpServerTracer
+    extends HttpServerTracer<HttpRequest, HttpResponse, HttpRequest, Void> {
   public static final AkkaHttpServerTracer TRACER = new AkkaHttpServerTracer();
 
   @Override
@@ -37,6 +39,11 @@ public class AkkaHttpServerTracer extends HttpServerTracer<HttpRequest, HttpRequ
     return httpRequest.getHeader(name).map(HttpHeader::value).orElse(null);
   }
 
+  @Override
+  protected int responseStatus(HttpResponse httpResponse) {
+    return httpResponse.status().intValue();
+  }
+
   @Override
   protected void attachServerContext(Context context, Void none) {}
 

instrumentation/armeria-1.0/library/src/main/java/io/opentelemetry/instrumentation/armeria/v1_0/server/ArmeriaServerTracer.java

@@ -17,6 +17,7 @@
 package io.opentelemetry.instrumentation.armeria.v1_0.server;
 
 import com.linecorp.armeria.common.HttpRequest;
+import com.linecorp.armeria.common.logging.RequestLog;
 import com.linecorp.armeria.server.ServiceRequestContext;
 import io.grpc.Context;
 import io.opentelemetry.auto.bootstrap.instrumentation.decorator.HttpServerTracer;
@@ -27,7 +28,7 @@ import java.net.SocketAddress;
 import java.net.URI;
 
 public class ArmeriaServerTracer
-    extends HttpServerTracer<HttpRequest, ServiceRequestContext, Void> {
+    extends HttpServerTracer<HttpRequest, RequestLog, ServiceRequestContext, Void> {
 
   ArmeriaServerTracer() {}
 
@@ -95,6 +96,11 @@ public class ArmeriaServerTracer
     return httpRequest.headers().get(name);
   }
 
+  @Override
+  protected int responseStatus(RequestLog httpResponse) {
+    return httpResponse.responseHeaders().status().code();
+  }
+
   @Override
   protected void attachServerContext(Context context, Void ctx) {}
 

instrumentation/armeria-1.0/library/src/main/java/io/opentelemetry/instrumentation/armeria/v1_0/server/OpenTelemetryService.java

@@ -18,7 +18,6 @@ package io.opentelemetry.instrumentation.armeria.v1_0.server;
 
 import com.linecorp.armeria.common.HttpRequest;
 import com.linecorp.armeria.common.HttpResponse;
-import com.linecorp.armeria.common.HttpStatus;
 import com.linecorp.armeria.common.logging.RequestLogProperty;
 import com.linecorp.armeria.server.HttpService;
 import com.linecorp.armeria.server.Route;
@@ -74,13 +73,12 @@ public class OpenTelemetryService extends SimpleDecoratingHttpService {
           .whenComplete()
           .thenAccept(
               log -> {
-                HttpStatus status = log.responseHeaders().status();
                 long requestEndTimeNanos = requestStartTimeNanos + log.responseDurationNanos();
                 if (log.responseCause() != null) {
                   serverTracer.endExceptionally(
-                      span, log.responseCause(), status.code(), requestEndTimeNanos);
+                      span, log.responseCause(), log, requestEndTimeNanos);
                 } else {
-                  serverTracer.end(span, status.code(), requestEndTimeNanos);
+                  serverTracer.end(span, log, requestEndTimeNanos);
                 }
               });
     }

instrumentation/grizzly-2.0/src/main/java/io/opentelemetry/auto/instrumentation/grizzly/GrizzlyHttpServerTracer.java

@@ -23,9 +23,11 @@ import java.net.URI;
 import java.net.URISyntaxException;
 import org.glassfish.grizzly.filterchain.FilterChainContext;
 import org.glassfish.grizzly.http.HttpRequestPacket;
+import org.glassfish.grizzly.http.HttpResponsePacket;
 
 public class GrizzlyHttpServerTracer
-    extends HttpServerTracer<HttpRequestPacket, HttpRequestPacket, FilterChainContext> {
+    extends HttpServerTracer<
+        HttpRequestPacket, HttpResponsePacket, HttpRequestPacket, FilterChainContext> {
 
   public static final GrizzlyHttpServerTracer TRACER = new GrizzlyHttpServerTracer();
 
@@ -39,6 +41,11 @@ public class GrizzlyHttpServerTracer
     return httpRequestPacket.getHeader(name);
   }
 
+  @Override
+  protected int responseStatus(HttpResponsePacket httpResponsePacket) {
+    return httpResponsePacket.getStatus();
+  }
+
   @Override
   protected void attachServerContext(Context context, FilterChainContext filterChainContext) {
     filterChainContext.getAttributes().setAttribute(CONTEXT_ATTRIBUTE, context);

instrumentation/grizzly-2.0/src/main/java/io/opentelemetry/auto/instrumentation/grizzly/HttpServerFilterAdvice.java

@@ -30,7 +30,7 @@ public class HttpServerFilterAdvice {
       @Advice.Argument(2) final HttpResponsePacket response) {
     Span span = TRACER.getServerSpan(ctx);
     if (span != null) {
-      TRACER.end(span, response.getStatus());
+      TRACER.end(span, response);
     }
   }
 }

instrumentation/jetty-8.0/src/main/java/io/opentelemetry/auto/instrumentation/jetty/JettyHandlerAdvice.java

@@ -76,7 +76,7 @@ public class JettyHandlerAdvice {
     TRACER.setPrincipal(span, request);
 
     if (throwable != null) {
-      TRACER.endExceptionally(span, throwable, response.getStatus());
+      TRACER.endExceptionally(span, throwable, response);
       return;
     }
 
@@ -94,8 +94,7 @@ public class JettyHandlerAdvice {
 
     // Check again in case the request finished before adding the listener.
     if (!request.isAsyncStarted() && responseHandled.compareAndSet(false, true)) {
-      JettyHttpServerTracer.contentLengthHelper(span, response);
+      TRACER.end(span, response);
-      TRACER.end(span, response.getStatus());
     }
   }
 }

instrumentation/jetty-8.0/src/main/java/io/opentelemetry/auto/instrumentation/jetty/JettyHttpServerTracer.java

@@ -16,20 +16,11 @@
 
 package io.opentelemetry.auto.instrumentation.jetty;
 
-import io.opentelemetry.auto.instrumentation.servlet.v3_0.CountingHttpServletResponse;
 import io.opentelemetry.auto.instrumentation.servlet.v3_0.Servlet3HttpServerTracer;
-import io.opentelemetry.trace.Span;
-import javax.servlet.ServletResponse;
 
 public class JettyHttpServerTracer extends Servlet3HttpServerTracer {
   public static final JettyHttpServerTracer TRACER = new JettyHttpServerTracer();
 
-  public static void contentLengthHelper(Span span, ServletResponse response) {
-    if (response instanceof CountingHttpServletResponse) {
-      TRACER.setContentLength(span, ((CountingHttpServletResponse) response).getContentLength());
-    }
-  }
-
   @Override
   protected String getInstrumentationName() {
     return "io.opentelemetry.auto.jetty-8.0";

instrumentation/netty/netty-3.8/src/main/java/io/opentelemetry/auto/instrumentation/netty/v3_8/server/HttpServerResponseTracingHandler.java

@@ -58,6 +58,6 @@ public class HttpServerResponseTracingHandler extends SimpleChannelDownstreamHan
       TRACER.endExceptionally(span, throwable);
       throw throwable;
     }
-    TRACER.end(span, ((HttpResponse) msg.getMessage()).getStatus().getCode());
+    TRACER.end(span, (HttpResponse) msg.getMessage());
   }
 }

instrumentation/netty/netty-3.8/src/main/java/io/opentelemetry/auto/instrumentation/netty/v3_8/server/NettyHttpServerTracer.java

@@ -28,9 +28,10 @@ import java.net.URI;
 import java.net.URISyntaxException;
 import org.jboss.netty.channel.Channel;
 import org.jboss.netty.handler.codec.http.HttpRequest;
+import org.jboss.netty.handler.codec.http.HttpResponse;
 
 public class NettyHttpServerTracer
-    extends HttpServerTracer<HttpRequest, Channel, ChannelTraceContext> {
+    extends HttpServerTracer<HttpRequest, HttpResponse, Channel, ChannelTraceContext> {
   public static final NettyHttpServerTracer TRACER = new NettyHttpServerTracer();
 
   @Override
@@ -43,6 +44,11 @@ public class NettyHttpServerTracer
     return httpRequest.headers().get(name);
   }
 
+  @Override
+  protected int responseStatus(HttpResponse httpResponse) {
+    return httpResponse.getStatus().getCode();
+  }
+
   @Override
   protected void attachServerContext(Context context, ChannelTraceContext channelTraceContext) {
     channelTraceContext.setContext(context);

instrumentation/netty/netty-4.0/src/main/java/io/opentelemetry/auto/instrumentation/netty/v4_0/server/HttpServerResponseTracingHandler.java

@@ -45,6 +45,6 @@ public class HttpServerResponseTracingHandler extends ChannelOutboundHandlerAdap
       TRACER.endExceptionally(span, throwable);
       throw throwable;
     }
-    TRACER.end(span, ((HttpResponse) msg).getStatus().code());
+    TRACER.end(span, (HttpResponse) msg);
   }
 }

instrumentation/netty/netty-4.0/src/main/java/io/opentelemetry/auto/instrumentation/netty/v4_0/server/NettyHttpServerTracer.java

@@ -21,6 +21,7 @@ import static io.netty.handler.codec.http.HttpHeaders.Names.HOST;
 import io.grpc.Context;
 import io.netty.channel.Channel;
 import io.netty.handler.codec.http.HttpRequest;
+import io.netty.handler.codec.http.HttpResponse;
 import io.opentelemetry.auto.bootstrap.instrumentation.decorator.HttpServerTracer;
 import io.opentelemetry.auto.instrumentation.netty.v4_0.AttributeKeys;
 import io.opentelemetry.context.propagation.HttpTextFormat.Getter;
@@ -29,7 +30,8 @@ import java.net.SocketAddress;
 import java.net.URI;
 import java.net.URISyntaxException;
 
-public class NettyHttpServerTracer extends HttpServerTracer<HttpRequest, Channel, Channel> {
+public class NettyHttpServerTracer
+    extends HttpServerTracer<HttpRequest, HttpResponse, Channel, Channel> {
   public static final NettyHttpServerTracer TRACER = new NettyHttpServerTracer();
 
   @Override
@@ -42,6 +44,11 @@ public class NettyHttpServerTracer extends HttpServerTracer<HttpRequest, Channel
     return httpRequest.headers().get(name);
   }
 
+  @Override
+  protected int responseStatus(HttpResponse httpResponse) {
+    return httpResponse.getStatus().code();
+  }
+
   @Override
   protected void attachServerContext(Context context, Channel channel) {
     channel.attr(AttributeKeys.SERVER_ATTRIBUTE_KEY).set(context);

instrumentation/netty/netty-4.1/src/main/java/io/opentelemetry/auto/instrumentation/netty/v4_1/server/HttpServerResponseTracingHandler.java

@@ -45,6 +45,6 @@ public class HttpServerResponseTracingHandler extends ChannelOutboundHandlerAdap
       TRACER.endExceptionally(span, throwable);
       throw throwable;
     }
-    TRACER.end(span, ((HttpResponse) msg).status().code());
+    TRACER.end(span, (HttpResponse) msg);
   }
 }

instrumentation/netty/netty-4.1/src/main/java/io/opentelemetry/auto/instrumentation/netty/v4_1/server/NettyHttpServerTracer.java

@@ -21,6 +21,7 @@ import static io.netty.handler.codec.http.HttpHeaderNames.HOST;
 import io.grpc.Context;
 import io.netty.channel.Channel;
 import io.netty.handler.codec.http.HttpRequest;
+import io.netty.handler.codec.http.HttpResponse;
 import io.opentelemetry.auto.bootstrap.instrumentation.decorator.HttpServerTracer;
 import io.opentelemetry.auto.instrumentation.netty.v4_1.AttributeKeys;
 import io.opentelemetry.context.propagation.HttpTextFormat.Getter;
@@ -29,7 +30,8 @@ import java.net.SocketAddress;
 import java.net.URI;
 import java.net.URISyntaxException;
 
-public class NettyHttpServerTracer extends HttpServerTracer<HttpRequest, Channel, Channel> {
+public class NettyHttpServerTracer
+    extends HttpServerTracer<HttpRequest, HttpResponse, Channel, Channel> {
   public static final NettyHttpServerTracer TRACER = new NettyHttpServerTracer();
 
   @Override
@@ -42,6 +44,11 @@ public class NettyHttpServerTracer extends HttpServerTracer<HttpRequest, Channel
     return httpRequest.headers().get(name);
   }
 
+  @Override
+  protected int responseStatus(HttpResponse httpResponse) {
+    return httpResponse.status().code();
+  }
+
   @Override
   protected void attachServerContext(Context context, Channel channel) {
     channel.attr(AttributeKeys.SERVER_ATTRIBUTE_KEY).set(context);

instrumentation/servlet/servlet-2.2/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_2/ResponseWithStatus.java

@@ -0,0 +1,38 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.opentelemetry.auto.instrumentation.servlet.v2_2;
+
+import javax.servlet.http.HttpServletResponse;
+
+public class ResponseWithStatus {
+
+  private final HttpServletResponse response;
+  private final int status;
+
+  public ResponseWithStatus(HttpServletResponse response, int status) {
+    this.response = response;
+    this.status = status;
+  }
+
+  HttpServletResponse getResponse() {
+    return response;
+  }
+
+  int getStatus() {
+    return status;
+  }
+}

instrumentation/servlet/servlet-2.2/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_2/Servlet2Advice.java

@@ -69,10 +69,12 @@ public class Servlet2Advice {
     Integer responseStatus =
         InstrumentationContext.get(ServletResponse.class, Integer.class).get(response);
 
+    ResponseWithStatus responseWithStatus =
+        new ResponseWithStatus((HttpServletResponse) response, responseStatus);
     if (throwable == null) {
-      TRACER.end(span, responseStatus);
+      TRACER.end(span, responseWithStatus);
     } else {
-      TRACER.endExceptionally(span, throwable, responseStatus);
+      TRACER.endExceptionally(span, throwable, responseWithStatus);
     }
   }
 }

instrumentation/servlet/servlet-2.2/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_2/Servlet2HttpServerTracer.java

@@ -18,10 +18,15 @@ package io.opentelemetry.auto.instrumentation.servlet.v2_2;
 
 import io.opentelemetry.instrumentation.servlet.ServletHttpServerTracer;
 
-public class Servlet2HttpServerTracer extends ServletHttpServerTracer {
+public class Servlet2HttpServerTracer extends ServletHttpServerTracer<ResponseWithStatus> {
   public static final Servlet2HttpServerTracer TRACER = new Servlet2HttpServerTracer();
 
   protected String getInstrumentationName() {
     return "io.opentelemetry.auto.servlet-2.2";
   }
+
+  @Override
+  protected int responseStatus(ResponseWithStatus responseWithStatus) {
+    return responseWithStatus.getStatus();
+  }
 }

instrumentation/servlet/servlet-2.2/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_2/Servlet2Instrumentation.java

@@ -63,6 +63,7 @@ public final class Servlet2Instrumentation extends Instrumenter.Default {
     return new String[] {
       "io.opentelemetry.instrumentation.servlet.ServletHttpServerTracer",
       "io.opentelemetry.instrumentation.servlet.HttpServletRequestGetter",
+      packageName + ".ResponseWithStatus",
       packageName + ".Servlet2HttpServerTracer"
     };
   }

instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v3_0/Servlet3Advice.java

@@ -83,8 +83,7 @@ public class Servlet3Advice {
 
     TRACER.setPrincipal(span, (HttpServletRequest) request);
     if (throwable != null) {
-      contentLengthHelper(span, response);
+      TRACER.endExceptionally(span, throwable, (HttpServletResponse) response);
-      TRACER.endExceptionally(span, throwable, ((HttpServletResponse) response).getStatus());
       return;
     }
 
@@ -102,14 +101,7 @@ public class Servlet3Advice {
 
     // Check again in case the request finished before adding the listener.
     if (!request.isAsyncStarted() && responseHandled.compareAndSet(false, true)) {
-      contentLengthHelper(span, response);
+      TRACER.end(span, (HttpServletResponse) response);
-      TRACER.end(span, ((HttpServletResponse) response).getStatus());
-    }
-  }
-
-  public static void contentLengthHelper(Span span, ServletResponse response) {
-    if (response instanceof CountingHttpServletResponse) {
-      TRACER.setContentLength(span, ((CountingHttpServletResponse) response).getContentLength());
     }
   }
 }

instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v3_0/Servlet3HttpServerTracer.java

@@ -22,9 +22,11 @@ import io.grpc.Context;
 import io.opentelemetry.instrumentation.servlet.ServletHttpServerTracer;
 import io.opentelemetry.trace.Span;
 import io.opentelemetry.trace.Status;
+import io.opentelemetry.trace.attributes.SemanticAttributes;
 import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 
-public class Servlet3HttpServerTracer extends ServletHttpServerTracer {
+public class Servlet3HttpServerTracer extends ServletHttpServerTracer<HttpServletResponse> {
 
   public static final Servlet3HttpServerTracer TRACER = new Servlet3HttpServerTracer();
 
@@ -38,12 +40,45 @@ public class Servlet3HttpServerTracer extends ServletHttpServerTracer {
     return connection.getRemotePort();
   }
 
+  @Override
+  protected int responseStatus(HttpServletResponse httpServletResponse) {
+    return httpServletResponse.getStatus();
+  }
+
+  @Override
+  public void endExceptionally(
+      Span span, Throwable throwable, HttpServletResponse response, long timestamp) {
+    captureContentLength(span, response);
+    if (response.isCommitted()) {
+      super.endExceptionally(span, throwable, response, timestamp);
+    } else {
+      // passing null response to super, in order to capture as 500 / INTERNAL, due to servlet spec
+      // https://javaee.github.io/servlet-spec/downloads/servlet-4.0/servlet-4_0_FINAL.pdf:
+      // "If a servlet generates an error that is not handled by the error page mechanism as
+      // described above, the container must ensure to send a response with status 500."
+      super.endExceptionally(span, throwable, null, timestamp);
+    }
+  }
+
+  @Override
+  public void end(Span span, HttpServletResponse response, long timestamp) {
+    captureContentLength(span, response);
+    super.end(span, response, timestamp);
+  }
+
   public void onTimeout(Span span, long timeout) {
     span.setStatus(Status.DEADLINE_EXCEEDED);
     span.setAttribute("timeout", timeout);
     span.end();
   }
 
+  private static void captureContentLength(Span span, HttpServletResponse response) {
+    if (response instanceof CountingHttpServletResponse) {
+      SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.set(
+          span, ((CountingHttpServletResponse) response).getContentLength());
+    }
+  }
+
   /*
   Given request already has a context associated with it.
   As there should not be nested spans of kind SERVER, we should NOT create a new span here.
@@ -56,7 +91,7 @@ public class Servlet3HttpServerTracer extends ServletHttpServerTracer {
 
   In this case we have to put the span from the request into current context before continuing.
   */
-  public boolean needsRescoping(Context attachedContext) {
+  public static boolean needsRescoping(Context attachedContext) {
     return !sameTrace(getSpan(Context.current()), getSpan(attachedContext));
   }
 

instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v3_0/TagSettingAsyncListener.java

@@ -20,7 +20,6 @@ import io.opentelemetry.trace.Span;
 import java.util.concurrent.atomic.AtomicBoolean;
 import javax.servlet.AsyncEvent;
 import javax.servlet.AsyncListener;
-import javax.servlet.ServletResponse;
 import javax.servlet.http.HttpServletResponse;
 
 public class TagSettingAsyncListener implements AsyncListener {
@@ -38,16 +37,13 @@ public class TagSettingAsyncListener implements AsyncListener {
   @Override
   public void onComplete(final AsyncEvent event) {
     if (responseHandled.compareAndSet(false, true)) {
-      contentLengthHelper(span, event);
+      servletHttpServerTracer.end(span, (HttpServletResponse) event.getSuppliedResponse());
-      servletHttpServerTracer.end(
-          span, ((HttpServletResponse) event.getSuppliedResponse()).getStatus());
     }
   }
 
   @Override
   public void onTimeout(final AsyncEvent event) {
     if (responseHandled.compareAndSet(false, true)) {
-      contentLengthHelper(span, event);
       servletHttpServerTracer.onTimeout(span, event.getAsyncContext().getTimeout());
     }
   }
@@ -55,11 +51,8 @@ public class TagSettingAsyncListener implements AsyncListener {
   @Override
   public void onError(final AsyncEvent event) {
     if (responseHandled.compareAndSet(false, true)) {
-      contentLengthHelper(span, event);
       servletHttpServerTracer.endExceptionally(
-          span,
+          span, event.getThrowable(), (HttpServletResponse) event.getSuppliedResponse());
-          event.getThrowable(),
-          ((HttpServletResponse) event.getSuppliedResponse()).getStatus());
     }
   }
 
@@ -68,12 +61,4 @@ public class TagSettingAsyncListener implements AsyncListener {
   public void onStartAsync(final AsyncEvent event) {
     event.getAsyncContext().addListener(this);
   }
-
-  public static void contentLengthHelper(Span span, AsyncEvent event) {
-    ServletResponse response = event.getSuppliedResponse();
-    if (response instanceof CountingHttpServletResponse) {
-      servletHttpServerTracer.setContentLength(
-          span, ((CountingHttpServletResponse) response).getContentLength());
-    }
-  }
 }