From cc483aeeb0fec12dd9dcc9f35a3956b07877dc0a Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 13 Oct 2026 17:23:23 -0400 Subject: [PATCH 1/8] Add @NoSyntheticAccessors perf contract Marks a top-level class whose nest must contain no synthetic accessors (access$* methods or synthetic constructors), which javac generates for cross-nest private access when targeting Java 8. Co-Authored-By: Claude Opus 5.5 --- .../skills/perf-review/references/checks.md | 1 + .../perfcontract/NoSyntheticAccessors.java | 44 +++++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100644 internal-api/src/main/java/datadog/perfcontract/NoSyntheticAccessors.java diff --git a/.agents/skills/perf-review/references/checks.md b/.agents/skills/perf-review/references/checks.md index 3de39dec2a0..59b0b551036 100644 --- a/.agents/skills/perf-review/references/checks.md +++ b/.agents/skills/perf-review/references/checks.md @@ -34,6 +34,7 @@ Fixed-signature, mechanically checkable: - boxing in specific hot APIs - using a string-API where an id-API exists on a hot decorator - the existing convention rules (e.g. don't extract one-shot instrumentation methods to constants) +- synthetic accessors (`access$*` methods, synthetic constructors) in a nest marked `@NoSyntheticAccessors` (`datadog.perfcontract.NoSyntheticAccessors`) — **already a real check**: a test per annotated class calls `datadog.trace.test.util.SyntheticAccessors.assertNone`, so don't spend AI review on it - *(grows as patterns prove mechanically checkable — migrate them off the AI as they stabilize; the `@NoEscape` and `@StaticLifetime` field-storage checks above belong here once their checkers land)* ## Java addendum (JVM-specific — mechanism authored with JIT-developer authority; **calibrate production-priority against your own escalation history**) diff --git a/internal-api/src/main/java/datadog/perfcontract/NoSyntheticAccessors.java b/internal-api/src/main/java/datadog/perfcontract/NoSyntheticAccessors.java new file mode 100644 index 00000000000..8eb1135c312 --- /dev/null +++ b/internal-api/src/main/java/datadog/perfcontract/NoSyntheticAccessors.java @@ -0,0 +1,44 @@ +package datadog.perfcontract; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Declares that no class in the annotated top-level class's nest needs a synthetic accessor. The + * nest is the top-level class and every class nested in it, including inner, local and anonymous + * classes. + * + *

The tracer compiles to Java 8 bytecode, which has no nestmates. When one class of a nest uses + * a {@code private} member of another, javac generates a static {@code access$NNN} method in the + * class that declares the member, and the caller goes through it. Calling another nest member's + * {@code private} constructor also generates a synthetic constructor, with an extra parameter typed + * as a generated class. Each accessor is a real call in the interpreter and C1, and takes an + * inlining level in C2, so on a hot path it can push a callee past {@code MaxInlineLevel}. + * Accessors also add methods and classes to load at startup. + * + *

The fix is to make the member package-private, or to stop using it across the nest. Which side + * the accessor lands on doesn't matter to this contract: it covers the whole nest. + * + *

This marker changes no runtime behavior. + * + *

Checker contract. Inspect the class files of the annotated class's nest: + * + *

+ * + *

The check is deterministic: tests call {@code + * datadog.trace.test.util.SyntheticAccessors.assertNone} with the annotated class. + */ +@Documented +@PerfContract +@Retention(RetentionPolicy.CLASS) +@Target(ElementType.TYPE) +public @interface NoSyntheticAccessors {} From 24ceeb8b03925084555be4b790f3f790329a8416 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 13 Oct 2026 17:23:26 -0400 Subject: [PATCH 2/8] Add SyntheticAccessors test helper Lists the synthetic accessors in a top-level class's nest by loading every TopLevel$* class file beside it, so tests can enforce @NoSyntheticAccessors deterministically. Co-Authored-By: Claude Opus 5.5 --- .../trace/test/util/SyntheticAccessors.java | 118 ++++++++++++++++++ .../test/util/SyntheticAccessorsTest.java | 70 +++++++++++ 2 files changed, 188 insertions(+) create mode 100644 utils/test-utils/src/main/java/datadog/trace/test/util/SyntheticAccessors.java create mode 100644 utils/test-utils/src/test/java/datadog/trace/test/util/SyntheticAccessorsTest.java diff --git a/utils/test-utils/src/main/java/datadog/trace/test/util/SyntheticAccessors.java b/utils/test-utils/src/main/java/datadog/trace/test/util/SyntheticAccessors.java new file mode 100644 index 00000000000..26dfb22522c --- /dev/null +++ b/utils/test-utils/src/main/java/datadog/trace/test/util/SyntheticAccessors.java @@ -0,0 +1,118 @@ +package datadog.trace.test.util; + +import static java.util.Collections.list; + +import java.io.File; +import java.io.IOException; +import java.lang.reflect.Constructor; +import java.lang.reflect.Method; +import java.net.JarURLConnection; +import java.net.URISyntaxException; +import java.net.URL; +import java.util.ArrayList; +import java.util.List; +import java.util.jar.JarEntry; +import java.util.jar.JarFile; + +/** + * Finds the synthetic accessors javac generates when classes in one nest use each other's {@code + * private} members. Backs the checker contract of {@code + * datadog.perfcontract.NoSyntheticAccessors}. + */ +public final class SyntheticAccessors { + private SyntheticAccessors() {} + + /** Fails if any class in {@code topLevel}'s nest has a synthetic accessor. */ + public static void assertNone(Class topLevel) { + List accessors = find(topLevel); + if (!accessors.isEmpty()) { + throw new AssertionError( + "Synthetic accessors in the nest of " + + topLevel.getName() + + "; make the accessed members package-private:\n " + + String.join("\n ", accessors)); + } + } + + /** + * Lists the synthetic accessors in {@code topLevel}'s nest: methods named {@code access$*} and + * synthetic constructors, each as {@code Class.member}. + */ + public static List find(Class topLevel) { + if (topLevel.getEnclosingClass() != null) { + throw new IllegalArgumentException("not a top-level class: " + topLevel.getName()); + } + List accessors = new ArrayList<>(); + for (Class nestMember : nestMembers(topLevel)) { + for (Method method : nestMember.getDeclaredMethods()) { + if (method.isSynthetic() && method.getName().startsWith("access$")) { + accessors.add(nestMember.getName() + "." + method.getName()); + } + } + for (Constructor constructor : nestMember.getDeclaredConstructors()) { + if (constructor.isSynthetic()) { + accessors.add(nestMember.getName() + "." + constructor.getParameterCount()); + } + } + } + return accessors; + } + + /** + * Loads {@code topLevel} and every class file named {@code TopLevel$*} beside it. Anonymous and + * local classes aren't reachable through reflection, so the class files are listed directly. + */ + private static List> nestMembers(Class topLevel) { + String binaryPrefix = topLevel.getName() + "$"; + String filePrefix = topLevel.getSimpleName() + "$"; + URL url = topLevel.getResource(topLevel.getSimpleName() + ".class"); + if (url == null) { + throw new IllegalStateException("no class file for " + topLevel.getName()); + } + List nestedNames = new ArrayList<>(); + try { + if ("file".equals(url.getProtocol())) { + File[] files = new File(url.toURI()).getParentFile().listFiles(); + if (files != null) { + for (File file : files) { + addNestedName(file.getName(), filePrefix, binaryPrefix, nestedNames); + } + } + } else if ("jar".equals(url.getProtocol())) { + String entryDir = topLevel.getName().replace('.', '/'); + entryDir = entryDir.substring(0, entryDir.lastIndexOf('/') + 1); + JarFile jar = ((JarURLConnection) url.openConnection()).getJarFile(); + for (JarEntry entry : list(jar.entries())) { + String name = entry.getName(); + if (name.startsWith(entryDir) && name.indexOf('/', entryDir.length()) < 0) { + addNestedName(name.substring(entryDir.length()), filePrefix, binaryPrefix, nestedNames); + } + } + } else { + throw new IllegalStateException("unsupported class location: " + url); + } + } catch (IOException | URISyntaxException e) { + throw new IllegalStateException("cannot list the nest of " + topLevel.getName(), e); + } + + List> members = new ArrayList<>(); + members.add(topLevel); + ClassLoader loader = topLevel.getClassLoader(); + for (String name : nestedNames) { + try { + members.add(Class.forName(name, false, loader)); + } catch (ClassNotFoundException e) { + throw new IllegalStateException("cannot load nest member " + name, e); + } + } + return members; + } + + private static void addNestedName( + String fileName, String filePrefix, String binaryPrefix, List nestedNames) { + if (fileName.startsWith(filePrefix) && fileName.endsWith(".class")) { + String suffix = fileName.substring(filePrefix.length(), fileName.length() - 6); + nestedNames.add(binaryPrefix + suffix); + } + } +} diff --git a/utils/test-utils/src/test/java/datadog/trace/test/util/SyntheticAccessorsTest.java b/utils/test-utils/src/test/java/datadog/trace/test/util/SyntheticAccessorsTest.java new file mode 100644 index 00000000000..e1d3e5e339a --- /dev/null +++ b/utils/test-utils/src/test/java/datadog/trace/test/util/SyntheticAccessorsTest.java @@ -0,0 +1,70 @@ +package datadog.trace.test.util; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.List; +import java.util.function.Supplier; +import org.junit.jupiter.api.Test; + +class SyntheticAccessorsTest { + @Test + void findsAccessorsForPrivateMembersUsedAcrossTheNest() { + List accessors = SyntheticAccessors.find(WithAccessors.class); + + // WithAccessors.secret is read by Inner, and Inner's private constructor is called by + // WithAccessors: one access$ method and one synthetic constructor. + assertEquals(2, accessors.size(), accessors.toString()); + assertTrue( + accessors.stream().anyMatch(a -> a.contains("WithAccessors.access$")), + accessors.toString()); + assertTrue( + accessors.stream().anyMatch(a -> a.contains("WithAccessors$Inner.")), + accessors.toString()); + assertThrows(AssertionError.class, () -> SyntheticAccessors.assertNone(WithAccessors.class)); + } + + @Test + void acceptsPackagePrivateMembersAndLambdas() { + SyntheticAccessors.assertNone(WithoutAccessors.class); + } + + @Test + void rejectsNestedClasses() { + assertThrows( + IllegalArgumentException.class, () -> SyntheticAccessors.find(WithAccessors.Inner.class)); + } +} + +final class WithAccessors { + private int secret = 42; + + Inner newInner() { + return new Inner(); + } + + final class Inner { + private Inner() {} + + int read() { + return secret; + } + } +} + +final class WithoutAccessors { + int visible = 42; + + Supplier supplier() { + return () -> visible; + } + + final class Inner { + Inner() {} + + int read() { + return visible; + } + } +} From d5b816ce3367b0c77e372aae400b212033f26084 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 13 Oct 2026 17:23:27 -0400 Subject: [PATCH 3/8] Remove CoreTracer's synthetic accessors CoreSpanBuilder read ten private CoreTracer fields through access$ methods on every span. Make the members used across the nest package-private and mark CoreTracer @NoSyntheticAccessors. Co-Authored-By: Claude Opus 5.5 --- .../java/datadog/trace/core/CoreTracer.java | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 8f52b49052c..5cbed5bc9aa 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -33,6 +33,7 @@ import datadog.metrics.api.Monitoring; import datadog.metrics.api.Recording; import datadog.metrics.api.statsd.StatsDClient; +import datadog.perfcontract.NoSyntheticAccessors; import datadog.trace.api.ClassloaderConfigurationOverrides; import datadog.trace.api.Config; import datadog.trace.api.DDSpanId; @@ -144,6 +145,7 @@ * datadog.trace.api.Tracer and TracerAPI, it coordinates many functions necessary creating, * reporting, and propagating traces */ +@NoSyntheticAccessors public class CoreTracer implements AgentTracer.TracerAPI, TracerFlare.Reporter { private static final Logger log = LoggerFactory.getLogger(CoreTracer.class); @@ -161,7 +163,7 @@ public static CoreTracerBuilder builder() { private final long clockSyncPeriod; /** If the tracer can create inferred services */ - private final boolean allowInferredServices; + final boolean allowInferredServices; /** Last time (in nanosecond ticks) the clock was checked for drift */ private volatile long lastSyncTicks; @@ -213,7 +215,7 @@ private static final class LocalRootSpanTags { } } - private volatile LocalRootSpanTags localRootSpanTags; + volatile LocalRootSpanTags localRootSpanTags; /** * When {@code false}, every exported span is stamped with the {@code _dd.apm.enabled:0} billing @@ -222,9 +224,9 @@ private static final class LocalRootSpanTags { private final boolean apmTracingEnabled; /** A set of tags that are added to every span */ - private final TagMap defaultSpanTags; + final TagMap defaultSpanTags; - private final boolean defaultSpanTagsNeedsIntercept; + final boolean defaultSpanTagsNeedsIntercept; /** number of spans in a pending trace before they get flushed */ private final int partialFlushMinSpans; @@ -233,15 +235,15 @@ private static final class LocalRootSpanTags { private final HealthMetrics healthMetrics; private final Recording traceWriteTimer; - private final IdGenerationStrategy idGenerationStrategy; + final IdGenerationStrategy idGenerationStrategy; private final TraceCollector.Factory traceCollectorFactory; - private final DataStreamsMonitoring dataStreamsMonitoring; + final DataStreamsMonitoring dataStreamsMonitoring; private final ExternalAgentLauncher externalAgentLauncher; - private final boolean disableSamplingMechanismValidation; + final boolean disableSamplingMechanismValidation; private final TimeSource timeSource; - private final ProfilingContextIntegration profilingContextIntegration; - private final boolean injectBaggageAsTags; - private final boolean injectLinksAsTags; + final ProfilingContextIntegration profilingContextIntegration; + final boolean injectBaggageAsTags; + final boolean injectLinksAsTags; private final boolean flushOnClose; private final Collection shutdownListeners = new CopyOnWriteArrayList<>(); @@ -255,7 +257,7 @@ private static final class LocalRootSpanTags { * Span tag interceptors. This Map is only ever added to during initialization, so it doesn't need * to be concurrent. */ - private final TagInterceptor tagInterceptor; + final TagInterceptor tagInterceptor; private final TraceInterceptors interceptors = new TraceInterceptors(); @@ -266,7 +268,7 @@ private static final class LocalRootSpanTags { private final CallbackProvider callbackProviderIast; private final CallbackProvider universalCallbackProvider; - private final PropagationTags.Factory propagationTagsFactory; + final PropagationTags.Factory propagationTagsFactory; // DQH - storing into a static constant, so value will constant propagate and dead code eliminate // the other branch in singleSpanBuilder @@ -276,7 +278,7 @@ private static final class LocalRootSpanTags { // Instance field (not static final) so it honors per-tracer config, e.g. an embedded tracer // built via CoreTracerBuilder#withProperties/#config rather than the global Config.get() // singleton. See the tag-ordering block in buildSpanContext. - private final boolean builderTagsPrecedence; + final boolean builderTagsPrecedence; // Cache used by buildSpan - instance so it can capture the CoreTracer private final ReusableSingleSpanBuilderThreadLocalCache spanBuilderThreadLocalCache = @@ -649,7 +651,7 @@ private CoreTracer( } // These field names must be stable to ensure the builder api is stable. - private CoreTracer( + CoreTracer( final Config config, final String serviceName, SharedCommunicationObjects sharedCommunicationObjects, @@ -1672,7 +1674,7 @@ Recording writeTimer() { return traceWriteTimer.start(); } - private static Map invertMap(Map map) { + static Map invertMap(Map map) { Map inverted = new HashMap<>(map.size()); for (Map.Entry entry : map.entrySet()) { inverted.put(entry.getValue(), entry.getKey()); @@ -2573,7 +2575,7 @@ public synchronized TraceInterceptor add(TraceInterceptor newInterceptor) { private static class ShutdownHook extends Thread { private final WeakReference reference; - private ShutdownHook(final CoreTracer tracer) { + ShutdownHook(final CoreTracer tracer) { super(AGENT_THREAD_GROUP, "dd-tracer-shutdown-hook"); reference = new WeakReference<>(tracer); } From 52b997b787470cff9f8513f9e74e0bc940bba90c Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 13 Oct 2026 17:23:28 -0400 Subject: [PATCH 4/8] Test that CoreTracer's nest has no synthetic accessors Co-Authored-By: Claude Opus 5.5 --- .../core/CoreTracerSyntheticAccessorsTest.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 dd-trace-core/src/test/java/datadog/trace/core/CoreTracerSyntheticAccessorsTest.java diff --git a/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerSyntheticAccessorsTest.java b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerSyntheticAccessorsTest.java new file mode 100644 index 00000000000..355d9536ef7 --- /dev/null +++ b/dd-trace-core/src/test/java/datadog/trace/core/CoreTracerSyntheticAccessorsTest.java @@ -0,0 +1,13 @@ +package datadog.trace.core; + +import static datadog.trace.test.util.SyntheticAccessors.assertNone; + +import org.junit.jupiter.api.Test; + +/** {@link CoreTracer} is {@code @NoSyntheticAccessors}: {@code CoreSpanBuilder} runs per span. */ +class CoreTracerSyntheticAccessorsTest { + @Test + void nestHasNoSyntheticAccessors() { + assertNone(CoreTracer.class); + } +} From 7fb8822adc3c788864bc5f983b65a8a0c04c2e97 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 13 Oct 2026 17:50:27 -0400 Subject: [PATCH 5/8] Exclude SyntheticAccessors from coverage and test jar-hosted nests Its uncovered code is defensive error handling that a normal test run can't reach, so it joins the module's other excluded test helpers. Co-Authored-By: Claude Opus 5.5 --- utils/test-utils/build.gradle.kts | 1 + .../datadog/trace/test/util/SyntheticAccessorsTest.java | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/utils/test-utils/build.gradle.kts b/utils/test-utils/build.gradle.kts index bb2a07b6da7..53d2a4c1e86 100644 --- a/utils/test-utils/build.gradle.kts +++ b/utils/test-utils/build.gradle.kts @@ -23,6 +23,7 @@ extra["excludedClassesCoverage"] = listOf( "datadog.trace.test.util.FlakySpockExtension*", "datadog.trace.test.util.MultipartRequestParser*", "datadog.trace.test.util.NonRetryable", + "datadog.trace.test.util.SyntheticAccessors", ) dependencies { diff --git a/utils/test-utils/src/test/java/datadog/trace/test/util/SyntheticAccessorsTest.java b/utils/test-utils/src/test/java/datadog/trace/test/util/SyntheticAccessorsTest.java index e1d3e5e339a..f817f14c77f 100644 --- a/utils/test-utils/src/test/java/datadog/trace/test/util/SyntheticAccessorsTest.java +++ b/utils/test-utils/src/test/java/datadog/trace/test/util/SyntheticAccessorsTest.java @@ -6,6 +6,7 @@ import java.util.List; import java.util.function.Supplier; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; class SyntheticAccessorsTest { @@ -30,6 +31,13 @@ void acceptsPackagePrivateMembersAndLambdas() { SyntheticAccessors.assertNone(WithoutAccessors.class); } + @Test + void readsNestsFromJars() { + // Assertions comes from the junit-jupiter-api jar, so this lists its nest from jar entries. + assertTrue(Assertions.class.getResource("Assertions.class").getProtocol().equals("jar")); + SyntheticAccessors.find(Assertions.class); + } + @Test void rejectsNestedClasses() { assertThrows( From 3def798ff8d2be90feacba04133b13914ed38a02 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 14 Oct 2026 22:02:33 -0400 Subject: [PATCH 6/8] Move buildSpanContext into CoreTracer buildSpanContext was a static method on CoreSpanBuilder that read twelve of CoreTracer's private fields through synthetic accessors on every span. As a package-private instance method on CoreTracer it reads them directly, so those fields go back to private. The move is mechanical: the tracer parameter becomes this. Co-Authored-By: Claude Opus 5.5 --- .../java/datadog/trace/core/CoreTracer.java | 624 +++++++++--------- 1 file changed, 311 insertions(+), 313 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 5cbed5bc9aa..1b0110aebd5 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -163,7 +163,7 @@ public static CoreTracerBuilder builder() { private final long clockSyncPeriod; /** If the tracer can create inferred services */ - final boolean allowInferredServices; + private final boolean allowInferredServices; /** Last time (in nanosecond ticks) the clock was checked for drift */ private volatile long lastSyncTicks; @@ -215,7 +215,7 @@ private static final class LocalRootSpanTags { } } - volatile LocalRootSpanTags localRootSpanTags; + private volatile LocalRootSpanTags localRootSpanTags; /** * When {@code false}, every exported span is stamped with the {@code _dd.apm.enabled:0} billing @@ -235,15 +235,15 @@ private static final class LocalRootSpanTags { private final HealthMetrics healthMetrics; private final Recording traceWriteTimer; - final IdGenerationStrategy idGenerationStrategy; + private final IdGenerationStrategy idGenerationStrategy; private final TraceCollector.Factory traceCollectorFactory; - final DataStreamsMonitoring dataStreamsMonitoring; + private final DataStreamsMonitoring dataStreamsMonitoring; private final ExternalAgentLauncher externalAgentLauncher; - final boolean disableSamplingMechanismValidation; + private final boolean disableSamplingMechanismValidation; private final TimeSource timeSource; - final ProfilingContextIntegration profilingContextIntegration; - final boolean injectBaggageAsTags; - final boolean injectLinksAsTags; + private final ProfilingContextIntegration profilingContextIntegration; + private final boolean injectBaggageAsTags; + private final boolean injectLinksAsTags; private final boolean flushOnClose; private final Collection shutdownListeners = new CopyOnWriteArrayList<>(); @@ -268,7 +268,7 @@ private static final class LocalRootSpanTags { private final CallbackProvider callbackProviderIast; private final CallbackProvider universalCallbackProvider; - final PropagationTags.Factory propagationTagsFactory; + private final PropagationTags.Factory propagationTagsFactory; // DQH - storing into a static constant, so value will constant propagate and dead code eliminate // the other branch in singleSpanBuilder @@ -278,7 +278,7 @@ private static final class LocalRootSpanTags { // Instance field (not static final) so it honors per-tracer config, e.g. an embedded tracer // built via CoreTracerBuilder#withProperties/#config rather than the global Config.get() // singleton. See the tag-ordering block in buildSpanContext. - final boolean builderTagsPrecedence; + private final boolean builderTagsPrecedence; // Cache used by buildSpan - instance so it can capture the CoreTracer private final ReusableSingleSpanBuilderThreadLocalCache spanBuilderThreadLocalCache = @@ -1682,6 +1682,306 @@ static Map invertMap(Map map) { return Collections.unmodifiableMap(inverted); } + /** + * Build the SpanContext, if the actual span has a parent, the following attributes must be + * propagated: - ServiceName - Baggage - Trace (a list of all spans related) - SpanType + * + * @return the context + */ + DDSpanContext buildSpanContext( + long spanId, + String serviceName, + CharSequence operationName, + String resourceName, + AgentSpanContext resolvedParentSpanContext, + boolean errorFlag, + CharSequence spanType, + TagMap.Ledger tagLedger, + SpanPrototype spanPrototype, + List links, + Object builderRequestContextDataAppSec, + Object builderRequestContextDataIast, + Object builderCiVisibilityContextData) { + DDTraceId traceId; + long parentSpanId; + final Map baggage; + final Baggage w3cBaggage; + final TraceCollector parentTraceCollector; + final int samplingPriority; + final CharSequence origin; + final TagMap coreTags; + final boolean coreTagsNeedsIntercept; + final TagMap rootSpanTags; + final boolean rootSpanTagsNeedsIntercept; + final DDSpanContext context; + Object requestContextDataAppSec; + Object requestContextDataIast; + Object ciVisibilityContextData; + final PathwayContext pathwayContext; + final PropagationTags propagationTags; + + if (spanId == CoreSpanBuilder.AUTO_ASSIGN_SPAN_ID) { + spanId = this.idGenerationStrategy.generateSpanId(); + } + + String parentServiceName = null; + CharSequence serviceNameSource = MANUAL; + // Propagate internal trace. + // Note: if we are not in the context of distributed tracing, and we are starting the first + // root span, parentSpanContext will be null at this point. + if (resolvedParentSpanContext instanceof DDSpanContext) { + final DDSpanContext ddsc = (DDSpanContext) resolvedParentSpanContext; + traceId = ddsc.getTraceId(); + parentSpanId = ddsc.getSpanId(); + baggage = ddsc.getBaggageItems(); + w3cBaggage = null; + parentTraceCollector = ddsc.getTraceCollector(); + samplingPriority = PrioritySampling.UNSET; + origin = null; + coreTags = null; + coreTagsNeedsIntercept = false; + rootSpanTags = null; + rootSpanTagsNeedsIntercept = false; + parentServiceName = ddsc.getServiceName(); + serviceNameSource = ddsc.getServiceNameSource(); + if (serviceName == null) { + serviceName = parentServiceName; + } + RequestContext requestContext = + ((DDSpanContext) resolvedParentSpanContext).getRequestContext(); + if (requestContext != null) { + requestContextDataAppSec = requestContext.getData(RequestContextSlot.APPSEC); + requestContextDataIast = requestContext.getData(RequestContextSlot.IAST); + ciVisibilityContextData = requestContext.getData(RequestContextSlot.CI_VISIBILITY); + } else { + requestContextDataAppSec = null; + requestContextDataIast = null; + ciVisibilityContextData = null; + } + propagationTags = this.propagationTagsFactory.empty(); + } else { + long endToEndStartTime; + + if (resolvedParentSpanContext instanceof ExtractedContext) { + // Propagate external trace + final ExtractedContext extractedContext = (ExtractedContext) resolvedParentSpanContext; + traceId = extractedContext.getTraceId(); + parentSpanId = extractedContext.getSpanId(); + samplingPriority = extractedContext.getSamplingPriority(); + endToEndStartTime = extractedContext.getEndToEndStartTime(); + propagationTags = extractedContext.getPropagationTags(); + } else if (resolvedParentSpanContext != null) { + traceId = + resolvedParentSpanContext.getTraceId() == DDTraceId.ZERO + ? this.idGenerationStrategy.generateTraceId() + : resolvedParentSpanContext.getTraceId(); + parentSpanId = resolvedParentSpanContext.getSpanId(); + samplingPriority = resolvedParentSpanContext.getSamplingPriority(); + endToEndStartTime = 0; + propagationTags = this.propagationTagsFactory.empty(); + } else { + // Start a new trace + traceId = this.idGenerationStrategy.generateTraceId(); + parentSpanId = DDSpanId.ZERO; + samplingPriority = PrioritySampling.UNSET; + endToEndStartTime = 0; + propagationTags = this.propagationTagsFactory.empty(); + } + + ConfigSnapshot traceConfig; + + // Get header tags and set origin whether propagating or not. + if (resolvedParentSpanContext instanceof TagContext) { + TagContext tc = (TagContext) resolvedParentSpanContext; + traceConfig = (ConfigSnapshot) tc.getTraceConfig(); + coreTags = tc.getTags(); + coreTagsNeedsIntercept = true; // maybe intercept isn't needed? + origin = tc.getOrigin(); + baggage = tc.getBaggage(); + w3cBaggage = tc.getW3CBaggage(); + requestContextDataAppSec = tc.getRequestContextDataAppSec(); + requestContextDataIast = tc.getRequestContextDataIast(); + ciVisibilityContextData = tc.getCiVisibilityContextData(); + } else { + traceConfig = null; + coreTags = null; + coreTagsNeedsIntercept = false; + origin = null; + baggage = null; + w3cBaggage = null; + requestContextDataAppSec = null; + requestContextDataIast = null; + ciVisibilityContextData = null; + } + + LocalRootSpanTags localRootSpanTags = this.localRootSpanTags; + rootSpanTags = localRootSpanTags.tags; + rootSpanTagsNeedsIntercept = localRootSpanTags.needsIntercept; + + parentTraceCollector = this.createTraceCollector(traceId, traceConfig); + + if (endToEndStartTime > 0) { + parentTraceCollector.beginEndToEnd(endToEndStartTime); + } + } + + ConfigSnapshot traceConfig = parentTraceCollector.getTraceConfig(); + + // Use parent pathwayContext if present and started + pathwayContext = + resolvedParentSpanContext != null + && resolvedParentSpanContext.getPathwayContext() != null + && resolvedParentSpanContext.getPathwayContext().isStarted() + ? resolvedParentSpanContext.getPathwayContext() + : this.dataStreamsMonitoring.newPathwayContext(); + + // when removing fake services the best upward service name to pick is the local root one + // since a split by tag (i.e. servlet context) might have happened on it. + if (!this.allowInferredServices) { + final DDSpan rootSpan = parentTraceCollector.getRootSpan(); + if (rootSpan != null) { + // An inferred proxy represents the gateway, not the application service. + // Preserve the service and source already resolved for spans beneath it. + // Avoid the synchronized tag lookup when inferred proxies are disabled. + if (!this.initialConfig.isInferredProxyPropagationEnabled() + || rootSpan.getTag("_dd.inferred_span") == null) { + serviceName = rootSpan.getServiceName(); + serviceNameSource = rootSpan.getServiceNameSource(); + } + } else { + serviceName = null; + } + } + if (serviceName == null) { + final Pair serviceNameAndSource = + traceConfig.getPreferredServiceNameAndSource(); + ; + if (serviceNameAndSource != null && serviceNameAndSource.hasLeft()) { + serviceName = serviceNameAndSource.getLeft(); + serviceNameSource = serviceNameAndSource.getRight(); + } + } + Map contextualTags = null; + if (parentServiceName == null) { + // only fetch this on local root spans + final ClassloaderConfigurationOverrides.ContextualInfo contextualInfo = + ClassloaderConfigurationOverrides.maybeGetContextualInfo(); + if (contextualInfo != null) { + // in this case we have a local root without service name. + // We can try to see if we can find one from the thread context classloader + if (serviceName == null) { + serviceName = contextualInfo.getServiceName(); + if (serviceName != null) { + serviceNameSource = contextualInfo.getServiceNameSource(); + } + } + contextualTags = contextualInfo.getTags(); + } + } + if (serviceName == null) { + // it could be on the initial snapshot but may be overridden to null and service name + // cannot be null + serviceName = this.serviceName; + // do not mark as manual when the service name is coming from the tracer default + serviceNameSource = null; + } + + if (operationName == null) { + operationName = resourceName; + } + + final TagMap mergedTracerTags = traceConfig.mergedTracerTags; + boolean mergedTracerTagsNeedsIntercept = traceConfig.mergedTracerTagsNeedsIntercept; + + final int tagsSize = + mergedTracerTags.size() + + (null == tagLedger ? 0 : tagLedger.estimateSize()) + + (null == coreTags ? 0 : coreTags.size()) + + (null == rootSpanTags ? 0 : rootSpanTags.size()) + + (null == contextualTags ? 0 : contextualTags.size()); + + if (builderRequestContextDataAppSec != null) { + requestContextDataAppSec = builderRequestContextDataAppSec; + } + if (builderCiVisibilityContextData != null) { + ciVisibilityContextData = builderCiVisibilityContextData; + } + if (builderRequestContextDataIast != null) { + requestContextDataIast = builderRequestContextDataIast; + } + + // some attributes are inherited from the parent + context = + new DDSpanContext( + traceId, + spanId, + parentSpanId, + parentServiceName, + serviceNameSource, + serviceName, + operationName, + resourceName, + samplingPriority, + origin, + baggage, + w3cBaggage, + errorFlag, + spanType, + tagsSize, + parentTraceCollector, + requestContextDataAppSec, + requestContextDataIast, + ciVisibilityContextData, + pathwayContext, + this.disableSamplingMechanismValidation, + propagationTags, + this.profilingContextIntegration, + this.injectBaggageAsTags, + this.injectLinksAsTags, + mergedTracerTagsNeedsIntercept ? null : mergedTracerTags); + + // By setting the tags on the context we apply decorators to any tags that have been set via + // the builder. The `mergedTracerTags` are always applied first (the precedence floor: + // everything overrides them). The remaining contributors are applied last-wins; with + // `builderTagsPrecedence` enabled, `tagLedger` (the explicit builder tags) moves to last so + // it wins collisions instead of being overridden by `coreTags`/`rootSpanTags`/ + // `contextualTags` -- see the PR description for the historical context on this ordering. + // + // mergedTracerTags is trace-level shared state and the precedence floor (everything below + // overrides it). When it carries no interceptable tags it is attached as a read-through + // PARENT at construction (shared by reference, no per-span copy). When it does need + // interception, copy its entries in (the interceptor's per-span side-effects can't be + // shared by reference). + if (mergedTracerTagsNeedsIntercept) { + context.setAllTags(mergedTracerTags, true); + } + if (spanPrototype != SpanPrototype.NONE) { + // Seed the prototype's constant tags + integration name as fallback defaults (span type was + // already seeded onto the builder). apply never clobbers, so tags set below still win, and + // this is the same seam decorator afterStart uses. + context.apply(spanPrototype); + } + if (this.builderTagsPrecedence) { + context.setAllTags(coreTags, coreTagsNeedsIntercept); + context.setAllTags(rootSpanTags, rootSpanTagsNeedsIntercept); + context.setAllTags(contextualTags); + context.setAllTags(tagLedger); + } else { + context.setAllTags(tagLedger); + context.setAllTags(coreTags, coreTagsNeedsIntercept); + context.setAllTags(rootSpanTags, rootSpanTagsNeedsIntercept); + context.setAllTags(contextualTags); + } + // Version is added later by the postProcessor (InternalTagsAdder), only if not already set + // during the request. Config version is kept out of the trace-level bundle (see + // withTracerTags), so this removal now only wipes a version set via the span builder — + // keeping the existing semantics where a builder-set version is replaced by the config + // version. Under read-through this is a cheap local removal (version isn't in the parent, + // so no tombstone). + context.removeTag(Tags.VERSION); + return context; + } + /** Spans are built using this builder */ public abstract static class CoreSpanBuilder implements AgentTracer.SpanBuilder { protected static final boolean USE_SCOPE = false; @@ -1746,8 +2046,7 @@ protected static final DDSpan buildSpan( instrumentationName, timestampMicro, links, - buildSpanContext( - tracer, + tracer.buildSpanContext( spanId, serviceName, operationName, @@ -2100,307 +2399,6 @@ public final CoreSpanBuilder withSpanId(final long spanId) { this.spanId = spanId; return this; } - - /** - * Build the SpanContext, if the actual span has a parent, the following attributes must be - * propagated: - ServiceName - Baggage - Trace (a list of all spans related) - SpanType - * - * @return the context - */ - protected static final DDSpanContext buildSpanContext( - final CoreTracer tracer, - long spanId, - String serviceName, - CharSequence operationName, - String resourceName, - AgentSpanContext resolvedParentSpanContext, - boolean errorFlag, - CharSequence spanType, - TagMap.Ledger tagLedger, - SpanPrototype spanPrototype, - List links, - Object builderRequestContextDataAppSec, - Object builderRequestContextDataIast, - Object builderCiVisibilityContextData) { - DDTraceId traceId; - long parentSpanId; - final Map baggage; - final Baggage w3cBaggage; - final TraceCollector parentTraceCollector; - final int samplingPriority; - final CharSequence origin; - final TagMap coreTags; - final boolean coreTagsNeedsIntercept; - final TagMap rootSpanTags; - final boolean rootSpanTagsNeedsIntercept; - final DDSpanContext context; - Object requestContextDataAppSec; - Object requestContextDataIast; - Object ciVisibilityContextData; - final PathwayContext pathwayContext; - final PropagationTags propagationTags; - - if (spanId == AUTO_ASSIGN_SPAN_ID) { - spanId = tracer.idGenerationStrategy.generateSpanId(); - } - - String parentServiceName = null; - CharSequence serviceNameSource = MANUAL; - // Propagate internal trace. - // Note: if we are not in the context of distributed tracing, and we are starting the first - // root span, parentSpanContext will be null at this point. - if (resolvedParentSpanContext instanceof DDSpanContext) { - final DDSpanContext ddsc = (DDSpanContext) resolvedParentSpanContext; - traceId = ddsc.getTraceId(); - parentSpanId = ddsc.getSpanId(); - baggage = ddsc.getBaggageItems(); - w3cBaggage = null; - parentTraceCollector = ddsc.getTraceCollector(); - samplingPriority = PrioritySampling.UNSET; - origin = null; - coreTags = null; - coreTagsNeedsIntercept = false; - rootSpanTags = null; - rootSpanTagsNeedsIntercept = false; - parentServiceName = ddsc.getServiceName(); - serviceNameSource = ddsc.getServiceNameSource(); - if (serviceName == null) { - serviceName = parentServiceName; - } - RequestContext requestContext = - ((DDSpanContext) resolvedParentSpanContext).getRequestContext(); - if (requestContext != null) { - requestContextDataAppSec = requestContext.getData(RequestContextSlot.APPSEC); - requestContextDataIast = requestContext.getData(RequestContextSlot.IAST); - ciVisibilityContextData = requestContext.getData(RequestContextSlot.CI_VISIBILITY); - } else { - requestContextDataAppSec = null; - requestContextDataIast = null; - ciVisibilityContextData = null; - } - propagationTags = tracer.propagationTagsFactory.empty(); - } else { - long endToEndStartTime; - - if (resolvedParentSpanContext instanceof ExtractedContext) { - // Propagate external trace - final ExtractedContext extractedContext = (ExtractedContext) resolvedParentSpanContext; - traceId = extractedContext.getTraceId(); - parentSpanId = extractedContext.getSpanId(); - samplingPriority = extractedContext.getSamplingPriority(); - endToEndStartTime = extractedContext.getEndToEndStartTime(); - propagationTags = extractedContext.getPropagationTags(); - } else if (resolvedParentSpanContext != null) { - traceId = - resolvedParentSpanContext.getTraceId() == DDTraceId.ZERO - ? tracer.idGenerationStrategy.generateTraceId() - : resolvedParentSpanContext.getTraceId(); - parentSpanId = resolvedParentSpanContext.getSpanId(); - samplingPriority = resolvedParentSpanContext.getSamplingPriority(); - endToEndStartTime = 0; - propagationTags = tracer.propagationTagsFactory.empty(); - } else { - // Start a new trace - traceId = tracer.idGenerationStrategy.generateTraceId(); - parentSpanId = DDSpanId.ZERO; - samplingPriority = PrioritySampling.UNSET; - endToEndStartTime = 0; - propagationTags = tracer.propagationTagsFactory.empty(); - } - - ConfigSnapshot traceConfig; - - // Get header tags and set origin whether propagating or not. - if (resolvedParentSpanContext instanceof TagContext) { - TagContext tc = (TagContext) resolvedParentSpanContext; - traceConfig = (ConfigSnapshot) tc.getTraceConfig(); - coreTags = tc.getTags(); - coreTagsNeedsIntercept = true; // maybe intercept isn't needed? - origin = tc.getOrigin(); - baggage = tc.getBaggage(); - w3cBaggage = tc.getW3CBaggage(); - requestContextDataAppSec = tc.getRequestContextDataAppSec(); - requestContextDataIast = tc.getRequestContextDataIast(); - ciVisibilityContextData = tc.getCiVisibilityContextData(); - } else { - traceConfig = null; - coreTags = null; - coreTagsNeedsIntercept = false; - origin = null; - baggage = null; - w3cBaggage = null; - requestContextDataAppSec = null; - requestContextDataIast = null; - ciVisibilityContextData = null; - } - - LocalRootSpanTags localRootSpanTags = tracer.localRootSpanTags; - rootSpanTags = localRootSpanTags.tags; - rootSpanTagsNeedsIntercept = localRootSpanTags.needsIntercept; - - parentTraceCollector = tracer.createTraceCollector(traceId, traceConfig); - - if (endToEndStartTime > 0) { - parentTraceCollector.beginEndToEnd(endToEndStartTime); - } - } - - ConfigSnapshot traceConfig = parentTraceCollector.getTraceConfig(); - - // Use parent pathwayContext if present and started - pathwayContext = - resolvedParentSpanContext != null - && resolvedParentSpanContext.getPathwayContext() != null - && resolvedParentSpanContext.getPathwayContext().isStarted() - ? resolvedParentSpanContext.getPathwayContext() - : tracer.dataStreamsMonitoring.newPathwayContext(); - - // when removing fake services the best upward service name to pick is the local root one - // since a split by tag (i.e. servlet context) might have happened on it. - if (!tracer.allowInferredServices) { - final DDSpan rootSpan = parentTraceCollector.getRootSpan(); - if (rootSpan != null) { - // An inferred proxy represents the gateway, not the application service. - // Preserve the service and source already resolved for spans beneath it. - // Avoid the synchronized tag lookup when inferred proxies are disabled. - if (!tracer.initialConfig.isInferredProxyPropagationEnabled() - || rootSpan.getTag("_dd.inferred_span") == null) { - serviceName = rootSpan.getServiceName(); - serviceNameSource = rootSpan.getServiceNameSource(); - } - } else { - serviceName = null; - } - } - if (serviceName == null) { - final Pair serviceNameAndSource = - traceConfig.getPreferredServiceNameAndSource(); - ; - if (serviceNameAndSource != null && serviceNameAndSource.hasLeft()) { - serviceName = serviceNameAndSource.getLeft(); - serviceNameSource = serviceNameAndSource.getRight(); - } - } - Map contextualTags = null; - if (parentServiceName == null) { - // only fetch this on local root spans - final ClassloaderConfigurationOverrides.ContextualInfo contextualInfo = - ClassloaderConfigurationOverrides.maybeGetContextualInfo(); - if (contextualInfo != null) { - // in this case we have a local root without service name. - // We can try to see if we can find one from the thread context classloader - if (serviceName == null) { - serviceName = contextualInfo.getServiceName(); - if (serviceName != null) { - serviceNameSource = contextualInfo.getServiceNameSource(); - } - } - contextualTags = contextualInfo.getTags(); - } - } - if (serviceName == null) { - // it could be on the initial snapshot but may be overridden to null and service name - // cannot be null - serviceName = tracer.serviceName; - // do not mark as manual when the service name is coming from the tracer default - serviceNameSource = null; - } - - if (operationName == null) { - operationName = resourceName; - } - - final TagMap mergedTracerTags = traceConfig.mergedTracerTags; - boolean mergedTracerTagsNeedsIntercept = traceConfig.mergedTracerTagsNeedsIntercept; - - final int tagsSize = - mergedTracerTags.size() - + (null == tagLedger ? 0 : tagLedger.estimateSize()) - + (null == coreTags ? 0 : coreTags.size()) - + (null == rootSpanTags ? 0 : rootSpanTags.size()) - + (null == contextualTags ? 0 : contextualTags.size()); - - if (builderRequestContextDataAppSec != null) { - requestContextDataAppSec = builderRequestContextDataAppSec; - } - if (builderCiVisibilityContextData != null) { - ciVisibilityContextData = builderCiVisibilityContextData; - } - if (builderRequestContextDataIast != null) { - requestContextDataIast = builderRequestContextDataIast; - } - - // some attributes are inherited from the parent - context = - new DDSpanContext( - traceId, - spanId, - parentSpanId, - parentServiceName, - serviceNameSource, - serviceName, - operationName, - resourceName, - samplingPriority, - origin, - baggage, - w3cBaggage, - errorFlag, - spanType, - tagsSize, - parentTraceCollector, - requestContextDataAppSec, - requestContextDataIast, - ciVisibilityContextData, - pathwayContext, - tracer.disableSamplingMechanismValidation, - propagationTags, - tracer.profilingContextIntegration, - tracer.injectBaggageAsTags, - tracer.injectLinksAsTags, - mergedTracerTagsNeedsIntercept ? null : mergedTracerTags); - - // By setting the tags on the context we apply decorators to any tags that have been set via - // the builder. The `mergedTracerTags` are always applied first (the precedence floor: - // everything overrides them). The remaining contributors are applied last-wins; with - // `builderTagsPrecedence` enabled, `tagLedger` (the explicit builder tags) moves to last so - // it wins collisions instead of being overridden by `coreTags`/`rootSpanTags`/ - // `contextualTags` -- see the PR description for the historical context on this ordering. - // - // mergedTracerTags is trace-level shared state and the precedence floor (everything below - // overrides it). When it carries no interceptable tags it is attached as a read-through - // PARENT at construction (shared by reference, no per-span copy). When it does need - // interception, copy its entries in (the interceptor's per-span side-effects can't be - // shared by reference). - if (mergedTracerTagsNeedsIntercept) { - context.setAllTags(mergedTracerTags, true); - } - if (spanPrototype != SpanPrototype.NONE) { - // Seed the prototype's constant tags + integration name as fallback defaults (span type was - // already seeded onto the builder). apply never clobbers, so tags set below still win, and - // this is the same seam decorator afterStart uses. - context.apply(spanPrototype); - } - if (tracer.builderTagsPrecedence) { - context.setAllTags(coreTags, coreTagsNeedsIntercept); - context.setAllTags(rootSpanTags, rootSpanTagsNeedsIntercept); - context.setAllTags(contextualTags); - context.setAllTags(tagLedger); - } else { - context.setAllTags(tagLedger); - context.setAllTags(coreTags, coreTagsNeedsIntercept); - context.setAllTags(rootSpanTags, rootSpanTagsNeedsIntercept); - context.setAllTags(contextualTags); - } - // Version is added later by the postProcessor (InternalTagsAdder), only if not already set - // during the request. Config version is kept out of the trace-level bundle (see - // withTracerTags), so this removal now only wipes a version set via the span builder — - // keeping the existing semantics where a builder-set version is replaced by the config - // version. Under read-through this is a cheap local removal (version isn't in the parent, - // so no tombstone). - context.removeTag(Tags.VERSION); - return context; - } } /** CoreSpanBuilder that can be used to produce multiple spans */ From 34ff801f24fbe8681e343fc636375e78ae3ad27a Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 14 Oct 2026 22:23:21 -0400 Subject: [PATCH 7/8] Let @SuppressPerfContract exempt a member from NoSyntheticAccessors SyntheticAccessors now reads the nest's class files with ASM instead of reflecting: only the bytecode says which member an access$ method or synthetic constructor reaches, and @SuppressPerfContract has CLASS retention. An accessor is exempt when its target member carries a suppression naming NoSyntheticAccessors with a non-blank reason, directly or through a reusable suppression annotation. Reports now name each accessor's target. Co-Authored-By: Claude Opus 5.5 --- .../perfcontract/NoSyntheticAccessors.java | 10 +- utils/test-utils/build.gradle.kts | 2 + .../trace/test/util/SyntheticAccessors.java | 330 ++++++++++++++++-- .../test/util/SyntheticAccessorsTest.java | 96 +++++ 4 files changed, 401 insertions(+), 37 deletions(-) diff --git a/internal-api/src/main/java/datadog/perfcontract/NoSyntheticAccessors.java b/internal-api/src/main/java/datadog/perfcontract/NoSyntheticAccessors.java index 8eb1135c312..cde2365a881 100644 --- a/internal-api/src/main/java/datadog/perfcontract/NoSyntheticAccessors.java +++ b/internal-api/src/main/java/datadog/perfcontract/NoSyntheticAccessors.java @@ -19,8 +19,9 @@ * inlining level in C2, so on a hot path it can push a callee past {@code MaxInlineLevel}. * Accessors also add methods and classes to load at startup. * - *

The fix is to make the member package-private, or to stop using it across the nest. Which side - * the accessor lands on doesn't matter to this contract: it covers the whole nest. + *

The fix is to make the member package-private, to stop using it across the nest, or, for a + * member only used off the hot path, to suppress the contract on it. Which side the accessor lands + * on doesn't matter to this contract: it covers the whole nest. * *

This marker changes no runtime behavior. * @@ -31,7 +32,10 @@ * constructor. *

  • Not a violation: other synthetic members, such as lambda bodies ({@code lambda$}) * and bridge methods. - *
  • Exemption: none. To accept accessors in a nest, don't annotate it. + *
  • Exemption: an accessor whose target member, the field, method or constructor it + * reaches, carries {@link SuppressPerfContract} naming this contract, directly or through a + * reusable suppression annotation. Use it for members used across the nest only off the hot + * path, so they can stay {@code private}. * * *

    The check is deterministic: tests call {@code diff --git a/utils/test-utils/build.gradle.kts b/utils/test-utils/build.gradle.kts index 53d2a4c1e86..8349e8eee27 100644 --- a/utils/test-utils/build.gradle.kts +++ b/utils/test-utils/build.gradle.kts @@ -47,4 +47,6 @@ dependencies { compileOnly(libs.bundles.spock) testImplementation(libs.junit.platform.launcher) + // @SuppressPerfContract fixtures; the checker matches annotation descriptors and never loads them. + testCompileOnly(project(":internal-api")) } diff --git a/utils/test-utils/src/main/java/datadog/trace/test/util/SyntheticAccessors.java b/utils/test-utils/src/main/java/datadog/trace/test/util/SyntheticAccessors.java index 26dfb22522c..3ff4b4aa6bd 100644 --- a/utils/test-utils/src/main/java/datadog/trace/test/util/SyntheticAccessors.java +++ b/utils/test-utils/src/main/java/datadog/trace/test/util/SyntheticAccessors.java @@ -4,78 +4,142 @@ import java.io.File; import java.io.IOException; -import java.lang.reflect.Constructor; -import java.lang.reflect.Method; +import java.io.InputStream; import java.net.JarURLConnection; import java.net.URISyntaxException; import java.net.URL; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.jar.JarEntry; import java.util.jar.JarFile; +import net.bytebuddy.jar.asm.AnnotationVisitor; +import net.bytebuddy.jar.asm.ClassReader; +import net.bytebuddy.jar.asm.ClassVisitor; +import net.bytebuddy.jar.asm.FieldVisitor; +import net.bytebuddy.jar.asm.MethodVisitor; +import net.bytebuddy.jar.asm.Opcodes; +import net.bytebuddy.jar.asm.Type; /** * Finds the synthetic accessors javac generates when classes in one nest use each other's {@code * private} members. Backs the checker contract of {@code * datadog.perfcontract.NoSyntheticAccessors}. + * + *

    It reads the nest's class files rather than reflecting on loaded classes, for two reasons: + * only the bytecode says which member an accessor reaches, and {@code @SuppressPerfContract} has + * {@code CLASS} retention, so reflection cannot see it. */ public final class SyntheticAccessors { + private static final String SUPPRESS_DESC = "Ldatadog/perfcontract/SuppressPerfContract;"; + private static final String CONTRACT_DESC = "Ldatadog/perfcontract/NoSyntheticAccessors;"; + private SyntheticAccessors() {} - /** Fails if any class in {@code topLevel}'s nest has a synthetic accessor. */ + /** Fails if any class in {@code topLevel}'s nest has an unsuppressed synthetic accessor. */ public static void assertNone(Class topLevel) { List accessors = find(topLevel); if (!accessors.isEmpty()) { throw new AssertionError( "Synthetic accessors in the nest of " + topLevel.getName() - + "; make the accessed members package-private:\n " + + "; make the accessed members package-private, or suppress them with" + + " @SuppressPerfContract(value = NoSyntheticAccessors.class, reason = ...):\n " + String.join("\n ", accessors)); } } /** - * Lists the synthetic accessors in {@code topLevel}'s nest: methods named {@code access$*} and - * synthetic constructors, each as {@code Class.member}. + * Lists the unsuppressed synthetic accessors in {@code topLevel}'s nest: methods named {@code + * access$*} and synthetic constructors, each as {@code Class.member -> Owner.target}. + * + *

    An accessor is suppressed when the member it reaches carries {@code @SuppressPerfContract} + * naming {@code NoSyntheticAccessors} with a non-blank reason, directly or through an annotation + * that is itself annotated that way. */ public static List find(Class topLevel) { if (topLevel.getEnclosingClass() != null) { throw new IllegalArgumentException("not a top-level class: " + topLevel.getName()); } + ClassLoader loader = topLevel.getClassLoader(); + Map nest = new HashMap<>(); + for (String binaryName : nestClassNames(topLevel)) { + NestClass nestClass = NestClass.read(loader, binaryName.replace('.', '/')); + nest.put(nestClass.internalName, nestClass); + } + List accessors = new ArrayList<>(); - for (Class nestMember : nestMembers(topLevel)) { - for (Method method : nestMember.getDeclaredMethods()) { - if (method.isSynthetic() && method.getName().startsWith("access$")) { - accessors.add(nestMember.getName() + "." + method.getName()); - } - } - for (Constructor constructor : nestMember.getDeclaredConstructors()) { - if (constructor.isSynthetic()) { - accessors.add(nestMember.getName() + "." + constructor.getParameterCount()); + Map suppressingAnnotations = new HashMap<>(); + for (NestClass nestClass : nest.values()) { + for (Accessor accessor : nestClass.accessors) { + Member target = accessor.target == null ? null : resolve(nest, accessor.target); + String problem = suppressionProblem(loader, target, suppressingAnnotations); + if (problem != null) { + accessors.add(accessor.describe(nestClass, target) + problem); } } } + accessors.sort(null); return accessors; } + /** The target as declared in the nest, or {@code null} when it is declared outside it. */ + private static Member resolve(Map nest, MemberRef ref) { + NestClass owner = nest.get(ref.owner); + return owner == null ? null : owner.members.get(ref.key()); + } + + /** + * Returns {@code null} when {@code target} suppresses the contract, else the reason it doesn't, + * formatted as a suffix for the report. + */ + private static String suppressionProblem( + ClassLoader loader, Member target, Map suppressingAnnotations) { + if (target == null) { + return ""; + } + if (target.suppression != null) { + return target.suppression.problem(); + } + for (String annotationDesc : target.annotations) { + if (suppressingAnnotations.computeIfAbsent( + annotationDesc, desc -> isSuppressingAnnotation(loader, desc))) { + return null; + } + } + return ""; + } + + /** Whether the annotation type {@code desc} is meta-annotated with a valid suppression. */ + private static boolean isSuppressingAnnotation(ClassLoader loader, String desc) { + String internalName = Type.getType(desc).getInternalName(); + if (loader.getResource(internalName + ".class") == null) { + return false; + } + Suppression suppression = NestClass.read(loader, internalName).typeSuppression; + return suppression != null && suppression.problem() == null; + } + /** - * Loads {@code topLevel} and every class file named {@code TopLevel$*} beside it. Anonymous and + * Lists {@code topLevel} and every class file named {@code TopLevel$*} beside it. Anonymous and * local classes aren't reachable through reflection, so the class files are listed directly. */ - private static List> nestMembers(Class topLevel) { + private static List nestClassNames(Class topLevel) { String binaryPrefix = topLevel.getName() + "$"; String filePrefix = topLevel.getSimpleName() + "$"; URL url = topLevel.getResource(topLevel.getSimpleName() + ".class"); if (url == null) { throw new IllegalStateException("no class file for " + topLevel.getName()); } - List nestedNames = new ArrayList<>(); + List names = new ArrayList<>(); + names.add(topLevel.getName()); try { if ("file".equals(url.getProtocol())) { File[] files = new File(url.toURI()).getParentFile().listFiles(); if (files != null) { for (File file : files) { - addNestedName(file.getName(), filePrefix, binaryPrefix, nestedNames); + addNestedName(file.getName(), filePrefix, binaryPrefix, names); } } } else if ("jar".equals(url.getProtocol())) { @@ -85,7 +149,7 @@ private static List> nestMembers(Class topLevel) { for (JarEntry entry : list(jar.entries())) { String name = entry.getName(); if (name.startsWith(entryDir) && name.indexOf('/', entryDir.length()) < 0) { - addNestedName(name.substring(entryDir.length()), filePrefix, binaryPrefix, nestedNames); + addNestedName(name.substring(entryDir.length()), filePrefix, binaryPrefix, names); } } } else { @@ -94,25 +158,223 @@ private static List> nestMembers(Class topLevel) { } catch (IOException | URISyntaxException e) { throw new IllegalStateException("cannot list the nest of " + topLevel.getName(), e); } - - List> members = new ArrayList<>(); - members.add(topLevel); - ClassLoader loader = topLevel.getClassLoader(); - for (String name : nestedNames) { - try { - members.add(Class.forName(name, false, loader)); - } catch (ClassNotFoundException e) { - throw new IllegalStateException("cannot load nest member " + name, e); - } - } - return members; + return names; } private static void addNestedName( - String fileName, String filePrefix, String binaryPrefix, List nestedNames) { + String fileName, String filePrefix, String binaryPrefix, List names) { if (fileName.startsWith(filePrefix) && fileName.endsWith(".class")) { String suffix = fileName.substring(filePrefix.length(), fileName.length() - 6); - nestedNames.add(binaryPrefix + suffix); + names.add(binaryPrefix + suffix); + } + } + + /** A field or method reference: owner, name and descriptor, as in the bytecode. */ + private static final class MemberRef { + final String owner; + final String name; + final String desc; + + MemberRef(String owner, String name, String desc) { + this.owner = owner; + this.name = name; + this.desc = desc; + } + + String key() { + return name + desc; + } + } + + /** A declared field, method or constructor, with the annotations the checker needs. */ + private static final class Member { + final String name; + final List annotations = new ArrayList<>(); + Suppression suppression; + + Member(String name) { + this.name = name; + } + } + + /** An {@code @SuppressPerfContract}'s contracts and reason. */ + private static final class Suppression { + final List contracts = new ArrayList<>(); + String reason; + + /** + * Returns {@code null} when this suppresses {@code NoSyntheticAccessors}, else the reason it + * doesn't, formatted as a report suffix. + */ + String problem() { + if (!contracts.contains(CONTRACT_DESC)) { + return " (its @SuppressPerfContract does not name NoSyntheticAccessors)"; + } + if (reason == null || reason.trim().isEmpty()) { + return " (its @SuppressPerfContract has a blank reason)"; + } + return null; + } + + AnnotationVisitor visitor() { + return new AnnotationVisitor(Opcodes.ASM9) { + @Override + public void visit(String name, Object value) { + if ("reason".equals(name)) { + reason = (String) value; + } + } + + @Override + public AnnotationVisitor visitArray(String name) { + if (!"value".equals(name)) { + return null; + } + return new AnnotationVisitor(Opcodes.ASM9) { + @Override + public void visit(String unused, Object value) { + contracts.add(((Type) value).getDescriptor()); + } + }; + } + }; + } + } + + /** A synthetic accessor and the member it reaches, when that can be told from its bytecode. */ + private static final class Accessor { + final String name; + final String desc; + MemberRef target; + + Accessor(String name, String desc) { + this.name = name; + this.desc = desc; + } + + String describe(NestClass owner, Member resolvedTarget) { + String self = + owner.binaryName() + + ("".equals(name) + ? "." + Type.getArgumentTypes(desc).length + : "." + name); + if (target == null) { + return self; + } + String targetName = resolvedTarget == null ? target.name : resolvedTarget.name; + return self + " -> " + target.owner.replace('/', '.') + "." + targetName; + } + } + + /** The parts of one nest class file the checker needs. */ + private static final class NestClass extends ClassVisitor { + String internalName; + final Map members = new HashMap<>(); + final List accessors = new ArrayList<>(); + Suppression typeSuppression; + + private NestClass() { + super(Opcodes.ASM9); + } + + static NestClass read(ClassLoader loader, String internalName) { + NestClass nestClass = new NestClass(); + try (InputStream in = loader.getResourceAsStream(internalName + ".class")) { + if (in == null) { + throw new IllegalStateException("no class file for " + internalName); + } + new ClassReader(in).accept(nestClass, ClassReader.SKIP_DEBUG | ClassReader.SKIP_FRAMES); + } catch (IOException e) { + throw new IllegalStateException("cannot read " + internalName, e); + } + return nestClass; + } + + String binaryName() { + return internalName.replace('/', '.'); + } + + @Override + public void visit( + int version, + int access, + String name, + String signature, + String superName, + String[] interfaces) { + this.internalName = name; + } + + @Override + public AnnotationVisitor visitAnnotation(String desc, boolean visible) { + if (SUPPRESS_DESC.equals(desc)) { + typeSuppression = new Suppression(); + return typeSuppression.visitor(); + } + return null; + } + + @Override + public FieldVisitor visitField( + int access, String name, String desc, String signature, Object value) { + Member member = new Member(name); + members.put(name + desc, member); + return new FieldVisitor(Opcodes.ASM9) { + @Override + public AnnotationVisitor visitAnnotation(String annotationDesc, boolean visible) { + return annotate(member, annotationDesc); + } + }; + } + + @Override + public MethodVisitor visitMethod( + int access, String name, String desc, String signature, String[] exceptions) { + Member member = new Member(name); + members.put(name + desc, member); + boolean synthetic = (access & Opcodes.ACC_SYNTHETIC) != 0; + Accessor accessor = + synthetic && (name.startsWith("access$") || "".equals(name)) + ? new Accessor(name, desc) + : null; + if (accessor != null) { + accessors.add(accessor); + } + return new MethodVisitor(Opcodes.ASM9) { + @Override + public AnnotationVisitor visitAnnotation(String annotationDesc, boolean visible) { + return annotate(member, annotationDesc); + } + + @Override + public void visitFieldInsn(int opcode, String owner, String fieldName, String fieldDesc) { + if (accessor != null && accessor.target == null) { + accessor.target = new MemberRef(owner, fieldName, fieldDesc); + } + } + + @Override + public void visitMethodInsn( + int opcode, String owner, String methodName, String methodDesc, boolean itf) { + if (accessor == null || accessor.target != null) { + return; + } + // A synthetic constructor reaches the private constructor of its own class. + boolean constructor = "".equals(accessor.name); + if (!constructor || ("".equals(methodName) && owner.equals(internalName))) { + accessor.target = new MemberRef(owner, methodName, methodDesc); + } + } + }; + } + + private static AnnotationVisitor annotate(Member member, String annotationDesc) { + if (SUPPRESS_DESC.equals(annotationDesc)) { + member.suppression = new Suppression(); + return member.suppression.visitor(); + } + member.annotations.add(annotationDesc); + return null; } } } diff --git a/utils/test-utils/src/test/java/datadog/trace/test/util/SyntheticAccessorsTest.java b/utils/test-utils/src/test/java/datadog/trace/test/util/SyntheticAccessorsTest.java index f817f14c77f..b0f79d39f37 100644 --- a/utils/test-utils/src/test/java/datadog/trace/test/util/SyntheticAccessorsTest.java +++ b/utils/test-utils/src/test/java/datadog/trace/test/util/SyntheticAccessorsTest.java @@ -4,6 +4,13 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import datadog.perfcontract.NoSyntheticAccessors; +import datadog.perfcontract.Singleton; +import datadog.perfcontract.SuppressPerfContract; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; import java.util.List; import java.util.function.Supplier; import org.junit.jupiter.api.Assertions; @@ -23,6 +30,10 @@ void findsAccessorsForPrivateMembersUsedAcrossTheNest() { assertTrue( accessors.stream().anyMatch(a -> a.contains("WithAccessors$Inner.")), accessors.toString()); + assertTrue( + accessors.stream() + .anyMatch(a -> a.endsWith("-> datadog.trace.test.util.WithAccessors.secret")), + accessors.toString()); assertThrows(AssertionError.class, () -> SyntheticAccessors.assertNone(WithAccessors.class)); } @@ -31,6 +42,35 @@ void acceptsPackagePrivateMembersAndLambdas() { SyntheticAccessors.assertNone(WithoutAccessors.class); } + @Test + void acceptsAccessorsToSuppressedMembers() { + SyntheticAccessors.assertNone(WithSuppressedAccessors.class); + } + + @Test + void acceptsSuppressionsThroughAMetaAnnotation() { + SyntheticAccessors.assertNone(WithMetaSuppressedAccessor.class); + } + + @Test + void rejectsASuppressionWithABlankReason() { + List accessors = SyntheticAccessors.find(WithBlankReasonSuppression.class); + + assertEquals(1, accessors.size(), accessors.toString()); + assertTrue(accessors.get(0).endsWith("(its @SuppressPerfContract has a blank reason)")); + } + + @Test + void rejectsASuppressionOfAnotherContract() { + List accessors = SyntheticAccessors.find(WithOtherContractSuppression.class); + + assertEquals(1, accessors.size(), accessors.toString()); + assertTrue( + accessors + .get(0) + .endsWith("(its @SuppressPerfContract does not name NoSyntheticAccessors)")); + } + @Test void readsNestsFromJars() { // Assertions comes from the junit-jupiter-api jar, so this lists its nest from jar entries. @@ -76,3 +116,59 @@ int read() { } } } + +final class WithSuppressedAccessors { + @SuppressPerfContract(value = NoSyntheticAccessors.class, reason = "read once, at setup") + private int secret = 42; + + Inner newInner() { + return new Inner(); + } + + final class Inner { + @SuppressPerfContract(value = NoSyntheticAccessors.class, reason = "built once, at setup") + private Inner() {} + + int read() { + return secret; + } + } +} + +/** A reusable suppression, applied through a meta-annotation. */ +@SuppressPerfContract(value = NoSyntheticAccessors.class, reason = "off the hot path") +@Retention(RetentionPolicy.CLASS) +@Target({ElementType.FIELD, ElementType.METHOD, ElementType.CONSTRUCTOR}) +@interface ColdPath {} + +final class WithMetaSuppressedAccessor { + @ColdPath private int secret = 42; + + final class Inner { + int read() { + return secret; + } + } +} + +final class WithBlankReasonSuppression { + @SuppressPerfContract(value = NoSyntheticAccessors.class, reason = " ") + private int secret = 42; + + final class Inner { + int read() { + return secret; + } + } +} + +final class WithOtherContractSuppression { + @SuppressPerfContract(value = Singleton.class, reason = "not this contract") + private int secret = 42; + + final class Inner { + int read() { + return secret; + } + } +} From 07e5191fb9bd53e719cd9492ae6b23d2b958a1a5 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 14 Oct 2026 22:23:23 -0400 Subject: [PATCH 8/8] Keep CoreTracer's cold cross-nest members private The constructor, invertMap, ShutdownHook's constructor and the three fields ConfigSnapshot reads are used across the nest only during setup or config reloads. They go back to private with a suppression that records why, instead of being opened to the package. Co-Authored-By: Claude Opus 5.5 --- .../java/datadog/trace/core/CoreTracer.java | 31 +++++++++++++++---- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 1b0110aebd5..921e01b4767 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -34,6 +34,7 @@ import datadog.metrics.api.Recording; import datadog.metrics.api.statsd.StatsDClient; import datadog.perfcontract.NoSyntheticAccessors; +import datadog.perfcontract.SuppressPerfContract; import datadog.trace.api.ClassloaderConfigurationOverrides; import datadog.trace.api.Config; import datadog.trace.api.DDSpanId; @@ -224,9 +225,15 @@ private static final class LocalRootSpanTags { private final boolean apmTracingEnabled; /** A set of tags that are added to every span */ - final TagMap defaultSpanTags; + @SuppressPerfContract( + value = NoSyntheticAccessors.class, + reason = "read by ConfigSnapshot, once per config snapshot") + private final TagMap defaultSpanTags; - final boolean defaultSpanTagsNeedsIntercept; + @SuppressPerfContract( + value = NoSyntheticAccessors.class, + reason = "read by ConfigSnapshot, once per config snapshot") + private final boolean defaultSpanTagsNeedsIntercept; /** number of spans in a pending trace before they get flushed */ private final int partialFlushMinSpans; @@ -257,7 +264,10 @@ private static final class LocalRootSpanTags { * Span tag interceptors. This Map is only ever added to during initialization, so it doesn't need * to be concurrent. */ - final TagInterceptor tagInterceptor; + @SuppressPerfContract( + value = NoSyntheticAccessors.class, + reason = "read by ConfigSnapshot, on a config reload that changes tracing tags") + private final TagInterceptor tagInterceptor; private final TraceInterceptors interceptors = new TraceInterceptors(); @@ -651,7 +661,10 @@ private CoreTracer( } // These field names must be stable to ensure the builder api is stable. - CoreTracer( + @SuppressPerfContract( + value = NoSyntheticAccessors.class, + reason = "called once, by CoreTracerBuilder.build()") + private CoreTracer( final Config config, final String serviceName, SharedCommunicationObjects sharedCommunicationObjects, @@ -1674,7 +1687,10 @@ Recording writeTimer() { return traceWriteTimer.start(); } - static Map invertMap(Map map) { + @SuppressPerfContract( + value = NoSyntheticAccessors.class, + reason = "called once, while building the tracer") + private static Map invertMap(Map map) { Map inverted = new HashMap<>(map.size()); for (Map.Entry entry : map.entrySet()) { inverted.put(entry.getValue(), entry.getKey()); @@ -2573,7 +2589,10 @@ public synchronized TraceInterceptor add(TraceInterceptor newInterceptor) { private static class ShutdownHook extends Thread { private final WeakReference reference; - ShutdownHook(final CoreTracer tracer) { + @SuppressPerfContract( + value = NoSyntheticAccessors.class, + reason = "constructed once, when the tracer registers its shutdown hook") + private ShutdownHook(final CoreTracer tracer) { super(AGENT_THREAD_GROUP, "dd-tracer-shutdown-hook"); reference = new WeakReference<>(tracer); }