fix: Try fixing spotted bugs

Author: bric3

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;
@@ -27,9 +28,15 @@ public class SharedCommunicationObjects {
   private final List<Runnable> pausedComponents = new ArrayList<>();
   private volatile boolean paused;
 
+  @SuppressFBWarnings("PA_PUBLIC_PRIMITIVE_ATTRIBUTE")
   public OkHttpClient okHttpClient;
+
+  @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;
@@ -189,6 +190,7 @@ public boolean isEnabledByDefault() {
    * <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,
@@ -436,6 +438,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-smoke-tests/appsec/springboot-graphql/build.gradle

@@ -40,7 +40,7 @@ tasks.withType(Test).configureEach {
 
 tasks.register('testRuntimeActivation', Test) {
   jvmArgs '-Dsmoke_test.appsec.enabled=inactive',
-      "-Ddatadog.smoketest.appsec.springboot-graphql.shadowJar.path=${tasks.shadowJar.archiveFile.get()}"
+    "-Ddatadog.smoketest.appsec.springboot-graphql.shadowJar.path=${tasks.shadowJar.archiveFile.get()}"
 }
 
 tasks.named('check') {

dd-smoke-tests/appsec/springboot-security/build.gradle

@@ -46,7 +46,7 @@ tasks.withType(Test).configureEach {
 
 tasks.register('testRuntimeActivation', Test) {
   jvmArgs '-Dsmoke_test.appsec.enabled=inactive',
-      "-Ddatadog.smoketest.appsec.springbootsecurity.shadowJar.path=${tasks.shadowJar.archiveFile.get()}"
+    "-Ddatadog.smoketest.appsec.springbootsecurity.shadowJar.path=${tasks.shadowJar.archiveFile.get()}"
 }
 
 tasks.named('check') {

dd-trace-core/src/main/java/datadog/trace/common/metrics/AggregateMetric.java

@@ -2,9 +2,17 @@
 
 import datadog.trace.core.histogram.Histogram;
 import datadog.trace.core.histogram.Histograms;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.concurrent.atomic.AtomicLongArray;
 
 /** Not thread-safe. Accumulates counts and durations. */
+@SuppressFBWarnings(
+    value = {
+      "AT_NONATOMIC_OPERATIONS_ON_SHARED_VARIABLE",
+      "AT_NONATOMIC_64BIT_PRIMITIVE",
+      "AT_STALE_THREAD_WRITE_OF_PRIMITIVE"
+    },
+    justification = "Explicitly not thread-safe. Accumulates counts and durations.")
 public final class AggregateMetric {
 
   static final long ERROR_TAG = 0x8000000000000000L;

dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/GenerationalUtf8Cache.java

@@ -65,7 +65,11 @@
  * provide better cache utilization.
  */
 @SuppressFBWarnings(
-    value = "IS2_INCONSISTENT_SYNC",
+    value = {
+      "IS2_INCONSISTENT_SYNC",
+      "AT_NONATOMIC_64BIT_PRIMITIVE",
+      "AT_STALE_THREAD_WRITE_OF_PRIMITIVE"
+    },
     justification =
         "stat updates are deliberately racy - sync is only used to prevent simultaneous bulk updates")
 public final class GenerationalUtf8Cache implements EncodingCache {