Code review sweep (run 24918537506) (open-telemetry#18281)

Author: otelbot[bot]

instrumentation/tomcat/tomcat-7.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/Tomcat7AttachResponseAdvice.java

@@ -6,14 +6,23 @@
 package io.opentelemetry.javaagent.instrumentation.tomcat.v7_0;
 
 import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
+import static io.opentelemetry.javaagent.instrumentation.tomcat.v7_0.Tomcat7Singletons.helper;
 import static java.util.Collections.singletonList;
 
 import com.google.auto.service.AutoService;
+import io.opentelemetry.context.Context;
+import io.opentelemetry.context.Scope;
+import io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge;
+import io.opentelemetry.javaagent.bootstrap.http.HttpServerResponseCustomizerHolder;
 import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import io.opentelemetry.javaagent.instrumentation.tomcat.common.TomcatServerHandlerInstrumentation;
 import java.util.List;
+import javax.annotation.Nullable;
+import net.bytebuddy.asm.Advice;
 import net.bytebuddy.matcher.ElementMatcher;
+import org.apache.coyote.Request;
+import org.apache.coyote.Response;
 
 @AutoService(InstrumentationModule.class)
 public class Tomcat7InstrumentationModule extends InstrumentationModule {
@@ -24,19 +33,84 @@ public Tomcat7InstrumentationModule() {
 
   @Override
   public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
-    // does not match tomcat 10.0+
     return hasClassesNamed(
-        "javax.servlet.http.HttpServletRequest", "org.apache.catalina.loader.Constants");
+        // removed in Servlet 5.0 (renamed to jakarta.servlet)
+        "javax.servlet.http.HttpServletRequest",
+        // removed in 10.0
+        "org.apache.catalina.loader.Constants");
   }
 
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
-    // Tomcat 10+ is excluded by making sure Request does not have any methods returning
-    // jakarta.servlet.ReadListener which is returned by getReadListener method on Tomcat 10+
-    String packageName = Tomcat7InstrumentationModule.class.getPackage().getName();
     return singletonList(
         new TomcatServerHandlerInstrumentation(
-            packageName + ".Tomcat7ServerHandlerAdvice",
-            packageName + ".Tomcat7AttachResponseAdvice"));
+            getClass().getName() + "$Tomcat7ServerHandlerAdvice",
+            getClass().getName() + "$Tomcat7AttachResponseAdvice"));
+  }
+
+  @SuppressWarnings("unused")
+  public static class Tomcat7AttachResponseAdvice {
+
+    @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class, inline = false)
+    public static void attachResponse(
+        @Advice.Argument(2) Response response, @Advice.Return boolean success) {
+
+      if (success) {
+        helper().attachResponseToRequest(Java8BytecodeBridge.currentContext(), response);
+      }
+    }
+  }
+
+  @SuppressWarnings("unused")
+  public static class Tomcat7ServerHandlerAdvice {
+
+    public static class AdviceScope {
+      private final Context context;
+      private final Scope scope;
+
+      private AdviceScope(Context context, Scope scope) {
+        this.context = context;
+        this.scope = scope;
+      }
+
+      @Nullable
+      public static AdviceScope start(Request request, Response response) {
+        Context parentContext = Context.current();
+        if (!helper().shouldStart(parentContext, request)) {
+          return null;
+        }
+
+        Context context = helper().start(parentContext, request);
+
+        Scope scope = context.makeCurrent();
+
+        HttpServerResponseCustomizerHolder.getCustomizer()
+            .customize(context, response, Tomcat7ResponseMutator.INSTANCE);
+
+        return new AdviceScope(context, scope);
+      }
+
+      public void end(Request request, Response response, Throwable throwable) {
+        helper().end(request, response, throwable, context, scope);
+      }
+    }
+
+    @Nullable
+    @Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
+    public static AdviceScope onEnter(
+        @Advice.Argument(0) Request request, @Advice.Argument(1) Response response) {
+      return AdviceScope.start(request, response);
+    }
+
+    @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class, inline = false)
+    public static void stopSpan(
+        @Advice.Argument(0) Request request,
+        @Advice.Argument(1) Response response,
+        @Advice.Thrown @Nullable Throwable throwable,
+        @Advice.Enter @Nullable AdviceScope adviceScope) {
+      if (adviceScope != null) {
+        adviceScope.end(request, response, throwable);
+      }
+    }
   }
 }

