Code review sweep (run 24942513837) (#18299)

Co-authored-by: otelbot <197425009+otelbot@users.noreply.github.com>
Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>

Author: otelbot[bot]

instrumentation/apache-httpclient/apache-httpclient-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v2_0/ApacheHttpClientSingletons.java

@@ -9,7 +9,7 @@
 import io.opentelemetry.javaagent.bootstrap.internal.JavaagentHttpClientInstrumenters;
 import org.apache.commons.httpclient.HttpMethod;
 
-public class ApacheHttpClientSingletons {
+class ApacheHttpClientSingletons {
   private static final String INSTRUMENTATION_NAME = "io.opentelemetry.apache-httpclient-2.0";
 
   private static final Instrumenter<HttpMethod, HttpMethod> instrumenter;
@@ -22,7 +22,7 @@ public class ApacheHttpClientSingletons {
             new HttpHeaderSetter());
   }
 
-  public static Instrumenter<HttpMethod, HttpMethod> instrumenter() {
+  static Instrumenter<HttpMethod, HttpMethod> instrumenter() {
     return instrumenter;
   }
 

instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientInstrumentation.java

@@ -46,8 +46,8 @@ public ElementMatcher<TypeDescription> typeMatcher() {
   public void transform(TypeTransformer transformer) {
     // There are 8 execute(...) methods.  Depending on the version, they may or may not delegate
     // to each other. Thus, all methods need to be instrumented.  Because of argument position and
-    // type, some methods can share the same advice class.  The call depth tracking ensures only 1
-    // span is created
+    // type, some methods can share the same advice class. Span suppression ensures only one span
+    // is created.
 
     transformer.applyAdviceToMethod(
         named("execute")
@@ -161,6 +161,7 @@ public void end(@Nullable Object result, @Nullable Throwable throwable) {
   @SuppressWarnings("unused")
   public static class UriRequestAdvice {
 
+    @Nullable
     @Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
     public static AdviceScope methodEnter(@Advice.Argument(0) HttpUriRequest request) {
       return AdviceScope.start(new ApacheHttpClientRequest(request));
@@ -210,9 +211,10 @@ public static void methodExit(
   @SuppressWarnings("unused")
   public static class RequestAdvice {
 
+    @Nullable
     @Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
     public static AdviceScope methodEnter(
-        @Advice.Argument(0) HttpHost host, @Advice.Argument(1) HttpRequest request) {
+        @Advice.Argument(0) @Nullable HttpHost host, @Advice.Argument(1) HttpRequest request) {
       return AdviceScope.start(new ApacheHttpClientRequest(host, request));
     }
 
@@ -234,7 +236,7 @@ public static class RequestWithHandlerAdvice {
     @AssignReturned.ToArguments(@ToArgument(value = 2, index = 1))
     @Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
     public static Object[] methodEnter(
-        @Advice.Argument(0) HttpHost host,
+        @Advice.Argument(0) @Nullable HttpHost host,
         @Advice.Argument(1) HttpRequest request,
         @Advice.Argument(2) ResponseHandler<?> handler) {
 

instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientRequest.java

@@ -29,7 +29,7 @@ public class ApacheHttpClientRequest {
   private final HttpRequest delegate;
   @Nullable private final HttpHost target;
 
-  public ApacheHttpClientRequest(HttpHost httpHost, HttpRequest httpRequest) {
+  public ApacheHttpClientRequest(@Nullable HttpHost httpHost, HttpRequest httpRequest) {
     URI calculatedUri = getUri(httpRequest);
     if (calculatedUri != null && httpHost != null) {
       uri = getCalculatedUri(httpHost, calculatedUri);
@@ -46,7 +46,7 @@ public ApacheHttpClientRequest(HttpUriRequest httpRequest) {
     target = null;
   }
 
-  public List<String> getHeader(String name) {
+  List<String> getHeader(String name) {
     return headersToList(delegate.getHeaders(name));
   }
 
@@ -62,16 +62,16 @@ static List<String> headersToList(Header[] headers) {
     return headersList;
   }
 
-  public void setHeader(String name, String value) {
+  void setHeader(String name, String value) {
     delegate.setHeader(name, value);
   }
 
-  public String getMethod() {
+  String getMethod() {
     return delegate.getRequestLine().getMethod();
   }
 
   @Nullable
-  public String getUrl() {
+  String getUrl() {
     return uri != null ? uri.toString() : null;
   }
 
@@ -88,7 +88,7 @@ String getProtocolVersion() {
   }
 
   @Nullable
-  public String getServerAddress() {
+  String getServerAddress() {
     if (uri != null) {
       return uri.getHost();
     }
@@ -99,7 +99,7 @@ public String getServerAddress() {
   }
 
   @Nullable
-  public Integer getServerPort() {
+  Integer getServerPort() {
     if (uri != null) {
       return uri.getPort();
     }
@@ -110,7 +110,7 @@ public Integer getServerPort() {
   }
 
   @Nullable
-  public String getScheme() {
+  String getScheme() {
     if (uri != null) {
       return uri.getScheme();
     }

instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/WrappingStatusSettingResponseHandler.java

@@ -13,13 +13,13 @@
 import org.apache.http.HttpResponse;
 import org.apache.http.client.ResponseHandler;
 
-public class WrappingStatusSettingResponseHandler<T> implements ResponseHandler<T> {
+class WrappingStatusSettingResponseHandler<T> implements ResponseHandler<T> {
   private final Context context;
   private final Context parentContext;
   private final ApacheHttpClientRequest request;
   private final ResponseHandler<T> handler;
 
-  public WrappingStatusSettingResponseHandler(
+  WrappingStatusSettingResponseHandler(
       Context context,
       Context parentContext,
       ApacheHttpClientRequest request,

instrumentation/apache-httpclient/apache-httpclient-4.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientTest.java

@@ -76,13 +76,13 @@ public ClientConnectionManager newInstance(
   @Nested
   class ApacheClientHostRequestTest extends AbstractTest<BasicHttpRequest> {
     @Override
-    public BasicHttpRequest createRequest(String method, URI uri) {
+    BasicHttpRequest createRequest(String method, URI uri) {
       // also testing with an absolute path below
       return new BasicHttpRequest(method, fullPathFromUri(uri));
     }
 
     @Override
-    public HttpResponse doExecuteRequest(BasicHttpRequest request, URI uri) throws Exception {
+    HttpResponse doExecuteRequest(BasicHttpRequest request, URI uri) throws Exception {
       return getClient(uri).execute(getHost(uri), request);
     }
 

instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientInstrumentation.java

@@ -202,7 +202,6 @@ public static Object[] methodEnter(
 
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class, inline = false)
     public static void methodExit(
-        @Advice.Argument(0) ClassicHttpRequest request,
         @Advice.Return @Nullable Object result,
         @Advice.Thrown @Nullable Throwable throwable,
         @Advice.Enter Object[] enterResult) {
@@ -238,7 +237,6 @@ public static Object[] methodEnter(
 
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class, inline = false)
     public static void methodExit(
-        @Advice.Argument(0) ClassicHttpRequest request,
         @Advice.Return @Nullable Object result,
         @Advice.Thrown @Nullable Throwable throwable,
         @Advice.Enter Object[] enterResult) {

instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientSingletons.java

@@ -10,7 +10,7 @@
 import org.apache.hc.core5.http.HttpRequest;
 import org.apache.hc.core5.http.HttpResponse;
 
-public class ApacheHttpClientSingletons {
+class ApacheHttpClientSingletons {
   private static final String INSTRUMENTATION_NAME = "io.opentelemetry.apache-httpclient-5.0";
 
   private static final Instrumenter<HttpRequest, HttpResponse> instrumenter;
@@ -23,7 +23,7 @@ public class ApacheHttpClientSingletons {
             new HttpHeaderSetter());
   }
 
-  public static Instrumenter<HttpRequest, HttpResponse> instrumenter() {
+  static Instrumenter<HttpRequest, HttpResponse> instrumenter() {
     return instrumenter;
   }
 

instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/WrappingStatusSettingResponseHandler.java

@@ -15,13 +15,13 @@
 import org.apache.hc.core5.http.HttpException;
 import org.apache.hc.core5.http.io.HttpClientResponseHandler;
 
-public class WrappingStatusSettingResponseHandler<T> implements HttpClientResponseHandler<T> {
+class WrappingStatusSettingResponseHandler<T> implements HttpClientResponseHandler<T> {
   private final Context context;
   private final Context parentContext;
   private final ClassicHttpRequest request;
   private final HttpClientResponseHandler<T> handler;
 
-  public WrappingStatusSettingResponseHandler(
+  WrappingStatusSettingResponseHandler(
       Context context,
       Context parentContext,
       ClassicHttpRequest request,

instrumentation/apache-httpclient/apache-httpclient-5.2/library/src/main/java/io/opentelemetry/instrumentation/apachehttpclient/v5_2/OtelExecChainHandler.java

@@ -70,9 +70,9 @@ private ClassicHttpResponse execute(
     try (io.opentelemetry.context.Scope ignored = context.makeCurrent()) {
       response = chain.proceed(request, scope);
       return response;
-    } catch (Exception e) {
-      error = e;
-      throw e;
+    } catch (Throwable t) {
+      error = t;
+      throw t;
     } finally {
       instrumenter.end(context, instrumenterRequest, response, error);
     }

instrumentation/apache-shenyu-2.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apacheshenyu/v2_4/ContextBuilderInstrumentation.java

@@ -5,6 +5,7 @@
 
 package io.opentelemetry.javaagent.instrumentation.apacheshenyu.v2_4;
 
+import static io.opentelemetry.javaagent.instrumentation.apacheshenyu.v2_4.ApacheShenYuSingletons.httpRouteGetter;
 import static net.bytebuddy.matcher.ElementMatchers.isPublic;
 import static net.bytebuddy.matcher.ElementMatchers.named;
 import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
@@ -46,10 +47,7 @@ public static void onExit(@Advice.Argument(0) ServerWebExchange exchange) {
         return;
       }
       HttpServerRoute.update(
-          context,
-          HttpServerRouteSource.NESTED_CONTROLLER,
-          ApacheShenYuSingletons.httpRouteGetter(),
-          metaData);
+          context, HttpServerRouteSource.NESTED_CONTROLLER, httpRouteGetter(), metaData);
       MetaDataHelper.extractAttributes(metaData, context);
     }
   }