Bump spotbugs gradle plugin to 6.4.4

build.gradle.kts

@@ -7,8 +7,8 @@ plugins {
   id("datadog.dump-hanged-test")
 
   id("com.diffplug.spotless") version "6.13.0"
-  id("com.github.spotbugs") version "5.0.14"
-  id("de.thetaphi.forbiddenapis") version "3.8"
+  id("com.github.spotbugs") version "6.4.4"
+  id("de.thetaphi.forbiddenapis") version "3.10"
   id("io.github.gradle-nexus.publish-plugin") version "2.0.0"
   id("com.gradleup.shadow") version "8.3.6" apply false
   id("me.champeau.jmh") version "0.7.3" apply false

buildSrc/call-site-instrumentation-plugin/build.gradle.kts

@@ -39,7 +39,7 @@ dependencies {
   testImplementation("org.objenesis", "objenesis", "3.0.1")
   testImplementation(libs.groovy)
   testImplementation("javax.servlet", "javax.servlet-api", "3.0.1")
-  testImplementation("com.github.spotbugs", "spotbugs-annotations", "4.2.0")
+  testImplementation(libs.spotbugs.annotations)
 }
 
 sourceSets {

communication/src/main/java/datadog/communication/ddagent/SharedCommunicationObjects.java

@@ -11,6 +11,7 @@
 import datadog.remoteconfig.DefaultConfigurationPoller;
 import datadog.trace.api.Config;
 import datadog.trace.util.AgentTaskScheduler;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.security.Security;
 import java.util.ArrayList;
 import java.util.List;
@@ -31,6 +32,7 @@ public class SharedCommunicationObjects {
    * HTTP client for making requests to Datadog agent. Depending on configuration, this client may
    * use regular HTTP, UDS or named pipe.
    */
+  @SuppressFBWarnings("PA_PUBLIC_PRIMITIVE_ATTRIBUTE")
   public OkHttpClient agentHttpClient;
 
   /**
@@ -39,10 +41,18 @@ public class SharedCommunicationObjects {
    */
   private volatile OkHttpClient intakeHttpClient;
 
+  @SuppressFBWarnings("PA_PUBLIC_PRIMITIVE_ATTRIBUTE")
   public long httpClientTimeout;
+
+  @SuppressFBWarnings("PA_PUBLIC_PRIMITIVE_ATTRIBUTE")
   public boolean forceClearTextHttpForIntakeClient;
+
+  @SuppressFBWarnings("PA_PUBLIC_PRIMITIVE_ATTRIBUTE")
   public HttpUrl agentUrl;
+
+  @SuppressFBWarnings("PA_PUBLIC_PRIMITIVE_ATTRIBUTE")
   public Monitoring monitoring;
+
   private volatile DDAgentFeaturesDiscovery featuresDiscovery;
   private ConfigurationPoller configurationPoller;
 

communication/src/main/java/datadog/communication/monitor/DDAgentStatsDClientManager.java

@@ -14,6 +14,8 @@
 public final class DDAgentStatsDClientManager implements StatsDClientManager {
   private static final DDAgentStatsDClientManager INSTANCE = new DDAgentStatsDClientManager();
 
+  private DDAgentStatsDClientManager() {}
+
   private static final boolean USE_LOGGING_CLIENT =
       LOGGING_WRITER_TYPE.equals(Config.get().getWriterType());
 

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

@@ -56,6 +56,7 @@
 import datadog.trace.util.AgentTaskScheduler;
 import datadog.trace.util.AgentThreadFactory.AgentThread;
 import datadog.trace.util.throwable.FatalAgentMisconfigurationError;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.lang.instrument.Instrumentation;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
@@ -198,6 +199,7 @@ private static void safelySetContextClassLoader(ClassLoader classLoader) {
    * <p>The Agent is considered to start successfully if Instrumentation can be activated. All other
    * pieces are considered optional.
    */
+  @SuppressFBWarnings("AT_STALE_THREAD_WRITE_OF_PRIMITIVE")
   public static void start(
       final Object bootstrapInitTelemetry,
       final Instrumentation inst,
@@ -446,6 +448,7 @@ private static void injectAgentArgsConfig(String agentArgs) {
     }
   }
 
+  @SuppressFBWarnings("AT_STALE_THREAD_WRITE_OF_PRIMITIVE")
   private static void configureCiVisibility(URL agentJarURL) {
     // Retro-compatibility for the old way to configure CI Visibility
     if ("true".equals(ddGetProperty("dd.integration.junit.enabled"))

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

@@ -29,7 +29,7 @@ public static PatchLogger getAnonymousLogger(final String resourceBundleName) {
     return SAFE_LOGGER;
   }
 
-  protected PatchLogger(final String name, final String resourceBundleName) {
+  private PatchLogger(final String name, final String resourceBundleName) {
     // super(name, resourceBundleName);
   }
 

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/jms/SessionState.java

@@ -2,7 +2,6 @@
 
 import datadog.trace.api.Config;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.ArrayDeque;
 import java.util.Collections;
 import java.util.Comparator;
@@ -65,7 +64,6 @@ public int compare(Map.Entry<Thread, TimeInQueue> o1, Map.Entry<Thread, TimeInQu
   private volatile int timeInQueueSpanCount = 0;
 
   // this field is protected by synchronization of capturedSpans, but SpotBugs miss that
-  @SuppressFBWarnings("IS2_INCONSISTENT_SYNC")
   private boolean capturingFlipped = false;
 
   public SessionState(int ackMode, boolean timeInQueueEnabled) {

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RetryUntilSuccessful.java

@@ -3,15 +3,16 @@
 import datadog.trace.api.civisibility.execution.TestExecutionPolicy;
 import datadog.trace.api.civisibility.execution.TestStatus;
 import datadog.trace.api.civisibility.telemetry.tag.RetryReason;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 
 /** Retries a test case if it failed, up to a maximum number of times. */
 public class RetryUntilSuccessful implements TestExecutionPolicy {
 
   private final int maxExecutions;
   private final boolean suppressFailures;
-  private int executions;
-  private boolean successfulExecutionSeen;
+  private final AtomicInteger executions = new AtomicInteger(0);
+  private final AtomicBoolean successfulExecutionSeen = new AtomicBoolean(false);
 
   /** Total retry counter that is shared by all retry until successful policies (currently ATR) */
   private final AtomicInteger totalRetryCount;
@@ -21,29 +22,29 @@ public RetryUntilSuccessful(
     this.maxExecutions = maxExecutions;
     this.suppressFailures = suppressFailures;
     this.totalRetryCount = totalRetryCount;
-    this.executions = 0;
   }
 
   @Override
   public ExecutionOutcome registerExecution(TestStatus status, long durationMillis) {
-    ++executions;
-    successfulExecutionSeen |= (status != TestStatus.fail);
-    if (executions > 1) {
+    int execs = executions.incrementAndGet();
+    boolean success = successfulExecutionSeen.get() | (status != TestStatus.fail);
+    successfulExecutionSeen.set(success);
+    if (execs > 1) {
       totalRetryCount.incrementAndGet();
     }
 
     boolean lastExecution = !retriesLeft();
-    boolean retry = executions > 1; // first execution is not a retry
+    boolean retry = execs > 1; // first execution is not a retry
     return new ExecutionOutcomeImpl(
         status == TestStatus.fail && (!lastExecution || suppressFailures),
         lastExecution,
-        lastExecution && !successfulExecutionSeen,
+        lastExecution && !success,
         false,
         retry ? RetryReason.atr : null);
   }
 
   private boolean retriesLeft() {
-    return !successfulExecutionSeen && executions < maxExecutions;
+    return !successfulExecutionSeen.get() && executions.get() < maxExecutions;
   }
 
   @Override
@@ -57,7 +58,7 @@ public boolean applicable() {
   public boolean suppressFailures() {
     // do not suppress failures for last execution (unless flag to suppress all failures is set);
     // the +1 is because this method is called _before_ subsequent execution is registered
-    return executions + 1 < maxExecutions || suppressFailures;
+    return executions.get() + 1 < maxExecutions || suppressFailures;
   }
 
   @Override

dd-java-agent/agent-iast/iast-test-fixtures/src/main/groovy/com/datadog/iast/test/IastRequestTestRunner.groovy

@@ -6,11 +6,13 @@ import datadog.trace.agent.test.utils.OkHttpUtils
 import datadog.trace.api.gateway.IGSpanInfo
 import datadog.trace.api.gateway.RequestContext
 import datadog.trace.api.gateway.RequestContextSlot
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings
 import okhttp3.OkHttpClient
 
 import java.util.concurrent.LinkedBlockingQueue
 import java.util.concurrent.TimeUnit
 
+@SuppressFBWarnings("HSM_HIDING_METHOD")
 class IastRequestTestRunner extends IastAgentTestRunner implements IastRequestContextPreparationTrait {
 
   private static final LinkedBlockingQueue<TaintedObjectCollection> TAINTED_OBJECTS = new LinkedBlockingQueue<>()

dd-java-agent/agent-iast/src/main/java/com/datadog/iast/IastSystem.java

@@ -55,6 +55,7 @@
 import datadog.trace.api.iast.telemetry.Verbosity;
 import datadog.trace.util.AgentTaskScheduler;
 import datadog.trace.util.stacktrace.StackWalkerFactory;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.lang.instrument.Instrumentation;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.UndeclaredThrowableException;
@@ -70,7 +71,11 @@
 public class IastSystem {
 
   private static final Logger LOGGER = LoggerFactory.getLogger(IastSystem.class);
+
+  @SuppressFBWarnings("PA_PUBLIC_PRIMITIVE_ATTRIBUTE")
   public static boolean DEBUG = false;
+
+  @SuppressFBWarnings("PA_PUBLIC_PRIMITIVE_ATTRIBUTE")
   public static Verbosity VERBOSITY = Verbosity.OFF;
 
   public static void start(final SubscriptionService ss) {

dd-java-agent/agent-iast/src/main/java/com/datadog/iast/propagation/StringModuleImpl.java

@@ -224,7 +224,6 @@ public void onStringConcatFactory(
   }
 
   @Override
-  @SuppressFBWarnings("ES_COMPARING_PARAMETER_STRING_WITH_EQ")
   public void onStringSubSequence(
       @Nonnull CharSequence self, int beginIndex, int endIndex, @Nullable CharSequence result) {
     if (self == result || !canBeTainted(result)) {
@@ -722,7 +721,6 @@ public void onStringReplace(
 
   /** This method is used to make an {@code CallSite.Around} of the {@code String.replace} method */
   @Override
-  @SuppressFBWarnings("ES_COMPARING_PARAMETER_STRING_WITH_EQ")
   public String onStringReplace(
       @Nonnull String self, CharSequence oldCharSeq, CharSequence newCharSeq) {
     final IastContext ctx = IastContext.Provider.get();
@@ -805,7 +803,6 @@ public String onStringReplace(
   }
 
   @Override
-  @SuppressFBWarnings("ES_COMPARING_PARAMETER_STRING_WITH_EQ")
   public void onStringValueOf(Object param, @Nonnull String result) {
     if (param == null || !canBeTainted(result)) {
       return;

dd-java-agent/agent-iast/src/main/java/com/datadog/iast/taint/TaintedMap.java

@@ -102,7 +102,7 @@ class TaintedMapImpl implements TaintedMap, Runnable {
     /**
      * Flag for the current alive tainted objects (red/black style marking for max age calculation).
      */
-    protected boolean generation;
+    protected volatile boolean generation;
 
     /** Whether to collect the {@link IastMetric#TAINTED_FLAT_MODE} metric or not */
     protected boolean collectFlatBucketMetric;

dd-java-agent/agent-profiling/profiling-controller/src/main/java/com/datadog/profiling/controller/EnvironmentChecker.java

@@ -50,7 +50,7 @@ public static boolean checkEnvironment(String temp, StringBuilder sb) {
     boolean result = false;
     result |= checkJFR(sb);
     result |= checkDdprof(sb);
-    if (!result) {;
+    if (!result) {
       appendLine("Profiler is not supported on this JVM.", sb);
       return false;
     } else {

dd-java-agent/agent-profiling/profiling-utils/src/main/java/com/datadog/profiling/utils/Timestamper.java

@@ -1,5 +1,6 @@
 package com.datadog.profiling.utils;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
 
 public interface Timestamper {
@@ -33,6 +34,7 @@ static boolean override(Timestamper timestamper) {
         Registration.INSTANCE, Timestamper.DEFAULT, timestamper);
   }
 
+  @SuppressFBWarnings("SING_SINGLETON_HAS_NONPRIVATE_CONSTRUCTOR")
   final class Singleton {
     //
     static final Timestamper TIMESTAMPER = Registration.INSTANCE.pending;

dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/usm/UsmMessageImpl.java

@@ -22,7 +22,7 @@ enum MessageType {
   private static final Logger log = LoggerFactory.getLogger(BaseUsmMessage.class);
 
   // TODO: sync with systemprobe code
-  static final NativeLong USM_IOCTL_ID = new NativeLong(0xda7ad09L);;
+  static final NativeLong USM_IOCTL_ID = new NativeLong(0xda7ad09L);
 
   abstract static class BaseUsmMessage implements UsmMessage {
 

dd-java-agent/appsec/src/main/java/com/datadog/appsec/config/AppSecConfigServiceImpl.java

@@ -100,8 +100,8 @@ public class AppSecConfigServiceImpl implements AppSecConfigService {
           .build()
           .adapter(Types.newParameterizedType(Map.class, String.class, Object.class));
 
-  private boolean hasUserWafConfig;
-  private boolean defaultConfigActivated;
+  private volatile boolean hasUserWafConfig;
+  private volatile boolean defaultConfigActivated;
   private final AtomicBoolean subscribedToRulesAndData = new AtomicBoolean();
   private final Set<String> usedDDWafConfigKeys =
       Collections.newSetFromMap(new ConcurrentHashMap<>());

dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java

@@ -14,6 +14,7 @@
 import datadog.trace.api.http.StoredBodySupplier;
 import datadog.trace.api.internal.TraceSegment;
 import datadog.trace.util.stacktrace.StackTraceEvent;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.io.Closeable;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -40,6 +41,7 @@
 
 // TODO: different methods to be called by different parts perhaps splitting it would make sense
 // or at least create separate interfaces
+@SuppressFBWarnings("AT_STALE_THREAD_WRITE_OF_PRIMITIVE")
 public class AppSecRequestContext implements DataBundle, Closeable {
   private static final Logger log = LoggerFactory.getLogger(AppSecRequestContext.class);
 

dd-java-agent/build.gradle

@@ -1,5 +1,4 @@
 import com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar
-
 import java.util.concurrent.atomic.AtomicBoolean
 
 plugins {
@@ -29,7 +28,6 @@ tasks.named("processResources") {
 sourceSets {
   "main_java6" {
     java.srcDirs "${project.projectDir}/src/main/java6"
-
   }
   main.resources.srcDir(includedAgentDir)
 }
@@ -43,7 +41,7 @@ tasks.named("compileJava") {
 }
 
 dependencies {
-  main_java6CompileOnly 'de.thetaphi:forbiddenapis:3.8'
+  main_java6CompileOnly libs.forbiddenapis
   testImplementation sourceSets.main_java6.output
 }
 

dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy

@@ -251,7 +251,7 @@ abstract class InstrumentationSpecification extends DDSpecification implements A
     }
   }
 
-  boolean originalAppSecRuntimeValue
+  volatile boolean originalAppSecRuntimeValue
 
   @Shared
   ConcurrentHashMap<DDSpan, List<Exception>> spanFinishLocations = new ConcurrentHashMap<>()

dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/build.gradle

@@ -117,7 +117,7 @@ dependencies {
   baseTestImplementation group: 'com.typesafe.akka', name: 'akka-http-spray-json_2.11', version: '10.0.10'
 
   iastTestImplementation project(':dd-java-agent:agent-iast:iast-test-fixtures')
-  iastTestCompileOnly group: 'de.thetaphi', name: 'forbiddenapis', version: '3.4'
+  iastTestCompileOnly libs.forbiddenapis
   iastTestRuntimeOnly project(':dd-java-agent:instrumentation:jackson-core')
   iastTestRuntimeOnly project(':dd-java-agent:instrumentation:jackson-core:jackson-core-2.8')
   iastTestRuntimeOnly project(':dd-java-agent:instrumentation:iast-instrumenter')

dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/iast/HttpRequestInstrumentation.java

@@ -24,7 +24,6 @@
 import datadog.trace.api.iast.Source;
 import datadog.trace.api.iast.SourceTypes;
 import datadog.trace.api.iast.propagation.PropagationModule;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import net.bytebuddy.asm.Advice;
 import scala.collection.Iterator;
 import scala.collection.immutable.Seq;
@@ -61,7 +60,6 @@ public void methodAdvice(MethodTransformer transformer) {
         HttpRequestInstrumentation.class.getName() + "$EntityAdvice");
   }
 
-  @SuppressFBWarnings("BC_IMPOSSIBLE_INSTANCEOF")
   @RequiresRequestContext(RequestContextSlot.IAST)
   static class RequestHeadersAdvice {
     @Advice.OnMethodExit(suppress = Throwable.class)

dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/iast/RequestContextInstrumentation.java

@@ -19,7 +19,6 @@
 import datadog.trace.api.iast.InstrumentationBridge;
 import datadog.trace.api.iast.Propagation;
 import datadog.trace.api.iast.propagation.PropagationModule;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import net.bytebuddy.asm.Advice;
 
 /** Propagates taint when fetching the {@link HttpRequest} from the {@link RequestContext}. */
@@ -46,7 +45,6 @@ public void methodAdvice(MethodTransformer transformer) {
         RequestContextInstrumentation.class.getName() + "$GetRequestAdvice");
   }
 
-  @SuppressFBWarnings("BC_IMPOSSIBLE_INSTANCEOF")
   @RequiresRequestContext(RequestContextSlot.IAST)
   static class GetRequestAdvice {
     @Advice.OnMethodExit(suppress = Throwable.class)

dd-java-agent/instrumentation/akka/akka-http/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/iast/helpers/TaintRequestFunction.java

@@ -6,15 +6,13 @@
 import datadog.trace.api.iast.SourceTypes;
 import datadog.trace.api.iast.propagation.PropagationModule;
 import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import scala.Tuple1;
 import scala.compat.java8.JFunction1;
 
 public class TaintRequestFunction implements JFunction1<Tuple1<HttpRequest>, Tuple1<HttpRequest>> {
   public static final TaintRequestFunction INSTANCE = new TaintRequestFunction();
 
   @Override
-  @SuppressFBWarnings("BC_IMPOSSIBLE_INSTANCEOF")
   public Tuple1<HttpRequest> apply(Tuple1<HttpRequest> v1) {
     HttpRequest httpRequest = v1._1();