Repository navigation
Conversation
in JDK8 debugger agent run async and SourceFileTracking is therefore racy. Test need to be performed since JDK11 Make SourceFileTracking Synchronous for test
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
More details
Synchronous source-file tracking makes the removed wait unnecessary, while the JDK 11 minimum excludes asynchronous Java 8 debugger startup. The completed static review identified no actionable regression.
🤖 Bits Code Review · Commit ca9c80a
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca9c80a0c1
ℹ️ 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".
| value = "datadog.environment.JavaVirtualMachine#isJ9", | ||
| disabledReason = "Flaky on J9 JVMs") | ||
| // Test disabled for JDK8 as the debuggerAgent is started async for JDK < 9 | ||
| @EnabledForJreRange(min = JRE.JAVA_11) |
There was a problem hiding this comment.
Remove the flaky quarantine after stabilizing the test
The new synchronization and JRE guard leave this method annotated with @Flaky; because CI skips flaky tests by default, the HugeClass regression still receives no normal CI coverage and this fix is not exercised there. Remove the existing @Flaky annotation once the race is addressed.
AGENTS.md reference: AGENTS.md:L68-L69
Useful? React with 👍 / 👎.
| additionalJvmArgs.add( | ||
| "-Ddd.internal.dynamic.instrumentation.synchronous.source.file.tracking.enabled=true"); |
There was a problem hiding this comment.
Scope synchronous tracking to the HugeClass test
Because this setup runs before every method, the internal switch makes the entire ProbeStateIntegrationTest class use SynchronousSourceFileTrackingTransformer, whose production default is false. In particular, testAddSourceFileProbeLargeInnerClasses() no longer exercises the normal asynchronous queue/flush path, so regressions in that path can pass this smoke test; use TestInfo to add the argument only for the HugeClass case (or isolate that case in a separate class).
Useful? React with 👍 / 👎.
🟢 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. |
What Does This Do
in JDK8 debugger agent run async and SourceFileTracking is therefore racy.
Test need to be performed since JDK11
Make SourceFileTracking Synchronous for test
Motivation
Additional Notes
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]