From e6e7f6e3989bb0c0cd3ee3633344b4572689209a Mon Sep 17 00:00:00 2001 From: Alexey Kuznetsov Date: Wed, 17 Jun 2026 22:07:20 -0400 Subject: [PATCH 1/7] Bump spotbugs plugin. --- build.gradle.kts | 2 +- dd-smoke-tests/apm-tracing-disabled/application/build.gradle | 2 +- gradle/libs.versions.toml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index f09064ed883..3a550a54e8e 100644 --- a/build.gradle.kts +++ b/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" id("com.gradleup.shadow") version "8.3.9" apply false diff --git a/dd-smoke-tests/apm-tracing-disabled/application/build.gradle b/dd-smoke-tests/apm-tracing-disabled/application/build.gradle index b5653805e95..c5f2d8455dc 100644 --- a/dd-smoke-tests/apm-tracing-disabled/application/build.gradle +++ b/dd-smoke-tests/apm-tracing-disabled/application/build.gradle @@ -34,7 +34,7 @@ if (hasProperty('apiJar')) { } dependencies { - compileOnly 'com.github.spotbugs:spotbugs-annotations:4.9.8' + compileOnly libs.spotbugs.annotations implementation 'org.springframework.boot:spring-boot-starter-web' // OpenTracing 0.32.0 is the last release and is intentionally pinned: this smoke test // exercises the legacy OpenTracing bridge in dd-trace-ot. Do not "upgrade" — there is diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 79ec4ba3b45..9241afbed4a 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -5,7 +5,7 @@ develocity = "4.4.1" forbiddenapis = "3.10" gradle-tooling-api = "8.14.5" -spotbugs_annotations = "4.9.8" +spotbugs_annotations = "4.10.2" # DataDog libs and forks ddprof = "1.44.0" From d3723cc000fa57fbdf320d3699e185c4ecd6ae60 Mon Sep 17 00:00:00 2001 From: Alexey Kuznetsov Date: Wed, 17 Jun 2026 23:50:07 -0400 Subject: [PATCH 2/7] Initial processing on new findings. --- .../build.gradle.kts | 4 ++- .../java/datadog/trace/bootstrap/Agent.java | 8 ++++++ .../java/datadog/trace/bootstrap/WeakMap.java | 8 ++++++ .../instrumentation/usm/UsmExtractor.java | 9 ++++++ .../usm/UsmMessageFactory.java | 9 ++++++ .../datadog/debugger/agent/DebuggerAgent.java | 8 ++++++ .../debugger/util/DebuggerMetrics.java | 10 +++++++ .../trace/agent/tooling/TracerInstaller.java | 9 ++++++ .../profiling/agent/ProfilingAgent.java | 9 ++++++ .../agent/tooling/InstrumenterState.java | 10 +++++++ .../tooling/bytebuddy/SharedTypePools.java | 8 ++++++ .../bytebuddy/matcher/HierarchyMatchers.java | 8 ++++++ .../junit4/TestEventsHandlerHolder.java | 15 ++++++++++ .../junit5/TestDataFactory.java | 8 ++++++ .../junit5/TestEventsHandlerHolder.java | 22 +++++++++++++++ .../spark/SparkLauncherListener.java | 28 +++++++++++++++++++ .../testng/TestEventsHandlerHolder.java | 7 +++++ .../weaver/DatadogWeaverReporter.java | 19 +++++++++++++ .../datastreams/DefaultPathwayContext.java | 26 +++++++++++++++++ gradle/spotbugFilters/exclude.xml | 6 ++++ gradle/spotbugs.gradle | 11 ++------ .../java/datadog/trace/api/ProcessTags.java | 8 ++++++ .../api/debugger/DebuggerConfigBridge.java | 13 +++++++++ .../api/telemetry/IntegrationsCollector.java | 17 +++++++++++ .../api/telemetry/ProductChangeCollector.java | 17 +++++++++++ .../instrumentation/api/AgentTracer.java | 13 +++++++++ .../trace/util/AgentTaskScheduler.java | 9 +++++- .../java/datadog/trace/util/UnsafeUtils.java | 8 ++++++ .../datadog/metrics/agent/AgentMeter.java | 9 ++++++ .../telemetry/dependency/Dependency.java | 13 +++++++++ 30 files changed, 339 insertions(+), 10 deletions(-) diff --git a/buildSrc/call-site-instrumentation-plugin/build.gradle.kts b/buildSrc/call-site-instrumentation-plugin/build.gradle.kts index 562ab980e91..62d4ecb62c4 100644 --- a/buildSrc/call-site-instrumentation-plugin/build.gradle.kts +++ b/buildSrc/call-site-instrumentation-plugin/build.gradle.kts @@ -37,7 +37,9 @@ dependencies { testRuntimeOnly(libs.junit.platform.launcher) testImplementation(libs.bundles.mockito) testImplementation("javax.servlet", "javax.servlet-api", "3.0.1") - testImplementation(libs.spotbugs.annotations) + // Only the annotation classes are needed; exclude transitive deps to avoid + // spotbugs-annotations pulling in junit-bom 6.x (requires JVM 17, this plugin targets JVM 8). + testImplementation(libs.spotbugs.annotations) { isTransitive = false } } sourceSets { diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java index aae283da55c..b0cdf977445 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java @@ -533,6 +533,14 @@ public static void shutdown(final boolean sync) { } } + // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. + // Agent is an agent-internal bootstrap class whose Class object is loaded in the isolated agent + // context; application code cannot realistically acquire the same lock, and the lock only guards + // the internal AGENT_CLASSLOADER state. A private static lock object would be cleaner but is not + // required here. + @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 diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/WeakMap.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/WeakMap.java index 1aa859ea3d8..92770e95a95 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/WeakMap.java +++ b/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 { @@ -26,6 +27,13 @@ public static WeakMap newWeakMap() { return SUPPLIER.get(); } + // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. + // WeakMap.Supplier is an agent-internal holder loaded in the isolated agent context; its Class + // lock does not escape to application code, and the lock only guards the private SUPPLIER + // field. + @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; diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/usm/UsmExtractor.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/usm/UsmExtractor.java index 2dbf35b79d3..f9686bb32a8 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/usm/UsmExtractor.java +++ b/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,13 @@ public static void send(UsmMessage message) { SUPPLIER.send(message); } + // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. + // UsmExtractor.Supplier is an agent-internal holder loaded in the isolated agent context; its + // Class lock does not escape to application code, and the lock only guards the private SUPPLIER + // field. + @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; diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/usm/UsmMessageFactory.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/usm/UsmMessageFactory.java index a1320d789b8..363adea11b2 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/usm/UsmMessageFactory.java +++ b/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,13 @@ public static UsmMessage getRequestMessage( return SUPPLIER.getRequestMessage(connection, buffer, bufferOffset, len); } + // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. + // UsmMessageFactory.Supplier is an agent-internal holder loaded in the isolated agent context; + // its Class lock does not escape to application code, and the lock only guards the private + // SUPPLIER field. + @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; diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerAgent.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerAgent.java index 843d2504b15..0045768d61a 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerAgent.java +++ b/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,13 @@ public class DebuggerAgent { static final AtomicBoolean symDBEnabled = new AtomicBoolean(); private static ClassesToRetransformFinder classesToRetransformFinder; + // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. + // DebuggerAgent is an agent-internal bootstrap class loaded in the agent's isolated classloader; + // its Class object is not exposed to application or third-party code that could lock on it, so + // there is no realistic risk of an external monitor collision or deadlock. + @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; diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/DebuggerMetrics.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/DebuggerMetrics.java index 4ebc243e0dc..4e30e5e3e9e 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/DebuggerMetrics.java +++ b/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,15 @@ private DebuggerMetrics(Config config) { } } + // SpotBugs USO_UNSAFE_METHOD_SYNCHRONIZATION, USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false + // positive, can be suppressed. + // This is a static synchronized singleton accessor in an agent-internal class loaded by the + // agent's isolated classloader. The lock guards only the lazy INSTANCE initialization; neither + // the DebuggerMetrics Class object nor the returned instance's monitor is exposed to or locked on + // by application/third-party code, and no instance synchronized methods lock on `this`. + @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); diff --git a/dd-java-agent/agent-installer/src/main/java/datadog/trace/agent/tooling/TracerInstaller.java b/dd-java-agent/agent-installer/src/main/java/datadog/trace/agent/tooling/TracerInstaller.java index 1a73b7cdba4..484bf14e527 100644 --- a/dd-java-agent/agent-installer/src/main/java/datadog/trace/agent/tooling/TracerInstaller.java +++ b/dd-java-agent/agent-installer/src/main/java/datadog/trace/agent/tooling/TracerInstaller.java @@ -12,6 +12,7 @@ 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; @@ -19,6 +20,14 @@ public class TracerInstaller { private static final Logger log = LoggerFactory.getLogger(TracerInstaller.class); /** Register a global tracer if no global tracer is already registered. */ + // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. + // TracerInstaller is an agent-internal class loaded in the agent class loader; its Class object + // is not exposed to application code that could lock it, and the lock only serializes one-time + // global tracer installation. + @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) { diff --git a/dd-java-agent/agent-profiling/src/main/java/com/datadog/profiling/agent/ProfilingAgent.java b/dd-java-agent/agent-profiling/src/main/java/com/datadog/profiling/agent/ProfilingAgent.java index c7a3b64fd79..8065b069761 100644 --- a/dd-java-agent/agent-profiling/src/main/java/com/datadog/profiling/agent/ProfilingAgent.java +++ b/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,14 @@ 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 */ + // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. + // ProfilingAgent is an agent-internal class; its Class object does not escape to application + // code, and the lock exists to make this reentrant startup method serialize one-time profiler + // initialization. + @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) { diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/InstrumenterState.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/InstrumenterState.java index cc2fc14ec45..4b2119d8781 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/InstrumenterState.java +++ b/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. */ + // 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; diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/SharedTypePools.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/SharedTypePools.java index 070ccc5383a..7cb78628fc6 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/SharedTypePools.java +++ b/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,13 @@ public static void clear() { SUPPLIER.clear(); } + // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. + // SharedTypePools is an agent-internal holder; its Class object is not exposed to application + // code, and the lock only guards one-time registration of the supplier. + @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; diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/HierarchyMatchers.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/HierarchyMatchers.java index f874b2d7e93..7af9a3049b1 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/HierarchyMatchers.java +++ b/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,13 @@ ElementMatcher.Junction isAnnotatedWith(NameMatchers.OneOf> HANDLERS = new ConcurrentEnumMap<>(TestFrameworkInstrumentation.class); + // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. + // This holder lives in the agent's instrumentation classloader and is not exposed to application + // code that could lock on its Class. Real-world contention risk is low; a private static final + // lock object would be a cleaner fix. + @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 capabilities) { TestEventsHandler handler = HANDLERS.get(framework); @@ -29,6 +37,13 @@ public static synchronized void start( } /** Used by instrumentation tests */ + // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. + // This holder lives in the agent's instrumentation classloader and is not exposed to application + // code that could lock on its Class. Real-world contention risk is low; a private static final + // lock object would be a cleaner fix. + @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 handler = HANDLERS.remove(framework); if (handler != null) { diff --git a/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/TestDataFactory.java b/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/TestDataFactory.java index 0a3d673b0c2..bfe231992c4 100644 --- a/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/TestDataFactory.java +++ b/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,13 @@ private TestDataFactory() {} private static volatile Map> TEST_DESCRIPTOR_FILTER_BY_ENGINE_ID = Collections.emptyMap(); + // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. + // This factory lives in the agent's instrumentation classloader and is not exposed to application + // code that could lock on its Class. Real-world contention risk is low; a private static final + // lock object would be a cleaner fix. + @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 testIdentifierFactory, diff --git a/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/TestEventsHandlerHolder.java b/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/TestEventsHandlerHolder.java index 38e93ba77b5..ead31526704 100644 --- a/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/TestEventsHandlerHolder.java +++ b/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,13 @@ public abstract class TestEventsHandlerHolder { private static volatile ContextStore EXECUTION_TRACKER_STORE; + // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. + // This holder lives in the agent's instrumentation classloader and is not exposed to application + // code that could lock on its Class. Real-world contention risk is low; a private static final + // lock object would be a cleaner fix. + @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 executionTrackerStore) { if (EXECUTION_TRACKER_STORE == null) { @@ -44,6 +52,13 @@ public static TestExecutionTracker getExecutionTracker(TestDescriptor testDescri } } + // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. + // This holder lives in the agent's instrumentation classloader and is not exposed to application + // code that could lock on its Class. Real-world contention risk is low; a private static final + // lock object would be a cleaner fix. + @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 suiteStore, @@ -62,6 +77,13 @@ public static synchronized void start( } /** Used by instrumentation tests */ + // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. + // This holder lives in the agent's instrumentation classloader and is not exposed to application + // code that could lock on its Class. Real-world contention risk is low; a private static final + // lock object would be a cleaner fix. + @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 handler : HANDLERS.values()) { handler.close(); diff --git a/dd-java-agent/instrumentation/spark/spark-common/src/main/java/datadog/trace/instrumentation/spark/SparkLauncherListener.java b/dd-java-agent/instrumentation/spark/spark-common/src/main/java/datadog/trace/instrumentation/spark/SparkLauncherListener.java index 643953b22c8..92e671f0f14 100644 --- a/dd-java-agent/instrumentation/spark/spark-common/src/main/java/datadog/trace/instrumentation/spark/SparkLauncherListener.java +++ b/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,15 @@ public class SparkLauncherListener implements SparkAppHandle.Listener { private static long submittedTimeMs = 0L; private static long runningTimeMs = 0L; + // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. + // This listener lives in the agent's instrumentation classloader and is not handed to application + // code in a way that lets it lock on the same Class. The Class lock is deliberately shared with + // the synchronized blocks in stateChanged/infoChanged/shutdown-hook to guard the launcherSpan + // state; a private static final lock object would be a cleaner equivalent. Risk is low. + @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 +80,15 @@ public static synchronized void createLauncherSpan(Object launcher) { } } + // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. + // This listener lives in the agent's instrumentation classloader and is not handed to application + // code in a way that lets it lock on the same Class. The Class lock is deliberately shared with + // the synchronized blocks in stateChanged/infoChanged/shutdown-hook to guard the launcherSpan + // state; a private static final lock object would be a cleaner equivalent. Risk is low. + @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 +104,15 @@ public static synchronized void finishSpan(boolean isError, String errorMessage) launcherSpan = null; } + // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. + // This listener lives in the agent's instrumentation classloader and is not handed to application + // code in a way that lets it lock on the same Class. The Class lock is deliberately shared with + // the synchronized blocks in stateChanged/infoChanged/shutdown-hook to guard the launcherSpan + // state; a private static final lock object would be a cleaner equivalent. Risk is low. + @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) { diff --git a/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/TestEventsHandlerHolder.java b/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/TestEventsHandlerHolder.java index 3988c18b8f1..2d1a89f7ba0 100644 --- a/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/TestEventsHandlerHolder.java +++ b/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/TestEventsHandlerHolder.java @@ -15,6 +15,13 @@ public abstract class TestEventsHandlerHolder { private static ContextStore TEST_STORE; + // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. + // This holder lives in the agent's instrumentation classloader and is not exposed to application + // code that could lock on its Class. Real-world contention risk is low; a private static final + // lock object would be a cleaner fix. + @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 testStore) { if (TEST_STORE == null) { TEST_STORE = testStore; diff --git a/dd-java-agent/instrumentation/weaver-0.9/src/main/java/datadog/trace/instrumentation/weaver/DatadogWeaverReporter.java b/dd-java-agent/instrumentation/weaver-0.9/src/main/java/datadog/trace/instrumentation/weaver/DatadogWeaverReporter.java index 61d74192e06..7fa37c7c997 100644 --- a/dd-java-agent/instrumentation/weaver-0.9/src/main/java/datadog/trace/instrumentation/weaver/DatadogWeaverReporter.java +++ b/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,15 @@ public class DatadogWeaverReporter { private static volatile TestEventsHandler TEST_EVENTS_HANDLER; + // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. + // This reporter lives in the agent's instrumentation classloader and is not exposed to + // application + // code that could lock on its Class. Real-world contention risk is low; a private static final + // lock object would be a cleaner fix. + @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 +47,15 @@ public static synchronized void start() { } /** Used by instrumentation tests */ + // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. + // This reporter lives in the agent's instrumentation classloader and is not exposed to + // application + // code that could lock on its Class. Real-world contention risk is low; a private static final + // lock object would be a cleaner fix. + @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(); diff --git a/dd-trace-core/src/main/java/datadog/trace/core/datastreams/DefaultPathwayContext.java b/dd-trace-core/src/main/java/datadog/trace/core/datastreams/DefaultPathwayContext.java index 167d6b12bb2..633a136133f 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/datastreams/DefaultPathwayContext.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/datastreams/DefaultPathwayContext.java @@ -14,6 +14,7 @@ import datadog.trace.api.datastreams.StatsPoint; import datadog.trace.api.time.TimeSource; import datadog.trace.util.FNV64Hash; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.io.IOException; import java.util.Base64; import java.util.concurrent.TimeUnit; @@ -74,7 +75,16 @@ public long getHash() { return hash; } + // SpotBugs USO_UNSAFE_METHOD_SYNCHRONIZATION: false positive, can be suppressed. + // DefaultPathwayContext is an agent-internal data-streams object reached only through internal + // abstractions (PathwayContext lives in internal-api and is attached to AgentSpanContext); + // instances are not handed to application or third-party code that could lock on the monitor. + // The lock guards mutable internal pathway state (hash, started, nano ticks) and outputBuffer. @Override + @SuppressFBWarnings( + value = "USO_UNSAFE_METHOD_SYNCHRONIZATION", + justification = + "Agent-internal pathway object; instances do not escape to application code that could synchronize on the monitor.") public synchronized void setCheckpoint( DataStreamsContext context, Consumer pointConsumer) { long startNanos = timeSource.getCurrentTimeNanos(); @@ -148,7 +158,15 @@ public StatsPoint getSavedStats() { return this.savedStats; } + // SpotBugs USO_UNSAFE_METHOD_SYNCHRONIZATION: false positive, can be suppressed. + // Same rationale as setCheckpoint: instances stay agent-internal and never reach application or + // third-party code that could lock on the monitor. The lock here is needed to safely read the + // mutable hash/nano-tick fields and to guard the shared outputBuffer it writes into. @Override + @SuppressFBWarnings( + value = "USO_UNSAFE_METHOD_SYNCHRONIZATION", + justification = + "Agent-internal pathway object; instances do not escape to application code that could synchronize on the monitor.") public synchronized String encode() throws IOException { if (!started) { throw new IllegalStateException("Context must be started to encode"); @@ -169,7 +187,15 @@ public synchronized String encode() throws IOException { return new String(base64, ISO_8859_1); } + // SpotBugs USO_UNSAFE_METHOD_SYNCHRONIZATION: false positive, can be suppressed. + // toString only reads the mutable state under the same monitor for a consistent snapshot; the + // object is agent-internal (used in debug logging) and never exposed to application code that + // could synchronize on it. @Override + @SuppressFBWarnings( + value = "USO_UNSAFE_METHOD_SYNCHRONIZATION", + justification = + "Agent-internal pathway object; instances do not escape to application code that could synchronize on the monitor.") public synchronized String toString() { if (started) { return "PathwayContext[ Hash " diff --git a/gradle/spotbugFilters/exclude.xml b/gradle/spotbugFilters/exclude.xml index 2276ed212e0..2822e0e0ac7 100644 --- a/gradle/spotbugFilters/exclude.xml +++ b/gradle/spotbugFilters/exclude.xml @@ -23,4 +23,10 @@ + + + + diff --git a/gradle/spotbugs.gradle b/gradle/spotbugs.gradle index 2fea20bcdcf..f96c63ba1ce 100644 --- a/gradle/spotbugs.gradle +++ b/gradle/spotbugs.gradle @@ -62,13 +62,8 @@ dependencies { compileOnly(libs.spotbugs.annotations) testImplementation(libs.spotbugs.annotations) { - // Exclude conflicting JUnit5. - exclude group: 'org.junit' - exclude group: 'org.junit.jupiter' - exclude group: 'org.junit.platform' - // Exclude conflicting logback. - exclude group: 'ch.qos.logback' - // Exclude conflicting log4j. - exclude group: 'org.apache.logging.log4j' + // Only the annotation classes are needed; drop transitive deps to avoid pulling in + // conflicting JUnit5 / logback / log4j (and junit-bom 6.x, which requires JVM 17). + transitive = false } } diff --git a/internal-api/src/main/java/datadog/trace/api/ProcessTags.java b/internal-api/src/main/java/datadog/trace/api/ProcessTags.java index 40b23faa45c..f27191a9efa 100644 --- a/internal-api/src/main/java/datadog/trace/api/ProcessTags.java +++ b/internal-api/src/main/java/datadog/trace/api/ProcessTags.java @@ -6,6 +6,7 @@ import datadog.trace.api.internal.VisibleForTesting; import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; import datadog.trace.util.TraceUtils; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.nio.file.Path; import java.nio.file.Paths; import java.util.Collections; @@ -191,6 +192,13 @@ private static boolean fillWebsphereTags(Map tags) { return false; } + // SpotBugs USO_UNSAFE_OBJECT_SYNCHRONIZATION: should be reviewed by team. + // The lock object is the package-private static-final TAGS collection, which is visible to + // other code in the package and is itself concurrently mutated (put/clear elsewhere also + // synchronize on it). A dedicated private lock object would be cleaner and clearly correct. + @SuppressFBWarnings( + value = "USO_UNSAFE_OBJECT_SYNCHRONIZATION", + justification = "Lock is a package-private static collection; consider a dedicated lock") static void calculate() { if (serializedForm != null || TAGS.isEmpty()) { return; diff --git a/internal-api/src/main/java/datadog/trace/api/debugger/DebuggerConfigBridge.java b/internal-api/src/main/java/datadog/trace/api/debugger/DebuggerConfigBridge.java index 4a85740c133..c0e47dfea51 100644 --- a/internal-api/src/main/java/datadog/trace/api/debugger/DebuggerConfigBridge.java +++ b/internal-api/src/main/java/datadog/trace/api/debugger/DebuggerConfigBridge.java @@ -1,6 +1,7 @@ package datadog.trace.api.debugger; import datadog.trace.api.Config; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import javax.annotation.Nonnull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -11,6 +12,12 @@ public final class DebuggerConfigBridge { private static DebuggerConfigUpdate DEFERRED_UPDATE; private static volatile DebuggerConfigUpdater UPDATER; + // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. + // Agent-internal final bridge class; the lock guards the private static UPDATER/DEFERRED_UPDATE + // holder state and is not a lock external app code would acquire on this class. + @SuppressFBWarnings( + value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION", + justification = "Agent-internal static holder; class lock guards private static fields") public static synchronized void updateConfig(DebuggerConfigUpdate update) { if (!update.hasUpdates()) { LOGGER.debug("No config update detected, skipping"); @@ -25,6 +32,12 @@ public static synchronized void updateConfig(DebuggerConfigUpdate update) { } } + // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. + // Agent-internal final bridge class; the lock guards the private static UPDATER/DEFERRED_UPDATE + // holder state and is not a lock external app code would acquire on this class. + @SuppressFBWarnings( + value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION", + justification = "Agent-internal static holder; class lock guards private static fields") public static synchronized void setUpdater(@Nonnull DebuggerConfigUpdater updater) { UPDATER = updater; if (DEFERRED_UPDATE != null && DEFERRED_UPDATE.hasUpdates()) { diff --git a/internal-api/src/main/java/datadog/trace/api/telemetry/IntegrationsCollector.java b/internal-api/src/main/java/datadog/trace/api/telemetry/IntegrationsCollector.java index 2b85a444e6e..426cfab49ba 100644 --- a/internal-api/src/main/java/datadog/trace/api/telemetry/IntegrationsCollector.java +++ b/internal-api/src/main/java/datadog/trace/api/telemetry/IntegrationsCollector.java @@ -1,5 +1,6 @@ package datadog.trace.api.telemetry; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.util.Collections; import java.util.LinkedHashMap; import java.util.Map; @@ -17,6 +18,14 @@ public static IntegrationsCollector get() { return INSTANCE; } + // SpotBugs USO_UNSAFE_METHOD_SYNCHRONIZATION: should be reviewed by team. + // This is a singleton exposed via the static get()/INSTANCE accessor, so any code holding the + // instance synchronizes on the same monitor that this telemetry path uses. The backing queue is + // already a LinkedBlockingQueue, so the method-level lock looks redundant and could be dropped or + // replaced with a private lock. + @SuppressFBWarnings( + value = "USO_UNSAFE_METHOD_SYNCHRONIZATION", + justification = "Singleton exposed via static accessor; review whether monitor is needed") public synchronized void update(Iterable names, boolean enabled) { Integration i = new Integration(); i.names = names; @@ -25,6 +34,14 @@ public synchronized void update(Iterable names, boolean enabled) { integrations.offer(i); } + // SpotBugs USO_UNSAFE_METHOD_SYNCHRONIZATION: should be reviewed by team. + // This is a singleton exposed via the static get()/INSTANCE accessor, so any code holding the + // instance synchronizes on the same monitor that this telemetry path uses. The backing queue is + // already a LinkedBlockingQueue, so the method-level lock looks redundant and could be dropped or + // replaced with a private lock. + @SuppressFBWarnings( + value = "USO_UNSAFE_METHOD_SYNCHRONIZATION", + justification = "Singleton exposed via static accessor; review whether monitor is needed") public synchronized Map drain() { if (integrations.isEmpty()) { return Collections.emptyMap(); diff --git a/internal-api/src/main/java/datadog/trace/api/telemetry/ProductChangeCollector.java b/internal-api/src/main/java/datadog/trace/api/telemetry/ProductChangeCollector.java index e6164819ff2..174338b469c 100644 --- a/internal-api/src/main/java/datadog/trace/api/telemetry/ProductChangeCollector.java +++ b/internal-api/src/main/java/datadog/trace/api/telemetry/ProductChangeCollector.java @@ -1,5 +1,6 @@ package datadog.trace.api.telemetry; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.util.Collections; import java.util.LinkedList; import java.util.List; @@ -17,10 +18,26 @@ public static ProductChangeCollector get() { return INSTANCE; } + // SpotBugs USO_UNSAFE_METHOD_SYNCHRONIZATION: should be reviewed by team. + // This is a singleton exposed via the static get()/INSTANCE accessor, so any code holding the + // instance synchronizes on the same monitor that this telemetry path uses. The backing queue is + // already a LinkedBlockingQueue, so the method-level lock looks redundant and could be dropped or + // replaced with a private lock. + @SuppressFBWarnings( + value = "USO_UNSAFE_METHOD_SYNCHRONIZATION", + justification = "Singleton exposed via static accessor; review whether monitor is needed") public synchronized void update(final ProductChange productChange) { productChanges.offer(productChange); } + // SpotBugs USO_UNSAFE_METHOD_SYNCHRONIZATION: should be reviewed by team. + // This is a singleton exposed via the static get()/INSTANCE accessor, so any code holding the + // instance synchronizes on the same monitor that this telemetry path uses. The backing queue is + // already a LinkedBlockingQueue, so the method-level lock looks redundant and could be dropped or + // replaced with a private lock. + @SuppressFBWarnings( + value = "USO_UNSAFE_METHOD_SYNCHRONIZATION", + justification = "Singleton exposed via static accessor; review whether monitor is needed") public synchronized List drain() { if (productChanges.isEmpty()) { return Collections.emptyList(); diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java index 14a5b5d6d21..0a8cffbee64 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java @@ -19,6 +19,7 @@ import datadog.trace.api.sampling.SamplingRule; import datadog.trace.api.scopemanager.ScopeListener; import datadog.trace.context.TraceScope; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.util.Collections; import java.util.List; import java.util.Map; @@ -251,12 +252,24 @@ public static boolean isRegistered() { return provider != NOOP_TRACER; } + // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. + // Agent-internal static holder; the class lock only serializes registration of the private + // static volatile provider field and is not a monitor app code would contend on. + @SuppressFBWarnings( + value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION", + justification = "Agent-internal static holder; class lock guards private static provider") public static synchronized void registerIfAbsent(final TracerAPI tracer) { if (tracer != null && tracer != NOOP_TRACER) { provider = tracer; } } + // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. + // Agent-internal static holder; the class lock only serializes registration of the private + // static volatile provider field and is not a monitor app code would contend on. + @SuppressFBWarnings( + value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION", + justification = "Agent-internal static holder; class lock guards private static provider") public static synchronized void forceRegister(TracerAPI tracer) { provider = tracer; } diff --git a/internal-api/src/main/java/datadog/trace/util/AgentTaskScheduler.java b/internal-api/src/main/java/datadog/trace/util/AgentTaskScheduler.java index 01e5eb33c63..24955d7a509 100644 --- a/internal-api/src/main/java/datadog/trace/util/AgentTaskScheduler.java +++ b/internal-api/src/main/java/datadog/trace/util/AgentTaskScheduler.java @@ -183,7 +183,14 @@ public Scheduled scheduleWithFixedDelay( return scheduleWithFixedDelay(RunnableTask.INSTANCE, target, initialDelay, period, unit); } - @SuppressFBWarnings("JLM_JSR166_UTILCONCURRENT_MONITORENTER") + // SpotBugs USO_UNSAFE_ACCESSIBLE_OBJECT_SYNCHRONIZATION: false positive, can be suppressed. + // The lock object is the private final workQueue, which never escapes this class and only guards + // one-time worker startup. (The pre-existing JLM warning is about locking a JSR166 concurrent + // collection, which is intentional here.) + @SuppressFBWarnings({ + "JLM_JSR166_UTILCONCURRENT_MONITORENTER", + "USO_UNSAFE_ACCESSIBLE_OBJECT_SYNCHRONIZATION" + }) private void scheduleTarget( final Task task, final Target target, diff --git a/internal-api/src/main/java/datadog/trace/util/UnsafeUtils.java b/internal-api/src/main/java/datadog/trace/util/UnsafeUtils.java index 06cd5f832a0..e09e3532245 100644 --- a/internal-api/src/main/java/datadog/trace/util/UnsafeUtils.java +++ b/internal-api/src/main/java/datadog/trace/util/UnsafeUtils.java @@ -1,6 +1,7 @@ package datadog.trace.util; import de.thetaphi.forbiddenapis.SuppressForbidden; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.lang.reflect.Field; import java.lang.reflect.Modifier; import org.slf4j.Logger; @@ -36,6 +37,13 @@ private static Unsafe getUnsafe() { * @return A shallow clone * @param Type of the object being cloned */ + // SpotBugs UNS_UNSAFE_CALL: false positive, can be suppressed. + // This class is a deliberate wrapper around sun.misc.Unsafe to allocate an instance without + // running constructors for shallow cloning; the Unsafe usage is intentional and necessary, and + // already guarded by a null check with a safe fallback to the original object. + @SuppressFBWarnings( + value = "UNS_UNSAFE_CALL", + justification = "Intentional sun.misc.Unsafe wrapper for shallow cloning") @SuppressWarnings("unchecked") public static T tryShallowClone(T original) { if (UNSAFE == null) { diff --git a/products/metrics/metrics-agent/src/main/java/datadog/metrics/agent/AgentMeter.java b/products/metrics/metrics-agent/src/main/java/datadog/metrics/agent/AgentMeter.java index ec11d10dfcd..e499fb7cec8 100644 --- a/products/metrics/metrics-agent/src/main/java/datadog/metrics/agent/AgentMeter.java +++ b/products/metrics/metrics-agent/src/main/java/datadog/metrics/agent/AgentMeter.java @@ -5,6 +5,7 @@ import datadog.metrics.api.Histograms; import datadog.metrics.api.Monitoring; import datadog.metrics.api.statsd.StatsDClient; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; public class AgentMeter { private static volatile StatsDClient statsdClient = StatsDClient.NO_OP; @@ -20,6 +21,14 @@ public static Monitoring monitoring() { return monitoring; } + // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. + // AgentMeter is an agent-internal holder whose Class object is not exposed to instrumented + // application code, so external lock contention on AgentMeter.class is not a realistic risk. + // The clean fix would be a private static final lock object, but suppression is fine here. + @SuppressFBWarnings( + value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION", + justification = + "Agent-internal holder; AgentMeter.class is not exposed to instrumented app code") public static synchronized void registerIfAbsent( StatsDClient statsDClient, Monitoring monitoring, Histograms.Factory historgramFactory) { if (statsDClient != null && AgentMeter.statsdClient == StatsDClient.NO_OP) { diff --git a/telemetry/src/main/java/datadog/telemetry/dependency/Dependency.java b/telemetry/src/main/java/datadog/telemetry/dependency/Dependency.java index 7d8b36bf489..1092ec0db34 100644 --- a/telemetry/src/main/java/datadog/telemetry/dependency/Dependency.java +++ b/telemetry/src/main/java/datadog/telemetry/dependency/Dependency.java @@ -1,5 +1,6 @@ package datadog.telemetry.dependency; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.io.IOException; import java.io.InputStream; import java.math.BigInteger; @@ -129,6 +130,18 @@ public static List fromMavenPom( return dependencies; } + // SpotBugs USO_UNSAFE_METHOD_SYNCHRONIZATION / USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false + // positive, can be suppressed. + // The lock is the Dependency.class monitor, used to serialize access to the static MessageDigest + // md and static byte[] buf scratch buffers (md.reset()/read into buf/md.digest()). No application + // code locks on this internal telemetry class, and these statics never escape; the + // synchronization + // simply guards shared dependency-resolution state. + @SuppressFBWarnings( + value = {"USO_UNSAFE_METHOD_SYNCHRONIZATION", "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION"}, + justification = + "Static synchronized method guards the shared static MessageDigest and byte[] buffer; the" + + " Dependency.class lock is internal and not exposed to application code.") public static synchronized Dependency guessFallbackNoPom( Attributes manifest, String source, InputStream is) throws IOException { final int slashIndex = source.lastIndexOf('/'); From 4d5318aa4799271db0a5263fe3513c36c02fcdcc Mon Sep 17 00:00:00 2001 From: Alexey Kuznetsov Date: Thu, 18 Jun 2026 09:56:21 -0400 Subject: [PATCH 3/7] Fixed compilation. --- buildSrc/call-site-instrumentation-plugin/build.gradle.kts | 2 +- .../opentelemetry/opentelemetry-0.3/build.gradle | 2 -- gradle/libs.versions.toml | 2 ++ gradle/spotbugs.gradle | 1 + 4 files changed, 4 insertions(+), 3 deletions(-) diff --git a/buildSrc/call-site-instrumentation-plugin/build.gradle.kts b/buildSrc/call-site-instrumentation-plugin/build.gradle.kts index 62d4ecb62c4..12744e0d5d1 100644 --- a/buildSrc/call-site-instrumentation-plugin/build.gradle.kts +++ b/buildSrc/call-site-instrumentation-plugin/build.gradle.kts @@ -25,7 +25,7 @@ apply { } dependencies { - compileOnly("com.google.code.findbugs", "jsr305", "3.0.2") + compileOnly(libs.jsr305) implementation("org.freemarker", "freemarker", "2.3.30") implementation(libs.asm) diff --git a/dd-java-agent/instrumentation/opentelemetry/opentelemetry-0.3/build.gradle b/dd-java-agent/instrumentation/opentelemetry/opentelemetry-0.3/build.gradle index 61a3cf66caf..40024727e01 100644 --- a/dd-java-agent/instrumentation/opentelemetry/opentelemetry-0.3/build.gradle +++ b/dd-java-agent/instrumentation/opentelemetry/opentelemetry-0.3/build.gradle @@ -17,8 +17,6 @@ addTestSuiteForDir('latestDepTest', 'test') dependencies { compileOnly group: 'io.opentelemetry', name: 'opentelemetry-api', version: otelVersion - - compileOnly group: 'com.google.code.findbugs', name: 'jsr305', version: '3.0.2' compileOnly group: 'com.google.auto.value', name: 'auto-value-annotations', version: '1.6.6' testImplementation group: 'io.opentelemetry', name: 'opentelemetry-api', version: otelVersion diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 9241afbed4a..aa473e5745f 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -5,6 +5,7 @@ develocity = "4.4.1" forbiddenapis = "3.10" gradle-tooling-api = "8.14.5" +jsr305 = "3.0.2" spotbugs_annotations = "4.10.2" # DataDog libs and forks @@ -80,6 +81,7 @@ testcontainers = "1.21.4" develocity = { module = "com.gradle:develocity-gradle-plugin", version.ref = "develocity" } forbiddenapis = { module = "de.thetaphi:forbiddenapis", version.ref = "forbiddenapis" } gradle-tooling-api = { module = "org.gradle:gradle-tooling-api", version.ref = "gradle-tooling-api" } +jsr305 = { module = "com.google.code.findbugs:jsr305", version.ref = "jsr305" } spotbugs-annotations = { module = "com.github.spotbugs:spotbugs-annotations", version.ref = "spotbugs_annotations" } # DataDog libs and forks diff --git a/gradle/spotbugs.gradle b/gradle/spotbugs.gradle index f96c63ba1ce..987ae1df866 100644 --- a/gradle/spotbugs.gradle +++ b/gradle/spotbugs.gradle @@ -61,6 +61,7 @@ tasks.matching { it.name.startsWith('spotbugs') }.configureEach { dependencies { compileOnly(libs.spotbugs.annotations) + testImplementation(libs.jsr305) testImplementation(libs.spotbugs.annotations) { // Only the annotation classes are needed; drop transitive deps to avoid pulling in // conflicting JUnit5 / logback / log4j (and junit-bom 6.x, which requires JVM 17). From ba635adaedcd65928bfa0e93fe408852d28817b8 Mon Sep 17 00:00:00 2001 From: Alexey Kuznetsov Date: Thu, 18 Jun 2026 11:06:15 -0400 Subject: [PATCH 4/7] Fixed compilation. --- dd-smoke-tests/apm-tracing-disabled/application/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-smoke-tests/apm-tracing-disabled/application/build.gradle b/dd-smoke-tests/apm-tracing-disabled/application/build.gradle index c5f2d8455dc..aff95d8dd15 100644 --- a/dd-smoke-tests/apm-tracing-disabled/application/build.gradle +++ b/dd-smoke-tests/apm-tracing-disabled/application/build.gradle @@ -34,7 +34,7 @@ if (hasProperty('apiJar')) { } dependencies { - compileOnly libs.spotbugs.annotations + compileOnly 'com.github.spotbugs:spotbugs-annotations:4.10.2' implementation 'org.springframework.boot:spring-boot-starter-web' // OpenTracing 0.32.0 is the last release and is intentionally pinned: this smoke test // exercises the legacy OpenTracing bridge in dd-trace-ot. Do not "upgrade" — there is From 247f390fbe4b4ae8c053443e8b63b472de17d52f Mon Sep 17 00:00:00 2001 From: Alexey Kuznetsov Date: Thu, 18 Jun 2026 17:46:23 -0400 Subject: [PATCH 5/7] Cleanup of Spotbugs annotations. --- .../build.gradle.kts | 3 --- .../csi/impl/AsmSpecificationBuilderTest.java | 4 ++-- .../java/datadog/communication/util/IOUtils.java | 12 ++++++------ .../instrumentation/decorator/SampleJavaClass.java | 3 --- .../trace/civisibility/git/tree/GitDiffParser.java | 4 ++-- .../java/datadog/crashtracking/CrashUploader.java | 3 +-- .../com/datadog/debugger/el/ProbeCondition.java | 4 ++-- .../opentelemetry/shim/trace/OtelSpanEvent.java | 4 ++-- .../agent/tooling/iast/TaintableEnumeration.java | 14 +++++++------- .../datastax/cassandra4/TracingSession.java | 2 +- .../datadog/smoketest/MoshiConfigTestHelper.java | 4 ++-- .../common/writer/ddintake/DDEvpProxyApi.java | 6 +++--- .../src/main/java/datadog/trace/core/DDSpan.java | 3 +-- gradle/spotbugs.gradle | 6 ------ .../metric/CiVisibilityMetricPeriodicAction.java | 4 ++-- .../ConfigInversionMetricPeriodicAction.java | 4 ++-- .../metric/CoreMetricsPeriodicAction.java | 4 ++-- .../telemetry/metric/IastMetricPeriodicAction.java | 4 ++-- .../metric/LLMObsMetricPeriodicAction.java | 4 ++-- .../telemetry/metric/MetricPeriodicAction.java | 4 ++-- .../metric/OtelEnvMetricPeriodicAction.java | 4 ++-- .../metric/OtelSpiMetricPeriodicAction.java | 4 ++-- .../telemetry/metric/WafMetricPeriodicAction.java | 4 ++-- .../metric/MetricPeriodicActionTest.groovy | 6 +++--- .../junit/utils/config/WithConfigExtension.java | 4 ++-- 25 files changed, 52 insertions(+), 66 deletions(-) diff --git a/buildSrc/call-site-instrumentation-plugin/build.gradle.kts b/buildSrc/call-site-instrumentation-plugin/build.gradle.kts index 12744e0d5d1..e55c422a824 100644 --- a/buildSrc/call-site-instrumentation-plugin/build.gradle.kts +++ b/buildSrc/call-site-instrumentation-plugin/build.gradle.kts @@ -37,9 +37,6 @@ dependencies { testRuntimeOnly(libs.junit.platform.launcher) testImplementation(libs.bundles.mockito) testImplementation("javax.servlet", "javax.servlet-api", "3.0.1") - // Only the annotation classes are needed; exclude transitive deps to avoid - // spotbugs-annotations pulling in junit-bom 6.x (requires JVM 17, this plugin targets JVM 8). - testImplementation(libs.spotbugs.annotations) { isTransitive = false } } sourceSets { diff --git a/buildSrc/call-site-instrumentation-plugin/src/test/java/datadog/trace/plugin/csi/impl/AsmSpecificationBuilderTest.java b/buildSrc/call-site-instrumentation-plugin/src/test/java/datadog/trace/plugin/csi/impl/AsmSpecificationBuilderTest.java index ec37662ff58..3f3fb54cacc 100644 --- a/buildSrc/call-site-instrumentation-plugin/src/test/java/datadog/trace/plugin/csi/impl/AsmSpecificationBuilderTest.java +++ b/buildSrc/call-site-instrumentation-plugin/src/test/java/datadog/trace/plugin/csi/impl/AsmSpecificationBuilderTest.java @@ -15,7 +15,6 @@ import datadog.trace.plugin.csi.impl.CallSiteSpecification.AroundSpecification; import datadog.trace.plugin.csi.impl.CallSiteSpecification.BeforeSpecification; import datadog.trace.plugin.csi.util.Types; -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.io.File; import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodType; @@ -553,12 +552,13 @@ void testSpecificationBuilderWithEnabledProperty() { assertEquals(Arrays.asList("true"), result.getEnabled().getArguments()); } + // Mirrors the real StringBuilderCallSite.aroundAppend advice: a single method carrying two + // @CallSite.Around annotations. The test below asserts the builder yields two advices from it. @CallSite(spi = CallSites.class) static class TestWithOtherAnnotations { @CallSite.Around("java.lang.StringBuilder java.lang.StringBuilder.append(java.lang.Object)") @CallSite.Around("java.lang.StringBuffer java.lang.StringBuffer.append(java.lang.Object)") @Nonnull - @SuppressFBWarnings("NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE") static Appendable aroundAppend( @CallSite.This @Nullable Appendable self, @CallSite.Argument(0) @Nullable Object param) throws Throwable { diff --git a/communication/src/main/java/datadog/communication/util/IOUtils.java b/communication/src/main/java/datadog/communication/util/IOUtils.java index 23b41fd6c4d..819e4bb3e18 100644 --- a/communication/src/main/java/datadog/communication/util/IOUtils.java +++ b/communication/src/main/java/datadog/communication/util/IOUtils.java @@ -1,6 +1,5 @@ package datadog.communication.util; -import edu.umd.cs.findbugs.annotations.NonNull; import java.io.BufferedReader; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -16,6 +15,7 @@ import java.nio.file.attribute.BasicFileAttributes; import java.util.ArrayList; import java.util.List; +import javax.annotation.Nonnull; public abstract class IOUtils { @@ -23,11 +23,11 @@ public abstract class IOUtils { private IOUtils() {} - public static @NonNull String readFully(InputStream input) throws IOException { + public static @Nonnull String readFully(InputStream input) throws IOException { return readFully(input, Charset.defaultCharset()); } - public static @NonNull String readFully(InputStream input, Charset charset) throws IOException { + public static @Nonnull String readFully(InputStream input, Charset charset) throws IOException { ByteArrayOutputStream output = new ByteArrayOutputStream(); readFully(input, output); return new String(output.toByteArray(), charset); @@ -41,17 +41,17 @@ public static void readFully(InputStream input, OutputStream output) throws IOEx } } - public static @NonNull List readLines(final InputStream input) throws IOException { + public static @Nonnull List readLines(final InputStream input) throws IOException { return readLines(input, Charset.defaultCharset()); } - public static @NonNull List readLines(final InputStream input, final Charset charset) + public static @Nonnull List readLines(final InputStream input, final Charset charset) throws IOException { final InputStreamReader reader = new InputStreamReader(input, charset); return readLines(reader); } - public static @NonNull List readLines(final Reader input) throws IOException { + public static @Nonnull List readLines(final Reader input) throws IOException { final BufferedReader reader = new BufferedReader(input, DEFAULT_BUFFER_SIZE); final List list = new ArrayList<>(); String line = reader.readLine(); diff --git a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/SampleJavaClass.java b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/SampleJavaClass.java index 7a2f61f2923..7897cd83037 100644 --- a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/SampleJavaClass.java +++ b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/SampleJavaClass.java @@ -1,13 +1,10 @@ package datadog.trace.bootstrap.instrumentation.decorator; -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; - /** * Used by {@link BaseDecoratorTest}. Groovy with Java 10+ doesn't seem to treat it properly as an * anonymous class, so use a Java class instead. */ public class SampleJavaClass { - @SuppressFBWarnings("DM_NEW_FOR_GETCLASS") public static Class anonymousClass = new Runnable() { diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitDiffParser.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitDiffParser.java index 374e3a75f6b..e1f8aefdb73 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitDiffParser.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitDiffParser.java @@ -1,7 +1,6 @@ package datadog.trace.civisibility.git.tree; import datadog.trace.civisibility.diff.LineDiff; -import edu.umd.cs.findbugs.annotations.NonNull; import java.io.BufferedReader; import java.io.IOException; import java.io.InputStream; @@ -12,6 +11,7 @@ import java.util.Map; import java.util.regex.Matcher; import java.util.regex.Pattern; +import javax.annotation.Nonnull; public class GitDiffParser { @@ -20,7 +20,7 @@ public class GitDiffParser { private static final Pattern CHANGED_LINES_PATTERN = Pattern.compile("^@@ -\\d+(,\\d+)? \\+(?\\d+)(,(?\\d+))? @@"); - public static @NonNull LineDiff parse(InputStream input) throws IOException { + public static @Nonnull LineDiff parse(InputStream input) throws IOException { Map linesByRelativePath = new HashMap<>(); BufferedReader bufferedReader = diff --git a/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/CrashUploader.java b/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/CrashUploader.java index 5c37b7c5908..7a07c11dbfb 100644 --- a/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/CrashUploader.java +++ b/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/CrashUploader.java @@ -29,7 +29,6 @@ import datadog.trace.util.AgentThreadFactory; import datadog.trace.util.PidHelper; import de.thetaphi.forbiddenapis.SuppressForbidden; -import edu.umd.cs.findbugs.annotations.NonNull; import java.io.*; import java.nio.charset.Charset; import java.nio.file.Files; @@ -130,7 +129,7 @@ public CrashUploader(@Nonnull final ConfigManager.StoredConfig storedConfig) { } CrashUploader( - @NonNull final Config config, @Nonnull final ConfigManager.StoredConfig storedConfig) { + @Nonnull final Config config, @Nonnull final ConfigManager.StoredConfig storedConfig) { this.config = config; this.storedConfig = storedConfig; this.uploaderSettings = storedConfig.toCrashUploaderSettings(); diff --git a/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/ProbeCondition.java b/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/ProbeCondition.java index 8ded283ec93..f81c92a8b19 100644 --- a/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/ProbeCondition.java +++ b/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/ProbeCondition.java @@ -9,8 +9,8 @@ import com.squareup.moshi.JsonWriter; import datadog.trace.bootstrap.debugger.el.DebuggerScript; import datadog.trace.bootstrap.debugger.el.ValueReferenceResolver; -import edu.umd.cs.findbugs.annotations.NonNull; import java.io.IOException; +import javax.annotation.Nonnull; /** Implements expression language for probe condition */ public final class ProbeCondition implements DebuggerScript { @@ -48,7 +48,7 @@ public ProbeCondition fromJson(JsonReader reader) throws IOException { } @Override - public void toJson(@NonNull JsonWriter jsonWriter, ProbeCondition value) throws IOException { + public void toJson(@Nonnull JsonWriter jsonWriter, ProbeCondition value) throws IOException { if (value == null) { jsonWriter.nullValue(); return; diff --git a/dd-java-agent/agent-otel/otel-shim/src/main/java/datadog/opentelemetry/shim/trace/OtelSpanEvent.java b/dd-java-agent/agent-otel/otel-shim/src/main/java/datadog/opentelemetry/shim/trace/OtelSpanEvent.java index 6bd1802aebb..a49cab4b877 100644 --- a/dd-java-agent/agent-otel/otel-shim/src/main/java/datadog/opentelemetry/shim/trace/OtelSpanEvent.java +++ b/dd-java-agent/agent-otel/otel-shim/src/main/java/datadog/opentelemetry/shim/trace/OtelSpanEvent.java @@ -2,7 +2,6 @@ import datadog.trace.api.time.SystemTimeSource; import datadog.trace.api.time.TimeSource; -import edu.umd.cs.findbugs.annotations.NonNull; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.common.AttributesBuilder; @@ -12,6 +11,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; +import javax.annotation.Nonnull; public class OtelSpanEvent { public static final String EXCEPTION_SPAN_EVENT_NAME = "exception"; @@ -43,7 +43,7 @@ public OtelSpanEvent(String name, Attributes attributes, long timestamp, TimeUni this.timestamp = unit.toNanos(timestamp); } - @NonNull + @Nonnull public static String toTag(List events) { StringBuilder builder = new StringBuilder("["); for (OtelSpanEvent event : events) { diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/iast/TaintableEnumeration.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/iast/TaintableEnumeration.java index f1d67e3a8a0..bf53c00c341 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/iast/TaintableEnumeration.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/iast/TaintableEnumeration.java @@ -3,8 +3,8 @@ import datadog.trace.api.iast.IastContext; import datadog.trace.api.iast.propagation.PropagationModule; import datadog.trace.util.stacktrace.StackUtils; -import edu.umd.cs.findbugs.annotations.NonNull; import java.util.Enumeration; +import javax.annotation.Nonnull; import javax.annotation.Nullable; public class TaintableEnumeration implements Enumeration { @@ -25,8 +25,8 @@ public class TaintableEnumeration implements Enumeration { private TaintableEnumeration( final IastContext ctx, - @NonNull final Enumeration delegate, - @NonNull final PropagationModule module, + @Nonnull final Enumeration delegate, + @Nonnull final PropagationModule module, final byte origin, @Nullable final CharSequence name, final boolean useValueAsName) { @@ -78,8 +78,8 @@ private static boolean nonTaintableEnumerationStack(final StackTraceElement elem public static Enumeration wrap( final IastContext ctx, - @NonNull final Enumeration delegate, - @NonNull final PropagationModule module, + @Nonnull final Enumeration delegate, + @Nonnull final PropagationModule module, final byte origin, @Nullable final CharSequence name) { return new TaintableEnumeration(ctx, delegate, module, origin, name, false); @@ -87,8 +87,8 @@ public static Enumeration wrap( public static Enumeration wrap( final IastContext ctx, - @NonNull final Enumeration delegate, - @NonNull final PropagationModule module, + @Nonnull final Enumeration delegate, + @Nonnull final PropagationModule module, final byte origin, boolean useValueAsName) { return new TaintableEnumeration(ctx, delegate, module, origin, null, useValueAsName); diff --git a/dd-java-agent/instrumentation/datastax-cassandra/datastax-cassandra-4.0/src/main/java/datadog/trace/instrumentation/datastax/cassandra4/TracingSession.java b/dd-java-agent/instrumentation/datastax-cassandra/datastax-cassandra-4.0/src/main/java/datadog/trace/instrumentation/datastax/cassandra4/TracingSession.java index 090f6983310..9b9dc069420 100644 --- a/dd-java-agent/instrumentation/datastax-cassandra/datastax-cassandra-4.0/src/main/java/datadog/trace/instrumentation/datastax/cassandra4/TracingSession.java +++ b/dd-java-agent/instrumentation/datastax-cassandra/datastax-cassandra-4.0/src/main/java/datadog/trace/instrumentation/datastax/cassandra4/TracingSession.java @@ -21,12 +21,12 @@ import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.InstrumentationTags; import datadog.trace.util.AgentThreadFactory; -import edu.umd.cs.findbugs.annotations.Nullable; import java.util.concurrent.CompletionException; import java.util.concurrent.CompletionStage; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import javax.annotation.Nonnull; +import javax.annotation.Nullable; public class TracingSession extends SessionWrapper implements CqlSession { private static final ExecutorService EXECUTOR_SERVICE = diff --git a/dd-smoke-tests/debugger-integration-tests/src/test/java/datadog/smoketest/MoshiConfigTestHelper.java b/dd-smoke-tests/debugger-integration-tests/src/test/java/datadog/smoketest/MoshiConfigTestHelper.java index ce702c585b3..a19c5833212 100644 --- a/dd-smoke-tests/debugger-integration-tests/src/test/java/datadog/smoketest/MoshiConfigTestHelper.java +++ b/dd-smoke-tests/debugger-integration-tests/src/test/java/datadog/smoketest/MoshiConfigTestHelper.java @@ -40,8 +40,8 @@ import com.squareup.moshi.JsonWriter; import com.squareup.moshi.Moshi; import datadog.trace.bootstrap.debugger.el.DebuggerScript; -import edu.umd.cs.findbugs.annotations.NonNull; import java.io.IOException; +import javax.annotation.Nonnull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -61,7 +61,7 @@ public static Moshi createMoshiConfig() { private static class ProbeConditionJsonAdapter extends ProbeCondition.ProbeConditionJsonAdapter { @Override - public void toJson(@NonNull JsonWriter jsonWriter, ProbeCondition value) throws IOException { + public void toJson(@Nonnull JsonWriter jsonWriter, ProbeCondition value) throws IOException { if (value == null) { jsonWriter.nullValue(); return; diff --git a/dd-trace-core/src/main/java/datadog/trace/common/writer/ddintake/DDEvpProxyApi.java b/dd-trace-core/src/main/java/datadog/trace/common/writer/ddintake/DDEvpProxyApi.java index 8926d17e200..e911a980c31 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/writer/ddintake/DDEvpProxyApi.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/writer/ddintake/DDEvpProxyApi.java @@ -10,11 +10,11 @@ import datadog.trace.api.intake.TrackType; import datadog.trace.common.writer.Payload; import datadog.trace.common.writer.RemoteApi; -import edu.umd.cs.findbugs.annotations.NonNull; import java.io.IOException; import java.net.ConnectException; import java.util.Locale; import java.util.concurrent.TimeUnit; +import javax.annotation.Nonnull; import okhttp3.HttpUrl; import okhttp3.OkHttpClient; import okhttp3.Request; @@ -36,7 +36,7 @@ public static DDEvpProxyApiBuilder builder() { public static class DDEvpProxyApiBuilder { private String apiVersion = DEFAULT_INTAKE_VERSION; - @NonNull private TrackType trackType = TrackType.NOOP; + @Nonnull private TrackType trackType = TrackType.NOOP; private long timeoutMillis = TimeUnit.SECONDS.toMillis(DEFAULT_INTAKE_TIMEOUT); HttpUrl agentUrl = null; @@ -44,7 +44,7 @@ public static class DDEvpProxyApiBuilder { String evpProxyEndpoint; boolean compressionEnabled; - public DDEvpProxyApiBuilder trackType(@NonNull final TrackType trackType) { + public DDEvpProxyApiBuilder trackType(@Nonnull final TrackType trackType) { this.trackType = trackType; return this; } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java index 8ffcc77b49c..213f27825bf 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java @@ -32,7 +32,6 @@ import datadog.trace.bootstrap.instrumentation.api.ResourceNamePriorities; import datadog.trace.bootstrap.instrumentation.api.SpanWrapper; import datadog.trace.core.util.StackTraces; -import edu.umd.cs.findbugs.annotations.NonNull; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.util.Collections; import java.util.List; @@ -876,7 +875,7 @@ public String toString() { } @Override - public void attachWrapper(@NonNull SpanWrapper wrapper) { + public void attachWrapper(@Nonnull SpanWrapper wrapper) { WRAPPER_FIELD_UPDATER.compareAndSet(this, null, wrapper); } diff --git a/gradle/spotbugs.gradle b/gradle/spotbugs.gradle index 987ae1df866..a69304a7728 100644 --- a/gradle/spotbugs.gradle +++ b/gradle/spotbugs.gradle @@ -60,11 +60,5 @@ tasks.matching { it.name.startsWith('spotbugs') }.configureEach { dependencies { compileOnly(libs.spotbugs.annotations) - testImplementation(libs.jsr305) - testImplementation(libs.spotbugs.annotations) { - // Only the annotation classes are needed; drop transitive deps to avoid pulling in - // conflicting JUnit5 / logback / log4j (and junit-bom 6.x, which requires JVM 17). - transitive = false - } } diff --git a/telemetry/src/main/java/datadog/telemetry/metric/CiVisibilityMetricPeriodicAction.java b/telemetry/src/main/java/datadog/telemetry/metric/CiVisibilityMetricPeriodicAction.java index 2638f609d83..15d40436a89 100644 --- a/telemetry/src/main/java/datadog/telemetry/metric/CiVisibilityMetricPeriodicAction.java +++ b/telemetry/src/main/java/datadog/telemetry/metric/CiVisibilityMetricPeriodicAction.java @@ -2,10 +2,10 @@ import datadog.trace.api.civisibility.InstrumentationBridge; import datadog.trace.api.telemetry.MetricCollector; -import edu.umd.cs.findbugs.annotations.NonNull; +import javax.annotation.Nonnull; public class CiVisibilityMetricPeriodicAction extends MetricPeriodicAction { - @NonNull + @Nonnull @Override public MetricCollector collector() { return InstrumentationBridge.getMetricCollector(); diff --git a/telemetry/src/main/java/datadog/telemetry/metric/ConfigInversionMetricPeriodicAction.java b/telemetry/src/main/java/datadog/telemetry/metric/ConfigInversionMetricPeriodicAction.java index d1d526fb2e9..d115aa9abeb 100644 --- a/telemetry/src/main/java/datadog/telemetry/metric/ConfigInversionMetricPeriodicAction.java +++ b/telemetry/src/main/java/datadog/telemetry/metric/ConfigInversionMetricPeriodicAction.java @@ -2,11 +2,11 @@ import datadog.trace.api.telemetry.ConfigInversionMetricCollectorImpl; import datadog.trace.api.telemetry.MetricCollector; -import edu.umd.cs.findbugs.annotations.NonNull; +import javax.annotation.Nonnull; public class ConfigInversionMetricPeriodicAction extends MetricPeriodicAction { @Override - @NonNull + @Nonnull public MetricCollector collector() { return ConfigInversionMetricCollectorImpl.getInstance(); } diff --git a/telemetry/src/main/java/datadog/telemetry/metric/CoreMetricsPeriodicAction.java b/telemetry/src/main/java/datadog/telemetry/metric/CoreMetricsPeriodicAction.java index 83ee04c661c..f7475929ba7 100644 --- a/telemetry/src/main/java/datadog/telemetry/metric/CoreMetricsPeriodicAction.java +++ b/telemetry/src/main/java/datadog/telemetry/metric/CoreMetricsPeriodicAction.java @@ -2,10 +2,10 @@ import datadog.trace.api.telemetry.CoreMetricCollector; import datadog.trace.api.telemetry.MetricCollector; -import edu.umd.cs.findbugs.annotations.NonNull; +import javax.annotation.Nonnull; public class CoreMetricsPeriodicAction extends MetricPeriodicAction { - @NonNull + @Nonnull @Override public MetricCollector collector() { return CoreMetricCollector.getInstance(); diff --git a/telemetry/src/main/java/datadog/telemetry/metric/IastMetricPeriodicAction.java b/telemetry/src/main/java/datadog/telemetry/metric/IastMetricPeriodicAction.java index f45889a753c..a4f434f5b82 100644 --- a/telemetry/src/main/java/datadog/telemetry/metric/IastMetricPeriodicAction.java +++ b/telemetry/src/main/java/datadog/telemetry/metric/IastMetricPeriodicAction.java @@ -2,12 +2,12 @@ import datadog.trace.api.iast.telemetry.IastMetricCollector; import datadog.trace.api.telemetry.MetricCollector; -import edu.umd.cs.findbugs.annotations.NonNull; +import javax.annotation.Nonnull; public class IastMetricPeriodicAction extends MetricPeriodicAction { @Override - @NonNull + @Nonnull public MetricCollector collector() { return IastMetricCollector.get(); } diff --git a/telemetry/src/main/java/datadog/telemetry/metric/LLMObsMetricPeriodicAction.java b/telemetry/src/main/java/datadog/telemetry/metric/LLMObsMetricPeriodicAction.java index 7b4e65ff713..91e3cb69eec 100644 --- a/telemetry/src/main/java/datadog/telemetry/metric/LLMObsMetricPeriodicAction.java +++ b/telemetry/src/main/java/datadog/telemetry/metric/LLMObsMetricPeriodicAction.java @@ -2,10 +2,10 @@ import datadog.trace.api.telemetry.LLMObsMetricCollector; import datadog.trace.api.telemetry.MetricCollector; -import edu.umd.cs.findbugs.annotations.NonNull; +import javax.annotation.Nonnull; public class LLMObsMetricPeriodicAction extends MetricPeriodicAction { - @NonNull + @Nonnull @Override public MetricCollector collector() { return LLMObsMetricCollector.get(); diff --git a/telemetry/src/main/java/datadog/telemetry/metric/MetricPeriodicAction.java b/telemetry/src/main/java/datadog/telemetry/metric/MetricPeriodicAction.java index c3088907a00..579eb6c761d 100644 --- a/telemetry/src/main/java/datadog/telemetry/metric/MetricPeriodicAction.java +++ b/telemetry/src/main/java/datadog/telemetry/metric/MetricPeriodicAction.java @@ -5,11 +5,11 @@ import datadog.telemetry.api.DistributionSeries; import datadog.telemetry.api.Metric; import datadog.trace.api.telemetry.MetricCollector; -import edu.umd.cs.findbugs.annotations.NonNull; import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; import java.util.Map; +import javax.annotation.Nonnull; public abstract class MetricPeriodicAction implements TelemetryRunnable.TelemetryPeriodicAction { @Override @@ -23,7 +23,7 @@ public final void doIteration(final TelemetryService service) { toDistributionSeries(rawDistributionSeriesPoints).forEach(service::addDistributionSeries); } - @NonNull + @Nonnull public abstract MetricCollector collector(); private Collection toTelemetryMetrics(final Collection metrics) { diff --git a/telemetry/src/main/java/datadog/telemetry/metric/OtelEnvMetricPeriodicAction.java b/telemetry/src/main/java/datadog/telemetry/metric/OtelEnvMetricPeriodicAction.java index 6a9999ff5a3..2729d48ff2d 100644 --- a/telemetry/src/main/java/datadog/telemetry/metric/OtelEnvMetricPeriodicAction.java +++ b/telemetry/src/main/java/datadog/telemetry/metric/OtelEnvMetricPeriodicAction.java @@ -2,11 +2,11 @@ import datadog.trace.api.telemetry.MetricCollector; import datadog.trace.api.telemetry.OtelEnvMetricCollectorImpl; -import edu.umd.cs.findbugs.annotations.NonNull; +import javax.annotation.Nonnull; public class OtelEnvMetricPeriodicAction extends MetricPeriodicAction { @Override - @NonNull + @Nonnull public MetricCollector collector() { return OtelEnvMetricCollectorImpl.getInstance(); } diff --git a/telemetry/src/main/java/datadog/telemetry/metric/OtelSpiMetricPeriodicAction.java b/telemetry/src/main/java/datadog/telemetry/metric/OtelSpiMetricPeriodicAction.java index bbdd9bfc89c..6016f71d195 100644 --- a/telemetry/src/main/java/datadog/telemetry/metric/OtelSpiMetricPeriodicAction.java +++ b/telemetry/src/main/java/datadog/telemetry/metric/OtelSpiMetricPeriodicAction.java @@ -2,11 +2,11 @@ import datadog.trace.api.telemetry.MetricCollector; import datadog.trace.api.telemetry.OtelSpiCollector; -import edu.umd.cs.findbugs.annotations.NonNull; +import javax.annotation.Nonnull; public class OtelSpiMetricPeriodicAction extends MetricPeriodicAction { @Override - @NonNull + @Nonnull public MetricCollector collector() { return OtelSpiCollector.getInstance(); } diff --git a/telemetry/src/main/java/datadog/telemetry/metric/WafMetricPeriodicAction.java b/telemetry/src/main/java/datadog/telemetry/metric/WafMetricPeriodicAction.java index 6f3d10ba794..90dcedc9f97 100644 --- a/telemetry/src/main/java/datadog/telemetry/metric/WafMetricPeriodicAction.java +++ b/telemetry/src/main/java/datadog/telemetry/metric/WafMetricPeriodicAction.java @@ -2,12 +2,12 @@ import datadog.trace.api.telemetry.MetricCollector; import datadog.trace.api.telemetry.WafMetricCollector; -import edu.umd.cs.findbugs.annotations.NonNull; +import javax.annotation.Nonnull; public class WafMetricPeriodicAction extends MetricPeriodicAction { @Override - @NonNull + @Nonnull public MetricCollector collector() { return WafMetricCollector.get(); } diff --git a/telemetry/src/test/groovy/datadog/telemetry/metric/MetricPeriodicActionTest.groovy b/telemetry/src/test/groovy/datadog/telemetry/metric/MetricPeriodicActionTest.groovy index 0bc65695f55..649260c38d9 100644 --- a/telemetry/src/test/groovy/datadog/telemetry/metric/MetricPeriodicActionTest.groovy +++ b/telemetry/src/test/groovy/datadog/telemetry/metric/MetricPeriodicActionTest.groovy @@ -5,9 +5,9 @@ import datadog.telemetry.TelemetryService import datadog.telemetry.api.DistributionSeries import datadog.telemetry.api.Metric import datadog.trace.api.telemetry.MetricCollector -import edu.umd.cs.findbugs.annotations.NonNull import groovy.transform.NamedDelegate import groovy.transform.NamedVariant +import javax.annotation.Nonnull import spock.lang.Specification class MetricPeriodicActionTest extends Specification { @@ -159,12 +159,12 @@ class MetricPeriodicActionTest extends Specification { private final MetricCollector collector - DefaultMetricPeriodicAction(@NonNull final MetricCollector collector) { + DefaultMetricPeriodicAction(@Nonnull final MetricCollector collector) { this.collector = collector } @Override - @NonNull + @Nonnull MetricCollector collector() { return collector } diff --git a/utils/junit-utils/src/main/java/datadog/trace/junit/utils/config/WithConfigExtension.java b/utils/junit-utils/src/main/java/datadog/trace/junit/utils/config/WithConfigExtension.java index 4c1108f5f58..6b9fe8a1039 100644 --- a/utils/junit-utils/src/main/java/datadog/trace/junit/utils/config/WithConfigExtension.java +++ b/utils/junit-utils/src/main/java/datadog/trace/junit/utils/config/WithConfigExtension.java @@ -2,7 +2,6 @@ import datadog.environment.EnvironmentVariables; import de.thetaphi.forbiddenapis.SuppressForbidden; -import edu.umd.cs.findbugs.annotations.NonNull; import java.lang.reflect.Constructor; import java.lang.reflect.Field; import java.lang.reflect.Modifier; @@ -11,6 +10,7 @@ import java.util.List; import java.util.Map; import java.util.Properties; +import javax.annotation.Nonnull; import org.junit.jupiter.api.extension.AfterAllCallback; import org.junit.jupiter.api.extension.AfterEachCallback; import org.junit.jupiter.api.extension.BeforeAllCallback; @@ -267,7 +267,7 @@ public static class TestEnvironmentVariables } @Override - public String get(@NonNull String name) { + public String get(@Nonnull String name) { return env.get(name); } From 2809641b2fff9a036fe0f9467f2b0b57d669f19b Mon Sep 17 00:00:00 2001 From: Alexey Kuznetsov Date: Wed, 1 Jul 2026 15:27:33 -0400 Subject: [PATCH 6/7] WIP --- .../csi/impl/AsmSpecificationBuilderTest.java | 2 -- .../main/java/datadog/trace/bootstrap/Agent.java | 5 ----- .../java/datadog/trace/bootstrap/WeakMap.java | 4 ---- .../instrumentation/usm/UsmExtractor.java | 4 ---- .../instrumentation/usm/UsmMessageFactory.java | 4 ---- .../com/datadog/debugger/agent/DebuggerAgent.java | 4 ---- .../datadog/debugger/util/DebuggerMetrics.java | 6 ------ .../trace/agent/tooling/TracerInstaller.java | 4 ---- .../datadog/profiling/agent/ProfilingAgent.java | 4 ---- .../agent/tooling/bytebuddy/SharedTypePools.java | 3 --- .../bytebuddy/matcher/HierarchyMatchers.java | 3 --- .../junit4/TestEventsHandlerHolder.java | 8 -------- .../instrumentation/junit5/TestDataFactory.java | 4 ---- .../junit5/TestEventsHandlerHolder.java | 12 ------------ .../spark/SparkLauncherListener.java | 15 --------------- .../testng/TestEventsHandlerHolder.java | 4 ---- .../weaver/DatadogWeaverReporter.java | 10 ---------- .../apm-tracing-disabled/application/build.gradle | 1 - .../smoketest/apmtracingdisabled/Controller.java | 2 -- 19 files changed, 99 deletions(-) diff --git a/buildSrc/call-site-instrumentation-plugin/src/test/java/datadog/trace/plugin/csi/impl/AsmSpecificationBuilderTest.java b/buildSrc/call-site-instrumentation-plugin/src/test/java/datadog/trace/plugin/csi/impl/AsmSpecificationBuilderTest.java index 3f3fb54cacc..14ad65bfb0b 100644 --- a/buildSrc/call-site-instrumentation-plugin/src/test/java/datadog/trace/plugin/csi/impl/AsmSpecificationBuilderTest.java +++ b/buildSrc/call-site-instrumentation-plugin/src/test/java/datadog/trace/plugin/csi/impl/AsmSpecificationBuilderTest.java @@ -552,8 +552,6 @@ void testSpecificationBuilderWithEnabledProperty() { assertEquals(Arrays.asList("true"), result.getEnabled().getArguments()); } - // Mirrors the real StringBuilderCallSite.aroundAppend advice: a single method carrying two - // @CallSite.Around annotations. The test below asserts the builder yields two advices from it. @CallSite(spi = CallSites.class) static class TestWithOtherAnnotations { @CallSite.Around("java.lang.StringBuilder java.lang.StringBuilder.append(java.lang.Object)") diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java index b0cdf977445..b61fea8bf64 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java @@ -533,11 +533,6 @@ public static void shutdown(final boolean sync) { } } - // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. - // Agent is an agent-internal bootstrap class whose Class object is loaded in the isolated agent - // context; application code cannot realistically acquire the same lock, and the lock only guards - // the internal AGENT_CLASSLOADER state. A private static lock object would be cleaner but is not - // required here. @SuppressFBWarnings( value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION", justification = "Agent-internal class; Class lock does not escape to application code") diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/WeakMap.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/WeakMap.java index 92770e95a95..2fedd001284 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/WeakMap.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/WeakMap.java @@ -27,10 +27,6 @@ public static WeakMap newWeakMap() { return SUPPLIER.get(); } - // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. - // WeakMap.Supplier is an agent-internal holder loaded in the isolated agent context; its Class - // lock does not escape to application code, and the lock only guards the private SUPPLIER - // field. @SuppressFBWarnings( value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION", justification = "Agent-internal holder; Class lock does not escape to application code") diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/usm/UsmExtractor.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/usm/UsmExtractor.java index f9686bb32a8..a7373c2e09a 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/usm/UsmExtractor.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/usm/UsmExtractor.java @@ -12,10 +12,6 @@ public static void send(UsmMessage message) { SUPPLIER.send(message); } - // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. - // UsmExtractor.Supplier is an agent-internal holder loaded in the isolated agent context; its - // Class lock does not escape to application code, and the lock only guards the private SUPPLIER - // field. @SuppressFBWarnings( value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION", justification = "Agent-internal holder; Class lock does not escape to application code") diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/usm/UsmMessageFactory.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/usm/UsmMessageFactory.java index 363adea11b2..d580b505fb8 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/usm/UsmMessageFactory.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/usm/UsmMessageFactory.java @@ -19,10 +19,6 @@ public static UsmMessage getRequestMessage( return SUPPLIER.getRequestMessage(connection, buffer, bufferOffset, len); } - // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. - // UsmMessageFactory.Supplier is an agent-internal holder loaded in the isolated agent context; - // its Class lock does not escape to application code, and the lock only guards the private - // SUPPLIER field. @SuppressFBWarnings( value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION", justification = "Agent-internal holder; Class lock does not escape to application code") diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerAgent.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerAgent.java index 0045768d61a..d8dd9f5b8f5 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerAgent.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerAgent.java @@ -76,10 +76,6 @@ public class DebuggerAgent { static final AtomicBoolean symDBEnabled = new AtomicBoolean(); private static ClassesToRetransformFinder classesToRetransformFinder; - // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. - // DebuggerAgent is an agent-internal bootstrap class loaded in the agent's isolated classloader; - // its Class object is not exposed to application or third-party code that could lock on it, so - // there is no realistic risk of an external monitor collision or deadlock. @SuppressFBWarnings( value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION", justification = "Agent-internal class; Class object does not escape to application code") diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/DebuggerMetrics.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/DebuggerMetrics.java index 4e30e5e3e9e..098c183c849 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/DebuggerMetrics.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/DebuggerMetrics.java @@ -30,12 +30,6 @@ private DebuggerMetrics(Config config) { } } - // SpotBugs USO_UNSAFE_METHOD_SYNCHRONIZATION, USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false - // positive, can be suppressed. - // This is a static synchronized singleton accessor in an agent-internal class loaded by the - // agent's isolated classloader. The lock guards only the lazy INSTANCE initialization; neither - // the DebuggerMetrics Class object nor the returned instance's monitor is exposed to or locked on - // by application/third-party code, and no instance synchronized methods lock on `this`. @SuppressFBWarnings( value = {"USO_UNSAFE_METHOD_SYNCHRONIZATION", "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION"}, justification = "Agent-internal singleton; neither Class object nor instance monitor escapes") diff --git a/dd-java-agent/agent-installer/src/main/java/datadog/trace/agent/tooling/TracerInstaller.java b/dd-java-agent/agent-installer/src/main/java/datadog/trace/agent/tooling/TracerInstaller.java index 484bf14e527..10d31c89bd7 100644 --- a/dd-java-agent/agent-installer/src/main/java/datadog/trace/agent/tooling/TracerInstaller.java +++ b/dd-java-agent/agent-installer/src/main/java/datadog/trace/agent/tooling/TracerInstaller.java @@ -20,10 +20,6 @@ public class TracerInstaller { private static final Logger log = LoggerFactory.getLogger(TracerInstaller.class); /** Register a global tracer if no global tracer is already registered. */ - // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. - // TracerInstaller is an agent-internal class loaded in the agent class loader; its Class object - // is not exposed to application code that could lock it, and the lock only serializes one-time - // global tracer installation. @SuppressFBWarnings( value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION", justification = diff --git a/dd-java-agent/agent-profiling/src/main/java/com/datadog/profiling/agent/ProfilingAgent.java b/dd-java-agent/agent-profiling/src/main/java/com/datadog/profiling/agent/ProfilingAgent.java index 8065b069761..9175e0bc40f 100644 --- a/dd-java-agent/agent-profiling/src/main/java/com/datadog/profiling/agent/ProfilingAgent.java +++ b/dd-java-agent/agent-profiling/src/main/java/com/datadog/profiling/agent/ProfilingAgent.java @@ -96,10 +96,6 @@ 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 */ - // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. - // ProfilingAgent is an agent-internal class; its Class object does not escape to application - // code, and the lock exists to make this reentrant startup method serialize one-time profiler - // initialization. @SuppressFBWarnings( value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION", justification = diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/SharedTypePools.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/SharedTypePools.java index 7cb78628fc6..f6029fc8f97 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/SharedTypePools.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/SharedTypePools.java @@ -41,9 +41,6 @@ public static void clear() { SUPPLIER.clear(); } - // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. - // SharedTypePools is an agent-internal holder; its Class object is not exposed to application - // code, and the lock only guards one-time registration of the supplier. @SuppressFBWarnings( value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION", justification = diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/HierarchyMatchers.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/HierarchyMatchers.java index 7af9a3049b1..079e80d49d0 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/HierarchyMatchers.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/HierarchyMatchers.java @@ -99,9 +99,6 @@ ElementMatcher.Junction isAnnotatedWith(NameMatchers.OneOf> HANDLERS = new ConcurrentEnumMap<>(TestFrameworkInstrumentation.class); - // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. - // This holder lives in the agent's instrumentation classloader and is not exposed to application - // code that could lock on its Class. Real-world contention risk is low; a private static final - // lock object would be a cleaner fix. @SuppressFBWarnings( value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION", justification = "Holder class not exposed to application code; locking on its Class is safe") @@ -37,10 +33,6 @@ public static synchronized void start( } /** Used by instrumentation tests */ - // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. - // This holder lives in the agent's instrumentation classloader and is not exposed to application - // code that could lock on its Class. Real-world contention risk is low; a private static final - // lock object would be a cleaner fix. @SuppressFBWarnings( value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION", justification = "Holder class not exposed to application code; locking on its Class is safe") diff --git a/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/TestDataFactory.java b/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/TestDataFactory.java index bfe231992c4..b6d43946a6e 100644 --- a/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/TestDataFactory.java +++ b/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/TestDataFactory.java @@ -25,10 +25,6 @@ private TestDataFactory() {} private static volatile Map> TEST_DESCRIPTOR_FILTER_BY_ENGINE_ID = Collections.emptyMap(); - // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. - // This factory lives in the agent's instrumentation classloader and is not exposed to application - // code that could lock on its Class. Real-world contention risk is low; a private static final - // lock object would be a cleaner fix. @SuppressFBWarnings( value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION", justification = "Holder class not exposed to application code; locking on its Class is safe") diff --git a/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/TestEventsHandlerHolder.java b/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/TestEventsHandlerHolder.java index ead31526704..1c8656a07cd 100644 --- a/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/TestEventsHandlerHolder.java +++ b/dd-java-agent/instrumentation/junit/junit-5/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/TestEventsHandlerHolder.java @@ -23,10 +23,6 @@ public abstract class TestEventsHandlerHolder { private static volatile ContextStore EXECUTION_TRACKER_STORE; - // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. - // This holder lives in the agent's instrumentation classloader and is not exposed to application - // code that could lock on its Class. Real-world contention risk is low; a private static final - // lock object would be a cleaner fix. @SuppressFBWarnings( value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION", justification = "Holder class not exposed to application code; locking on its Class is safe") @@ -52,10 +48,6 @@ public static TestExecutionTracker getExecutionTracker(TestDescriptor testDescri } } - // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. - // This holder lives in the agent's instrumentation classloader and is not exposed to application - // code that could lock on its Class. Real-world contention risk is low; a private static final - // lock object would be a cleaner fix. @SuppressFBWarnings( value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION", justification = "Holder class not exposed to application code; locking on its Class is safe") @@ -77,10 +69,6 @@ public static synchronized void start( } /** Used by instrumentation tests */ - // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. - // This holder lives in the agent's instrumentation classloader and is not exposed to application - // code that could lock on its Class. Real-world contention risk is low; a private static final - // lock object would be a cleaner fix. @SuppressFBWarnings( value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION", justification = "Holder class not exposed to application code; locking on its Class is safe") diff --git a/dd-java-agent/instrumentation/spark/spark-common/src/main/java/datadog/trace/instrumentation/spark/SparkLauncherListener.java b/dd-java-agent/instrumentation/spark/spark-common/src/main/java/datadog/trace/instrumentation/spark/SparkLauncherListener.java index 92e671f0f14..fa050768917 100644 --- a/dd-java-agent/instrumentation/spark/spark-common/src/main/java/datadog/trace/instrumentation/spark/SparkLauncherListener.java +++ b/dd-java-agent/instrumentation/spark/spark-common/src/main/java/datadog/trace/instrumentation/spark/SparkLauncherListener.java @@ -31,11 +31,6 @@ public class SparkLauncherListener implements SparkAppHandle.Listener { private static long submittedTimeMs = 0L; private static long runningTimeMs = 0L; - // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. - // This listener lives in the agent's instrumentation classloader and is not handed to application - // code in a way that lets it lock on the same Class. The Class lock is deliberately shared with - // the synchronized blocks in stateChanged/infoChanged/shutdown-hook to guard the launcherSpan - // state; a private static final lock object would be a cleaner equivalent. Risk is low. @SuppressFBWarnings( value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION", justification = @@ -80,11 +75,6 @@ public static synchronized void createLauncherSpan(Object launcher) { } } - // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. - // This listener lives in the agent's instrumentation classloader and is not handed to application - // code in a way that lets it lock on the same Class. The Class lock is deliberately shared with - // the synchronized blocks in stateChanged/infoChanged/shutdown-hook to guard the launcherSpan - // state; a private static final lock object would be a cleaner equivalent. Risk is low. @SuppressFBWarnings( value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION", justification = @@ -104,11 +94,6 @@ public static synchronized void finishSpan(boolean isError, String errorMessage) launcherSpan = null; } - // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. - // This listener lives in the agent's instrumentation classloader and is not handed to application - // code in a way that lets it lock on the same Class. The Class lock is deliberately shared with - // the synchronized blocks in stateChanged/infoChanged/shutdown-hook to guard the launcherSpan - // state; a private static final lock object would be a cleaner equivalent. Risk is low. @SuppressFBWarnings( value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION", justification = diff --git a/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/TestEventsHandlerHolder.java b/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/TestEventsHandlerHolder.java index 2d1a89f7ba0..09be2e9ce64 100644 --- a/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/TestEventsHandlerHolder.java +++ b/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/TestEventsHandlerHolder.java @@ -15,10 +15,6 @@ public abstract class TestEventsHandlerHolder { private static ContextStore TEST_STORE; - // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. - // This holder lives in the agent's instrumentation classloader and is not exposed to application - // code that could lock on its Class. Real-world contention risk is low; a private static final - // lock object would be a cleaner fix. @SuppressFBWarnings( value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION", justification = "Holder class not exposed to application code; locking on its Class is safe") diff --git a/dd-java-agent/instrumentation/weaver-0.9/src/main/java/datadog/trace/instrumentation/weaver/DatadogWeaverReporter.java b/dd-java-agent/instrumentation/weaver-0.9/src/main/java/datadog/trace/instrumentation/weaver/DatadogWeaverReporter.java index 7fa37c7c997..aaf35631118 100644 --- a/dd-java-agent/instrumentation/weaver-0.9/src/main/java/datadog/trace/instrumentation/weaver/DatadogWeaverReporter.java +++ b/dd-java-agent/instrumentation/weaver-0.9/src/main/java/datadog/trace/instrumentation/weaver/DatadogWeaverReporter.java @@ -29,11 +29,6 @@ public class DatadogWeaverReporter { private static volatile TestEventsHandler TEST_EVENTS_HANDLER; - // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. - // This reporter lives in the agent's instrumentation classloader and is not exposed to - // application - // code that could lock on its Class. Real-world contention risk is low; a private static final - // lock object would be a cleaner fix. @SuppressFBWarnings( value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION", justification = @@ -47,11 +42,6 @@ public static synchronized void start() { } /** Used by instrumentation tests */ - // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. - // This reporter lives in the agent's instrumentation classloader and is not exposed to - // application - // code that could lock on its Class. Real-world contention risk is low; a private static final - // lock object would be a cleaner fix. @SuppressFBWarnings( value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION", justification = diff --git a/dd-smoke-tests/apm-tracing-disabled/application/build.gradle b/dd-smoke-tests/apm-tracing-disabled/application/build.gradle index aff95d8dd15..cac44f5c2dd 100644 --- a/dd-smoke-tests/apm-tracing-disabled/application/build.gradle +++ b/dd-smoke-tests/apm-tracing-disabled/application/build.gradle @@ -34,7 +34,6 @@ if (hasProperty('apiJar')) { } dependencies { - compileOnly 'com.github.spotbugs:spotbugs-annotations:4.10.2' implementation 'org.springframework.boot:spring-boot-starter-web' // OpenTracing 0.32.0 is the last release and is intentionally pinned: this smoke test // exercises the legacy OpenTracing bridge in dd-trace-ot. Do not "upgrade" — there is diff --git a/dd-smoke-tests/apm-tracing-disabled/application/src/main/java/datadog/smoketest/apmtracingdisabled/Controller.java b/dd-smoke-tests/apm-tracing-disabled/application/src/main/java/datadog/smoketest/apmtracingdisabled/Controller.java index 3bb55197614..8fdda3fd75e 100644 --- a/dd-smoke-tests/apm-tracing-disabled/application/src/main/java/datadog/smoketest/apmtracingdisabled/Controller.java +++ b/dd-smoke-tests/apm-tracing-disabled/application/src/main/java/datadog/smoketest/apmtracingdisabled/Controller.java @@ -1,6 +1,5 @@ package datadog.smoketest.apmtracingdisabled; -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import io.opentracing.Span; import io.opentracing.util.GlobalTracer; import java.io.IOException; @@ -51,7 +50,6 @@ public String pathParam( } @GetMapping("/iast") - @SuppressFBWarnings public void write( @RequestParam(name = "injection", required = false) String injection, @RequestParam(name = "url", required = false) String url, From afedd8518ea8049c5c68ee71464a19dffd50d864 Mon Sep 17 00:00:00 2001 From: Alexey Kuznetsov Date: Tue, 7 Jul 2026 14:04:47 -0400 Subject: [PATCH 7/7] Cleanup. --- .../trace/agent/tooling/InstrumenterState.java | 2 +- .../core/datastreams/DefaultPathwayContext.java | 13 ------------- gradle/spotbugFilters/exclude.xml | 6 ------ .../main/java/datadog/trace/api/ProcessTags.java | 2 +- .../trace/api/debugger/DebuggerConfigBridge.java | 6 ------ .../trace/api/telemetry/IntegrationsCollector.java | 4 ++-- .../trace/api/telemetry/ProductChangeCollector.java | 4 ++-- .../bootstrap/instrumentation/api/AgentTracer.java | 6 ------ .../java/datadog/trace/util/AgentTaskScheduler.java | 4 ---- .../main/java/datadog/trace/util/UnsafeUtils.java | 4 ---- .../main/java/datadog/metrics/agent/AgentMeter.java | 4 ---- .../datadog/telemetry/dependency/Dependency.java | 7 ------- 12 files changed, 6 insertions(+), 56 deletions(-) diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/InstrumenterState.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/InstrumenterState.java index 4b2119d8781..203ac7dab87 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/InstrumenterState.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/InstrumenterState.java @@ -122,7 +122,7 @@ public static void blockInstrumentation(ClassLoader classLoader, int instrumenta } /** Records that the instrumentation is blocked by default. */ - // SpotBugs USO_UNSAFE_ACCESSIBLE_OBJECT_SYNCHRONIZATION: should be reviewed by team. + // 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 diff --git a/dd-trace-core/src/main/java/datadog/trace/core/datastreams/DefaultPathwayContext.java b/dd-trace-core/src/main/java/datadog/trace/core/datastreams/DefaultPathwayContext.java index 633a136133f..a6a11610d33 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/datastreams/DefaultPathwayContext.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/datastreams/DefaultPathwayContext.java @@ -75,11 +75,6 @@ public long getHash() { return hash; } - // SpotBugs USO_UNSAFE_METHOD_SYNCHRONIZATION: false positive, can be suppressed. - // DefaultPathwayContext is an agent-internal data-streams object reached only through internal - // abstractions (PathwayContext lives in internal-api and is attached to AgentSpanContext); - // instances are not handed to application or third-party code that could lock on the monitor. - // The lock guards mutable internal pathway state (hash, started, nano ticks) and outputBuffer. @Override @SuppressFBWarnings( value = "USO_UNSAFE_METHOD_SYNCHRONIZATION", @@ -158,10 +153,6 @@ public StatsPoint getSavedStats() { return this.savedStats; } - // SpotBugs USO_UNSAFE_METHOD_SYNCHRONIZATION: false positive, can be suppressed. - // Same rationale as setCheckpoint: instances stay agent-internal and never reach application or - // third-party code that could lock on the monitor. The lock here is needed to safely read the - // mutable hash/nano-tick fields and to guard the shared outputBuffer it writes into. @Override @SuppressFBWarnings( value = "USO_UNSAFE_METHOD_SYNCHRONIZATION", @@ -187,10 +178,6 @@ public synchronized String encode() throws IOException { return new String(base64, ISO_8859_1); } - // SpotBugs USO_UNSAFE_METHOD_SYNCHRONIZATION: false positive, can be suppressed. - // toString only reads the mutable state under the same monitor for a consistent snapshot; the - // object is agent-internal (used in debug logging) and never exposed to application code that - // could synchronize on it. @Override @SuppressFBWarnings( value = "USO_UNSAFE_METHOD_SYNCHRONIZATION", diff --git a/gradle/spotbugFilters/exclude.xml b/gradle/spotbugFilters/exclude.xml index 2822e0e0ac7..2276ed212e0 100644 --- a/gradle/spotbugFilters/exclude.xml +++ b/gradle/spotbugFilters/exclude.xml @@ -23,10 +23,4 @@ - - - - diff --git a/internal-api/src/main/java/datadog/trace/api/ProcessTags.java b/internal-api/src/main/java/datadog/trace/api/ProcessTags.java index f27191a9efa..fa036ac7ac4 100644 --- a/internal-api/src/main/java/datadog/trace/api/ProcessTags.java +++ b/internal-api/src/main/java/datadog/trace/api/ProcessTags.java @@ -192,7 +192,7 @@ private static boolean fillWebsphereTags(Map tags) { return false; } - // SpotBugs USO_UNSAFE_OBJECT_SYNCHRONIZATION: should be reviewed by team. + // Claude: SpotBugs USO_UNSAFE_OBJECT_SYNCHRONIZATION: should be reviewed by team. // The lock object is the package-private static-final TAGS collection, which is visible to // other code in the package and is itself concurrently mutated (put/clear elsewhere also // synchronize on it). A dedicated private lock object would be cleaner and clearly correct. diff --git a/internal-api/src/main/java/datadog/trace/api/debugger/DebuggerConfigBridge.java b/internal-api/src/main/java/datadog/trace/api/debugger/DebuggerConfigBridge.java index c0e47dfea51..12e3a86cf86 100644 --- a/internal-api/src/main/java/datadog/trace/api/debugger/DebuggerConfigBridge.java +++ b/internal-api/src/main/java/datadog/trace/api/debugger/DebuggerConfigBridge.java @@ -12,9 +12,6 @@ public final class DebuggerConfigBridge { private static DebuggerConfigUpdate DEFERRED_UPDATE; private static volatile DebuggerConfigUpdater UPDATER; - // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. - // Agent-internal final bridge class; the lock guards the private static UPDATER/DEFERRED_UPDATE - // holder state and is not a lock external app code would acquire on this class. @SuppressFBWarnings( value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION", justification = "Agent-internal static holder; class lock guards private static fields") @@ -32,9 +29,6 @@ public static synchronized void updateConfig(DebuggerConfigUpdate update) { } } - // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. - // Agent-internal final bridge class; the lock guards the private static UPDATER/DEFERRED_UPDATE - // holder state and is not a lock external app code would acquire on this class. @SuppressFBWarnings( value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION", justification = "Agent-internal static holder; class lock guards private static fields") diff --git a/internal-api/src/main/java/datadog/trace/api/telemetry/IntegrationsCollector.java b/internal-api/src/main/java/datadog/trace/api/telemetry/IntegrationsCollector.java index 426cfab49ba..5e07b939e2b 100644 --- a/internal-api/src/main/java/datadog/trace/api/telemetry/IntegrationsCollector.java +++ b/internal-api/src/main/java/datadog/trace/api/telemetry/IntegrationsCollector.java @@ -18,7 +18,7 @@ public static IntegrationsCollector get() { return INSTANCE; } - // SpotBugs USO_UNSAFE_METHOD_SYNCHRONIZATION: should be reviewed by team. + // Claude: SpotBugs USO_UNSAFE_METHOD_SYNCHRONIZATION: should be reviewed by team. // This is a singleton exposed via the static get()/INSTANCE accessor, so any code holding the // instance synchronizes on the same monitor that this telemetry path uses. The backing queue is // already a LinkedBlockingQueue, so the method-level lock looks redundant and could be dropped or @@ -34,7 +34,7 @@ public synchronized void update(Iterable names, boolean enabled) { integrations.offer(i); } - // SpotBugs USO_UNSAFE_METHOD_SYNCHRONIZATION: should be reviewed by team. + // Claude: SpotBugs USO_UNSAFE_METHOD_SYNCHRONIZATION: should be reviewed by team. // This is a singleton exposed via the static get()/INSTANCE accessor, so any code holding the // instance synchronizes on the same monitor that this telemetry path uses. The backing queue is // already a LinkedBlockingQueue, so the method-level lock looks redundant and could be dropped or diff --git a/internal-api/src/main/java/datadog/trace/api/telemetry/ProductChangeCollector.java b/internal-api/src/main/java/datadog/trace/api/telemetry/ProductChangeCollector.java index 174338b469c..4e87ab7abb3 100644 --- a/internal-api/src/main/java/datadog/trace/api/telemetry/ProductChangeCollector.java +++ b/internal-api/src/main/java/datadog/trace/api/telemetry/ProductChangeCollector.java @@ -18,7 +18,7 @@ public static ProductChangeCollector get() { return INSTANCE; } - // SpotBugs USO_UNSAFE_METHOD_SYNCHRONIZATION: should be reviewed by team. + // Claude: SpotBugs USO_UNSAFE_METHOD_SYNCHRONIZATION: should be reviewed by team. // This is a singleton exposed via the static get()/INSTANCE accessor, so any code holding the // instance synchronizes on the same monitor that this telemetry path uses. The backing queue is // already a LinkedBlockingQueue, so the method-level lock looks redundant and could be dropped or @@ -30,7 +30,7 @@ public synchronized void update(final ProductChange productChange) { productChanges.offer(productChange); } - // SpotBugs USO_UNSAFE_METHOD_SYNCHRONIZATION: should be reviewed by team. + // Claude: SpotBugs USO_UNSAFE_METHOD_SYNCHRONIZATION: should be reviewed by team. // This is a singleton exposed via the static get()/INSTANCE accessor, so any code holding the // instance synchronizes on the same monitor that this telemetry path uses. The backing queue is // already a LinkedBlockingQueue, so the method-level lock looks redundant and could be dropped or diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java index 0a8cffbee64..cd69992a218 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java @@ -252,9 +252,6 @@ public static boolean isRegistered() { return provider != NOOP_TRACER; } - // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. - // Agent-internal static holder; the class lock only serializes registration of the private - // static volatile provider field and is not a monitor app code would contend on. @SuppressFBWarnings( value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION", justification = "Agent-internal static holder; class lock guards private static provider") @@ -264,9 +261,6 @@ public static synchronized void registerIfAbsent(final TracerAPI tracer) { } } - // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. - // Agent-internal static holder; the class lock only serializes registration of the private - // static volatile provider field and is not a monitor app code would contend on. @SuppressFBWarnings( value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION", justification = "Agent-internal static holder; class lock guards private static provider") diff --git a/internal-api/src/main/java/datadog/trace/util/AgentTaskScheduler.java b/internal-api/src/main/java/datadog/trace/util/AgentTaskScheduler.java index 24955d7a509..81ecb2a76d3 100644 --- a/internal-api/src/main/java/datadog/trace/util/AgentTaskScheduler.java +++ b/internal-api/src/main/java/datadog/trace/util/AgentTaskScheduler.java @@ -183,10 +183,6 @@ public Scheduled scheduleWithFixedDelay( return scheduleWithFixedDelay(RunnableTask.INSTANCE, target, initialDelay, period, unit); } - // SpotBugs USO_UNSAFE_ACCESSIBLE_OBJECT_SYNCHRONIZATION: false positive, can be suppressed. - // The lock object is the private final workQueue, which never escapes this class and only guards - // one-time worker startup. (The pre-existing JLM warning is about locking a JSR166 concurrent - // collection, which is intentional here.) @SuppressFBWarnings({ "JLM_JSR166_UTILCONCURRENT_MONITORENTER", "USO_UNSAFE_ACCESSIBLE_OBJECT_SYNCHRONIZATION" diff --git a/internal-api/src/main/java/datadog/trace/util/UnsafeUtils.java b/internal-api/src/main/java/datadog/trace/util/UnsafeUtils.java index e09e3532245..fc9fdabccd8 100644 --- a/internal-api/src/main/java/datadog/trace/util/UnsafeUtils.java +++ b/internal-api/src/main/java/datadog/trace/util/UnsafeUtils.java @@ -37,10 +37,6 @@ private static Unsafe getUnsafe() { * @return A shallow clone * @param Type of the object being cloned */ - // SpotBugs UNS_UNSAFE_CALL: false positive, can be suppressed. - // This class is a deliberate wrapper around sun.misc.Unsafe to allocate an instance without - // running constructors for shallow cloning; the Unsafe usage is intentional and necessary, and - // already guarded by a null check with a safe fallback to the original object. @SuppressFBWarnings( value = "UNS_UNSAFE_CALL", justification = "Intentional sun.misc.Unsafe wrapper for shallow cloning") diff --git a/products/metrics/metrics-agent/src/main/java/datadog/metrics/agent/AgentMeter.java b/products/metrics/metrics-agent/src/main/java/datadog/metrics/agent/AgentMeter.java index e499fb7cec8..2ad193dfeda 100644 --- a/products/metrics/metrics-agent/src/main/java/datadog/metrics/agent/AgentMeter.java +++ b/products/metrics/metrics-agent/src/main/java/datadog/metrics/agent/AgentMeter.java @@ -21,10 +21,6 @@ public static Monitoring monitoring() { return monitoring; } - // SpotBugs USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false positive, can be suppressed. - // AgentMeter is an agent-internal holder whose Class object is not exposed to instrumented - // application code, so external lock contention on AgentMeter.class is not a realistic risk. - // The clean fix would be a private static final lock object, but suppression is fine here. @SuppressFBWarnings( value = "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION", justification = diff --git a/telemetry/src/main/java/datadog/telemetry/dependency/Dependency.java b/telemetry/src/main/java/datadog/telemetry/dependency/Dependency.java index 1092ec0db34..6765f77f4de 100644 --- a/telemetry/src/main/java/datadog/telemetry/dependency/Dependency.java +++ b/telemetry/src/main/java/datadog/telemetry/dependency/Dependency.java @@ -130,13 +130,6 @@ public static List fromMavenPom( return dependencies; } - // SpotBugs USO_UNSAFE_METHOD_SYNCHRONIZATION / USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION: false - // positive, can be suppressed. - // The lock is the Dependency.class monitor, used to serialize access to the static MessageDigest - // md and static byte[] buf scratch buffers (md.reset()/read into buf/md.digest()). No application - // code locks on this internal telemetry class, and these statics never escape; the - // synchronization - // simply guards shared dependency-resolution state. @SuppressFBWarnings( value = {"USO_UNSAFE_METHOD_SYNCHRONIZATION", "USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION"}, justification =