instrumentation/tomcat/tomcat-7.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/Tomcat7InstrumentationModule.java

@@ -18,7 +18,8 @@ protected void report(Request request, Response response, Throwable t) {
       return;
     }
     try {
-      response.getWriter().print(t != null ? t.getCause().getMessage() : response.getMessage());
+      Throwable cause = t != null ? t.getCause() : null;
+      response.getWriter().print(cause != null ? cause.getMessage() : response.getMessage());
     } catch (IOException ignored) {
       // Ignore exception when writing exception message to response fails on IO - same as is done
       // by the superclass itself and by other built-in ErrorReportValve implementations.

instrumentation/tomcat/tomcat-7.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/Tomcat7ServerHandlerAdvice.java

@@ -147,7 +147,7 @@ public SpanFinishingCallback(Context context, String spanName) {
     }
 
     @Override
-    public void onSuccess(Object result) {
+    public void onSuccess(T result) {
       instrumenter().end(context, spanName, result, null);
     }
 

instrumentation/tomcat/tomcat-7.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/ErrorHandlerValve.java

@@ -92,7 +92,7 @@ private static void setTagIfPresent(
         attributes.put(tag, value.toString());
       }
 
-    } catch (Exception e) {
+    } catch (Exception ignored) {
       // Expected that this won't work for all result types
     }
   }

instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/v6_6/TwilioAsyncInstrumentation.java

@@ -5,6 +5,7 @@ description: >
 library_link: https://github.com/twilio/twilio-java
 configurations:
   - name: otel.instrumentation.twilio.experimental-span-attributes
+    declarative_name: java.twilio.experimental_span_attributes/development
     description: >
       Enables experimental span attributes `twilio.type`, `twilio.account`, `twilio.sid`, and
       `twilio.status`.

instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/v6_6/TwilioExperimentalAttributesExtractor.java

@@ -15,7 +15,7 @@
 import java.util.List;
 import javax.annotation.Nullable;
 
-public class UndertowHttpAttributesGetter
+class UndertowHttpAttributesGetter
     implements HttpServerAttributesGetter<HttpServerExchange, HttpServerExchange> {
 
   @Override

instrumentation/twilio-6.6/metadata.yaml

@@ -6,20 +6,24 @@ semantic_conventions:
   - HTTP_SERVER_METRICS
 configurations:
   - name: otel.instrumentation.http.known-methods
+    declarative_name: java.common.http.known_methods
     description: >
       Configures the instrumentation to recognize an alternative set of HTTP request methods. All
       other methods will be treated as `_OTHER`.
     type: list
     default: "CONNECT,DELETE,GET,HEAD,OPTIONS,PATCH,POST,PUT,TRACE"
   - name: otel.instrumentation.http.server.capture-request-headers
+    declarative_name: general.http.server.request_captured_headers
     description: List of HTTP request headers to capture in HTTP server telemetry.
     type: list
     default: ""
   - name: otel.instrumentation.http.server.capture-response-headers
+    declarative_name: general.http.server.response_captured_headers
     description: List of HTTP response headers to capture in HTTP server telemetry.
     type: list
     default: ""
   - name: otel.instrumentation.http.server.emit-experimental-telemetry
+    declarative_name: java.common.http.server.emit_experimental_telemetry/development
     description: >
       Enable the capture of experimental HTTP server telemetry. Adds the `http.request.body.size`
       and `http.response.body.size` attributes to spans, and records `http.server.request.body.size`

instrumentation/undertow-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/undertow/v1_4/UndertowHttpAttributesGetter.java

@@ -5,9 +5,11 @@
 
 package io.opentelemetry.javaagent.instrumentation.vaadin.v14_2;
 
+import javax.annotation.Nullable;
+
 public class VaadinServiceContext {
   private boolean requestHandled;
-  private String spanNameCandidate;
+  @Nullable private String spanNameCandidate;
 
   void setRequestHandled() {
     requestHandled = true;
@@ -21,6 +23,7 @@ void setSpanNameCandidate(String spanNameCandidate) {
     this.spanNameCandidate = spanNameCandidate;
   }
 
+  @Nullable
   String getSpanNameCandidate() {
     return spanNameCandidate;
   }