Repository navigation
Mute telemetry when Connection.getClientInfo fails - #11412
Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 2 commits intoMay 19, 2026
Merged
Conversation
bric3
approved these changes
May 19, 2026
ygree
approved these changes
May 19, 2026
Contributor
Author
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
gh-worker-dd-mergequeue-cf854d Bot
pushed a commit
that referenced
this pull request
Oct 5, 2026
Stop repeating AbstractMethodError from JDBC getClientInfo Connections from old JDBC drivers lack getClientInfo, so every parseDBInfoFromConnection call threw and caught an AbstractMethodError. #11412 muted the log line but not the throw. Add AbstractMethodGuard: a per-call-site object that treats AbstractMethodError and UnsupportedOperationException as "not supported" (returns null) and lets everything else, SQLException included, propagate through a type parameter. A class is latched, so later calls skip the call, only when the error message names exactly the receiver class, so a wrapper delegating to a deficient driver is never latched. Both HotSpot message formats (JDK 8 and 11+) are recognised. Anything unattributable keeps today's behaviour. The JDBC call site now narrows its catch from Throwable to SQLException, so unexpected failures reach the outer handler and stay visible. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Add AbstractMethodGuard benchmark; make the latch race-free Benchmark the guard against the status quo (throw and catch on every call), a wrapper that cannot be latched, and the working path, using a real AbstractMethodError built at setup. Replace the lazily created ClassValue with an eager final field, and gate the per-class lookup with a plain anyLatched flag. This removes the creation race (a lost update could discard every latch so far) and gives safe publication through the final field. A plain flag measured faster than a volatile one on the working path. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Tidy AbstractMethodGuard after review Make isLatched package-private (only tests and the benchmark use it) and have invokeOrNull call it so the check has a single definition. Move CLIENT_INFO_GUARD next to the other private statics in JDBCDecorator. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Rebuild the guard as Latch and ClassLatch with a bound helper Replace AbstractMethodGuard with two abstract types meant to be held in static final fields, one per call site: - Latch: a one-way, call-site-wide latch, for failures that are the same for everyone (e.g. a field missing from the classes on the classpath). - ClassLatch: a per-class latch keyed through an overridable keyOf, for failures that recur for every instance of a class. The per-class state is an eager final ClassValue behind a plain anyLatched flag, so it is safely published and cannot lose latches to a creation race. Subclasses implement get in an ordinary try/catch, so checked exceptions need no generics tricks, and change the state only through protected helpers (latch, unlatch, latchIfNamed). A protected higher-order handleAbstractMethod covers the common case: AbstractMethodError is latched only when its message names the key class, and UnsupportedOperationException is swallowed without latching. JDBCDecorator now holds a static final ClassLatch for getClientInfo; behaviour is unchanged. The benchmark is renamed to ClassLatchBenchmark, gives each arm its own method, and adds a comparison against a dedicated-subclass form. Its results are provisional: depth-50 latched arms showed per-fork JIT modes and a cleaner run with more forks is planned. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Move Latch out of this PR Latch has no user here; it moves to the Jackson NoSuchFieldError fix (#12670), which is where it is used. ClassLatch's Javadoc no longer links to it. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Let the caller choose the fallback: tryGetOrNull and tryGetOrDefault Drop the defaultValue hook from ClassLatch. The public methods are now tryGetOrNull (skip if the target is null or latched, otherwise get; null means nothing is available) and tryGetOrDefault (null-coalescing sugar over it), so a call that yields nothing and a skipped call always agree. handleAbstractMethod returns null on an unsupported call. The protected hook stays get. A null return needs no allocation and no escape analysis, unlike a wrapper result type. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Add ThrowingFunction and use it for ClassLatch.handleAbstractMethod Replace the nested ClassLatch.Call with a top-level ThrowingFunction next to the other functional interfaces (TriFunction, TriConsumer), so other toolbox types can share it. Behaviour is unchanged. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Mark handleAbstractMethod as a StrategyConsumer with a Strategy slot Annotate ClassLatch.handleAbstractMethod with @StrategyConsumer and its function parameter with @strategy, as ConcurrentHashtable does, and note in its Javadoc that callers should pass a method reference or a non-capturing lambda. Documentation and tooling markers only; behaviour is unchanged. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Add handleNoSuchMethod and handleNoSuchOrAbstractMethod Name the helpers after the JVM errors they handle, as handleNoSuchField does: handleAbstractMethod (AbstractMethodError), handleNoSuchMethod (NoSuchMethodError) and handleNoSuchOrAbstractMethod (both), and cross-link their Javadocs. The two errors are easy to confuse, so the combined helper is the documented default. A NoSuchMethodError names the declared type, not the receiver, and can come from a call made inside one receiver's implementation, so it latches the target's key and never the whole call site. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Name the JDBC latch constant CLIENT_INFO_LATCH Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Name the ClassLatch hook handle Rename the protected hook get to handle, so it reads the same across the Latch family and does not suggest a no-arg accessor. The public methods (tryGetOrNull, tryGetOrDefault) are unchanged. Naming only. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Add ClassLatchBenchmark results Record a five-fork run on Zulu 17 (M1) in the benchmark's Javadoc: a latched class against the status quo, a wrapper that cannot latch, and the working path, at two stack depths, with the method-reference form compared to a dedicated subclass. The two forms are indistinguishable. Notes one unexplained result: at depth 50 a latched skip is slower than a call that runs. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Rename the ClassLatch hook to apply and the public methods to tryApply* Rename the protected hook handle to apply, and the public methods tryGetOrNull and tryGetOrDefault to tryApplyOrNull and tryApplyOrDefault. apply matches ThrowingFunction.apply, which the handleX helpers take, and no longer overlaps with the handleX helper names. Naming only; benchmark results are unchanged. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Remove a stray blank line in JDBCDecorator Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Keep catching Throwable around getClientInfo Narrowing the catch to SQLException broke existing tests: TestConnection's getClientInfo throws a bare Throwable, and JDBCInstrumentationV0Test and JDBCWrappedInterfacesTest expect the URL-derived DB info to survive any failure of getClientInfo. Restore the old resilience at the call site, so the latch is purely an optimization, and update ParseDBInfoClientInfoTest to assert that any failure still yields URL-based DBInfo. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Fix ClassLatch's JDK-version NoSuchMethodError handling and require a method-name check tryApplyOrNull previously only caught NoSuchMethodError, missing the BootstrapMethodError-wrapped form that some JVM versions raise for the same linkage failure, which let it escape the latch uncaught. handleAbstractMethod/latchIfNamed/isNamedIn now also take the method name the call site expects, since a receiver's own implementation can raise an AbstractMethodError naming the key class while blaming a different method it calls internally; without the check that error would incorrectly latch the class for a method it does implement. Extracts the duplicated compile() test helper into CompilingClassLoaders. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Add an overridable fallback to ClassLatch A latched target, and the call that fails, now yield fallback(target) instead of a bare null. Call sites with a slower alternative (an older API) can put it in the latch rather than re-checking isLatched after tryApplyOrNull, which also removes the ambiguity between a skipped call and an operation that legitimately returned null. The default is null, so existing latches are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Rename ClassLatch.tryApplyOrNull to tryApply With an overridable fallback the result is no longer necessarily null when the operation is skipped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
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
3 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
Mute the telemetry for this kind of errors:
Those are unactionable and the current code already handle a fallback case for that
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]
Note: Once your PR is ready to merge, add it to the merge queue by commenting
/merge./merge -ccancels the queue request./merge -f --reason "reason"skips all merge queue checks; please use this judiciously, as some checks do not run at the PR-level. For more information, see this doc.