Repository navigation
Add @ForegroundSafe/@BackgroundOnly declared-contract reachability check (J16) (AI perf review) - #12474
Closed
dougqh wants to merge 65 commits into
Conversation
dougqh
added
tag: no release notes
Contributor
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6310c5078
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
This comment has been minimized.
This comment has been minimized.
Contributor
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
Contributor
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
LLM ValidationLLM Validation Gate — dd-apm-sdk-review✅ PASS
AnalysisChanged instruction file(s): 1 case(s) dipped but stayed within the pass bar.
Results
Cases
Per-dimension scores, token usage, latency, and estimated cost are in the CI job logs. |
bric3
added this pull request to stack #12505
September 15, 2026 12:29
dougqh
changed the title
Prevent Netty idle timers from retaining request context Co-authored-by: andrea.marziali <andrea.marziali@datadoghq.com>
Prevent Netty startup tasks from inheriting context Cover shaded Netty event executors Co-authored-by: andrea.marziali <andrea.marziali@datadoghq.com>
Avoid abandoning Guidewire test continuations Co-authored-by: andrea.marziali <andrea.marziali@datadoghq.com>
Close ServiceTalk captured context scope in test Co-authored-by: andrea.marziali <andrea.marziali@datadoghq.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8163b89741
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
The type annotation marks linear cache operations as foreground-safe. The interface rule also reports a contract violation when the runtime target is not known.
🤖 Datadog Autotest · Commit 8163b89 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
Prevent nested smoke builds from falling back to Maven Central Redirect nested Maven Central declarations Deduplicate nested Maven proxy repositories The init script injects the repository proxy and redirectMavenCentral() rewrites any mavenCentral() the nested build declares onto that same URL, so both the plugin-management and project repository lists end up holding the proxy twice. They stay distinct Gradle repositories, duplicating metadata and artifact requests on every cache miss. Drop the redundant entries, keeping the repository this script injected: it is the only one known to be unrestricted. A declared mavenCentral() may carry content filters or narrowed metadata sources that the redirect does not lift, so keeping such a declaration instead would stop resolving whatever it excludes. Containers this script never injected into are left untouched, so nothing removes a proxy the nested build owns outright. Also handle a repository declared without a URL, which would otherwise fail with a NullPointerException thrown from an injected init script rather than Gradle's own error. The test fixture now declares Maven Central with a content filter before the injection happens, which is the ordering that makes the choice of survivor observable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: alexey.kuznetsov <alexey.kuznetsov@datadoghq.com>
Fix ATR eligibility with fail-fast test ordering test: clarify ATR eligibility coverage Co-authored-by: daniel-mohedano <daniel.mohedano@datadoghq.com>
Fix Robolectric 4.17 muzzle validation Preserve Robolectric minor release codenames Convert Robolectric codename test to JUnit fix: avoid reflection for Robolectric Android codenames test: cover fixed and latest Robolectric versions fix: smoke test version resolution Merge branch 'master' into alexeyk/fix-robolectric-4-17-muzzle Rollback temporarey fix. Merge branch 'master' into alexeyk/fix-robolectric-4-17-muzzle refactor: simplify Robolectric codename mapping Co-authored-by: daniel-mohedano <daniel.mohedano@datadoghq.com> Co-authored-by: alexey.kuznetsov <alexey.kuznetsov@datadoghq.com>
Infer status 200 for implicit-success Lambda responses Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Update Lambda URL instrumentation test for inferred success status Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Merge branch 'master' into clara.poncet/api-security-lambda Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
…test version. (#12488) Use Apache archive for Tomcat smoke test, since dlcdn has ONLY the latest version. Merge branch 'master' into alexeyk/use-archived-tomcat-distribution Merge branch 'master' into alexeyk/use-archived-tomcat-distribution Merge branch 'master' into alexeyk/use-archived-tomcat-distribution Merge branch 'master' into alexeyk/use-archived-tomcat-distribution Co-authored-by: alexey.kuznetsov <alexey.kuznetsov@datadoghq.com>
fix(ci-visibility): align TIA missing coverage field Merge branch 'master' into daniel.mohedano/fix-tia-missing-line-coverage-field Merge branch 'master' into daniel.mohedano/fix-tia-missing-line-coverage-field Merge branch 'master' into daniel.mohedano/fix-tia-missing-line-coverage-field Merge branch 'master' into daniel.mohedano/fix-tia-missing-line-coverage-field Co-authored-by: daniel.mohedano <daniel.mohedano@datadoghq.com>
4 of 7 tasks
…#12433) Fix `Lettuce5MasterReplicaTest` span wait race Replace `writer.waitForTraces(1)` with a predicate-based wait for the Redis `SET` span. This avoids relying on `ListWriter`’s cumulative trace count after `writer.clear()` and prevents the assertion from running before the command span is written. Reproduce missing peer.hostname with Lettuce running with redis cluster Fix missing peer.hostname on Lettuce cluster spans Fixed the CI failure in Lettuce5ClusterTest Root cause: the test used `grokzen/redis-cluster:6.2.1`, and Docker now returns `NotFound` for that tag. Docker Hub currently lists newer `6.2.x` tags such as `6.2.14` Fixed Lettuce5 muzzle test Use the official Redis image to bootstrap a single-node cluster in Lettuce5ClusterTest available in the CI registry mirror. Fix Lettuce cluster peer tags for replica reads Decorate Lettuce cluster command spans from the physical connection selected by the cluster provider instead of deriving peer tags from the slot-owning master. When ReadFrom routes a cluster read to a replica, getConnectionAsync(READ, slot) can return a replica connection while the slot partition still points to the master. Track RedisURI on the selected write/read connection futures and use that connection context when decorating the active command span. Expand Lettuce5ClusterTest to run a single-shard master/replica Redis Cluster, verify replica-routed reads use the replica peer tags, and keep random exposed ports with room for Redis cluster bus ports. Fixed the Lettuce 6.2 failure. Cause: Lettuce 6.2’s replica read selection requires the replica partition metadata to have a non-zero replication offset. The test created the replica correctly, but the client could still have stale topology metadata when `ReadFrom.SLAVE` was enabled, causing `PartitionSelectorException`. Populate Lettuce cluster context at node connection creation Move `StatefulConnection -> RedisURI` context population from per-command pooled cluster connection lookups to `RedisClusterClient.connectToNodeAsync`, deriving the URI from Lettuce’s resolved `ConnectionFuture` remote address. This removes the hot-path `thenApply` wrappers from cluster read/write selection while preserving command span decoration for the selected physical node. Also add a completed-future fast path for cluster and master/replica span decoration callbacks. Fix Lettuce cluster SET span wait in tests Fix reactive Lettuce cluster peer tagging Activate the stored reactive Redis command span while `RedisSubscription.dispatchCommand()` selects the physical cluster connection, allowing existing connection decoration to add peer tags. Add a reactive cluster regression test covering `peer.hostname` on command spans. Merge branch 'master' into ygree/lettuce-cluster-mode-peer-hostname-fix Sequence Lettuce peer decoration before connection use Return the dependent connection future that performs peer decoration and replace the async advice return values so Lettuce dispatch waits until the selected connection has tagged the active Redis span. Keeps already-completed futures on the synchronous path. Merge branch 'master' into ygree/lettuce-cluster-mode-peer-hostname-fix Merge branch 'master' into ygree/lettuce-cluster-mode-peer-hostname-fix Co-authored-by: yury.gribkov <yury.gribkov@datadoghq.com>
chore: Update Gradle dependencies Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: alexey.kuznetsov <alexey.kuznetsov@datadoghq.com>
FixedSizeCache's per-method markers were unreachable by the checker at real call sites, since every caller holds a DDCache-typed reference. Move @BackgroundOnly onto DDCache.visit() itself (mirroring the EncodingCache precedent) and drop it from FixedSizeCache.clear(), which wasn't cost-justified at the small capacities real callers use. Also fixes an inverted "narrow"/"widen" phrase in both markers' javadoc (three independent reviewers flagged it) and updates the J16 example in performance.md to match.
dougqh
force-pushed
the
dougqh/foreground-background-only-check
branch
from
September 22, 2026 17:28
9a7f0e7 to
bad45b5
Compare
dougqh
requested review from
AlexeyKuznetsov-DD,
claponcet,
mhlidd,
randomanderson,
tylfin and
vandonr
and removed request for
a team
September 22, 2026 17:29
2 of 6 tasks
Contributor
Author
|
Unstacking: this branch was based on the now-paused |
2 of 6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
17 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What Does This Do
datadog.trace.api.function.ForegroundSafe/.BackgroundOnlyannotations declaring a one-way foreground/background cost contract, applied toFixedSizeCache/EncodingCacheas initial markers.background-only-contract) to thedd-apm-sdk-reviewskill's performance lens: flags a foreground call path reaching@BackgroundOnlycode, including a declaration-site inheritance-widening violation and a bounded two-hop interface-propagation extension.docs/instrumentation_design_guidelines.md.CLASSretention so a future bytecode-level checker (APMLP-1645) can consume the markers..llm-validationfixtures covering the interface-hop and inheritance-widening cases (13 gate cases total, up from 11).Motivation
Part of APMLP-1513 / APMLP-1645: giving the perf-review tooling a declared contract to check against, rather than relying on ad hoc judgment calls about whether code is cheap enough for a foreground thread.
Additional Notes
./gradlew :internal-api:spotlessJavaCheckpasses..llm-validationgate run (13 cases) and thedd-apm-sdk-review8-lens re-review are pending via the platform CI job.internal-apifor now, pending a future move of the whole toolbox into its own component.Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueJira ticket: APMLP-1513
🤖 Generated with Claude Code