Re-apply "Bump spotbugs gradle plugin to 6.4.7 (#10025)"

build.gradle.kts

@@ -10,8 +10,8 @@ plugins {
   id("dd-trace-java.ci-jobs")
 
   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.7"
+  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

@@ -38,7 +38,7 @@ dependencies {
   testImplementation(libs.groovy)
   testImplementation(libs.bundles.spock)
   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

@@ -16,6 +16,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

@@ -58,6 +58,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;
@@ -202,6 +203,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,
@@ -460,6 +462,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,9 +3,14 @@
 import datadog.trace.api.civisibility.execution.TestExecutionPolicy;
 import datadog.trace.api.civisibility.execution.TestStatus;
 import datadog.trace.api.civisibility.telemetry.tag.RetryReason;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.concurrent.atomic.AtomicInteger;
 
 /** Retries a test case if it failed, up to a maximum number of times. */
+@SuppressFBWarnings(
+    value = {"AT_NONATOMIC_OPERATIONS_ON_SHARED_VARIABLE", "AT_STALE_THREAD_WRITE_OF_PRIMITIVE"},
+    justification =
+        "TestExecutionPolicy instances are confined to a single thread and are not meant to be thread-safe")
 public class RetryUntilSuccessful implements TestExecutionPolicy {
 
   private final int maxExecutions;

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)) {
@@ -746,7 +745,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();
@@ -829,7 +827,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

@@ -7,6 +7,7 @@
 import datadog.trace.api.iast.telemetry.IastMetricCollector;
 import datadog.trace.api.iast.telemetry.Verbosity;
 import datadog.trace.util.AgentTaskScheduler;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Iterator;
@@ -102,6 +103,10 @@ class TaintedMapImpl implements TaintedMap, Runnable {
     /**
      * Flag for the current alive tainted objects (red/black style marking for max age calculation).
      */
+    @SuppressFBWarnings(
+        value = "AT_STALE_THREAD_WRITE_OF_PRIMITIVE",
+        justification =
+            "The design explicitly tolerates losses in high-concurrency scenarios. The delayed visibility of the generation flag can cause some entries to be purged a bit earlier or later than ideal, but this is still within the acceptable boundaries of the design.")
     protected boolean generation;
 
     /** Whether to collect the {@link IastMetric#TAINTED_FLAT_MODE} metric or not */

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-controller/src/test/java/com/datadog/profiling/controller/ProfilingSystemTest.java

@@ -20,6 +20,7 @@
 import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.awaitility.Awaitility.await;
 import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assumptions.assumeFalse;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.anyInt;
@@ -36,6 +37,7 @@
 import ch.qos.logback.classic.Level;
 import ch.qos.logback.classic.spi.ILoggingEvent;
 import ch.qos.logback.core.Appender;
+import datadog.environment.JavaVirtualMachine;
 import datadog.trace.api.profiling.RecordingData;
 import datadog.trace.api.profiling.RecordingDataListener;
 import datadog.trace.bootstrap.config.provider.ConfigProvider;
@@ -87,6 +89,12 @@ public class ProfilingSystemTest {
   @SuppressWarnings("unchecked")
   @BeforeEach
   public void setup() throws Exception {
+    assumeFalse(
+        JavaVirtualMachine.isOracleJDK8(),
+        "Oracle JDK 1.8 did not merge the fix in JDK-8058322, leading to the JVM failing to correctly "
+            + "extract method parameters without args, when the code is compiled on a later JDK (targeting 8). "
+            + "This can manifest when creating mocks.");
+
     when(controller.createRecording(ProfilingSystem.RECORDING_NAME, context)).thenReturn(recording);
     when(threadLocalRandom.nextInt(eq(1), anyInt())).thenReturn(1);
     when(recordingData.getEnd()).thenAnswer(mockInvocation -> Instant.now());

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

@@ -55,6 +55,7 @@
 import datadog.trace.api.ProductActivation;
 import datadog.trace.api.UserIdCollectionMode;
 import datadog.trace.api.telemetry.WafMetricCollector;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.io.ByteArrayInputStream;
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;
@@ -100,8 +101,18 @@ public class AppSecConfigServiceImpl implements AppSecConfigService {
           .build()
           .adapter(Types.newParameterizedType(Map.class, String.class, Object.class));
 
+  @SuppressFBWarnings(
+      value = "AT_STALE_THREAD_WRITE_OF_PRIMITIVE",
+      justification =
+          "The variable is only read and written by the single configuration-poller thread.")
   private boolean hasUserWafConfig;
+
+  @SuppressFBWarnings(
+      value = "AT_STALE_THREAD_WRITE_OF_PRIMITIVE",
+      justification =
+          "The variable is only read and written by the single configuration-poller thread.")
   private 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;
@@ -41,6 +42,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

@@ -252,6 +252,7 @@ abstract class InstrumentationSpecification extends DDSpecification implements A
     }
   }
 
+  @SuppressFBWarnings(value = "AT_STALE_THREAD_WRITE_OF_PRIMITIVE", justification = "The variable is accessed only by the test thread in setup and cleanup.")
   boolean originalAppSecRuntimeValue
 
   @Shared

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

@@ -119,7 +119,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();