Extract duplicated snapshot-cap literal in ExceptionProbeManager - #2
Draft
jaipilot[bot] wants to merge 1 commit into
Draft
Conversation
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
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.
Context
This is a bounded JAIPilot companion for the exact PR head
a8822169dad29d03a5c6222df9e545c4a1b805cd(mirror of DataDog#12254, which fixes an exception-debugger memory leak from HotSpot's FastThrow/OmitStackTraceInFastThrow optimization by capping captured snapshots per throwable and removing theThrowableStateonce its snapshots have been assigned to a span).Change
In
ExceptionProbeManager.ThrowableState#addSnapshot(code added by this PR), the cap value256was written twice within 4 lines: once in thesize() > 256guard and once hard-coded into theLOGGER.debugmessage text ("Too many (256) snapshots..."). Extracted a singleprivate static final int MAX_SNAPSHOTS = 256;field and referenced it from both places (the log message now uses a{}placeholder for the value). This removes a duplicate-literal drift risk without changing the cap value, the rendered log text, or any other behavior.Why not more
Every other line the PR touches (the
Throwable innerMostExceptionplumbing intoprocessSnapshotsAndSetTags, and the newExceptionProbeManager#removeThrowableState) is a minimal, already-clean fix for the leak/empty-snapshot-list bug fixed by the two upstream commits (15c924f4,a8822169). There is no unused code, no duplicate logic, and no measurable hot path to optimize in this bounded diff, and the two changed files touch no dependency/JDK/framework version, so cleanup, performance, and modernization passes beyond the one accepted edit arenot_applicable/already_satisfied.Verification
./gradlew :dd-java-agent:agent-debugger:test --tests "com.datadog.debugger.exception.*"-> 27/27 passed both times, including the PR's ownaddSnapshotCapsGrowthtest that directly locks the 256-item cap-then-stable-size behavior.spotlessCheckpasses (no formatting drift)../gradlew :dd-java-agent:agent-debugger:test-> 782/782 tests passed, 0 failures/errors.Environment note
This cloud sandbox's auto-provisioned Gradle toolchain JDK (Temurin 25, pinned by the repository's
gradle/gradle-daemon-jvm.properties) did not trust the sandbox's own TLS-inspecting egress gateway CA, which initially blocked all Gradle dependency/plugin resolution (whilecurland the system JDK worked fine). This was resolved by importing the sandbox's already-trusted gateway CA certificates (present in/etc/ssl/certs/ca-certificates.crt, the same bundleSSL_CERT_FILEalready points to) into that toolchain JDK'scacertstruststore -- a local environment fix only, not a repository change -- after which the real Gradle build and tests ran and passed as reported above.Generated by JAIPilot Cloud for #1 from Anthropic session
sesn_01R97M2SJtizJZqLCTJGioSx.