Code review sweep (run 24974154176)

instrumentation/jaxws/jaxws-metro-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/metro/MetroHelper.java

@@ -18,7 +18,7 @@ public class MetroHelper {
   private static final String SCOPE_KEY = MetroHelper.class.getName() + ".Scope";
   private static final String THROWABLE_KEY = MetroHelper.class.getName() + ".Throwable";
 
-  private static final MetroServerSpanNameUpdater SPAN_NAME_UPDATER =
+  private static final MetroServerSpanNameUpdater spanNameUpdater =
       new MetroServerSpanNameUpdater();
 
   private MetroHelper() {}
@@ -27,7 +27,7 @@ public static void start(WSEndpoint<?> endpoint, Packet packet) {
     Context parentContext = Context.current();
 
     MetroRequest request = new MetroRequest(endpoint, packet);
-    SPAN_NAME_UPDATER.updateServerSpanName(parentContext, request);
+    spanNameUpdater.updateServerSpanName(parentContext, request);
 
     if (!instrumenter().shouldStart(parentContext, request)) {
       return;

instrumentation/jaxws/jaxws-metro-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/metro/MetroRequest.java

@@ -9,20 +9,20 @@
 import com.sun.xml.ws.api.server.WSEndpoint;
 import javax.xml.namespace.QName;
 
-public class MetroRequest {
+class MetroRequest {
   private final Packet packet;
   private final String spanName;
 
-  public MetroRequest(WSEndpoint<?> endpoint, Packet packet) {
+  MetroRequest(WSEndpoint<?> endpoint, Packet packet) {
     this.packet = packet;
     this.spanName = getSpanName(endpoint, packet);
   }
 
-  public String spanName() {
+  String spanName() {
     return spanName;
   }
 
-  public Packet packet() {
+  Packet packet() {
     return packet;
   }
 

instrumentation/jaxws/jaxws-metro-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/metro/MetroServerSpanNameUpdater.java

@@ -39,7 +39,7 @@ final class MetroServerSpanNameUpdater {
    */
   private final Map<String, HttpServletRequestAdapter> servletRequestAdapters;
 
-  public MetroServerSpanNameUpdater() {
+  MetroServerSpanNameUpdater() {
     this.servletRequestAdapters = new LinkedHashMap<>();
 
     registerHttpServletRequestAdapter(
@@ -72,7 +72,7 @@ private void registerHttpServletRequestAdapter(
     logger.finest(() -> "Enabled " + name + " jaxws metro server span naming");
   }
 
-  public void updateServerSpanName(Context context, MetroRequest metroRequest) {
+  void updateServerSpanName(Context context, MetroRequest metroRequest) {
     String spanName = metroRequest.spanName();
 
     Span serverSpan = LocalRootSpan.fromContextOrNull(context);

instrumentation/jaxws/jaxws-metro-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/metro/MetroSingletons.java

@@ -9,7 +9,7 @@
 import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
 import io.opentelemetry.javaagent.bootstrap.internal.ExperimentalConfig;
 
-public class MetroSingletons {
+class MetroSingletons {
   private static final String INSTRUMENTATION_NAME = "io.opentelemetry.jaxws-metro-2.2";
 
   private static final Instrumenter<MetroRequest, Void> instrumenter;
@@ -22,7 +22,7 @@ public class MetroSingletons {
             .buildInstrumenter();
   }
 
-  public static Instrumenter<MetroRequest, Void> instrumenter() {
+  static Instrumenter<MetroRequest, Void> instrumenter() {
     return instrumenter;
   }
 

instrumentation/jboss-logmanager/jboss-logmanager-appender-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/appender/v1_1/LoggingEventMapper.java

@@ -32,12 +32,12 @@ public class LoggingEventMapper {
 
   private static final Cache<String, AttributeKey<String>> mdcAttributeKeys = Cache.bounded(100);
 
-  private final List<AttributeKey<String>> captureMdcAttributeKeys;
-
   private static final boolean captureExperimentalAttributes =
       DeclarativeConfigUtil.getInstrumentationConfig(GlobalOpenTelemetry.get(), "jboss_logmanager")
           .getBoolean("experimental_log_attributes/development", false);
 
+  private final List<AttributeKey<String>> captureMdcAttributeKeys;
+
   // cached as an optimization
   private final boolean captureAllMdcAttributes;
 

instrumentation/jboss-logmanager/jboss-logmanager-mdc-1.1/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/mdc/v1_1/JbossLogmanagerMdcTest.java

@@ -5,7 +5,7 @@
 
 package io.opentelemetry.javaagent.instrumentation.jbosslogmanager.mdc.v1_1;
 
-import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThat;
 
 import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
@@ -58,11 +58,12 @@ void noIdsGeneratedWhenNoSpanProvided() {
     try {
       logger.info("log message 1");
 
-      assertThat(logRecords).hasSize(1);
-      assertThat(logRecords.get(0).getMessage()).isEqualTo("log message 1");
-      assertThat(logRecords.get(0).getMdc("trace_id")).isNull();
-      assertThat(logRecords.get(0).getMdc("span_id")).isNull();
-      assertThat(logRecords.get(0).getMdc("trace_flags")).isNull();
+      assertThat(logRecords).extracting(ExtLogRecord::getMessage).containsExactly("log message 1");
+
+      ExtLogRecord logRecord = logRecords.get(0);
+      assertThat(logRecord.getMdc("trace_id")).isNull();
+      assertThat(logRecord.getMdc("span_id")).isNull();
+      assertThat(logRecord.getMdc("trace_flags")).isNull();
     } finally {
       logger.removeHandler(handler);
     }
@@ -94,46 +95,47 @@ void idsGeneratedWhenSpanProvided() throws ReflectiveOperationException {
                 return Span.current();
               });
 
-      assertThat(logRecords).hasSize(3);
+      assertThat(logRecords)
+          .extracting(ExtLogRecord::getMessage)
+          .containsExactly("log message 1", "log message 2", "log message 3");
+
+      ExtLogRecord firstLogRecord = logRecords.get(0);
+      ExtLogRecord secondLogRecord = logRecords.get(1);
+      ExtLogRecord thirdLogRecord = logRecords.get(2);
 
       Method getMdcCopy = null;
       try {
-        getMdcCopy = logRecords.get(0).getClass().getMethod("getMdcCopy");
+        getMdcCopy = firstLogRecord.getClass().getMethod("getMdcCopy");
       } catch (NoSuchMethodException ignored) {
         // ignored
       }
 
-      assertThat(logRecords.get(0).getMessage()).isEqualTo("log message 1");
-      assertThat(logRecords.get(0).getMdc("trace_id"))
-          .isEqualTo(span1.getSpanContext().getTraceId());
-      assertThat(logRecords.get(0).getMdc("span_id")).isEqualTo(span1.getSpanContext().getSpanId());
-      assertThat(logRecords.get(0).getMdc("trace_flags"))
+      assertThat(firstLogRecord.getMdc("trace_id")).isEqualTo(span1.getSpanContext().getTraceId());
+      assertThat(firstLogRecord.getMdc("span_id")).isEqualTo(span1.getSpanContext().getSpanId());
+      assertThat(firstLogRecord.getMdc("trace_flags"))
           .isEqualTo(span1.getSpanContext().getTraceFlags().asHex());
 
       if (getMdcCopy != null) {
         @SuppressWarnings("unchecked")
-        Map<String, String> copiedMdc = (Map<String, String>) getMdcCopy.invoke(logRecords.get(0));
+        Map<String, String> copiedMdc = (Map<String, String>) getMdcCopy.invoke(firstLogRecord);
         assertThat(copiedMdc.get("trace_id")).isEqualTo(span1.getSpanContext().getTraceId());
         assertThat(copiedMdc.get("span_id")).isEqualTo(span1.getSpanContext().getSpanId());
         assertThat(copiedMdc.get("trace_flags"))
             .isEqualTo(span1.getSpanContext().getTraceFlags().asHex());
       }
 
-      assertThat(logRecords.get(1).getMessage()).isEqualTo("log message 2");
-      assertThat(logRecords.get(1).getMdc("trace_id")).isNull();
-      assertThat(logRecords.get(1).getMdc("span_id")).isNull();
-      assertThat(logRecords.get(1).getMdc("trace_flags")).isNull();
+      assertThat(secondLogRecord.getMdc("trace_id")).isNull();
+      assertThat(secondLogRecord.getMdc("span_id")).isNull();
+      assertThat(secondLogRecord.getMdc("trace_flags")).isNull();
 
-      assertThat(logRecords.get(2).getMessage()).isEqualTo("log message 3");
-      assertThat(logRecords.get(2).getMdc("trace_id"))
-          .isEqualTo(span2.getSpanContext().getTraceId());
-      assertThat(logRecords.get(2).getMdc("span_id")).isEqualTo(span2.getSpanContext().getSpanId());
-      assertThat(logRecords.get(2).getMdc("trace_flags"))
+      assertThat(thirdLogRecord.getMdc("trace_id")).isEqualTo(span2.getSpanContext().getTraceId());
+      assertThat(thirdLogRecord.getMdc("span_id")).isEqualTo(span2.getSpanContext().getSpanId());
+      assertThat(thirdLogRecord.getMdc("trace_flags"))
           .isEqualTo(span2.getSpanContext().getTraceFlags().asHex());
 
       if (getMdcCopy != null) {
         @SuppressWarnings("unchecked")
-        Map<String, String> copiedMdc = (Map<String, String>) getMdcCopy.invoke(logRecords.get(2));
+        Map<String, String> copiedMdc = (Map<String, String>) getMdcCopy.invoke(thirdLogRecord);
         assertThat(copiedMdc.get("trace_id")).isEqualTo(span2.getSpanContext().getTraceId());
         assertThat(copiedMdc.get("span_id")).isEqualTo(span2.getSpanContext().getSpanId());
         assertThat(copiedMdc.get("trace_flags"))

instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/ConnectionInstrumentation.java

@@ -170,6 +170,7 @@ public void end(@Nullable Throwable throwable) {
       }
     }
 
+    @Nullable
     @Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
     public static AdviceScope onEnter(
         @Advice.This Connection connection, @Advice.Origin("#m") String methodName) {

instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcAdviceScope.java

@@ -74,6 +74,7 @@ private static JdbcAdviceScope start(CallDepth callDepth, Supplier<DbRequest> re
     return new JdbcAdviceScope(callDepth, request, context, context.makeCurrent());
   }
 
+  @Nullable
   private static DbRequest createBatchRequest(Statement statement) {
     if (statement instanceof PreparedStatement) {
       String sql = JdbcData.PREPARED_STATEMENT.get((PreparedStatement) statement);

instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/PreparedStatementInstrumentation.java

@@ -239,7 +239,7 @@ public static void onExit(
   public static class ClearParametersAdvice {
 
     @Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
-    public static void clearBatch(@Advice.This PreparedStatement statement) {
+    public static void clearParameters(@Advice.This PreparedStatement statement) {
       JdbcData.clearParameters(statement);
     }
   }

instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/OpenTelemetryDriver.java

@@ -240,7 +240,7 @@ public void setOpenTelemetry(OpenTelemetry openTelemetry) {
 
   @Nullable
   @Override
-  public Connection connect(String url, Properties info) throws SQLException {
+  public Connection connect(String url, @Nullable Properties info) throws SQLException {
     if (url == null || url.trim().isEmpty()) {
       throw new IllegalArgumentException("url is required");
     }
@@ -283,7 +283,8 @@ public boolean acceptsURL(String url) {
   }
 
   @Override
-  public DriverPropertyInfo[] getPropertyInfo(String url, Properties info) throws SQLException {
+  public DriverPropertyInfo[] getPropertyInfo(String url, @Nullable Properties info)
+      throws SQLException {
     if (url == null || url.trim().isEmpty()) {
       throw new IllegalArgumentException("url is required");
     }

instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/OpenTelemetryDataSource.java

@@ -88,7 +88,8 @@ public Connection getConnection() throws SQLException {
   }
 
   @Override
-  public Connection getConnection(String username, String password) throws SQLException {
+  public Connection getConnection(@Nullable String username, @Nullable String password)
+      throws SQLException {
     Connection connection = wrapCall(() -> delegate.getConnection(username, password));
     DbInfo dbInfo = getDbInfo(connection);
     return OpenTelemetryConnection.create(