Alexeyk/bump spotbugs

build.gradle.kts

@@ -14,7 +14,7 @@ plugins {
 
   id("com.diffplug.spotless") version "8.4.0"
   id("me.champeau.gradle.japicmp") version "0.4.3"
-  id("com.github.spotbugs") version "6.5.5"
+  id("com.github.spotbugs") version "6.5.6"
   id("de.thetaphi.forbiddenapis") version "3.10"
   id("io.github.gradle-nexus.publish-plugin") version "2.0.0"
   alias(libs.plugins.shadow) apply false

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java

@@ -534,6 +534,9 @@ public static void shutdown(final boolean sync) {
     }
   }
 
+  @SuppressFBWarnings(
+      value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION",
+      justification = "Agent-internal class; Class lock does not escape to application code")
   public static synchronized Class<?> installAgentCLI() throws Exception {
     if (null == AGENT_CLASSLOADER) {
       // in CLI mode we skip installation of instrumentation because we're not running as an agent

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/WeakMap.java

@@ -1,5 +1,6 @@
 package datadog.trace.bootstrap;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.function.Function;
 
 public interface WeakMap<K, V> {
@@ -26,6 +27,9 @@ public static <K, V> WeakMap<K, V> newWeakMap() {
       return SUPPLIER.get();
     }
 
+    @SuppressFBWarnings(
+        value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION",
+        justification = "Agent-internal holder; Class lock does not escape to application code")
     public static synchronized void registerIfAbsent(Supplier supplier) {
       if (null == SUPPLIER) {
         SUPPLIER = supplier;

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/usm/UsmExtractor.java

@@ -1,5 +1,7 @@
 package datadog.trace.bootstrap.instrumentation.usm;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+
 public interface UsmExtractor {
   void send(UsmMessage message);
 
@@ -10,6 +12,9 @@ public static void send(UsmMessage message) {
       SUPPLIER.send(message);
     }
 
+    @SuppressFBWarnings(
+        value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION",
+        justification = "Agent-internal holder; Class lock does not escape to application code")
     public static synchronized void registerIfAbsent(UsmExtractor supplier) {
       if (SUPPLIER == null) {
         SUPPLIER = supplier;

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/usm/UsmMessageFactory.java

@@ -1,5 +1,7 @@
 package datadog.trace.bootstrap.instrumentation.usm;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+
 public interface UsmMessageFactory {
   UsmMessage getCloseMessage(UsmConnection connection);
 
@@ -17,6 +19,9 @@ public static UsmMessage getRequestMessage(
       return SUPPLIER.getRequestMessage(connection, buffer, bufferOffset, len);
     }
 
+    @SuppressFBWarnings(
+        value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION",
+        justification = "Agent-internal holder; Class lock does not escape to application code")
     public static synchronized void registerIfAbsent(UsmMessageFactory supplier) {
       if (null == SUPPLIER) {
         SUPPLIER = supplier;

dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerAgent.java

@@ -39,6 +39,7 @@
 import datadog.trace.bootstrap.instrumentation.api.Tags;
 import datadog.trace.core.DDTraceCoreInfo;
 import datadog.trace.util.TagsHelper;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.io.IOException;
 import java.lang.instrument.ClassFileTransformer;
 import java.lang.instrument.Instrumentation;
@@ -75,6 +76,9 @@ public class DebuggerAgent {
   static final AtomicBoolean symDBEnabled = new AtomicBoolean();
   private static ClassesToRetransformFinder classesToRetransformFinder;
 
+  @SuppressFBWarnings(
+      value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION",
+      justification = "Agent-internal class; Class object does not escape to application code")
   public static synchronized void run(
       Config config, Instrumentation inst, SharedCommunicationObjects sco) {
     instrumentation = inst;

dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/DebuggerMetrics.java

@@ -3,6 +3,7 @@
 import datadog.metrics.api.statsd.StatsDClient;
 import datadog.metrics.impl.statsd.DDAgentStatsDClientManager;
 import datadog.trace.api.Config;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 
 /** implements a StatsD client for internal debugger agent metrics */
 public class DebuggerMetrics implements StatsDClient {
@@ -29,6 +30,9 @@ private DebuggerMetrics(Config config) {
     }
   }
 
+  @SuppressFBWarnings(
+      value = {"USO_UNSAFE_METHOD_SYNCHRONIZATION", "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION"},
+      justification = "Agent-internal singleton; neither Class object nor instance monitor escapes")
   public static synchronized DebuggerMetrics getInstance(Config config) {
     if (INSTANCE == null) {
       INSTANCE = new DebuggerMetrics(config);

dd-java-agent/agent-installer/src/main/java/datadog/trace/agent/tooling/TracerInstaller.java

@@ -12,13 +12,18 @@
 import datadog.trace.core.servicediscovery.ForeignMemoryWriterFactory;
 import datadog.trace.core.servicediscovery.ServiceDiscovery;
 import datadog.trace.core.servicediscovery.ServiceDiscoveryFactory;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 public class TracerInstaller {
   private static final Logger log = LoggerFactory.getLogger(TracerInstaller.class);
 
   /** Register a global tracer if no global tracer is already registered. */
+  @SuppressFBWarnings(
+      value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION",
+      justification =
+          "Agent-internal class; Class object does not escape to app code and lock only guards one-time tracer install.")
   public static synchronized void installGlobalTracer(
       SharedCommunicationObjects sharedCommunicationObjects,
       ProfilingContextIntegration profilingContextIntegration) {

dd-java-agent/agent-profiling/src/main/java/com/datadog/profiling/agent/ProfilingAgent.java

@@ -29,6 +29,7 @@
 import datadog.trace.api.profiling.RecordingDataListener;
 import datadog.trace.api.profiling.RecordingType;
 import datadog.trace.bootstrap.config.provider.ConfigProvider;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.io.IOException;
 import java.lang.instrument.Instrumentation;
 import java.lang.ref.WeakReference;
@@ -95,6 +96,10 @@ public void onNewData(RecordingType type, RecordingData data, boolean handleSync
    * Main entry point into profiling Note: this must be reentrant because we may want to start
    * profiling before any other tool, and then attempt to start it again at normal time
    */
+  @SuppressFBWarnings(
+      value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION",
+      justification =
+          "Agent-internal class; Class object does not escape to app code and lock only guards reentrant one-time profiler startup.")
   public static synchronized boolean run(final boolean earlyStart, Instrumentation inst)
       throws IllegalArgumentException, IOException {
     if (profiler == null) {

dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/InstrumenterState.java

@@ -1,6 +1,7 @@
 package datadog.trace.agent.tooling;
 
 import datadog.instrument.utils.ClassLoaderValue;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.Arrays;
 import java.util.concurrent.atomic.AtomicLongArray;
 import org.slf4j.Logger;
@@ -121,6 +122,15 @@ public static void blockInstrumentation(ClassLoader classLoader, int instrumenta
   }
 
   /** Records that the instrumentation is blocked by default. */
+  // Claude: SpotBugs USO_UNSAFE_ACCESSIBLE_OBJECT_SYNCHRONIZATION: should be reviewed by team.
+  // The lock object is the non-final 'defaultState' array, which is reassigned in
+  // resetDefaultState(); a reset racing with this method could leave threads synchronizing on
+  // different array instances, and reads in currentState/updateState are unsynchronized. The field
+  // is private so app code can't lock it, but the reassignable-lock smell warrants a look.
+  @SuppressFBWarnings(
+      value = "USO_UNSAFE_ACCESSIBLE_OBJECT_SYNCHRONIZATION",
+      justification =
+          "Locks the reassignable 'defaultState' array; private to the agent but lock-object identity can change on reset, so flagged for team review.")
   public static void blockInstrumentation(int instrumentationId) {
     int bitIndex = instrumentationId << 1;
     int wordIndex = bitIndex >> ADDRESS_BITS_PER_WORD;

dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/SharedTypePools.java

@@ -2,6 +2,7 @@
 
 import static datadog.trace.agent.tooling.bytebuddy.ClassFileLocators.classFileLocator;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.lang.instrument.ClassFileTransformer;
 import net.bytebuddy.pool.TypePool;
 
@@ -40,6 +41,10 @@ public static void clear() {
     SUPPLIER.clear();
   }
 
+  @SuppressFBWarnings(
+      value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION",
+      justification =
+          "Agent-internal holder; Class object does not escape to app code and lock only guards one-time supplier registration.")
   public static synchronized void registerIfAbsent(Supplier supplier) {
     if (null == SUPPLIER) {
       SUPPLIER = supplier;

dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/HierarchyMatchers.java

@@ -5,6 +5,7 @@
 
 import datadog.trace.agent.tooling.bytebuddy.SharedTypePools;
 import de.thetaphi.forbiddenapis.SuppressForbidden;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import net.bytebuddy.description.ByteCodeElement;
 import net.bytebuddy.description.NamedElement;
 import net.bytebuddy.description.annotation.AnnotationSource;
@@ -98,6 +99,10 @@ ElementMatcher.Junction<T> isAnnotatedWith(NameMatchers.OneOf<? super NamedEleme
     return ElementMatchers.isAnnotatedWith(matcher);
   }
 
+  @SuppressFBWarnings(
+      value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION",
+      justification =
+          "Agent-internal holder; Class object does not escape to app code and lock only guards one-time supplier registration.")
   public static synchronized void registerIfAbsent(Supplier supplier) {
     if (null == SUPPLIER) {
       SUPPLIER = supplier;

dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/TestEventsHandlerHolder.java

@@ -7,6 +7,7 @@
 import datadog.trace.api.civisibility.events.TestSuiteDescriptor;
 import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
 import datadog.trace.util.ConcurrentEnumMap;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.Collection;
 import java.util.Map;
 
@@ -17,6 +18,9 @@ public abstract class TestEventsHandlerHolder {
           TestFrameworkInstrumentation, TestEventsHandler<TestSuiteDescriptor, TestDescriptor>>
       HANDLERS = new ConcurrentEnumMap<>(TestFrameworkInstrumentation.class);
 
+  @SuppressFBWarnings(
+      value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION",
+      justification = "Holder class not exposed to application code; locking on its Class is safe")
   public static synchronized void start(
       TestFrameworkInstrumentation framework, Collection<LibraryCapability> capabilities) {
     TestEventsHandler<TestSuiteDescriptor, TestDescriptor> handler = HANDLERS.get(framework);
@@ -29,6 +33,9 @@ public static synchronized void start(
   }
 
   /** Used by instrumentation tests */
+  @SuppressFBWarnings(
+      value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION",
+      justification = "Holder class not exposed to application code; locking on its Class is safe")
   public static synchronized void stop(TestFrameworkInstrumentation framework) {
     TestEventsHandler<TestSuiteDescriptor, TestDescriptor> handler = HANDLERS.remove(framework);
     if (handler != null) {

dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/TestDataFactory.java

@@ -2,6 +2,7 @@
 
 import datadog.trace.api.civisibility.config.TestIdentifier;
 import datadog.trace.api.civisibility.config.TestSourceData;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
@@ -24,6 +25,9 @@ private TestDataFactory() {}
   private static volatile Map<String, Predicate<TestDescriptor>>
       TEST_DESCRIPTOR_FILTER_BY_ENGINE_ID = Collections.emptyMap();
 
+  @SuppressFBWarnings(
+      value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION",
+      justification = "Holder class not exposed to application code; locking on its Class is safe")
   public static synchronized void register(
       String engineId,
       Function<TestDescriptor, TestIdentifier> testIdentifierFactory,

dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/TestEventsHandlerHolder.java

@@ -8,6 +8,7 @@
 import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
 import datadog.trace.bootstrap.ContextStore;
 import datadog.trace.util.ConcurrentEnumMap;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.Map;
 import org.junit.platform.engine.TestDescriptor;
 import org.junit.platform.engine.TestEngine;
@@ -22,6 +23,9 @@ public abstract class TestEventsHandlerHolder {
   private static volatile ContextStore<TestDescriptor, TestExecutionTracker>
       EXECUTION_TRACKER_STORE;
 
+  @SuppressFBWarnings(
+      value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION",
+      justification = "Holder class not exposed to application code; locking on its Class is safe")
   public static synchronized void setExecutionTrackerStore(
       ContextStore<TestDescriptor, TestExecutionTracker> executionTrackerStore) {
     if (EXECUTION_TRACKER_STORE == null) {
@@ -44,6 +48,9 @@ public static TestExecutionTracker getExecutionTracker(TestDescriptor testDescri
     }
   }
 
+  @SuppressFBWarnings(
+      value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION",
+      justification = "Holder class not exposed to application code; locking on its Class is safe")
   public static synchronized void start(
       TestEngine testEngine,
       ContextStore<TestDescriptor, DDTestSuite> suiteStore,
@@ -62,6 +69,9 @@ public static synchronized void start(
   }
 
   /** Used by instrumentation tests */
+  @SuppressFBWarnings(
+      value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION",
+      justification = "Holder class not exposed to application code; locking on its Class is safe")
   public static synchronized void stop() {
     for (TestEventsHandler<TestDescriptor, TestDescriptor> handler : HANDLERS.values()) {
       handler.close();

dd-java-agent/instrumentation/spark/spark-common/src/main/java/datadog/trace/instrumentation/spark/SparkLauncherListener.java

@@ -5,6 +5,7 @@
 import datadog.trace.api.sampling.SamplingMechanism;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
 import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.lang.reflect.Field;
 import java.util.Map;
 import org.apache.spark.launcher.SparkAppHandle;
@@ -30,6 +31,10 @@ public class SparkLauncherListener implements SparkAppHandle.Listener {
   private static long submittedTimeMs = 0L;
   private static long runningTimeMs = 0L;
 
+  @SuppressFBWarnings(
+      value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION",
+      justification =
+          "Listener class not exposed to application code; locking on its Class is safe")
   public static synchronized void createLauncherSpan(Object launcher) {
     if (launcherSpan != null) {
       return;
@@ -70,6 +75,10 @@ public static synchronized void createLauncherSpan(Object launcher) {
     }
   }
 
+  @SuppressFBWarnings(
+      value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION",
+      justification =
+          "Listener class not exposed to application code; locking on its Class is safe")
   public static synchronized void finishSpan(boolean isError, String errorMessage) {
     AgentSpan span = launcherSpan;
     if (span == null) {
@@ -85,6 +94,10 @@ public static synchronized void finishSpan(boolean isError, String errorMessage)
     launcherSpan = null;
   }
 
+  @SuppressFBWarnings(
+      value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION",
+      justification =
+          "Listener class not exposed to application code; locking on its Class is safe")
   public static synchronized void finishSpanWithThrowable(Throwable throwable) {
     AgentSpan span = launcherSpan;
     if (span == null) {

dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/TestEventsHandlerHolder.java

@@ -15,6 +15,9 @@ public abstract class TestEventsHandlerHolder {
 
   private static ContextStore<ITestResult, DDTest> TEST_STORE;
 
+  @SuppressFBWarnings(
+      value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION",
+      justification = "Holder class not exposed to application code; locking on its Class is safe")
   public static synchronized void setContextStore(ContextStore<ITestResult, DDTest> testStore) {
     if (TEST_STORE == null) {
       TEST_STORE = testStore;

dd-java-agent/instrumentation/weaver-0.9/src/main/java/datadog/trace/instrumentation/weaver/DatadogWeaverReporter.java

@@ -8,6 +8,7 @@
 import datadog.trace.api.civisibility.execution.TestExecutionTracker;
 import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
 import datadog.trace.api.time.SystemTimeSource;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.lang.reflect.Method;
 import java.util.Collection;
 import java.util.Collections;
@@ -28,6 +29,10 @@ public class DatadogWeaverReporter {
   private static volatile TestEventsHandler<TestSuiteDescriptor, TestDescriptor>
       TEST_EVENTS_HANDLER;
 
+  @SuppressFBWarnings(
+      value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION",
+      justification =
+          "Reporter class not exposed to application code; locking on its Class is safe")
   public static synchronized void start() {
     if (TEST_EVENTS_HANDLER == null) {
       TEST_EVENTS_HANDLER =
@@ -37,6 +42,10 @@ public static synchronized void start() {
   }
 
   /** Used by instrumentation tests */
+  @SuppressFBWarnings(
+      value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION",
+      justification =
+          "Reporter class not exposed to application code; locking on its Class is safe")
   public static synchronized void stop() {
     if (TEST_EVENTS_HANDLER != null) {
       TEST_EVENTS_HANDLER.close();