From 0e357abeb24e90536c2203d23e3cc638e41ba648 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 30 Sep 2026 09:17:28 -0400 Subject: [PATCH 01/15] 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 --- .../instrumentation/jdbc/JDBCDecorator.java | 10 +- .../jdbc/ParseDBInfoClientInfoTest.java | 126 ++++++++ .../trace/util/AbstractMethodGuard.java | 128 ++++++++ .../trace/util/AbstractMethodGuardTest.java | 296 ++++++++++++++++++ 4 files changed, 557 insertions(+), 3 deletions(-) create mode 100644 dd-java-agent/instrumentation/jdbc/src/test/java/datadog/trace/instrumentation/jdbc/ParseDBInfoClientInfoTest.java create mode 100644 internal-api/src/main/java/datadog/trace/util/AbstractMethodGuard.java create mode 100644 internal-api/src/test/java/datadog/trace/util/AbstractMethodGuardTest.java diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java index fb66f082a3c..fe08bbff251 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java @@ -29,6 +29,7 @@ import datadog.trace.bootstrap.instrumentation.jdbc.DBQueryInfo; import datadog.trace.bootstrap.instrumentation.jdbc.JDBCConnectionContext; import datadog.trace.bootstrap.instrumentation.jdbc.JDBCConnectionUrlParser; +import datadog.trace.util.AbstractMethodGuard; import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.sql.ClientInfoStatus; @@ -51,6 +52,8 @@ public class JDBCDecorator extends DatabaseClientDecorator { public static final JDBCDecorator DECORATE = new JDBCDecorator(); public static final CharSequence JAVA_JDBC = UTF8BytesString.create("java-jdbc"); public static final CharSequence DATABASE_QUERY = UTF8BytesString.create("database.query"); + private static final AbstractMethodGuard CLIENT_INFO_GUARD = new AbstractMethodGuard(); + private static final UTF8BytesString DB_QUERY = UTF8BytesString.create("DB Query"); private static final UTF8BytesString JDBC_STATEMENT = UTF8BytesString.create("java-jdbc-statement"); @@ -246,9 +249,10 @@ public static DBInfo parseDBInfoFromConnection(final Connection connection) { if (metaData != null && (url = metaData.getURL()) != null) { Properties clientInfo = null; try { - clientInfo = connection.getClientInfo(); - } catch (final Throwable ex) { - // getClientInfo is likely not allowed, we can still extract info from the url alone + // old drivers and pool proxies may not implement getClientInfo at all + clientInfo = CLIENT_INFO_GUARD.invokeOrNull(connection, Connection::getClientInfo); + } catch (final SQLException ex) { + // getClientInfo is not allowed, we can still extract info from the url alone log.debug(LogCollector.EXCLUDE_TELEMETRY, "Could not get client info from DB", ex); } dbInfo = JDBCConnectionUrlParser.extractDBInfo(url, clientInfo); diff --git a/dd-java-agent/instrumentation/jdbc/src/test/java/datadog/trace/instrumentation/jdbc/ParseDBInfoClientInfoTest.java b/dd-java-agent/instrumentation/jdbc/src/test/java/datadog/trace/instrumentation/jdbc/ParseDBInfoClientInfoTest.java new file mode 100644 index 00000000000..3d3126fe1c7 --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/test/java/datadog/trace/instrumentation/jdbc/ParseDBInfoClientInfoTest.java @@ -0,0 +1,126 @@ +package datadog.trace.instrumentation.jdbc; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; + +import datadog.trace.bootstrap.instrumentation.jdbc.DBInfo; +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Proxy; +import java.sql.Connection; +import java.sql.DatabaseMetaData; +import java.sql.SQLException; +import java.util.Properties; +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.jupiter.api.Test; + +/** How {@code parseDBInfoFromConnection} copes with connections whose getClientInfo fails. */ +class ParseDBInfoClientInfoTest { + private static final String URL = "jdbc:postgresql://db.example.com:5432/orders"; + + interface ClientInfoAnswer { + Properties get() throws Throwable; + } + + private static Connection connection(AtomicInteger clientInfoCalls, ClientInfoAnswer answer) { + DatabaseMetaData metaData = + (DatabaseMetaData) + Proxy.newProxyInstance( + DatabaseMetaData.class.getClassLoader(), + new Class[] {DatabaseMetaData.class}, + (proxy, method, args) -> "getURL".equals(method.getName()) ? URL : null); + InvocationHandler handler = + (proxy, method, args) -> { + switch (method.getName()) { + case "getMetaData": + return metaData; + case "getClientInfo": + clientInfoCalls.incrementAndGet(); + try { + return answer.get(); + } catch (InvocationTargetException e) { + throw e.getCause(); + } + default: + return null; + } + }; + return (Connection) + Proxy.newProxyInstance( + Connection.class.getClassLoader(), new Class[] {Connection.class}, handler); + } + + @Test + void urlIsStillParsedWhenGetClientInfoThrowsSqlException() { + AtomicInteger calls = new AtomicInteger(); + Connection connection = + connection( + calls, + () -> { + throw new SQLException("not allowed"); + }); + + DBInfo info = JDBCDecorator.parseDBInfoFromConnection(connection); + + assertEquals("postgresql", info.getType()); + assertEquals("orders", info.getDb()); + } + + @Test + void urlIsStillParsedWhenGetClientInfoIsUnsupported() { + AtomicInteger calls = new AtomicInteger(); + Connection connection = + connection( + calls, + () -> { + throw new UnsupportedOperationException(); + }); + + DBInfo info = JDBCDecorator.parseDBInfoFromConnection(connection); + + assertEquals("postgresql", info.getType()); + assertEquals("orders", info.getDb()); + } + + @Test + void urlIsStillParsedWhenGetClientInfoIsMissing() { + AtomicInteger calls = new AtomicInteger(); + Connection connection = + connection( + calls, + () -> { + throw new AbstractMethodError("driver predates JDBC 4.0"); + }); + + DBInfo info = JDBCDecorator.parseDBInfoFromConnection(connection); + + assertEquals("postgresql", info.getType()); + assertEquals("orders", info.getDb()); + } + + @Test + void unexpectedFailuresFallBackToDefaultInsteadOfBeingSwallowed() { + AtomicInteger calls = new AtomicInteger(); + Connection connection = + connection( + calls, + () -> { + throw new IllegalStateException("unexpected"); + }); + + // not one of the expected "unsupported" shapes, so it reaches the outer handler + assertSame(DBInfo.DEFAULT, JDBCDecorator.parseDBInfoFromConnection(connection)); + } + + @Test + void returnsTheClientInfoWhenAvailable() { + AtomicInteger calls = new AtomicInteger(); + Properties clientInfo = new Properties(); + Connection connection = connection(calls, () -> clientInfo); + + DBInfo info = JDBCDecorator.parseDBInfoFromConnection(connection); + + assertEquals("postgresql", info.getType()); + assertEquals(1, calls.get()); + } +} diff --git a/internal-api/src/main/java/datadog/trace/util/AbstractMethodGuard.java b/internal-api/src/main/java/datadog/trace/util/AbstractMethodGuard.java new file mode 100644 index 00000000000..3464738210a --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/util/AbstractMethodGuard.java @@ -0,0 +1,128 @@ +package datadog.trace.util; + +import javax.annotation.Nullable; + +/** + * Calls a method that some implementations of an interface may lack, and stops paying for the + * resulting {@link AbstractMethodError} once it is known that a class lacks it. + * + *

Intended as a {@code static final} field, one per call site: the instance only holds state. + * Nothing is latched until the first failure, so on the happy path the cost is one plain field + * read. State is deliberately not atomic: a lost update costs one more caught error, never a wrong + * result. + * + *

A class is latched only when the error is attributed to exactly that class. HotSpot's message + * names the receiver class that lacks the implementation, so a wrapper delegating to a deficient + * object is never latched -- its contents may differ from one instance to the next. If the error + * cannot be attributed (wrapper, unparseable message, another VM) nothing is latched and every call + * behaves as it would without the guard. + * + *

{@link AbstractMethodError} and {@link UnsupportedOperationException} are both treated as + * "this implementation does not support the method" and yield {@code null}. Only the former can be + * attributed to a class, so only the former is latched; an unsupported operation is caught on every + * call. Anything else, checked exceptions included, propagates to the caller unchanged. + */ +public final class AbstractMethodGuard { + + /** A method reference such as {@code Connection::getClientInfo}. */ + @FunctionalInterface + public interface Call { + R apply(T target) throws E; + } + + private static final String RECEIVER_PREFIX = "Receiver class "; + + /** + * Per-class latch, created on the first failure. The value type is a JDK type so that nothing + * from the agent class loader is referenced from an application class. + */ + private ClassValue latched; + + /** + * Invokes {@code call} on {@code target}, returning {@code null} if the target's class is known + * to lack the method, if it just turned out to, or if it reported the operation as unsupported. A + * {@code null} target also returns {@code null}, without latching. + */ + @Nullable + public R invokeOrNull(@Nullable T target, Call call) + throws E { + if (target == null) { + return null; + } + final Class type = target.getClass(); + final ClassValue latched = this.latched; + if (latched != null && latched.get(type)[0]) { + return null; + } + try { + return call.apply(target); + } catch (AbstractMethodError e) { + if (isAttributedTo(e, type)) { + latch(type); + } + return null; + } catch (UnsupportedOperationException e) { + // no class to attribute it to, and it may come from a delegate: never latched + return null; + } + } + + /** Returns whether {@code type} is known to lack the method. */ + public boolean isLatched(Class type) { + final ClassValue latched = this.latched; + return latched != null && latched.get(type)[0]; + } + + private void latch(Class type) { + ClassValue latched = this.latched; + if (latched == null) { + // racy on purpose: if two threads get here, one latch may be lost and re-earned + latched = + new ClassValue() { + @Override + protected boolean[] computeValue(Class type) { + return new boolean[1]; + } + }; + this.latched = latched; + } + latched.get(type)[0] = true; + } + + /** + * Matches the receiver class named by HotSpot's message against the class of the object that was + * called. Two formats exist: + * + *

    + *
  • JDK 11+: {@code Receiver class X does not define or inherit an implementation of the + * resolved method ...} + *
  • JDK 8: {@code X.method(descriptor)} + *
+ * + * A concrete object's class is never the abstract method's declaring class or an interface, so an + * exact match can only be the receiver. + */ + static boolean isAttributedTo(AbstractMethodError e, Class type) { + final String message = e.getMessage(); + if (message == null) { + return false; + } + final String name = type.getName(); + if (message.startsWith(RECEIVER_PREFIX)) { + final int end = RECEIVER_PREFIX.length() + name.length(); + return message.startsWith(name, RECEIVER_PREFIX.length()) + && message.length() > end + && message.charAt(end) == ' '; + } + if (message.startsWith(name) && message.length() > name.length() + 1) { + // JDK 8: the method name follows the class name and runs up to the descriptor, so it + // contains no '.'; that rules out a longer class name sharing this one as a prefix + final int methodStart = name.length() + 1; + final int paren = message.indexOf('(', methodStart); + return message.charAt(name.length()) == '.' + && paren > methodStart + && message.indexOf('.', methodStart) < 0; + } + return false; + } +} diff --git a/internal-api/src/test/java/datadog/trace/util/AbstractMethodGuardTest.java b/internal-api/src/test/java/datadog/trace/util/AbstractMethodGuardTest.java new file mode 100644 index 00000000000..1d861e24385 --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/util/AbstractMethodGuardTest.java @@ -0,0 +1,296 @@ +package datadog.trace.util; + +import static java.util.Collections.singletonList; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +import java.io.File; +import java.io.IOException; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.net.URL; +import java.net.URLClassLoader; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.sql.SQLException; +import java.util.concurrent.atomic.AtomicInteger; +import javax.tools.JavaCompiler; +import javax.tools.ToolProvider; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +class AbstractMethodGuardTest { + + private static AbstractMethodError receiverError(Class type) { + return new AbstractMethodError( + "Receiver class " + + type.getName() + + " does not define or inherit an implementation of the resolved method 'abstract" + + " java.lang.String m()' of interface I."); + } + + @Test + void returnsTheResultAndLatchesNothing() { + AbstractMethodGuard guard = new AbstractMethodGuard(); + + assertEquals("ok", guard.invokeOrNull("x", s -> "ok")); + assertFalse(guard.isLatched(String.class)); + } + + @Test + void nullTargetReturnsNullWithoutCalling() { + AbstractMethodGuard guard = new AbstractMethodGuard(); + AtomicInteger calls = new AtomicInteger(); + + assertNull( + guard.invokeOrNull( + null, + t -> { + calls.incrementAndGet(); + return "never"; + })); + assertEquals(0, calls.get()); + } + + @Test + void latchesTheReceiverClassAndStopsCalling() { + AbstractMethodGuard guard = new AbstractMethodGuard(); + AtomicInteger calls = new AtomicInteger(); + AbstractMethodGuard.Call call = + t -> { + calls.incrementAndGet(); + throw receiverError(t.getClass()); + }; + + assertNull(guard.invokeOrNull("x", call)); + assertNull(guard.invokeOrNull("y", call)); + assertNull(guard.invokeOrNull("z", call)); + + assertEquals(1, calls.get()); + assertTrue(guard.isLatched(String.class)); + } + + @Test + void otherClassesAreUnaffectedByALatch() { + AbstractMethodGuard guard = new AbstractMethodGuard(); + guard.invokeOrNull( + "x", + t -> { + throw receiverError(t.getClass()); + }); + + assertEquals("called", guard.invokeOrNull(Integer.valueOf(1), t -> "called")); + assertFalse(guard.isLatched(Integer.class)); + } + + @Test + void doesNotLatchAWrapperWhoseDelegateIsTheDeficientClass() { + AbstractMethodGuard guard = new AbstractMethodGuard(); + AtomicInteger calls = new AtomicInteger(); + // the called object is a String, but the error names some other (delegate) class + AbstractMethodGuard.Call call = + t -> { + calls.incrementAndGet(); + throw receiverError(Integer.class); + }; + + assertNull(guard.invokeOrNull("x", call)); + assertNull(guard.invokeOrNull("x", call)); + + assertEquals(2, calls.get()); + assertFalse(guard.isLatched(String.class)); + } + + @Test + void doesNotLatchWhenTheMessageCannotBeAttributed() { + AbstractMethodGuard guard = new AbstractMethodGuard(); + AtomicInteger calls = new AtomicInteger(); + + for (AbstractMethodError error : + new AbstractMethodError[] { + new AbstractMethodError(), new AbstractMethodError("something else entirely") + }) { + assertNull( + guard.invokeOrNull( + "x", + t -> { + calls.incrementAndGet(); + throw error; + })); + } + + assertEquals(2, calls.get()); + assertFalse(guard.isLatched(String.class)); + } + + @Test + void attributionRequiresTheWholeClassName() { + assertTrue(AbstractMethodGuard.isAttributedTo(receiverError(String.class), String.class)); + // "java.lang.String" is a prefix of "java.lang.StringBuilder", but not the same class + assertFalse( + AbstractMethodGuard.isAttributedTo(receiverError(String.class), StringBuilder.class)); + assertFalse( + AbstractMethodGuard.isAttributedTo( + new AbstractMethodError("Receiver class " + String.class.getName()), String.class)); + } + + @Test + void attributesTheJdk8MessageFormat() { + // JDK 8 reports "." + String name = String.class.getName(); + + assertTrue( + AbstractMethodGuard.isAttributedTo( + new AbstractMethodError(name + ".getClientInfo()Ljava/util/Properties;"), + String.class)); + // a different class whose name merely starts with this one + assertFalse( + AbstractMethodGuard.isAttributedTo( + new AbstractMethodError(name + "Builder.getClientInfo()Ljava/util/Properties;"), + String.class)); + // a class in a package named like this class + assertFalse( + AbstractMethodGuard.isAttributedTo( + new AbstractMethodError(name + ".Inner.getClientInfo()Ljava/util/Properties;"), + String.class)); + assertFalse(AbstractMethodGuard.isAttributedTo(new AbstractMethodError(name), String.class)); + assertFalse( + AbstractMethodGuard.isAttributedTo(new AbstractMethodError(name + "."), String.class)); + } + + @Test + void checkedExceptionsPropagateAndDoNotLatch() { + AbstractMethodGuard guard = new AbstractMethodGuard(); + SQLException failure = new SQLException("boom"); + + SQLException thrown = + assertThrows( + SQLException.class, + () -> + guard.invokeOrNull( + "x", + t -> { + throw failure; + })); + + assertSame(failure, thrown); + assertFalse(guard.isLatched(String.class)); + } + + @Test + void unsupportedOperationYieldsNullOnEveryCallAndIsNeverLatched() { + AbstractMethodGuard guard = new AbstractMethodGuard(); + AtomicInteger calls = new AtomicInteger(); + AbstractMethodGuard.Call call = + t -> { + calls.incrementAndGet(); + throw new UnsupportedOperationException(); + }; + + assertNull(guard.invokeOrNull("x", call)); + assertNull(guard.invokeOrNull("x", call)); + + assertEquals(2, calls.get()); + assertFalse(guard.isLatched(String.class)); + } + + @Test + void otherUncheckedExceptionsPropagateAndDoNotLatch() { + AbstractMethodGuard guard = new AbstractMethodGuard(); + + assertThrows( + IllegalStateException.class, + () -> + guard.invokeOrNull( + "x", + t -> { + throw new IllegalStateException(); + })); + assertFalse(guard.isLatched(String.class)); + } + + /** + * Pins the HotSpot message format that attribution depends on, using a real error: a class built + * against an old interface, called through code built against a newer one. + */ + @Test + void latchesARealAbstractMethodError(@TempDir Path dir) throws Exception { + JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); + assumeTrue(compiler != null, "needs a JDK"); + String vm = System.getProperty("java.vm.name", ""); + assumeTrue(vm.contains("HotSpot") || vm.contains("OpenJDK"), "message format is HotSpot's"); + + Path oldDir = Files.createDirectory(dir.resolve("old")); + Path newDir = Files.createDirectory(dir.resolve("new")); + compile(compiler, oldDir, null, "I", "public interface I { String a(); }"); + compile( + compiler, + oldDir, + oldDir, + "Impl", + "public class Impl implements I { public String a() { return \"a\"; } }"); + compile(compiler, newDir, null, "I", "public interface I { String a(); String b(); }"); + compile( + compiler, + newDir, + newDir, + "Caller", + "public class Caller { public static String call(I i) { return i.b(); } }"); + + try (URLClassLoader loader = + new URLClassLoader( + new URL[] {newDir.toUri().toURL(), oldDir.toUri().toURL()}, + AbstractMethodGuardTest.class.getClassLoader())) { + Class iface = loader.loadClass("I"); + Object impl = loader.loadClass("Impl").getDeclaredConstructor().newInstance(); + Method call = loader.loadClass("Caller").getMethod("call", iface); + + AtomicInteger calls = new AtomicInteger(); + AbstractMethodGuard guard = new AbstractMethodGuard(); + AbstractMethodGuard.Call invoke = + target -> { + calls.incrementAndGet(); + try { + return call.invoke(null, target); + } catch (InvocationTargetException e) { + if (e.getCause() instanceof AbstractMethodError) { + throw (AbstractMethodError) e.getCause(); + } + throw e; + } + }; + + assertNull(guard.invokeOrNull(impl, invoke)); + assertNull(guard.invokeOrNull(impl, invoke)); + + assertEquals(1, calls.get(), "second call should be skipped"); + assertTrue(guard.isLatched(impl.getClass())); + } + } + + private static void compile( + JavaCompiler compiler, Path out, Path classpath, String name, String source) + throws IOException { + Path file = out.resolve(name + ".java"); + Files.write(file, singletonList(source), StandardCharsets.UTF_8); + int result = + classpath == null + ? compiler.run(null, null, null, "-d", out.toString(), file.toString()) + : compiler.run( + null, + null, + null, + "-cp", + classpath.toString() + File.pathSeparator, + "-d", + out.toString(), + file.toString()); + assertEquals(0, result, "compiling " + name); + } +} From 2e8a1927a4fd28c95f5bd7c2e93bcfeaaec976fc Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 30 Sep 2026 12:23:56 -0400 Subject: [PATCH 02/15] 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 --- .../util/AbstractMethodGuardBenchmark.java | 252 ++++++++++++++++++ .../trace/util/AbstractMethodGuard.java | 48 ++-- 2 files changed, 278 insertions(+), 22 deletions(-) create mode 100644 internal-api/src/jmh/java/datadog/trace/util/AbstractMethodGuardBenchmark.java diff --git a/internal-api/src/jmh/java/datadog/trace/util/AbstractMethodGuardBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/AbstractMethodGuardBenchmark.java new file mode 100644 index 00000000000..169a88b8223 --- /dev/null +++ b/internal-api/src/jmh/java/datadog/trace/util/AbstractMethodGuardBenchmark.java @@ -0,0 +1,252 @@ +package datadog.trace.util; + +import static java.util.Collections.singletonList; + +import java.io.File; +import java.io.IOException; +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodType; +import java.net.URL; +import java.net.URLClassLoader; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import javax.tools.JavaCompiler; +import javax.tools.ToolProvider; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Param; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.TearDown; +import org.openjdk.jmh.annotations.Threads; +import org.openjdk.jmh.annotations.Warmup; + +/** + * What {@link AbstractMethodGuard} saves when an implementation lacks an interface method. + * + *

The missing method is real: {@code Impl} is built against an old {@code I}, and the caller + * against a newer {@code I} that added {@code b()}, so the call raises the JVM's own {@link + * AbstractMethodError}. Every arm reaches it through the same {@link MethodHandle}. + * + *

    + *
  • {@code unguardedMissing}: the status quo -- throw and catch on every call. + *
  • {@code guardedMissing}: the guard has latched {@code Impl}, so the call is skipped. + *
  • {@code guardedWrapperMissing}: the called object is a wrapper whose delegate lacks the + * method. The error names the delegate, so nothing may be latched and every call still + * throws. This is the price of staying safe; it should match {@code unguardedMissing}. + *
  • {@code unguardedPresent} / {@code guardedPresent}: the method exists. The difference is the + * guard's overhead on the path that works. + *
+ * + *

The cost of a throw grows with the depth of the stack it fills in, which is why {@code depth} + * is a parameter: a benchmark thread's stack is shallow, a request thread's is not. + * + *

Run with {@code ./gradlew :internal-api:jmh -Pjmh.includes=AbstractMethodGuardBenchmark + * -Pjmh.profilers=gc}. + * + *

Results are ops/s, single thread, JDK 17.0.7 (Zulu), MacBook M1, 2 forks. They come from two + * separate runs, so compare arms within a group, not across groups. JDK 8 and x86 are not measured. + * + * 

+ * Status quo and wrapper (an earlier run; neither arm depends on the latch)
+ * Benchmark                                        (depth)      ops/s    B/op
+ * AbstractMethodGuardBenchmark.unguardedMissing          0    218,369     896
+ * AbstractMethodGuardBenchmark.unguardedMissing         50    162,090   2,256
+ * AbstractMethodGuardBenchmark.guardedWrapperMissing     0    226,385     896
+ * AbstractMethodGuardBenchmark.guardedWrapperMissing    50    165,602   2,256
+ *
+ * Latched and working path (final code, plain flag)
+ * AbstractMethodGuardBenchmark.guardedMissing            0  199,493,000     0
+ * AbstractMethodGuardBenchmark.guardedMissing           50   30,081,622     0
+ * AbstractMethodGuardBenchmark.guardedPresent            0  207,855,250     0
+ * AbstractMethodGuardBenchmark.guardedPresent           50   30,509,308     0
+ * AbstractMethodGuardBenchmark.unguardedPresent          0  223,546,636     0
+ * AbstractMethodGuardBenchmark.unguardedPresent         50   31,579,822     0
+ *
+ * + * A latched class costs about 5 ns instead of 4.6-6.2 us and allocates nothing. A wrapper, which + * cannot be latched, performs like the status quo. On the working path the guard adds about 0.3 ns + * (depth 0) to 1.1 ns (depth 50); a volatile flag measured 0.6 ns and 5.8 ns. + */ +@Fork(2) +@Warmup(iterations = 3) +@Measurement(iterations = 4) +@Threads(1) +@State(Scope.Benchmark) +public class AbstractMethodGuardBenchmark { + + @Param({"0", "50"}) + int depth; + + private Path dir; + private URLClassLoader loader; + private MethodHandle call; + private Object impl; + private Object full; + private Object wrapper; + + private AbstractMethodGuard.Call invoke; + + private static final AbstractMethodGuard MISSING = new AbstractMethodGuard(); + private static final AbstractMethodGuard WRAPPER = new AbstractMethodGuard(); + private static final AbstractMethodGuard PRESENT = new AbstractMethodGuard(); + + @Setup + public void setup() throws Throwable { + JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); + if (compiler == null) { + throw new IllegalStateException("needs a JDK to build the classes under test"); + } + dir = Files.createTempDirectory("abstract-method-guard"); + Path oldDir = Files.createDirectory(dir.resolve("old")); + Path newDir = Files.createDirectory(dir.resolve("new")); + + compile(compiler, oldDir, null, "I", "public interface I { String a(); }"); + compile( + compiler, + oldDir, + oldDir, + "Impl", + "public class Impl implements I { public String a() { return \"a\"; } }"); + + compile(compiler, newDir, null, "I", "public interface I { String a(); String b(); }"); + compile( + compiler, + newDir, + newDir, + "Full", + "public class Full implements I {" + + " public String a() { return \"a\"; } public String b() { return \"b\"; } }"); + compile( + compiler, + newDir, + newDir, + "Wrapper", + "public class Wrapper implements I { private final I delegate;" + + " public Wrapper(I delegate) { this.delegate = delegate; }" + + " public String a() { return delegate.a(); }" + + " public String b() { return delegate.b(); } }"); + compile( + compiler, + newDir, + newDir, + "Caller", + "public class Caller { public static String call(I i) { return i.b(); } }"); + + // the new interface shadows the old one; Impl was built against the old one + loader = + new URLClassLoader( + new URL[] {newDir.toUri().toURL(), oldDir.toUri().toURL()}, + AbstractMethodGuardBenchmark.class.getClassLoader()); + Class iface = loader.loadClass("I"); + impl = loader.loadClass("Impl").getDeclaredConstructor().newInstance(); + full = loader.loadClass("Full").getDeclaredConstructor().newInstance(); + wrapper = loader.loadClass("Wrapper").getDeclaredConstructor(iface).newInstance(impl); + + call = + MethodHandles.publicLookup() + .findStatic( + loader.loadClass("Caller"), "call", MethodType.methodType(String.class, iface)) + .asType(MethodType.methodType(Object.class, Object.class)); + + invoke = + target -> { + try { + return (Object) call.invokeExact(target); + } catch (RuntimeException | Error e) { + throw e; + } catch (Throwable e) { + throw new IllegalStateException(e); + } + }; + + // reach the steady state: the guard has already met the deficient class + MISSING.invokeOrNull(impl, invoke); + if (!MISSING.isLatched(impl.getClass())) { + throw new IllegalStateException("expected the guard to latch " + impl.getClass()); + } + WRAPPER.invokeOrNull(wrapper, invoke); + if (WRAPPER.isLatched(wrapper.getClass()) || WRAPPER.isLatched(impl.getClass())) { + throw new IllegalStateException("a wrapper must never be latched"); + } + } + + @TearDown + public void tearDown() throws IOException { + loader.close(); + } + + @Benchmark + public Object unguardedMissing() { + return descend(depth, 0); + } + + @Benchmark + public Object guardedMissing() { + return descend(depth, 1); + } + + @Benchmark + public Object guardedWrapperMissing() { + return descend(depth, 2); + } + + @Benchmark + public Object unguardedPresent() { + return descend(depth, 3); + } + + @Benchmark + public Object guardedPresent() { + return descend(depth, 4); + } + + /** Grows the stack so that a throw has a realistic amount to fill in. */ + private Object descend(int remaining, int kind) { + if (remaining > 0) { + return descend(remaining - 1, kind); + } + switch (kind) { + case 0: + try { + return invoke.apply(impl); + } catch (AbstractMethodError e) { + return null; + } + case 1: + return MISSING.invokeOrNull(impl, invoke); + case 2: + return WRAPPER.invokeOrNull(wrapper, invoke); + case 3: + return invoke.apply(full); + default: + return PRESENT.invokeOrNull(full, invoke); + } + } + + private static void compile( + JavaCompiler compiler, Path out, Path classpath, String name, String source) + throws IOException { + Path file = out.resolve(name + ".java"); + Files.write(file, singletonList(source), StandardCharsets.UTF_8); + int result = + classpath == null + ? compiler.run(null, null, null, "-d", out.toString(), file.toString()) + : compiler.run( + null, + null, + null, + "-cp", + classpath.toString() + File.pathSeparator, + "-d", + out.toString(), + file.toString()); + if (result != 0) { + throw new IllegalStateException("compiling " + name + " failed"); + } + } +} diff --git a/internal-api/src/main/java/datadog/trace/util/AbstractMethodGuard.java b/internal-api/src/main/java/datadog/trace/util/AbstractMethodGuard.java index 3464738210a..7c6bda20e6d 100644 --- a/internal-api/src/main/java/datadog/trace/util/AbstractMethodGuard.java +++ b/internal-api/src/main/java/datadog/trace/util/AbstractMethodGuard.java @@ -7,9 +7,9 @@ * resulting {@link AbstractMethodError} once it is known that a class lacks it. * *

Intended as a {@code static final} field, one per call site: the instance only holds state. - * Nothing is latched until the first failure, so on the happy path the cost is one plain field - * read. State is deliberately not atomic: a lost update costs one more caught error, never a wrong - * result. + * Until a class has been latched, the cost on the happy path is one plain flag read; the per-class + * lookup only happens once something has been latched. The per-class state is deliberately not + * atomic: a late-visible write costs one more caught error, never a wrong result. * *

A class is latched only when the error is attributed to exactly that class. HotSpot's message * names the receiver class that lacks the implementation, so a wrapper delegating to a deficient @@ -33,10 +33,25 @@ public interface Call { private static final String RECEIVER_PREFIX = "Receiver class "; /** - * Per-class latch, created on the first failure. The value type is a JDK type so that nothing - * from the agent class loader is referenced from an application class. + * Per-class latch. Created eagerly so that it is safely published through a final field: it holds + * nothing for a class until {@link ClassValue#get} is called for it. The value type is a JDK type + * so that nothing from the agent class loader is referenced from an application class. */ - private ClassValue latched; + private final ClassValue latched = new Latches(); + + /** + * Whether any class has been latched; keeps the per-class lookup off the common path. Plain on + * purpose: a stale read only costs one more caught error, and {@code volatile} measured + * noticeably slower on the working path (see {@code AbstractMethodGuardBenchmark}). + */ + private boolean anyLatched; + + private static final class Latches extends ClassValue { + @Override + protected boolean[] computeValue(Class type) { + return new boolean[1]; + } + } /** * Invokes {@code call} on {@code target}, returning {@code null} if the target's class is known @@ -50,8 +65,7 @@ public R invokeOrNull(@Nullable T target, Call type = target.getClass(); - final ClassValue latched = this.latched; - if (latched != null && latched.get(type)[0]) { + if (anyLatched && latched.get(type)[0]) { return null; } try { @@ -69,24 +83,14 @@ public R invokeOrNull(@Nullable T target, Call type) { - final ClassValue latched = this.latched; - return latched != null && latched.get(type)[0]; + return anyLatched && latched.get(type)[0]; } private void latch(Class type) { - ClassValue latched = this.latched; - if (latched == null) { - // racy on purpose: if two threads get here, one latch may be lost and re-earned - latched = - new ClassValue() { - @Override - protected boolean[] computeValue(Class type) { - return new boolean[1]; - } - }; - this.latched = latched; - } latched.get(type)[0] = true; + // after the write, so a reader that sees the flag can look the class up; a reader that sees + // the flag but not yet the write just calls once more + anyLatched = true; } /** From cf84e6845c7f05272c4bf3d87f78ebb2b9526530 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 30 Sep 2026 12:40:36 -0400 Subject: [PATCH 03/15] 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 --- .../datadog/trace/instrumentation/jdbc/JDBCDecorator.java | 2 +- .../main/java/datadog/trace/util/AbstractMethodGuard.java | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java index fe08bbff251..50f877bab1b 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java @@ -48,11 +48,11 @@ public class JDBCDecorator extends DatabaseClientDecorator { private static final Logger log = LoggerFactory.getLogger(JDBCDecorator.class); + private static final AbstractMethodGuard CLIENT_INFO_GUARD = new AbstractMethodGuard(); public static final JDBCDecorator DECORATE = new JDBCDecorator(); public static final CharSequence JAVA_JDBC = UTF8BytesString.create("java-jdbc"); public static final CharSequence DATABASE_QUERY = UTF8BytesString.create("database.query"); - private static final AbstractMethodGuard CLIENT_INFO_GUARD = new AbstractMethodGuard(); private static final UTF8BytesString DB_QUERY = UTF8BytesString.create("DB Query"); private static final UTF8BytesString JDBC_STATEMENT = diff --git a/internal-api/src/main/java/datadog/trace/util/AbstractMethodGuard.java b/internal-api/src/main/java/datadog/trace/util/AbstractMethodGuard.java index 7c6bda20e6d..9278471a8d8 100644 --- a/internal-api/src/main/java/datadog/trace/util/AbstractMethodGuard.java +++ b/internal-api/src/main/java/datadog/trace/util/AbstractMethodGuard.java @@ -65,7 +65,7 @@ public R invokeOrNull(@Nullable T target, Call type = target.getClass(); - if (anyLatched && latched.get(type)[0]) { + if (isLatched(type)) { return null; } try { @@ -81,8 +81,8 @@ public R invokeOrNull(@Nullable T target, Call type) { + /** Returns whether {@code type} is known to lack the method. Visible for tests and benchmarks. */ + boolean isLatched(Class type) { return anyLatched && latched.get(type)[0]; } From f4b29e67aa704898c6c4e8dde503c5cc37cfb9da Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 30 Sep 2026 14:13:14 -0400 Subject: [PATCH 04/15] 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 --- .../instrumentation/jdbc/JDBCDecorator.java | 15 +- .../util/AbstractMethodGuardBenchmark.java | 252 ------------- .../trace/util/ClassLatchBenchmark.java | 347 ++++++++++++++++++ .../trace/util/AbstractMethodGuard.java | 132 ------- .../java/datadog/trace/util/ClassLatch.java | 181 +++++++++ .../main/java/datadog/trace/util/Latch.java | 58 +++ .../trace/util/AbstractMethodGuardTest.java | 296 --------------- .../datadog/trace/util/ClassLatchTest.java | 217 +++++++++++ .../trace/util/HandleAbstractMethodTest.java | 258 +++++++++++++ .../java/datadog/trace/util/LatchTest.java | 115 ++++++ 10 files changed, 1187 insertions(+), 684 deletions(-) delete mode 100644 internal-api/src/jmh/java/datadog/trace/util/AbstractMethodGuardBenchmark.java create mode 100644 internal-api/src/jmh/java/datadog/trace/util/ClassLatchBenchmark.java delete mode 100644 internal-api/src/main/java/datadog/trace/util/AbstractMethodGuard.java create mode 100644 internal-api/src/main/java/datadog/trace/util/ClassLatch.java create mode 100644 internal-api/src/main/java/datadog/trace/util/Latch.java delete mode 100644 internal-api/src/test/java/datadog/trace/util/AbstractMethodGuardTest.java create mode 100644 internal-api/src/test/java/datadog/trace/util/ClassLatchTest.java create mode 100644 internal-api/src/test/java/datadog/trace/util/HandleAbstractMethodTest.java create mode 100644 internal-api/src/test/java/datadog/trace/util/LatchTest.java diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java index 50f877bab1b..a72eaf88356 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java @@ -29,7 +29,7 @@ import datadog.trace.bootstrap.instrumentation.jdbc.DBQueryInfo; import datadog.trace.bootstrap.instrumentation.jdbc.JDBCConnectionContext; import datadog.trace.bootstrap.instrumentation.jdbc.JDBCConnectionUrlParser; -import datadog.trace.util.AbstractMethodGuard; +import datadog.trace.util.ClassLatch; import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.sql.ClientInfoStatus; @@ -48,7 +48,15 @@ public class JDBCDecorator extends DatabaseClientDecorator { private static final Logger log = LoggerFactory.getLogger(JDBCDecorator.class); - private static final AbstractMethodGuard CLIENT_INFO_GUARD = new AbstractMethodGuard(); + + /** Old drivers and pool proxies may not implement getClientInfo at all. */ + private static final ClassLatch CLIENT_INFO = + new ClassLatch() { + @Override + protected Properties get(Connection connection) throws SQLException { + return handleAbstractMethod(connection, Connection::getClientInfo); + } + }; public static final JDBCDecorator DECORATE = new JDBCDecorator(); public static final CharSequence JAVA_JDBC = UTF8BytesString.create("java-jdbc"); @@ -249,8 +257,7 @@ public static DBInfo parseDBInfoFromConnection(final Connection connection) { if (metaData != null && (url = metaData.getURL()) != null) { Properties clientInfo = null; try { - // old drivers and pool proxies may not implement getClientInfo at all - clientInfo = CLIENT_INFO_GUARD.invokeOrNull(connection, Connection::getClientInfo); + clientInfo = CLIENT_INFO.getOrDefault(connection); } catch (final SQLException ex) { // getClientInfo is not allowed, we can still extract info from the url alone log.debug(LogCollector.EXCLUDE_TELEMETRY, "Could not get client info from DB", ex); diff --git a/internal-api/src/jmh/java/datadog/trace/util/AbstractMethodGuardBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/AbstractMethodGuardBenchmark.java deleted file mode 100644 index 169a88b8223..00000000000 --- a/internal-api/src/jmh/java/datadog/trace/util/AbstractMethodGuardBenchmark.java +++ /dev/null @@ -1,252 +0,0 @@ -package datadog.trace.util; - -import static java.util.Collections.singletonList; - -import java.io.File; -import java.io.IOException; -import java.lang.invoke.MethodHandle; -import java.lang.invoke.MethodHandles; -import java.lang.invoke.MethodType; -import java.net.URL; -import java.net.URLClassLoader; -import java.nio.charset.StandardCharsets; -import java.nio.file.Files; -import java.nio.file.Path; -import javax.tools.JavaCompiler; -import javax.tools.ToolProvider; -import org.openjdk.jmh.annotations.Benchmark; -import org.openjdk.jmh.annotations.Fork; -import org.openjdk.jmh.annotations.Measurement; -import org.openjdk.jmh.annotations.Param; -import org.openjdk.jmh.annotations.Scope; -import org.openjdk.jmh.annotations.Setup; -import org.openjdk.jmh.annotations.State; -import org.openjdk.jmh.annotations.TearDown; -import org.openjdk.jmh.annotations.Threads; -import org.openjdk.jmh.annotations.Warmup; - -/** - * What {@link AbstractMethodGuard} saves when an implementation lacks an interface method. - * - *

The missing method is real: {@code Impl} is built against an old {@code I}, and the caller - * against a newer {@code I} that added {@code b()}, so the call raises the JVM's own {@link - * AbstractMethodError}. Every arm reaches it through the same {@link MethodHandle}. - * - *

    - *
  • {@code unguardedMissing}: the status quo -- throw and catch on every call. - *
  • {@code guardedMissing}: the guard has latched {@code Impl}, so the call is skipped. - *
  • {@code guardedWrapperMissing}: the called object is a wrapper whose delegate lacks the - * method. The error names the delegate, so nothing may be latched and every call still - * throws. This is the price of staying safe; it should match {@code unguardedMissing}. - *
  • {@code unguardedPresent} / {@code guardedPresent}: the method exists. The difference is the - * guard's overhead on the path that works. - *
- * - *

The cost of a throw grows with the depth of the stack it fills in, which is why {@code depth} - * is a parameter: a benchmark thread's stack is shallow, a request thread's is not. - * - *

Run with {@code ./gradlew :internal-api:jmh -Pjmh.includes=AbstractMethodGuardBenchmark - * -Pjmh.profilers=gc}. - * - *

Results are ops/s, single thread, JDK 17.0.7 (Zulu), MacBook M1, 2 forks. They come from two - * separate runs, so compare arms within a group, not across groups. JDK 8 and x86 are not measured. - * - * 

- * Status quo and wrapper (an earlier run; neither arm depends on the latch)
- * Benchmark                                        (depth)      ops/s    B/op
- * AbstractMethodGuardBenchmark.unguardedMissing          0    218,369     896
- * AbstractMethodGuardBenchmark.unguardedMissing         50    162,090   2,256
- * AbstractMethodGuardBenchmark.guardedWrapperMissing     0    226,385     896
- * AbstractMethodGuardBenchmark.guardedWrapperMissing    50    165,602   2,256
- *
- * Latched and working path (final code, plain flag)
- * AbstractMethodGuardBenchmark.guardedMissing            0  199,493,000     0
- * AbstractMethodGuardBenchmark.guardedMissing           50   30,081,622     0
- * AbstractMethodGuardBenchmark.guardedPresent            0  207,855,250     0
- * AbstractMethodGuardBenchmark.guardedPresent           50   30,509,308     0
- * AbstractMethodGuardBenchmark.unguardedPresent          0  223,546,636     0
- * AbstractMethodGuardBenchmark.unguardedPresent         50   31,579,822     0
- *
- * - * A latched class costs about 5 ns instead of 4.6-6.2 us and allocates nothing. A wrapper, which - * cannot be latched, performs like the status quo. On the working path the guard adds about 0.3 ns - * (depth 0) to 1.1 ns (depth 50); a volatile flag measured 0.6 ns and 5.8 ns. - */ -@Fork(2) -@Warmup(iterations = 3) -@Measurement(iterations = 4) -@Threads(1) -@State(Scope.Benchmark) -public class AbstractMethodGuardBenchmark { - - @Param({"0", "50"}) - int depth; - - private Path dir; - private URLClassLoader loader; - private MethodHandle call; - private Object impl; - private Object full; - private Object wrapper; - - private AbstractMethodGuard.Call invoke; - - private static final AbstractMethodGuard MISSING = new AbstractMethodGuard(); - private static final AbstractMethodGuard WRAPPER = new AbstractMethodGuard(); - private static final AbstractMethodGuard PRESENT = new AbstractMethodGuard(); - - @Setup - public void setup() throws Throwable { - JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); - if (compiler == null) { - throw new IllegalStateException("needs a JDK to build the classes under test"); - } - dir = Files.createTempDirectory("abstract-method-guard"); - Path oldDir = Files.createDirectory(dir.resolve("old")); - Path newDir = Files.createDirectory(dir.resolve("new")); - - compile(compiler, oldDir, null, "I", "public interface I { String a(); }"); - compile( - compiler, - oldDir, - oldDir, - "Impl", - "public class Impl implements I { public String a() { return \"a\"; } }"); - - compile(compiler, newDir, null, "I", "public interface I { String a(); String b(); }"); - compile( - compiler, - newDir, - newDir, - "Full", - "public class Full implements I {" - + " public String a() { return \"a\"; } public String b() { return \"b\"; } }"); - compile( - compiler, - newDir, - newDir, - "Wrapper", - "public class Wrapper implements I { private final I delegate;" - + " public Wrapper(I delegate) { this.delegate = delegate; }" - + " public String a() { return delegate.a(); }" - + " public String b() { return delegate.b(); } }"); - compile( - compiler, - newDir, - newDir, - "Caller", - "public class Caller { public static String call(I i) { return i.b(); } }"); - - // the new interface shadows the old one; Impl was built against the old one - loader = - new URLClassLoader( - new URL[] {newDir.toUri().toURL(), oldDir.toUri().toURL()}, - AbstractMethodGuardBenchmark.class.getClassLoader()); - Class iface = loader.loadClass("I"); - impl = loader.loadClass("Impl").getDeclaredConstructor().newInstance(); - full = loader.loadClass("Full").getDeclaredConstructor().newInstance(); - wrapper = loader.loadClass("Wrapper").getDeclaredConstructor(iface).newInstance(impl); - - call = - MethodHandles.publicLookup() - .findStatic( - loader.loadClass("Caller"), "call", MethodType.methodType(String.class, iface)) - .asType(MethodType.methodType(Object.class, Object.class)); - - invoke = - target -> { - try { - return (Object) call.invokeExact(target); - } catch (RuntimeException | Error e) { - throw e; - } catch (Throwable e) { - throw new IllegalStateException(e); - } - }; - - // reach the steady state: the guard has already met the deficient class - MISSING.invokeOrNull(impl, invoke); - if (!MISSING.isLatched(impl.getClass())) { - throw new IllegalStateException("expected the guard to latch " + impl.getClass()); - } - WRAPPER.invokeOrNull(wrapper, invoke); - if (WRAPPER.isLatched(wrapper.getClass()) || WRAPPER.isLatched(impl.getClass())) { - throw new IllegalStateException("a wrapper must never be latched"); - } - } - - @TearDown - public void tearDown() throws IOException { - loader.close(); - } - - @Benchmark - public Object unguardedMissing() { - return descend(depth, 0); - } - - @Benchmark - public Object guardedMissing() { - return descend(depth, 1); - } - - @Benchmark - public Object guardedWrapperMissing() { - return descend(depth, 2); - } - - @Benchmark - public Object unguardedPresent() { - return descend(depth, 3); - } - - @Benchmark - public Object guardedPresent() { - return descend(depth, 4); - } - - /** Grows the stack so that a throw has a realistic amount to fill in. */ - private Object descend(int remaining, int kind) { - if (remaining > 0) { - return descend(remaining - 1, kind); - } - switch (kind) { - case 0: - try { - return invoke.apply(impl); - } catch (AbstractMethodError e) { - return null; - } - case 1: - return MISSING.invokeOrNull(impl, invoke); - case 2: - return WRAPPER.invokeOrNull(wrapper, invoke); - case 3: - return invoke.apply(full); - default: - return PRESENT.invokeOrNull(full, invoke); - } - } - - private static void compile( - JavaCompiler compiler, Path out, Path classpath, String name, String source) - throws IOException { - Path file = out.resolve(name + ".java"); - Files.write(file, singletonList(source), StandardCharsets.UTF_8); - int result = - classpath == null - ? compiler.run(null, null, null, "-d", out.toString(), file.toString()) - : compiler.run( - null, - null, - null, - "-cp", - classpath.toString() + File.pathSeparator, - "-d", - out.toString(), - file.toString()); - if (result != 0) { - throw new IllegalStateException("compiling " + name + " failed"); - } - } -} diff --git a/internal-api/src/jmh/java/datadog/trace/util/ClassLatchBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/ClassLatchBenchmark.java new file mode 100644 index 00000000000..d15c2658124 --- /dev/null +++ b/internal-api/src/jmh/java/datadog/trace/util/ClassLatchBenchmark.java @@ -0,0 +1,347 @@ +package datadog.trace.util; + +import static java.util.Collections.singletonList; + +import java.io.File; +import java.io.IOException; +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodType; +import java.net.URL; +import java.net.URLClassLoader; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import javax.tools.JavaCompiler; +import javax.tools.ToolProvider; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Param; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.TearDown; +import org.openjdk.jmh.annotations.Threads; +import org.openjdk.jmh.annotations.Warmup; + +/** + * What {@link ClassLatch#handleAbstractMethod} saves when an implementation lacks an interface + * method. + * + *

The missing method is real: {@code Impl} is built against an old {@code I}, and the caller + * against a newer {@code I} that added {@code b()}, so the call raises the JVM's own {@link + * AbstractMethodError}. Every arm reaches it through the same {@link MethodHandle}. The latches are + * {@code static final} anonymous subclasses, as they would be at a call site, and each arm has its + * own method so that no arm's profile is shaped by another's. + * + *

    + *
  • {@code unguardedMissing}: the status quo -- throw and catch on every call. + *
  • {@code latchedMissing}: the latch has latched {@code Impl}, so the call is skipped. + *
  • {@code latchedWrapperMissing}: the called object is a wrapper whose delegate lacks the + * method. The error names the delegate, so nothing may be latched and every call still + * throws. This is the price of staying safe; it should match {@code unguardedMissing}. + *
  • {@code unguardedPresent} / {@code latchedPresent}: the method exists. The difference is the + * latch's overhead on the path that works. + *
  • {@code subclassMissing} / {@code subclassPresent}: the same policy written as a reusable + * abstract subclass overriding {@code invoke}, instead of a method reference passed to {@code + * handleAbstractMethod}. It is a benchmark-local copy: it shows whether a dedicated class + * would be worth shipping. + *
+ * + *

The cost of a throw grows with the depth of the stack it fills in, which is why {@code depth} + * is a parameter: a benchmark thread's stack is shallow, a request thread's is not. + * + *

Run with {@code ./gradlew :internal-api:jmh -Pjmh.includes=ClassLatchBenchmark + * -Pjmh.profilers=gc}. + * + *

Results: ops/s, single thread, Zulu 17.0.7, MacBook M1, 2 forks, one run. JDK 8 and x86 are + * not measured. + * + * 

+ * Benchmark                                        (depth)       ops/s   B/op
+ * ClassLatchBenchmark.unguardedMissing                   0     220,977    896
+ * ClassLatchBenchmark.latchedWrapperMissing              0     228,684    896
+ * ClassLatchBenchmark.latchedMissing                     0 201,587,502      0
+ * ClassLatchBenchmark.subclassMissing                    0 201,915,340      0
+ * ClassLatchBenchmark.unguardedPresent                   0 223,537,261      0
+ * ClassLatchBenchmark.latchedPresent                     0 203,305,896      0
+ * ClassLatchBenchmark.subclassPresent                    0 227,339,864      0   (+-22%)
+ *
+ * ClassLatchBenchmark.unguardedMissing                  50     164,166  2,256
+ * ClassLatchBenchmark.latchedWrapperMissing             50     166,284  2,256
+ * ClassLatchBenchmark.unguardedPresent                  50  32,451,009      0
+ * ClassLatchBenchmark.latchedPresent                    50  28,001,510      0
+ *
+ * + * A latched class costs about 5 ns instead of about 4.5 us and allocates nothing instead of 896 B + * per call. A wrapper, which cannot be latched, performs like the status quo. On the working path + * the latch adds about 0.5 ns at depth 0. At depth 0 the method-reference form ({@code + * handleAbstractMethod}) and the dedicated-subclass form are indistinguishable on the latched path. + * + *

Depth 50, latched arms: not reported. Each fork was stable, but forks landed in + * different compiled states. {@code latchedMissing} ran at about 11.6M ops/s in one fork and about + * 31.7M in the other, {@code subclassMissing} at about 11.5M in both, {@code subclassPresent} at + * about 36M and 32M. So the mean and its error are two modes averaged, and the ranking of the two + * forms at depth 50 is not established. Both modes (roughly 85 ns and 32 ns) are far below the + * status quo of about 6 us. The cause was not investigated. + */ +@Fork(2) +@Warmup(iterations = 3) +@Measurement(iterations = 4) +@Threads(1) +@State(Scope.Benchmark) +public class ClassLatchBenchmark { + + @Param({"0", "50"}) + int depth; + + private Path dir; + private URLClassLoader loader; + private Object impl; + private Object full; + private Object wrapper; + + /** Set in {@link #setup}; every arm and latch calls through it. */ + private static MethodHandle handle; + + private static Object invokeHandle(Object target) { + try { + return (Object) handle.invokeExact(target); + } catch (RuntimeException | Error e) { + throw e; + } catch (Throwable e) { + throw new IllegalStateException(e); + } + } + + private static final ClassLatch MISSING = + new ClassLatch() { + @Override + protected Object get(Object target) { + return handleAbstractMethod(target, ClassLatchBenchmark::invokeHandle); + } + }; + + private static final ClassLatch WRAPPER = + new ClassLatch() { + @Override + protected Object get(Object target) { + return handleAbstractMethod(target, ClassLatchBenchmark::invokeHandle); + } + }; + + private static final ClassLatch PRESENT = + new ClassLatch() { + @Override + protected Object get(Object target) { + return handleAbstractMethod(target, ClassLatchBenchmark::invokeHandle); + } + }; + + // the policy as a reusable subclass, for comparison only + private abstract static class SubclassStyle extends ClassLatch { + protected abstract Object invoke(Object target); + + @Override + protected final Object get(Object target) { + try { + return invoke(target); + } catch (AbstractMethodError e) { + latchIfNamed(target, e); + return defaultValue(target); + } catch (UnsupportedOperationException e) { + return defaultValue(target); + } + } + } + + private static final SubclassStyle SUBCLASS_MISSING = + new SubclassStyle() { + @Override + protected Object invoke(Object target) { + return invokeHandle(target); + } + }; + + private static final SubclassStyle SUBCLASS_PRESENT = + new SubclassStyle() { + @Override + protected Object invoke(Object target) { + return invokeHandle(target); + } + }; + + @Setup + public void setup() throws Throwable { + JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); + if (compiler == null) { + throw new IllegalStateException("needs a JDK to build the classes under test"); + } + dir = Files.createTempDirectory("abstract-method-latch"); + Path oldDir = Files.createDirectory(dir.resolve("old")); + Path newDir = Files.createDirectory(dir.resolve("new")); + + compile(compiler, oldDir, null, "I", "public interface I { String a(); }"); + compile( + compiler, + oldDir, + oldDir, + "Impl", + "public class Impl implements I { public String a() { return \"a\"; } }"); + + compile(compiler, newDir, null, "I", "public interface I { String a(); String b(); }"); + compile( + compiler, + newDir, + newDir, + "Full", + "public class Full implements I {" + + " public String a() { return \"a\"; } public String b() { return \"b\"; } }"); + compile( + compiler, + newDir, + newDir, + "Wrapper", + "public class Wrapper implements I { private final I delegate;" + + " public Wrapper(I delegate) { this.delegate = delegate; }" + + " public String a() { return delegate.a(); }" + + " public String b() { return delegate.b(); } }"); + compile( + compiler, + newDir, + newDir, + "Caller", + "public class Caller { public static String call(I i) { return i.b(); } }"); + + // the new interface shadows the old one; Impl was built against the old one + loader = + new URLClassLoader( + new URL[] {newDir.toUri().toURL(), oldDir.toUri().toURL()}, + ClassLatchBenchmark.class.getClassLoader()); + Class iface = loader.loadClass("I"); + impl = loader.loadClass("Impl").getDeclaredConstructor().newInstance(); + full = loader.loadClass("Full").getDeclaredConstructor().newInstance(); + wrapper = loader.loadClass("Wrapper").getDeclaredConstructor(iface).newInstance(impl); + + handle = + MethodHandles.publicLookup() + .findStatic( + loader.loadClass("Caller"), "call", MethodType.methodType(String.class, iface)) + .asType(MethodType.methodType(Object.class, Object.class)); + + // reach the steady state: the latch has already met the deficient class + MISSING.getOrDefault(impl); + if (!MISSING.isLatched(impl)) { + throw new IllegalStateException("expected the latch to latch " + impl.getClass()); + } + WRAPPER.getOrDefault(wrapper); + if (WRAPPER.isLatched(wrapper) || WRAPPER.isLatched(impl)) { + throw new IllegalStateException("a wrapper must never be latched"); + } + SUBCLASS_MISSING.getOrDefault(impl); + if (!SUBCLASS_MISSING.isLatched(impl)) { + throw new IllegalStateException( + "expected the subclass-style latch to latch " + impl.getClass()); + } + } + + @TearDown + public void tearDown() throws IOException { + loader.close(); + } + + @Benchmark + public Object unguardedMissing() { + return unguardedMissing(depth); + } + + @Benchmark + public Object latchedMissing() { + return latchedMissing(depth); + } + + @Benchmark + public Object latchedWrapperMissing() { + return latchedWrapperMissing(depth); + } + + @Benchmark + public Object unguardedPresent() { + return unguardedPresent(depth); + } + + @Benchmark + public Object latchedPresent() { + return latchedPresent(depth); + } + + @Benchmark + public Object subclassMissing() { + return subclassMissing(depth); + } + + @Benchmark + public Object subclassPresent() { + return subclassPresent(depth); + } + + // Each arm descends on its own so that a throw has a realistic amount of stack to fill in. + + private Object unguardedMissing(int remaining) { + if (remaining > 0) { + return unguardedMissing(remaining - 1); + } + try { + return invokeHandle(impl); + } catch (AbstractMethodError e) { + return null; + } + } + + private Object latchedMissing(int remaining) { + return remaining > 0 ? latchedMissing(remaining - 1) : MISSING.getOrDefault(impl); + } + + private Object latchedWrapperMissing(int remaining) { + return remaining > 0 ? latchedWrapperMissing(remaining - 1) : WRAPPER.getOrDefault(wrapper); + } + + private Object unguardedPresent(int remaining) { + return remaining > 0 ? unguardedPresent(remaining - 1) : invokeHandle(full); + } + + private Object latchedPresent(int remaining) { + return remaining > 0 ? latchedPresent(remaining - 1) : PRESENT.getOrDefault(full); + } + + private Object subclassMissing(int remaining) { + return remaining > 0 ? subclassMissing(remaining - 1) : SUBCLASS_MISSING.getOrDefault(impl); + } + + private Object subclassPresent(int remaining) { + return remaining > 0 ? subclassPresent(remaining - 1) : SUBCLASS_PRESENT.getOrDefault(full); + } + + private static void compile( + JavaCompiler compiler, Path out, Path classpath, String name, String source) + throws IOException { + Path file = out.resolve(name + ".java"); + Files.write(file, singletonList(source), StandardCharsets.UTF_8); + int result = + classpath == null + ? compiler.run(null, null, null, "-d", out.toString(), file.toString()) + : compiler.run( + null, + null, + null, + "-cp", + classpath.toString() + File.pathSeparator, + "-d", + out.toString(), + file.toString()); + if (result != 0) { + throw new IllegalStateException("compiling " + name + " failed"); + } + } +} diff --git a/internal-api/src/main/java/datadog/trace/util/AbstractMethodGuard.java b/internal-api/src/main/java/datadog/trace/util/AbstractMethodGuard.java deleted file mode 100644 index 9278471a8d8..00000000000 --- a/internal-api/src/main/java/datadog/trace/util/AbstractMethodGuard.java +++ /dev/null @@ -1,132 +0,0 @@ -package datadog.trace.util; - -import javax.annotation.Nullable; - -/** - * Calls a method that some implementations of an interface may lack, and stops paying for the - * resulting {@link AbstractMethodError} once it is known that a class lacks it. - * - *

Intended as a {@code static final} field, one per call site: the instance only holds state. - * Until a class has been latched, the cost on the happy path is one plain flag read; the per-class - * lookup only happens once something has been latched. The per-class state is deliberately not - * atomic: a late-visible write costs one more caught error, never a wrong result. - * - *

A class is latched only when the error is attributed to exactly that class. HotSpot's message - * names the receiver class that lacks the implementation, so a wrapper delegating to a deficient - * object is never latched -- its contents may differ from one instance to the next. If the error - * cannot be attributed (wrapper, unparseable message, another VM) nothing is latched and every call - * behaves as it would without the guard. - * - *

{@link AbstractMethodError} and {@link UnsupportedOperationException} are both treated as - * "this implementation does not support the method" and yield {@code null}. Only the former can be - * attributed to a class, so only the former is latched; an unsupported operation is caught on every - * call. Anything else, checked exceptions included, propagates to the caller unchanged. - */ -public final class AbstractMethodGuard { - - /** A method reference such as {@code Connection::getClientInfo}. */ - @FunctionalInterface - public interface Call { - R apply(T target) throws E; - } - - private static final String RECEIVER_PREFIX = "Receiver class "; - - /** - * Per-class latch. Created eagerly so that it is safely published through a final field: it holds - * nothing for a class until {@link ClassValue#get} is called for it. The value type is a JDK type - * so that nothing from the agent class loader is referenced from an application class. - */ - private final ClassValue latched = new Latches(); - - /** - * Whether any class has been latched; keeps the per-class lookup off the common path. Plain on - * purpose: a stale read only costs one more caught error, and {@code volatile} measured - * noticeably slower on the working path (see {@code AbstractMethodGuardBenchmark}). - */ - private boolean anyLatched; - - private static final class Latches extends ClassValue { - @Override - protected boolean[] computeValue(Class type) { - return new boolean[1]; - } - } - - /** - * Invokes {@code call} on {@code target}, returning {@code null} if the target's class is known - * to lack the method, if it just turned out to, or if it reported the operation as unsupported. A - * {@code null} target also returns {@code null}, without latching. - */ - @Nullable - public R invokeOrNull(@Nullable T target, Call call) - throws E { - if (target == null) { - return null; - } - final Class type = target.getClass(); - if (isLatched(type)) { - return null; - } - try { - return call.apply(target); - } catch (AbstractMethodError e) { - if (isAttributedTo(e, type)) { - latch(type); - } - return null; - } catch (UnsupportedOperationException e) { - // no class to attribute it to, and it may come from a delegate: never latched - return null; - } - } - - /** Returns whether {@code type} is known to lack the method. Visible for tests and benchmarks. */ - boolean isLatched(Class type) { - return anyLatched && latched.get(type)[0]; - } - - private void latch(Class type) { - latched.get(type)[0] = true; - // after the write, so a reader that sees the flag can look the class up; a reader that sees - // the flag but not yet the write just calls once more - anyLatched = true; - } - - /** - * Matches the receiver class named by HotSpot's message against the class of the object that was - * called. Two formats exist: - * - *

    - *
  • JDK 11+: {@code Receiver class X does not define or inherit an implementation of the - * resolved method ...} - *
  • JDK 8: {@code X.method(descriptor)} - *
- * - * A concrete object's class is never the abstract method's declaring class or an interface, so an - * exact match can only be the receiver. - */ - static boolean isAttributedTo(AbstractMethodError e, Class type) { - final String message = e.getMessage(); - if (message == null) { - return false; - } - final String name = type.getName(); - if (message.startsWith(RECEIVER_PREFIX)) { - final int end = RECEIVER_PREFIX.length() + name.length(); - return message.startsWith(name, RECEIVER_PREFIX.length()) - && message.length() > end - && message.charAt(end) == ' '; - } - if (message.startsWith(name) && message.length() > name.length() + 1) { - // JDK 8: the method name follows the class name and runs up to the descriptor, so it - // contains no '.'; that rules out a longer class name sharing this one as a prefix - final int methodStart = name.length() + 1; - final int paren = message.indexOf('(', methodStart); - return message.charAt(name.length()) == '.' - && paren > methodStart - && message.indexOf('.', methodStart) < 0; - } - return false; - } -} diff --git a/internal-api/src/main/java/datadog/trace/util/ClassLatch.java b/internal-api/src/main/java/datadog/trace/util/ClassLatch.java new file mode 100644 index 00000000000..3a2860c67ef --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/util/ClassLatch.java @@ -0,0 +1,181 @@ +package datadog.trace.util; + +import javax.annotation.Nullable; + +/** + * A per-class latch for an operation that, once it has failed for a class, will fail the same way + * for every instance of that class: an interface method the class does not implement, for example. + * For a failure that is the same for everyone, use {@link Latch}. + * + *

Intended as a {@code static final} anonymous subclass, one per call site and per operation: a + * class lacking one method says nothing about another, so latches must not be shared. As a constant + * of a known exact type, the receiver lets the JIT inline {@link #get}, {@link #defaultValue} and + * {@link #keyOf}. Subclasses decide what counts as a failure in their own {@code try/catch} inside + * {@link #get}, so checked exceptions and a tight {@code try} scope come for free, and latch + * through the protected helpers. Only the declaring subclass can change the state. + * + *

{@link #keyOf} chooses the class the latch is keyed on, and is used by every operation, so the + * check and the latch cannot disagree. The default is the target's own class. Never key on a + * wrapper whose contents can differ from one instance to the next; override {@link #keyOf} to + * return the class of the object that is actually deficient. + * + *

This is a hint, not a lock. Nothing is stored for a class until it is latched, and until then + * the cost is one plain flag read. The state is deliberately not atomic. A stale read only costs + * another failure; a thread always sees its own write, so each thread pays for at most one failure + * after its own first. Other threads' writes become visible eventually, with no bound on how long + * that takes. A class is never latched unless the subclass latched it. + * + * @param the type of the value the operation is applied to + * @param the type of the result + * @param the checked exception {@link #get} may throw + */ +public abstract class ClassLatch { + /** + * A call that may fail, typically a method reference such as {@code Connection::getClientInfo}. + */ + @FunctionalInterface + public interface Call { + @Nullable + R apply(T target) throws E; + } + + private static final String RECEIVER_PREFIX = "Receiver class "; + + /** + * Per-class state. Created eagerly so that it is safely published through a final field; it holds + * nothing for a class until {@link ClassValue#get} is called for it. The value is a JDK type so + * that nothing from the agent class loader is referenced from an application class. + */ + private final ClassValue latched = new Latches(); + + /** Whether any class has been latched; keeps the per-class lookup off the common path. */ + private boolean anyLatched; + + private static final class Latches extends ClassValue { + @Override + protected boolean[] computeValue(Class type) { + return new boolean[1]; + } + } + + /** Performs the operation. Latch through the protected helpers when it failed for the class. */ + @Nullable + protected abstract R get(T target) throws E; + + /** The result for a {@code null} or latched target. {@code null} unless overridden. */ + @Nullable + protected R defaultValue(@Nullable T target) { + return null; + } + + /** The class the latch is keyed on. The target's own class unless overridden. */ + protected Class keyOf(T target) { + return target.getClass(); + } + + /** + * Performs the operation unless the target is {@code null} or latched, in which case returns + * {@link #defaultValue}. + */ + @Nullable + public final R getOrDefault(@Nullable T target) throws E { + return target == null || isLatched(target) ? defaultValue(target) : get(target); + } + + /** Returns whether the operation is being skipped for the target. */ + public final boolean isLatched(@Nullable T target) { + return target != null && anyLatched && latched.get(keyOf(target))[0]; + } + + /** Skips the operation for the target's key from now on. */ + protected final void latch(T target) { + latched.get(keyOf(target))[0] = true; + // after the write: a reader that sees the flag can look the class up, and one that sees the + // flag but not yet the write just performs the operation once more + anyLatched = true; + } + + /** Resumes performing the operation for the target's key, for tests or a policy that retries. */ + protected final void unlatch(T target) { + if (anyLatched) { + latched.get(keyOf(target))[0] = false; + } + } + + /** + * Latches the target's key if the error's message names that class as the receiver that lacks the + * method. Returns whether it latched. An error that does not name the key, for example one thrown + * inside a wrapper's delegate, is left alone. + */ + protected final boolean latchIfNamed(T target, AbstractMethodError error) { + if (isNamedIn(error, keyOf(target))) { + latch(target); + return true; + } + return false; + } + + /** + * For a call to a method that some implementations may lack: returns {@link #defaultValue} if the + * call raises {@link AbstractMethodError} or {@link UnsupportedOperationException}, latching the + * key first in the former case if, and only if, the error names it (see {@link #latchIfNamed}). + * An unsupported operation names no class, so it is never latched, and is caught on every call. + * Anything else, checked exceptions included, propagates unchanged. + * + * 

{@code
+   * protected Properties get(Connection c) throws SQLException {
+   *   return handleAbstractMethod(c, Connection::getClientInfo);
+   * }
+   * }
+ * + * Compose {@link #latchIfNamed} and {@link #latch} directly for anything more involved. + */ + @Nullable + protected final R handleAbstractMethod(T target, Call call) throws E { + try { + return call.apply(target); + } catch (AbstractMethodError e) { + latchIfNamed(target, e); + return defaultValue(target); + } catch (UnsupportedOperationException e) { + // no class to attribute it to, and it may come from a delegate: never latched + return defaultValue(target); + } + } + + /** + * Matches the receiver class named by HotSpot's message against a class. Two formats exist: + * + *
    + *
  • JDK 11+: {@code Receiver class X does not define or inherit an implementation of the + * resolved method ...} + *
  • JDK 8: {@code X.method(descriptor)} + *
+ * + * A concrete object's class is never the abstract method's declaring class or an interface, so an + * exact match can only be the receiver. An unparseable message never matches. + */ + static boolean isNamedIn(AbstractMethodError e, Class type) { + final String message = e.getMessage(); + if (message == null) { + return false; + } + final String name = type.getName(); + if (message.startsWith(RECEIVER_PREFIX)) { + final int end = RECEIVER_PREFIX.length() + name.length(); + return message.startsWith(name, RECEIVER_PREFIX.length()) + && message.length() > end + && message.charAt(end) == ' '; + } + if (message.startsWith(name) && message.length() > name.length() + 1) { + // JDK 8: the method name follows the class name and runs up to the descriptor, so it + // contains no '.'; that rules out a longer class name sharing this one as a prefix + final int methodStart = name.length() + 1; + final int paren = message.indexOf('(', methodStart); + return message.charAt(name.length()) == '.' + && paren > methodStart + && message.indexOf('.', methodStart) < 0; + } + return false; + } +} diff --git a/internal-api/src/main/java/datadog/trace/util/Latch.java b/internal-api/src/main/java/datadog/trace/util/Latch.java new file mode 100644 index 00000000000..9541b2a19c0 --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/util/Latch.java @@ -0,0 +1,58 @@ +package datadog.trace.util; + +import javax.annotation.Nullable; + +/** + * A one-way, call-site-wide latch for an operation that fails the same way for everyone once it has + * failed, such as reading a field that is missing from the classes on the classpath. For a failure + * that depends on the receiver's class, use {@link ClassLatch}. + * + *

Intended as a {@code static final} anonymous subclass, one per call site: the receiver is then + * a constant of a known exact type, so the JIT can inline {@link #get} and {@link #defaultValue}. + * Subclasses decide what counts as a failure in their own {@code try/catch} inside {@link #get}, so + * checked exceptions and a tight {@code try} scope come for free, and call {@link #latch()} + * themselves. + * + *

This is a hint, not a lock. The flag is deliberately plain. A stale read only costs another + * failure; a thread always sees its own write, so each thread pays for at most one failure after + * its own first. Other threads' writes become visible eventually, with no bound on how long that + * takes. + * + * @param the type of the value the operation is applied to + * @param the type of the result + * @param the checked exception {@link #get} may throw + */ +public abstract class Latch { + private boolean latched; + + /** Performs the operation. Call {@link #latch()} when it has failed in a way that will recur. */ + @Nullable + protected abstract R get(T target) throws E; + + /** The result once latched. {@code null} unless overridden. */ + @Nullable + protected R defaultValue(T target) { + return null; + } + + /** Performs the operation unless latched, in which case returns {@link #defaultValue}. */ + @Nullable + public final R getOrDefault(T target) throws E { + return latched ? defaultValue(target) : get(target); + } + + /** Returns whether the operation is being skipped. */ + public final boolean isLatched() { + return latched; + } + + /** Skips the operation from now on. */ + protected final void latch() { + latched = true; + } + + /** Resumes performing the operation, for tests or for a policy that retries. */ + protected final void unlatch() { + latched = false; + } +} diff --git a/internal-api/src/test/java/datadog/trace/util/AbstractMethodGuardTest.java b/internal-api/src/test/java/datadog/trace/util/AbstractMethodGuardTest.java deleted file mode 100644 index 1d861e24385..00000000000 --- a/internal-api/src/test/java/datadog/trace/util/AbstractMethodGuardTest.java +++ /dev/null @@ -1,296 +0,0 @@ -package datadog.trace.util; - -import static java.util.Collections.singletonList; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertSame; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assumptions.assumeTrue; - -import java.io.File; -import java.io.IOException; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; -import java.net.URL; -import java.net.URLClassLoader; -import java.nio.charset.StandardCharsets; -import java.nio.file.Files; -import java.nio.file.Path; -import java.sql.SQLException; -import java.util.concurrent.atomic.AtomicInteger; -import javax.tools.JavaCompiler; -import javax.tools.ToolProvider; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.io.TempDir; - -class AbstractMethodGuardTest { - - private static AbstractMethodError receiverError(Class type) { - return new AbstractMethodError( - "Receiver class " - + type.getName() - + " does not define or inherit an implementation of the resolved method 'abstract" - + " java.lang.String m()' of interface I."); - } - - @Test - void returnsTheResultAndLatchesNothing() { - AbstractMethodGuard guard = new AbstractMethodGuard(); - - assertEquals("ok", guard.invokeOrNull("x", s -> "ok")); - assertFalse(guard.isLatched(String.class)); - } - - @Test - void nullTargetReturnsNullWithoutCalling() { - AbstractMethodGuard guard = new AbstractMethodGuard(); - AtomicInteger calls = new AtomicInteger(); - - assertNull( - guard.invokeOrNull( - null, - t -> { - calls.incrementAndGet(); - return "never"; - })); - assertEquals(0, calls.get()); - } - - @Test - void latchesTheReceiverClassAndStopsCalling() { - AbstractMethodGuard guard = new AbstractMethodGuard(); - AtomicInteger calls = new AtomicInteger(); - AbstractMethodGuard.Call call = - t -> { - calls.incrementAndGet(); - throw receiverError(t.getClass()); - }; - - assertNull(guard.invokeOrNull("x", call)); - assertNull(guard.invokeOrNull("y", call)); - assertNull(guard.invokeOrNull("z", call)); - - assertEquals(1, calls.get()); - assertTrue(guard.isLatched(String.class)); - } - - @Test - void otherClassesAreUnaffectedByALatch() { - AbstractMethodGuard guard = new AbstractMethodGuard(); - guard.invokeOrNull( - "x", - t -> { - throw receiverError(t.getClass()); - }); - - assertEquals("called", guard.invokeOrNull(Integer.valueOf(1), t -> "called")); - assertFalse(guard.isLatched(Integer.class)); - } - - @Test - void doesNotLatchAWrapperWhoseDelegateIsTheDeficientClass() { - AbstractMethodGuard guard = new AbstractMethodGuard(); - AtomicInteger calls = new AtomicInteger(); - // the called object is a String, but the error names some other (delegate) class - AbstractMethodGuard.Call call = - t -> { - calls.incrementAndGet(); - throw receiverError(Integer.class); - }; - - assertNull(guard.invokeOrNull("x", call)); - assertNull(guard.invokeOrNull("x", call)); - - assertEquals(2, calls.get()); - assertFalse(guard.isLatched(String.class)); - } - - @Test - void doesNotLatchWhenTheMessageCannotBeAttributed() { - AbstractMethodGuard guard = new AbstractMethodGuard(); - AtomicInteger calls = new AtomicInteger(); - - for (AbstractMethodError error : - new AbstractMethodError[] { - new AbstractMethodError(), new AbstractMethodError("something else entirely") - }) { - assertNull( - guard.invokeOrNull( - "x", - t -> { - calls.incrementAndGet(); - throw error; - })); - } - - assertEquals(2, calls.get()); - assertFalse(guard.isLatched(String.class)); - } - - @Test - void attributionRequiresTheWholeClassName() { - assertTrue(AbstractMethodGuard.isAttributedTo(receiverError(String.class), String.class)); - // "java.lang.String" is a prefix of "java.lang.StringBuilder", but not the same class - assertFalse( - AbstractMethodGuard.isAttributedTo(receiverError(String.class), StringBuilder.class)); - assertFalse( - AbstractMethodGuard.isAttributedTo( - new AbstractMethodError("Receiver class " + String.class.getName()), String.class)); - } - - @Test - void attributesTheJdk8MessageFormat() { - // JDK 8 reports "." - String name = String.class.getName(); - - assertTrue( - AbstractMethodGuard.isAttributedTo( - new AbstractMethodError(name + ".getClientInfo()Ljava/util/Properties;"), - String.class)); - // a different class whose name merely starts with this one - assertFalse( - AbstractMethodGuard.isAttributedTo( - new AbstractMethodError(name + "Builder.getClientInfo()Ljava/util/Properties;"), - String.class)); - // a class in a package named like this class - assertFalse( - AbstractMethodGuard.isAttributedTo( - new AbstractMethodError(name + ".Inner.getClientInfo()Ljava/util/Properties;"), - String.class)); - assertFalse(AbstractMethodGuard.isAttributedTo(new AbstractMethodError(name), String.class)); - assertFalse( - AbstractMethodGuard.isAttributedTo(new AbstractMethodError(name + "."), String.class)); - } - - @Test - void checkedExceptionsPropagateAndDoNotLatch() { - AbstractMethodGuard guard = new AbstractMethodGuard(); - SQLException failure = new SQLException("boom"); - - SQLException thrown = - assertThrows( - SQLException.class, - () -> - guard.invokeOrNull( - "x", - t -> { - throw failure; - })); - - assertSame(failure, thrown); - assertFalse(guard.isLatched(String.class)); - } - - @Test - void unsupportedOperationYieldsNullOnEveryCallAndIsNeverLatched() { - AbstractMethodGuard guard = new AbstractMethodGuard(); - AtomicInteger calls = new AtomicInteger(); - AbstractMethodGuard.Call call = - t -> { - calls.incrementAndGet(); - throw new UnsupportedOperationException(); - }; - - assertNull(guard.invokeOrNull("x", call)); - assertNull(guard.invokeOrNull("x", call)); - - assertEquals(2, calls.get()); - assertFalse(guard.isLatched(String.class)); - } - - @Test - void otherUncheckedExceptionsPropagateAndDoNotLatch() { - AbstractMethodGuard guard = new AbstractMethodGuard(); - - assertThrows( - IllegalStateException.class, - () -> - guard.invokeOrNull( - "x", - t -> { - throw new IllegalStateException(); - })); - assertFalse(guard.isLatched(String.class)); - } - - /** - * Pins the HotSpot message format that attribution depends on, using a real error: a class built - * against an old interface, called through code built against a newer one. - */ - @Test - void latchesARealAbstractMethodError(@TempDir Path dir) throws Exception { - JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); - assumeTrue(compiler != null, "needs a JDK"); - String vm = System.getProperty("java.vm.name", ""); - assumeTrue(vm.contains("HotSpot") || vm.contains("OpenJDK"), "message format is HotSpot's"); - - Path oldDir = Files.createDirectory(dir.resolve("old")); - Path newDir = Files.createDirectory(dir.resolve("new")); - compile(compiler, oldDir, null, "I", "public interface I { String a(); }"); - compile( - compiler, - oldDir, - oldDir, - "Impl", - "public class Impl implements I { public String a() { return \"a\"; } }"); - compile(compiler, newDir, null, "I", "public interface I { String a(); String b(); }"); - compile( - compiler, - newDir, - newDir, - "Caller", - "public class Caller { public static String call(I i) { return i.b(); } }"); - - try (URLClassLoader loader = - new URLClassLoader( - new URL[] {newDir.toUri().toURL(), oldDir.toUri().toURL()}, - AbstractMethodGuardTest.class.getClassLoader())) { - Class iface = loader.loadClass("I"); - Object impl = loader.loadClass("Impl").getDeclaredConstructor().newInstance(); - Method call = loader.loadClass("Caller").getMethod("call", iface); - - AtomicInteger calls = new AtomicInteger(); - AbstractMethodGuard guard = new AbstractMethodGuard(); - AbstractMethodGuard.Call invoke = - target -> { - calls.incrementAndGet(); - try { - return call.invoke(null, target); - } catch (InvocationTargetException e) { - if (e.getCause() instanceof AbstractMethodError) { - throw (AbstractMethodError) e.getCause(); - } - throw e; - } - }; - - assertNull(guard.invokeOrNull(impl, invoke)); - assertNull(guard.invokeOrNull(impl, invoke)); - - assertEquals(1, calls.get(), "second call should be skipped"); - assertTrue(guard.isLatched(impl.getClass())); - } - } - - private static void compile( - JavaCompiler compiler, Path out, Path classpath, String name, String source) - throws IOException { - Path file = out.resolve(name + ".java"); - Files.write(file, singletonList(source), StandardCharsets.UTF_8); - int result = - classpath == null - ? compiler.run(null, null, null, "-d", out.toString(), file.toString()) - : compiler.run( - null, - null, - null, - "-cp", - classpath.toString() + File.pathSeparator, - "-d", - out.toString(), - file.toString()); - assertEquals(0, result, "compiling " + name); - } -} diff --git a/internal-api/src/test/java/datadog/trace/util/ClassLatchTest.java b/internal-api/src/test/java/datadog/trace/util/ClassLatchTest.java new file mode 100644 index 00000000000..d4c5193d61e --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/util/ClassLatchTest.java @@ -0,0 +1,217 @@ +package datadog.trace.util; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.sql.SQLException; +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.jupiter.api.Test; + +class ClassLatchTest { + + private static AbstractMethodError receiverError(Class type) { + return new AbstractMethodError( + "Receiver class " + + type.getName() + + " does not define or inherit an implementation of the resolved method 'abstract" + + " java.lang.String m()' of interface I."); + } + + /** Latches the target's class unconditionally on any IllegalStateException. */ + private static class Counting extends ClassLatch { + final AtomicInteger calls = new AtomicInteger(); + + @Override + protected String get(Object target) { + calls.incrementAndGet(); + try { + throw new IllegalStateException(); + } catch (IllegalStateException e) { + latch(target); + return "failed"; + } + } + } + + @Test + void latchesTheTargetsClassAndSkipsLaterCalls() { + Counting latch = new Counting(); + + assertEquals("failed", latch.getOrDefault("x")); + assertNull(latch.getOrDefault("y")); + assertNull(latch.getOrDefault("z")); + + assertEquals(1, latch.calls.get()); + assertTrue(latch.isLatched("x")); + } + + @Test + void otherClassesAreUnaffected() { + Counting latch = new Counting(); + latch.getOrDefault("x"); + + assertFalse(latch.isLatched(Integer.valueOf(1))); + assertEquals("failed", latch.getOrDefault(Integer.valueOf(1))); + assertEquals(2, latch.calls.get()); + } + + @Test + void nullTargetReturnsTheDefaultWithoutCalling() { + Counting latch = new Counting(); + + assertNull(latch.getOrDefault(null)); + assertFalse(latch.isLatched(null)); + assertEquals(0, latch.calls.get()); + } + + @Test + void defaultValueIsUsedWhenSkipped() { + ClassLatch latch = + new ClassLatch() { + @Override + protected String get(Object target) { + latch(target); + return "first"; + } + + @Override + protected String defaultValue(Object target) { + return "default"; + } + }; + + assertEquals("first", latch.getOrDefault("x")); + assertEquals("default", latch.getOrDefault("x")); + } + + @Test + void keyOfChoosesTheClassToLatch() { + // a wrapper whose contents differ: latch on what it holds, never on the wrapper itself + final class Wrapper { + final Object delegate; + + Wrapper(Object delegate) { + this.delegate = delegate; + } + } + ClassLatch latch = + new ClassLatch() { + @Override + protected String get(Wrapper target) { + latch(target); + return "called"; + } + + @Override + protected Class keyOf(Wrapper target) { + return target.delegate.getClass(); + } + }; + + assertEquals("called", latch.getOrDefault(new Wrapper("x"))); + + assertTrue(latch.isLatched(new Wrapper("another string"))); + assertFalse(latch.isLatched(new Wrapper(Integer.valueOf(1)))); + assertEquals("called", latch.getOrDefault(new Wrapper(Integer.valueOf(1)))); + } + + @Test + void unlatchResumesForThatKeyOnly() { + ClassLatch resuming = + new ClassLatch() { + @Override + protected String get(Object target) { + latch(target); + return "called"; + } + + @Override + protected String defaultValue(Object target) { + unlatch(target); + return "skipped"; + } + }; + resuming.getOrDefault("x"); + resuming.getOrDefault(Integer.valueOf(1)); + + assertEquals("skipped", resuming.getOrDefault("x")); + assertFalse(resuming.isLatched("x")); + assertTrue(resuming.isLatched(Integer.valueOf(1))); + } + + @Test + void latchIfNamedLatchesOnlyWhenTheErrorNamesTheKey() { + final boolean[] result = new boolean[1]; + ClassLatch latch = + new ClassLatch() { + @Override + protected String get(Object target) { + result[0] = latchIfNamed(target, receiverError(target.getClass())); + return "named"; + } + }; + latch.getOrDefault("x"); + assertTrue(result[0]); + assertTrue(latch.isLatched("x")); + + ClassLatch other = + new ClassLatch() { + @Override + protected String get(Object target) { + result[0] = latchIfNamed(target, receiverError(Integer.class)); + return "other"; + } + }; + other.getOrDefault("x"); + assertFalse(result[0]); + assertFalse(other.isLatched("x")); + } + + @Test + void attributesTheHotSpotMessageFormats() { + assertTrue(ClassLatch.isNamedIn(receiverError(String.class), String.class)); + // "java.lang.String" is a prefix of "java.lang.StringBuilder", but not the same class + assertFalse(ClassLatch.isNamedIn(receiverError(String.class), StringBuilder.class)); + assertFalse( + ClassLatch.isNamedIn( + new AbstractMethodError("Receiver class " + String.class.getName()), String.class)); + + // JDK 8 reports "." + String name = String.class.getName(); + assertTrue( + ClassLatch.isNamedIn( + new AbstractMethodError(name + ".getClientInfo()Ljava/util/Properties;"), + String.class)); + // a different class whose name merely starts with this one + assertFalse( + ClassLatch.isNamedIn( + new AbstractMethodError(name + "Builder.getClientInfo()Ljava/util/Properties;"), + String.class)); + // a class in a package named like this class + assertFalse( + ClassLatch.isNamedIn( + new AbstractMethodError(name + ".Inner.getClientInfo()Ljava/util/Properties;"), + String.class)); + assertFalse(ClassLatch.isNamedIn(new AbstractMethodError(name), String.class)); + assertFalse(ClassLatch.isNamedIn(new AbstractMethodError(name + "."), String.class)); + assertFalse(ClassLatch.isNamedIn(new AbstractMethodError(), String.class)); + assertFalse(ClassLatch.isNamedIn(new AbstractMethodError("something else"), String.class)); + } + + @Test + void checkedExceptionsPropagateWithoutLatching() { + ClassLatch latch = + new ClassLatch() { + @Override + protected String get(Object target) throws SQLException { + throw new SQLException("boom"); + } + }; + + assertThrows(SQLException.class, () -> latch.getOrDefault("x")); + assertFalse(latch.isLatched("x")); + } +} diff --git a/internal-api/src/test/java/datadog/trace/util/HandleAbstractMethodTest.java b/internal-api/src/test/java/datadog/trace/util/HandleAbstractMethodTest.java new file mode 100644 index 00000000000..c8abd6f1220 --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/util/HandleAbstractMethodTest.java @@ -0,0 +1,258 @@ +package datadog.trace.util; + +import static java.util.Collections.singletonList; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +import java.io.File; +import java.io.IOException; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.net.URL; +import java.net.URLClassLoader; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.sql.SQLException; +import java.util.concurrent.atomic.AtomicInteger; +import javax.tools.JavaCompiler; +import javax.tools.ToolProvider; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +class HandleAbstractMethodTest { + + /** What a call site writes: {@code get} delegating to {@code handleAbstractMethod}. */ + private abstract static class Handling extends ClassLatch { + protected abstract R invoke(T target) throws E; + + @Override + protected final R get(T target) throws E { + return handleAbstractMethod(target, this::invoke); + } + } + + private static AbstractMethodError receiverError(Class type) { + return new AbstractMethodError( + "Receiver class " + + type.getName() + + " does not define or inherit an implementation of the resolved method 'abstract" + + " java.lang.String m()' of interface I."); + } + + private static final class Throwing extends Handling { + final AtomicInteger calls = new AtomicInteger(); + final Throwable failure; + + Throwing(Throwable failure) { + this.failure = failure; + } + + @Override + protected String invoke(Object target) throws Exception { + calls.incrementAndGet(); + if (failure instanceof AbstractMethodError) { + // name the class of whatever was called, as the JVM does for the receiver + throw receiverError(target.getClass()); + } + if (failure instanceof Exception) { + throw (Exception) failure; + } + throw (Error) failure; + } + } + + @Test + void returnsTheResultAndLatchesNothing() throws Exception { + Handling latch = + new Handling() { + @Override + protected String invoke(Object target) { + return "ok"; + } + }; + + assertEquals("ok", latch.getOrDefault("x")); + assertFalse(latch.isLatched("x")); + } + + @Test + void latchesTheReceiverClassAndStopsCalling() throws Exception { + Throwing latch = new Throwing(new AbstractMethodError()); + + assertNull(latch.getOrDefault("x")); + assertNull(latch.getOrDefault("y")); + assertNull(latch.getOrDefault("z")); + + assertEquals(1, latch.calls.get()); + assertTrue(latch.isLatched("x")); + } + + @Test + void otherClassesAreUnaffectedByALatch() throws Exception { + Throwing latch = new Throwing(new AbstractMethodError()); + latch.getOrDefault("x"); + + assertFalse(latch.isLatched(Integer.valueOf(1))); + assertNull(latch.getOrDefault(Integer.valueOf(1))); + assertEquals(2, latch.calls.get()); + } + + @Test + void doesNotLatchWhenTheErrorNamesAnotherClass() throws Exception { + // a wrapper whose delegate lacks the method: the error names the delegate, not the wrapper + AtomicInteger calls = new AtomicInteger(); + Handling latch = + new Handling() { + @Override + protected String invoke(Object target) { + calls.incrementAndGet(); + throw receiverError(Integer.class); + } + }; + + assertNull(latch.getOrDefault("x")); + assertNull(latch.getOrDefault("x")); + + assertEquals(2, calls.get()); + assertFalse(latch.isLatched("x")); + } + + @Test + void doesNotLatchWhenTheMessageCannotBeAttributed() throws Exception { + AtomicInteger calls = new AtomicInteger(); + for (AbstractMethodError error : + new AbstractMethodError[] { + new AbstractMethodError(), new AbstractMethodError("something else entirely") + }) { + Handling latch = + new Handling() { + @Override + protected String invoke(Object target) { + calls.incrementAndGet(); + throw error; + } + }; + + assertNull(latch.getOrDefault("x")); + assertFalse(latch.isLatched("x")); + } + assertEquals(2, calls.get()); + } + + @Test + void unsupportedOperationYieldsTheDefaultOnEveryCallAndIsNeverLatched() throws Exception { + Throwing latch = new Throwing(new UnsupportedOperationException()); + + assertNull(latch.getOrDefault("x")); + assertNull(latch.getOrDefault("x")); + + assertEquals(2, latch.calls.get()); + assertFalse(latch.isLatched("x")); + } + + @Test + void checkedExceptionsPropagateAndDoNotLatch() { + SQLException failure = new SQLException("boom"); + Throwing latch = new Throwing(failure); + + SQLException thrown = assertThrows(SQLException.class, () -> latch.getOrDefault("x")); + + assertSame(failure, thrown); + assertFalse(latch.isLatched("x")); + } + + @Test + void otherUncheckedExceptionsPropagateAndDoNotLatch() { + Throwing latch = new Throwing(new IllegalStateException()); + + assertThrows(IllegalStateException.class, () -> latch.getOrDefault("x")); + assertFalse(latch.isLatched("x")); + } + + /** + * Pins the HotSpot message format that attribution depends on, using a real error: a class built + * against an old interface, called through code built against a newer one. + */ + @Test + void latchesARealAbstractMethodError(@TempDir Path dir) throws Exception { + JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); + assumeTrue(compiler != null, "needs a JDK"); + String vm = System.getProperty("java.vm.name", ""); + assumeTrue(vm.contains("HotSpot") || vm.contains("OpenJDK"), "message format is HotSpot's"); + + Path oldDir = Files.createDirectory(dir.resolve("old")); + Path newDir = Files.createDirectory(dir.resolve("new")); + compile(compiler, oldDir, null, "I", "public interface I { String a(); }"); + compile( + compiler, + oldDir, + oldDir, + "Impl", + "public class Impl implements I { public String a() { return \"a\"; } }"); + compile(compiler, newDir, null, "I", "public interface I { String a(); String b(); }"); + compile( + compiler, + newDir, + newDir, + "Caller", + "public class Caller { public static String call(I i) { return i.b(); } }"); + + try (URLClassLoader loader = + new URLClassLoader( + new URL[] {newDir.toUri().toURL(), oldDir.toUri().toURL()}, + HandleAbstractMethodTest.class.getClassLoader())) { + Class iface = loader.loadClass("I"); + Object impl = loader.loadClass("Impl").getDeclaredConstructor().newInstance(); + Method call = loader.loadClass("Caller").getMethod("call", iface); + + AtomicInteger calls = new AtomicInteger(); + Handling latch = + new Handling() { + @Override + protected Object invoke(Object target) throws Exception { + calls.incrementAndGet(); + try { + return call.invoke(null, target); + } catch (InvocationTargetException e) { + if (e.getCause() instanceof AbstractMethodError) { + throw (AbstractMethodError) e.getCause(); + } + throw e; + } + } + }; + + assertNull(latch.getOrDefault(impl)); + assertNull(latch.getOrDefault(impl)); + + assertEquals(1, calls.get(), "second call should be skipped"); + assertTrue(latch.isLatched(impl)); + } + } + + private static void compile( + JavaCompiler compiler, Path out, Path classpath, String name, String source) + throws IOException { + Path file = out.resolve(name + ".java"); + Files.write(file, singletonList(source), StandardCharsets.UTF_8); + int result = + classpath == null + ? compiler.run(null, null, null, "-d", out.toString(), file.toString()) + : compiler.run( + null, + null, + null, + "-cp", + classpath.toString() + File.pathSeparator, + "-d", + out.toString(), + file.toString()); + assertEquals(0, result, "compiling " + name); + } +} diff --git a/internal-api/src/test/java/datadog/trace/util/LatchTest.java b/internal-api/src/test/java/datadog/trace/util/LatchTest.java new file mode 100644 index 00000000000..eee3c29a795 --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/util/LatchTest.java @@ -0,0 +1,115 @@ +package datadog.trace.util; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.sql.SQLException; +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.jupiter.api.Test; + +class LatchTest { + + /** The shape of a missing-field read: rethrow the first failure, then answer a default. */ + private static final class FieldLatch extends Latch { + final AtomicInteger calls = new AtomicInteger(); + boolean fieldPresent; + + @Override + protected Boolean get(String target) { + calls.incrementAndGet(); + try { + if (!fieldPresent) { + throw new NoSuchFieldError("_interner"); + } + return false; + } catch (NoSuchFieldError e) { + latch(); + throw e; + } + } + + @Override + protected Boolean defaultValue(String target) { + return Boolean.TRUE; + } + } + + @Test + void performsTheOperationUntilLatched() { + FieldLatch latch = new FieldLatch(); + latch.fieldPresent = true; + + assertEquals(false, latch.getOrDefault("x")); + assertEquals(false, latch.getOrDefault("x")); + + assertEquals(2, latch.calls.get()); + assertFalse(latch.isLatched()); + } + + @Test + void rethrowsTheFirstFailureThenAnswersTheDefaultWithoutCalling() { + FieldLatch latch = new FieldLatch(); + + assertThrows(NoSuchFieldError.class, () -> latch.getOrDefault("x")); + assertTrue(latch.isLatched()); + + assertEquals(true, latch.getOrDefault("x")); + assertEquals(true, latch.getOrDefault("y")); + assertEquals(1, latch.calls.get(), "later calls should be skipped"); + } + + @Test + void defaultsToNull() { + Latch latch = + new Latch() { + @Override + protected String get(String target) { + latch(); + return "first"; + } + }; + + assertEquals("first", latch.getOrDefault("x")); + assertNull(latch.getOrDefault("x")); + } + + @Test + void unlatchResumesTheOperation() { + Latch latch = + new Latch() { + @Override + protected String get(String target) { + latch(); + return "called"; + } + + @Override + protected String defaultValue(String target) { + unlatch(); + return "skipped"; + } + }; + + assertEquals("called", latch.getOrDefault("x")); + assertEquals("skipped", latch.getOrDefault("x")); + assertFalse(latch.isLatched()); + assertEquals("called", latch.getOrDefault("x")); + } + + @Test + void checkedExceptionsPropagate() { + Latch latch = + new Latch() { + @Override + protected String get(String target) throws SQLException { + throw new SQLException("boom"); + } + }; + + assertThrows(SQLException.class, () -> latch.getOrDefault("x")); + assertFalse(latch.isLatched()); + } +} From 5ceffc1f0d99435bffb0651a9d08142ebeaeef73 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 30 Sep 2026 14:45:27 -0400 Subject: [PATCH 05/15] 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 --- .../java/datadog/trace/util/ClassLatch.java | 3 +- .../main/java/datadog/trace/util/Latch.java | 58 --------- .../java/datadog/trace/util/LatchTest.java | 115 ------------------ 3 files changed, 2 insertions(+), 174 deletions(-) delete mode 100644 internal-api/src/main/java/datadog/trace/util/Latch.java delete mode 100644 internal-api/src/test/java/datadog/trace/util/LatchTest.java diff --git a/internal-api/src/main/java/datadog/trace/util/ClassLatch.java b/internal-api/src/main/java/datadog/trace/util/ClassLatch.java index 3a2860c67ef..e3d0d1ea118 100644 --- a/internal-api/src/main/java/datadog/trace/util/ClassLatch.java +++ b/internal-api/src/main/java/datadog/trace/util/ClassLatch.java @@ -5,7 +5,8 @@ /** * A per-class latch for an operation that, once it has failed for a class, will fail the same way * for every instance of that class: an interface method the class does not implement, for example. - * For a failure that is the same for everyone, use {@link Latch}. + * A failure that is the same for everyone, whatever the class, needs only a single flag and no + * per-class state. * *

Intended as a {@code static final} anonymous subclass, one per call site and per operation: a * class lacking one method says nothing about another, so latches must not be shared. As a constant diff --git a/internal-api/src/main/java/datadog/trace/util/Latch.java b/internal-api/src/main/java/datadog/trace/util/Latch.java deleted file mode 100644 index 9541b2a19c0..00000000000 --- a/internal-api/src/main/java/datadog/trace/util/Latch.java +++ /dev/null @@ -1,58 +0,0 @@ -package datadog.trace.util; - -import javax.annotation.Nullable; - -/** - * A one-way, call-site-wide latch for an operation that fails the same way for everyone once it has - * failed, such as reading a field that is missing from the classes on the classpath. For a failure - * that depends on the receiver's class, use {@link ClassLatch}. - * - *

Intended as a {@code static final} anonymous subclass, one per call site: the receiver is then - * a constant of a known exact type, so the JIT can inline {@link #get} and {@link #defaultValue}. - * Subclasses decide what counts as a failure in their own {@code try/catch} inside {@link #get}, so - * checked exceptions and a tight {@code try} scope come for free, and call {@link #latch()} - * themselves. - * - *

This is a hint, not a lock. The flag is deliberately plain. A stale read only costs another - * failure; a thread always sees its own write, so each thread pays for at most one failure after - * its own first. Other threads' writes become visible eventually, with no bound on how long that - * takes. - * - * @param the type of the value the operation is applied to - * @param the type of the result - * @param the checked exception {@link #get} may throw - */ -public abstract class Latch { - private boolean latched; - - /** Performs the operation. Call {@link #latch()} when it has failed in a way that will recur. */ - @Nullable - protected abstract R get(T target) throws E; - - /** The result once latched. {@code null} unless overridden. */ - @Nullable - protected R defaultValue(T target) { - return null; - } - - /** Performs the operation unless latched, in which case returns {@link #defaultValue}. */ - @Nullable - public final R getOrDefault(T target) throws E { - return latched ? defaultValue(target) : get(target); - } - - /** Returns whether the operation is being skipped. */ - public final boolean isLatched() { - return latched; - } - - /** Skips the operation from now on. */ - protected final void latch() { - latched = true; - } - - /** Resumes performing the operation, for tests or for a policy that retries. */ - protected final void unlatch() { - latched = false; - } -} diff --git a/internal-api/src/test/java/datadog/trace/util/LatchTest.java b/internal-api/src/test/java/datadog/trace/util/LatchTest.java deleted file mode 100644 index eee3c29a795..00000000000 --- a/internal-api/src/test/java/datadog/trace/util/LatchTest.java +++ /dev/null @@ -1,115 +0,0 @@ -package datadog.trace.util; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; - -import java.sql.SQLException; -import java.util.concurrent.atomic.AtomicInteger; -import org.junit.jupiter.api.Test; - -class LatchTest { - - /** The shape of a missing-field read: rethrow the first failure, then answer a default. */ - private static final class FieldLatch extends Latch { - final AtomicInteger calls = new AtomicInteger(); - boolean fieldPresent; - - @Override - protected Boolean get(String target) { - calls.incrementAndGet(); - try { - if (!fieldPresent) { - throw new NoSuchFieldError("_interner"); - } - return false; - } catch (NoSuchFieldError e) { - latch(); - throw e; - } - } - - @Override - protected Boolean defaultValue(String target) { - return Boolean.TRUE; - } - } - - @Test - void performsTheOperationUntilLatched() { - FieldLatch latch = new FieldLatch(); - latch.fieldPresent = true; - - assertEquals(false, latch.getOrDefault("x")); - assertEquals(false, latch.getOrDefault("x")); - - assertEquals(2, latch.calls.get()); - assertFalse(latch.isLatched()); - } - - @Test - void rethrowsTheFirstFailureThenAnswersTheDefaultWithoutCalling() { - FieldLatch latch = new FieldLatch(); - - assertThrows(NoSuchFieldError.class, () -> latch.getOrDefault("x")); - assertTrue(latch.isLatched()); - - assertEquals(true, latch.getOrDefault("x")); - assertEquals(true, latch.getOrDefault("y")); - assertEquals(1, latch.calls.get(), "later calls should be skipped"); - } - - @Test - void defaultsToNull() { - Latch latch = - new Latch() { - @Override - protected String get(String target) { - latch(); - return "first"; - } - }; - - assertEquals("first", latch.getOrDefault("x")); - assertNull(latch.getOrDefault("x")); - } - - @Test - void unlatchResumesTheOperation() { - Latch latch = - new Latch() { - @Override - protected String get(String target) { - latch(); - return "called"; - } - - @Override - protected String defaultValue(String target) { - unlatch(); - return "skipped"; - } - }; - - assertEquals("called", latch.getOrDefault("x")); - assertEquals("skipped", latch.getOrDefault("x")); - assertFalse(latch.isLatched()); - assertEquals("called", latch.getOrDefault("x")); - } - - @Test - void checkedExceptionsPropagate() { - Latch latch = - new Latch() { - @Override - protected String get(String target) throws SQLException { - throw new SQLException("boom"); - } - }; - - assertThrows(SQLException.class, () -> latch.getOrDefault("x")); - assertFalse(latch.isLatched()); - } -} From 3af387a66c62078c156d2f5214befed9fd84dd45 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 30 Sep 2026 15:05:54 -0400 Subject: [PATCH 06/15] 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 --- .../instrumentation/jdbc/JDBCDecorator.java | 2 +- .../trace/util/ClassLatchBenchmark.java | 20 ++-- .../java/datadog/trace/util/ClassLatch.java | 43 ++++---- .../datadog/trace/util/ClassLatchTest.java | 99 ++++++++++++------- .../trace/util/HandleAbstractMethodTest.java | 30 +++--- 5 files changed, 111 insertions(+), 83 deletions(-) diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java index a72eaf88356..f6d5fe0fd90 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java @@ -257,7 +257,7 @@ public static DBInfo parseDBInfoFromConnection(final Connection connection) { if (metaData != null && (url = metaData.getURL()) != null) { Properties clientInfo = null; try { - clientInfo = CLIENT_INFO.getOrDefault(connection); + clientInfo = CLIENT_INFO.tryGetOrNull(connection); } catch (final SQLException ex) { // getClientInfo is not allowed, we can still extract info from the url alone log.debug(LogCollector.EXCLUDE_TELEMETRY, "Could not get client info from DB", ex); diff --git a/internal-api/src/jmh/java/datadog/trace/util/ClassLatchBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/ClassLatchBenchmark.java index d15c2658124..69ea15a81d2 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/ClassLatchBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/ClassLatchBenchmark.java @@ -149,9 +149,9 @@ protected final Object get(Object target) { return invoke(target); } catch (AbstractMethodError e) { latchIfNamed(target, e); - return defaultValue(target); + return null; } catch (UnsupportedOperationException e) { - return defaultValue(target); + return null; } } } @@ -231,15 +231,15 @@ public void setup() throws Throwable { .asType(MethodType.methodType(Object.class, Object.class)); // reach the steady state: the latch has already met the deficient class - MISSING.getOrDefault(impl); + MISSING.tryGetOrNull(impl); if (!MISSING.isLatched(impl)) { throw new IllegalStateException("expected the latch to latch " + impl.getClass()); } - WRAPPER.getOrDefault(wrapper); + WRAPPER.tryGetOrNull(wrapper); if (WRAPPER.isLatched(wrapper) || WRAPPER.isLatched(impl)) { throw new IllegalStateException("a wrapper must never be latched"); } - SUBCLASS_MISSING.getOrDefault(impl); + SUBCLASS_MISSING.tryGetOrNull(impl); if (!SUBCLASS_MISSING.isLatched(impl)) { throw new IllegalStateException( "expected the subclass-style latch to latch " + impl.getClass()); @@ -300,11 +300,11 @@ private Object unguardedMissing(int remaining) { } private Object latchedMissing(int remaining) { - return remaining > 0 ? latchedMissing(remaining - 1) : MISSING.getOrDefault(impl); + return remaining > 0 ? latchedMissing(remaining - 1) : MISSING.tryGetOrNull(impl); } private Object latchedWrapperMissing(int remaining) { - return remaining > 0 ? latchedWrapperMissing(remaining - 1) : WRAPPER.getOrDefault(wrapper); + return remaining > 0 ? latchedWrapperMissing(remaining - 1) : WRAPPER.tryGetOrNull(wrapper); } private Object unguardedPresent(int remaining) { @@ -312,15 +312,15 @@ private Object unguardedPresent(int remaining) { } private Object latchedPresent(int remaining) { - return remaining > 0 ? latchedPresent(remaining - 1) : PRESENT.getOrDefault(full); + return remaining > 0 ? latchedPresent(remaining - 1) : PRESENT.tryGetOrNull(full); } private Object subclassMissing(int remaining) { - return remaining > 0 ? subclassMissing(remaining - 1) : SUBCLASS_MISSING.getOrDefault(impl); + return remaining > 0 ? subclassMissing(remaining - 1) : SUBCLASS_MISSING.tryGetOrNull(impl); } private Object subclassPresent(int remaining) { - return remaining > 0 ? subclassPresent(remaining - 1) : SUBCLASS_PRESENT.getOrDefault(full); + return remaining > 0 ? subclassPresent(remaining - 1) : SUBCLASS_PRESENT.tryGetOrNull(full); } private static void compile( diff --git a/internal-api/src/main/java/datadog/trace/util/ClassLatch.java b/internal-api/src/main/java/datadog/trace/util/ClassLatch.java index e3d0d1ea118..d95d207fb74 100644 --- a/internal-api/src/main/java/datadog/trace/util/ClassLatch.java +++ b/internal-api/src/main/java/datadog/trace/util/ClassLatch.java @@ -10,10 +10,10 @@ * *

Intended as a {@code static final} anonymous subclass, one per call site and per operation: a * class lacking one method says nothing about another, so latches must not be shared. As a constant - * of a known exact type, the receiver lets the JIT inline {@link #get}, {@link #defaultValue} and - * {@link #keyOf}. Subclasses decide what counts as a failure in their own {@code try/catch} inside - * {@link #get}, so checked exceptions and a tight {@code try} scope come for free, and latch - * through the protected helpers. Only the declaring subclass can change the state. + * of a known exact type, the receiver lets the JIT inline {@link #get} and {@link #keyOf}. + * Subclasses decide what counts as a failure in their own {@code try/catch} inside {@link #get}, so + * checked exceptions and a tight {@code try} scope come for free, and latch through the protected + * helpers. Only the declaring subclass can change the state. * *

{@link #keyOf} chooses the class the latch is keyed on, and is used by every operation, so the * check and the latch cannot disagree. The default is the target's own class. Never key on a @@ -63,12 +63,6 @@ protected boolean[] computeValue(Class type) { @Nullable protected abstract R get(T target) throws E; - /** The result for a {@code null} or latched target. {@code null} unless overridden. */ - @Nullable - protected R defaultValue(@Nullable T target) { - return null; - } - /** The class the latch is keyed on. The target's own class unless overridden. */ protected Class keyOf(T target) { return target.getClass(); @@ -76,11 +70,22 @@ protected Class keyOf(T target) { /** * Performs the operation unless the target is {@code null} or latched, in which case returns - * {@link #defaultValue}. + * {@code null}. A {@code null} result means nothing is available: the operation was skipped, or + * it produced no value. */ @Nullable - public final R getOrDefault(@Nullable T target) throws E { - return target == null || isLatched(target) ? defaultValue(target) : get(target); + public final R tryGetOrNull(@Nullable T target) throws E { + return target == null || isLatched(target) ? null : get(target); + } + + /** + * Like {@link #tryGetOrNull}, but returns {@code fallback} when there is nothing available. The + * fallback is also used when the operation itself produced {@code null}, so a call and a skipped + * call always agree. + */ + public final R tryGetOrDefault(@Nullable T target, R fallback) throws E { + final R result = tryGetOrNull(target); + return result != null ? result : fallback; } /** Returns whether the operation is being skipped for the target. */ @@ -117,10 +122,10 @@ protected final boolean latchIfNamed(T target, AbstractMethodError error) { } /** - * For a call to a method that some implementations may lack: returns {@link #defaultValue} if the - * call raises {@link AbstractMethodError} or {@link UnsupportedOperationException}, latching the - * key first in the former case if, and only if, the error names it (see {@link #latchIfNamed}). - * An unsupported operation names no class, so it is never latched, and is caught on every call. + * For a call to a method that some implementations may lack: returns {@code null} if the call + * raises {@link AbstractMethodError} or {@link UnsupportedOperationException}, latching the key + * first in the former case if, and only if, the error names it (see {@link #latchIfNamed}). An + * unsupported operation names no class, so it is never latched, and is caught on every call. * Anything else, checked exceptions included, propagates unchanged. * * 

{@code
@@ -137,10 +142,10 @@ protected final R handleAbstractMethod(T target, Call call) throws E {
       return call.apply(target);
     } catch (AbstractMethodError e) {
       latchIfNamed(target, e);
-      return defaultValue(target);
+      return null;
     } catch (UnsupportedOperationException e) {
       // no class to attribute it to, and it may come from a delegate: never latched
-      return defaultValue(target);
+      return null;
     }
   }
 
diff --git a/internal-api/src/test/java/datadog/trace/util/ClassLatchTest.java b/internal-api/src/test/java/datadog/trace/util/ClassLatchTest.java
index d4c5193d61e..7af43b9505d 100644
--- a/internal-api/src/test/java/datadog/trace/util/ClassLatchTest.java
+++ b/internal-api/src/test/java/datadog/trace/util/ClassLatchTest.java
@@ -40,9 +40,9 @@ protected String get(Object target) {
   void latchesTheTargetsClassAndSkipsLaterCalls() {
     Counting latch = new Counting();
 
-    assertEquals("failed", latch.getOrDefault("x"));
-    assertNull(latch.getOrDefault("y"));
-    assertNull(latch.getOrDefault("z"));
+    assertEquals("failed", latch.tryGetOrNull("x"));
+    assertNull(latch.tryGetOrNull("y"));
+    assertNull(latch.tryGetOrNull("z"));
 
     assertEquals(1, latch.calls.get());
     assertTrue(latch.isLatched("x"));
@@ -51,10 +51,10 @@ void latchesTheTargetsClassAndSkipsLaterCalls() {
   @Test
   void otherClassesAreUnaffected() {
     Counting latch = new Counting();
-    latch.getOrDefault("x");
+    latch.tryGetOrNull("x");
 
     assertFalse(latch.isLatched(Integer.valueOf(1)));
-    assertEquals("failed", latch.getOrDefault(Integer.valueOf(1)));
+    assertEquals("failed", latch.tryGetOrNull(Integer.valueOf(1)));
     assertEquals(2, latch.calls.get());
   }
 
@@ -62,29 +62,48 @@ void otherClassesAreUnaffected() {
   void nullTargetReturnsTheDefaultWithoutCalling() {
     Counting latch = new Counting();
 
-    assertNull(latch.getOrDefault(null));
+    assertNull(latch.tryGetOrNull(null));
     assertFalse(latch.isLatched(null));
     assertEquals(0, latch.calls.get());
   }
 
   @Test
-  void defaultValueIsUsedWhenSkipped() {
+  void tryGetOrDefaultReturnsTheResultWhenThereIsOne() {
+    ClassLatch latch =
+        new ClassLatch() {
+          @Override
+          protected Boolean get(Object target) {
+            return false;
+          }
+        };
+
+    // a real false must not be replaced by the fallback
+    assertEquals(false, latch.tryGetOrDefault("x", Boolean.TRUE));
+  }
+
+  @Test
+  void tryGetOrDefaultReturnsTheFallbackWhenSkippedOrNull() {
+    Counting latch = new Counting();
+
+    // the first call latches and yields a value; later calls are skipped
+    assertEquals("failed", latch.tryGetOrDefault("x", "fallback"));
+    assertEquals("fallback", latch.tryGetOrDefault("x", "fallback"));
+    assertEquals("fallback", latch.tryGetOrDefault(null, "fallback"));
+  }
+
+  @Test
+  void aCallThatYieldsNothingAndASkippedCallAgree() {
     ClassLatch latch =
         new ClassLatch() {
           @Override
           protected String get(Object target) {
             latch(target);
-            return "first";
-          }
-
-          @Override
-          protected String defaultValue(Object target) {
-            return "default";
+            return null;
           }
         };
 
-    assertEquals("first", latch.getOrDefault("x"));
-    assertEquals("default", latch.getOrDefault("x"));
+    assertEquals("fallback", latch.tryGetOrDefault("x", "fallback"));
+    assertEquals("fallback", latch.tryGetOrDefault("x", "fallback"));
   }
 
   @Test
@@ -111,35 +130,39 @@ protected Class keyOf(Wrapper target) {
           }
         };
 
-    assertEquals("called", latch.getOrDefault(new Wrapper("x")));
+    assertEquals("called", latch.tryGetOrNull(new Wrapper("x")));
 
     assertTrue(latch.isLatched(new Wrapper("another string")));
     assertFalse(latch.isLatched(new Wrapper(Integer.valueOf(1))));
-    assertEquals("called", latch.getOrDefault(new Wrapper(Integer.valueOf(1))));
+    assertEquals("called", latch.tryGetOrNull(new Wrapper(Integer.valueOf(1))));
+  }
+
+  /** A subclass may expose {@code unlatch}, for a policy that retries. */
+  private static final class Resumable extends ClassLatch {
+    @Override
+    protected String get(Object target) {
+      latch(target);
+      return "called";
+    }
+
+    void resume(Object target) {
+      unlatch(target);
+    }
   }
 
   @Test
   void unlatchResumesForThatKeyOnly() {
-    ClassLatch resuming =
-        new ClassLatch() {
-          @Override
-          protected String get(Object target) {
-            latch(target);
-            return "called";
-          }
+    Resumable latch = new Resumable();
+    latch.tryGetOrNull("x");
+    latch.tryGetOrNull(Integer.valueOf(1));
+    assertTrue(latch.isLatched("x"));
+    assertTrue(latch.isLatched(Integer.valueOf(1)));
 
-          @Override
-          protected String defaultValue(Object target) {
-            unlatch(target);
-            return "skipped";
-          }
-        };
-    resuming.getOrDefault("x");
-    resuming.getOrDefault(Integer.valueOf(1));
+    latch.resume("x");
 
-    assertEquals("skipped", resuming.getOrDefault("x"));
-    assertFalse(resuming.isLatched("x"));
-    assertTrue(resuming.isLatched(Integer.valueOf(1)));
+    assertFalse(latch.isLatched("x"));
+    assertTrue(latch.isLatched(Integer.valueOf(1)));
+    assertEquals("called", latch.tryGetOrNull("x"));
   }
 
   @Test
@@ -153,7 +176,7 @@ protected String get(Object target) {
             return "named";
           }
         };
-    latch.getOrDefault("x");
+    latch.tryGetOrNull("x");
     assertTrue(result[0]);
     assertTrue(latch.isLatched("x"));
 
@@ -165,7 +188,7 @@ protected String get(Object target) {
             return "other";
           }
         };
-    other.getOrDefault("x");
+    other.tryGetOrNull("x");
     assertFalse(result[0]);
     assertFalse(other.isLatched("x"));
   }
@@ -211,7 +234,7 @@ protected String get(Object target) throws SQLException {
           }
         };
 
-    assertThrows(SQLException.class, () -> latch.getOrDefault("x"));
+    assertThrows(SQLException.class, () -> latch.tryGetOrNull("x"));
     assertFalse(latch.isLatched("x"));
   }
 }
diff --git a/internal-api/src/test/java/datadog/trace/util/HandleAbstractMethodTest.java b/internal-api/src/test/java/datadog/trace/util/HandleAbstractMethodTest.java
index c8abd6f1220..1e8a48e444c 100644
--- a/internal-api/src/test/java/datadog/trace/util/HandleAbstractMethodTest.java
+++ b/internal-api/src/test/java/datadog/trace/util/HandleAbstractMethodTest.java
@@ -77,7 +77,7 @@ protected String invoke(Object target) {
           }
         };
 
-    assertEquals("ok", latch.getOrDefault("x"));
+    assertEquals("ok", latch.tryGetOrNull("x"));
     assertFalse(latch.isLatched("x"));
   }
 
@@ -85,9 +85,9 @@ protected String invoke(Object target) {
   void latchesTheReceiverClassAndStopsCalling() throws Exception {
     Throwing latch = new Throwing(new AbstractMethodError());
 
-    assertNull(latch.getOrDefault("x"));
-    assertNull(latch.getOrDefault("y"));
-    assertNull(latch.getOrDefault("z"));
+    assertNull(latch.tryGetOrNull("x"));
+    assertNull(latch.tryGetOrNull("y"));
+    assertNull(latch.tryGetOrNull("z"));
 
     assertEquals(1, latch.calls.get());
     assertTrue(latch.isLatched("x"));
@@ -96,10 +96,10 @@ void latchesTheReceiverClassAndStopsCalling() throws Exception {
   @Test
   void otherClassesAreUnaffectedByALatch() throws Exception {
     Throwing latch = new Throwing(new AbstractMethodError());
-    latch.getOrDefault("x");
+    latch.tryGetOrNull("x");
 
     assertFalse(latch.isLatched(Integer.valueOf(1)));
-    assertNull(latch.getOrDefault(Integer.valueOf(1)));
+    assertNull(latch.tryGetOrNull(Integer.valueOf(1)));
     assertEquals(2, latch.calls.get());
   }
 
@@ -116,8 +116,8 @@ protected String invoke(Object target) {
           }
         };
 
-    assertNull(latch.getOrDefault("x"));
-    assertNull(latch.getOrDefault("x"));
+    assertNull(latch.tryGetOrNull("x"));
+    assertNull(latch.tryGetOrNull("x"));
 
     assertEquals(2, calls.get());
     assertFalse(latch.isLatched("x"));
@@ -139,7 +139,7 @@ protected String invoke(Object target) {
             }
           };
 
-      assertNull(latch.getOrDefault("x"));
+      assertNull(latch.tryGetOrNull("x"));
       assertFalse(latch.isLatched("x"));
     }
     assertEquals(2, calls.get());
@@ -149,8 +149,8 @@ protected String invoke(Object target) {
   void unsupportedOperationYieldsTheDefaultOnEveryCallAndIsNeverLatched() throws Exception {
     Throwing latch = new Throwing(new UnsupportedOperationException());
 
-    assertNull(latch.getOrDefault("x"));
-    assertNull(latch.getOrDefault("x"));
+    assertNull(latch.tryGetOrNull("x"));
+    assertNull(latch.tryGetOrNull("x"));
 
     assertEquals(2, latch.calls.get());
     assertFalse(latch.isLatched("x"));
@@ -161,7 +161,7 @@ void checkedExceptionsPropagateAndDoNotLatch() {
     SQLException failure = new SQLException("boom");
     Throwing latch = new Throwing(failure);
 
-    SQLException thrown = assertThrows(SQLException.class, () -> latch.getOrDefault("x"));
+    SQLException thrown = assertThrows(SQLException.class, () -> latch.tryGetOrNull("x"));
 
     assertSame(failure, thrown);
     assertFalse(latch.isLatched("x"));
@@ -171,7 +171,7 @@ void checkedExceptionsPropagateAndDoNotLatch() {
   void otherUncheckedExceptionsPropagateAndDoNotLatch() {
     Throwing latch = new Throwing(new IllegalStateException());
 
-    assertThrows(IllegalStateException.class, () -> latch.getOrDefault("x"));
+    assertThrows(IllegalStateException.class, () -> latch.tryGetOrNull("x"));
     assertFalse(latch.isLatched("x"));
   }
 
@@ -228,8 +228,8 @@ protected Object invoke(Object target) throws Exception {
             }
           };
 
-      assertNull(latch.getOrDefault(impl));
-      assertNull(latch.getOrDefault(impl));
+      assertNull(latch.tryGetOrNull(impl));
+      assertNull(latch.tryGetOrNull(impl));
 
       assertEquals(1, calls.get(), "second call should be skipped");
       assertTrue(latch.isLatched(impl));

From 4c0745536ca66f9fbf5b6d06851e137a687de472 Mon Sep 17 00:00:00 2001
From: Douglas Q Hawkins 
Date: Wed, 30 Sep 2026 15:23:00 -0400
Subject: [PATCH 07/15] 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 
---
 .../datadog/trace/api/function/ThrowingFunction.java | 10 ++++++++++
 .../src/main/java/datadog/trace/util/ClassLatch.java | 12 ++----------
 2 files changed, 12 insertions(+), 10 deletions(-)
 create mode 100644 internal-api/src/main/java/datadog/trace/api/function/ThrowingFunction.java

diff --git a/internal-api/src/main/java/datadog/trace/api/function/ThrowingFunction.java b/internal-api/src/main/java/datadog/trace/api/function/ThrowingFunction.java
new file mode 100644
index 00000000000..9b2d2fe010c
--- /dev/null
+++ b/internal-api/src/main/java/datadog/trace/api/function/ThrowingFunction.java
@@ -0,0 +1,10 @@
+package datadog.trace.api.function;
+
+/**
+ * A function that may throw a checked exception, typically a method reference such as {@code
+ * Connection::getClientInfo}. The exception type is a parameter so that it flows through the
+ * caller: a function that throws nothing checked declares {@code RuntimeException}.
+ */
+public interface ThrowingFunction {
+  R apply(T t) throws E;
+}
diff --git a/internal-api/src/main/java/datadog/trace/util/ClassLatch.java b/internal-api/src/main/java/datadog/trace/util/ClassLatch.java
index d95d207fb74..2f0889b2827 100644
--- a/internal-api/src/main/java/datadog/trace/util/ClassLatch.java
+++ b/internal-api/src/main/java/datadog/trace/util/ClassLatch.java
@@ -1,5 +1,6 @@
 package datadog.trace.util;
 
+import datadog.trace.api.function.ThrowingFunction;
 import javax.annotation.Nullable;
 
 /**
@@ -31,15 +32,6 @@
  * @param  the checked exception {@link #get} may throw
  */
 public abstract class ClassLatch {
-  /**
-   * A call that may fail, typically a method reference such as {@code Connection::getClientInfo}.
-   */
-  @FunctionalInterface
-  public interface Call {
-    @Nullable
-    R apply(T target) throws E;
-  }
-
   private static final String RECEIVER_PREFIX = "Receiver class ";
 
   /**
@@ -137,7 +129,7 @@ protected final boolean latchIfNamed(T target, AbstractMethodError error) {
    * Compose {@link #latchIfNamed} and {@link #latch} directly for anything more involved.
    */
   @Nullable
-  protected final R handleAbstractMethod(T target, Call call) throws E {
+  protected final R handleAbstractMethod(T target, ThrowingFunction call) throws E {
     try {
       return call.apply(target);
     } catch (AbstractMethodError e) {

From 9dabe5396fdc73cfea3c5bea120fa322d68f9ed1 Mon Sep 17 00:00:00 2001
From: Douglas Q Hawkins 
Date: Wed, 30 Sep 2026 15:46:58 -0400
Subject: [PATCH 08/15] 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 
---
 .../src/main/java/datadog/trace/util/ClassLatch.java      | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/internal-api/src/main/java/datadog/trace/util/ClassLatch.java b/internal-api/src/main/java/datadog/trace/util/ClassLatch.java
index 2f0889b2827..281d40d65b9 100644
--- a/internal-api/src/main/java/datadog/trace/util/ClassLatch.java
+++ b/internal-api/src/main/java/datadog/trace/util/ClassLatch.java
@@ -1,5 +1,7 @@
 package datadog.trace.util;
 
+import datadog.trace.api.function.Strategy;
+import datadog.trace.api.function.StrategyConsumer;
 import datadog.trace.api.function.ThrowingFunction;
 import javax.annotation.Nullable;
 
@@ -126,10 +128,14 @@ protected final boolean latchIfNamed(T target, AbstractMethodError error) {
    * }
    * }
* + * Pass a method reference or a non-capturing lambda, and keep this method small so it inlines: + * that is what lets the JIT see the exact function at each call site (see {@link Strategy}). * Compose {@link #latchIfNamed} and {@link #latch} directly for anything more involved. */ @Nullable - protected final R handleAbstractMethod(T target, ThrowingFunction call) throws E { + @StrategyConsumer + protected final R handleAbstractMethod(T target, @Strategy ThrowingFunction call) + throws E { try { return call.apply(target); } catch (AbstractMethodError e) { From b4045082ecbca7fb77465be45e591b1496ffafd7 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 30 Sep 2026 16:05:38 -0400 Subject: [PATCH 09/15] 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 --- .../java/datadog/trace/util/ClassLatch.java | 65 +++++- .../trace/util/HandleNoSuchMethodTest.java | 82 +++++++ .../HandleNoSuchOrAbstractMethodTest.java | 203 ++++++++++++++++++ 3 files changed, 349 insertions(+), 1 deletion(-) create mode 100644 internal-api/src/test/java/datadog/trace/util/HandleNoSuchMethodTest.java create mode 100644 internal-api/src/test/java/datadog/trace/util/HandleNoSuchOrAbstractMethodTest.java diff --git a/internal-api/src/main/java/datadog/trace/util/ClassLatch.java b/internal-api/src/main/java/datadog/trace/util/ClassLatch.java index 281d40d65b9..7e57b6b07dd 100644 --- a/internal-api/src/main/java/datadog/trace/util/ClassLatch.java +++ b/internal-api/src/main/java/datadog/trace/util/ClassLatch.java @@ -128,7 +128,12 @@ protected final boolean latchIfNamed(T target, AbstractMethodError error) { * } * } * - * Pass a method reference or a non-capturing lambda, and keep this method small so it inlines: + * This covers only {@link AbstractMethodError}. A method that may also be missing from the + * classes on the classpath altogether raises {@link NoSuchMethodError}, which this does not + * handle; see {@link #handleNoSuchMethod} and {@link #handleNoSuchOrAbstractMethod}, and prefer + * the latter when unsure which a call site can see. + * + *

Pass a method reference or a non-capturing lambda, and keep this method small so it inlines: * that is what lets the JIT see the exact function at each call site (see {@link Strategy}). * Compose {@link #latchIfNamed} and {@link #latch} directly for anything more involved. */ @@ -147,6 +152,64 @@ protected final R handleAbstractMethod(T target, @Strategy ThrowingFunctionA {@link NoSuchMethodError} is a failure of resolution, which the JVM keeps for the call + * site, but it can equally be raised by a call made inside one receiver's + * implementation. Its message names the declared type, not the receiver, so it cannot be + * attributed to a class. It therefore latches the target's key, never the whole call site: a + * site-wide failure then costs one throw per receiver class, and an inner failure stays confined + * to the class that has it. + * + * 

{@code
+   * protected Properties get(Connection c) throws SQLException {
+   *   return handleNoSuchMethod(c, Connection::getClientInfo);
+   * }
+   * }
+ */ + @Nullable + @StrategyConsumer + protected final R handleNoSuchMethod(T target, @Strategy ThrowingFunction call) + throws E { + try { + return call.apply(target); + } catch (NoSuchMethodError e) { + latch(target); + return null; + } + } + + /** + * For a call to a method that may be missing ({@link NoSuchMethodError}) or unimplemented by some + * classes ({@link AbstractMethodError}): both yield {@code null}, as does {@link + * UnsupportedOperationException}. This is {@link #handleAbstractMethod} and {@link + * #handleNoSuchMethod} together, with the same latching rules: an {@link AbstractMethodError} + * latches only if its message names the key, and a {@link NoSuchMethodError} latches the target's + * key. The two are easy to confuse, so prefer this one unless you know which a call site can see. + * + * 
{@code
+   * protected Properties get(Connection c) throws SQLException {
+   *   return handleNoSuchOrAbstractMethod(c, Connection::getClientInfo);
+   * }
+   * }
+ */ + @Nullable + @StrategyConsumer + protected final R handleNoSuchOrAbstractMethod(T target, @Strategy ThrowingFunction call) + throws E { + try { + return handleAbstractMethod(target, call); + } catch (NoSuchMethodError e) { + latch(target); + return null; + } + } + /** * Matches the receiver class named by HotSpot's message against a class. Two formats exist: * diff --git a/internal-api/src/test/java/datadog/trace/util/HandleNoSuchMethodTest.java b/internal-api/src/test/java/datadog/trace/util/HandleNoSuchMethodTest.java new file mode 100644 index 00000000000..5b21e8fb405 --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/util/HandleNoSuchMethodTest.java @@ -0,0 +1,82 @@ +package datadog.trace.util; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.sql.SQLException; +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.jupiter.api.Test; + +class HandleNoSuchMethodTest { + + /** What a call site writes: {@code get} delegating to {@code handleNoSuchMethod}. */ + private static final class Throwing extends ClassLatch { + final AtomicInteger calls = new AtomicInteger(); + final Throwable failure; + + Throwing(Throwable failure) { + this.failure = failure; + } + + @Override + protected String get(Object target) throws Exception { + return handleNoSuchMethod( + target, + t -> { + calls.incrementAndGet(); + if (failure instanceof Exception) { + throw (Exception) failure; + } + throw (Error) failure; + }); + } + } + + @Test + void returnsTheResultAndLatchesNothing() throws Exception { + ClassLatch latch = + new ClassLatch() { + @Override + protected String get(Object target) { + return handleNoSuchMethod(target, t -> "ok"); + } + }; + + assertEquals("ok", latch.tryGetOrNull("x")); + assertFalse(latch.isLatched("x")); + } + + @Test + void noSuchMethodYieldsNullAndLatchesTheTargetsClass() throws Exception { + Throwing latch = new Throwing(new NoSuchMethodError("I.b()Ljava/lang/String;")); + + assertNull(latch.tryGetOrNull("x")); + assertNull(latch.tryGetOrNull("y")); + + assertEquals(1, latch.calls.get(), "later calls should be skipped"); + assertTrue(latch.isLatched("x")); + // the message names the declared type, not the receiver: another class gets its own attempt + assertFalse(latch.isLatched(Integer.valueOf(1))); + } + + @Test + void doesNotHandleAbstractMethodErrorOrUnsupportedOperation() { + Throwing abstractMethod = new Throwing(new AbstractMethodError("Impl.b()V")); + assertThrows(AbstractMethodError.class, () -> abstractMethod.tryGetOrNull("x")); + assertFalse(abstractMethod.isLatched("x")); + + Throwing unsupported = new Throwing(new UnsupportedOperationException()); + assertThrows(UnsupportedOperationException.class, () -> unsupported.tryGetOrNull("x")); + assertFalse(unsupported.isLatched("x")); + } + + @Test + void otherFailuresPropagateWithoutLatching() { + Throwing checked = new Throwing(new SQLException("boom")); + assertThrows(SQLException.class, () -> checked.tryGetOrNull("x")); + assertFalse(checked.isLatched("x")); + } +} diff --git a/internal-api/src/test/java/datadog/trace/util/HandleNoSuchOrAbstractMethodTest.java b/internal-api/src/test/java/datadog/trace/util/HandleNoSuchOrAbstractMethodTest.java new file mode 100644 index 00000000000..1be4e1f317b --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/util/HandleNoSuchOrAbstractMethodTest.java @@ -0,0 +1,203 @@ +package datadog.trace.util; + +import static java.util.Collections.singletonList; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +import java.io.File; +import java.io.IOException; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.net.URL; +import java.net.URLClassLoader; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.sql.SQLException; +import java.util.concurrent.atomic.AtomicInteger; +import javax.tools.JavaCompiler; +import javax.tools.ToolProvider; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +class HandleNoSuchOrAbstractMethodTest { + + /** What a call site writes: {@code get} delegating to {@code handleNoSuchOrAbstractMethod}. */ + private abstract static class Handling extends ClassLatch { + final AtomicInteger calls = new AtomicInteger(); + + protected abstract R invoke(T target) throws E; + + @Override + protected final R get(T target) throws E { + return handleNoSuchOrAbstractMethod( + target, + t -> { + calls.incrementAndGet(); + return invoke(t); + }); + } + } + + private static final class Throwing extends Handling { + final Throwable failure; + + Throwing(Throwable failure) { + this.failure = failure; + } + + @Override + protected String invoke(Object target) throws Exception { + if (failure instanceof Exception) { + throw (Exception) failure; + } + throw (Error) failure; + } + } + + @Test + void noSuchMethodYieldsNullAndLatchesTheTargetsClass() throws Exception { + Throwing latch = new Throwing(new NoSuchMethodError("I.b()Ljava/lang/String;")); + + assertNull(latch.tryGetOrNull("x")); + assertNull(latch.tryGetOrNull("y")); + assertNull(latch.tryGetOrNull("z")); + + assertEquals(1, latch.calls.get(), "later calls should be skipped"); + assertTrue(latch.isLatched("x")); + } + + @Test + void noSuchMethodNeverLatchesTheWholeSite() throws Exception { + // the message names the declared type, not the receiver, so it cannot be attributed to a class; + // latching only the target's key means another class still gets its own attempt + Throwing latch = new Throwing(new NoSuchMethodError("I.b()Ljava/lang/String;")); + latch.tryGetOrNull("x"); + + assertFalse(latch.isLatched(Integer.valueOf(1))); + assertNull(latch.tryGetOrNull(Integer.valueOf(1))); + assertEquals(2, latch.calls.get()); + assertTrue(latch.isLatched(Integer.valueOf(1))); + } + + @Test + void abstractMethodStillLatchesOnlyWhenTheErrorNamesTheKey() throws Exception { + Throwing named = + new Throwing( + new AbstractMethodError( + "Receiver class " + + String.class.getName() + + " does not define or inherit an implementation of the resolved method")); + assertNull(named.tryGetOrNull("x")); + assertTrue(named.isLatched("x")); + + Throwing unnamed = new Throwing(new AbstractMethodError("something else entirely")); + assertNull(unnamed.tryGetOrNull("x")); + assertNull(unnamed.tryGetOrNull("x")); + assertFalse(unnamed.isLatched("x")); + assertEquals(2, unnamed.calls.get()); + } + + @Test + void unsupportedOperationIsSwallowedAndNeverLatched() throws Exception { + Throwing latch = new Throwing(new UnsupportedOperationException()); + + assertNull(latch.tryGetOrNull("x")); + assertNull(latch.tryGetOrNull("x")); + + assertEquals(2, latch.calls.get()); + assertFalse(latch.isLatched("x")); + } + + @Test + void otherFailuresPropagateWithoutLatching() { + Throwing checked = new Throwing(new SQLException("boom")); + assertThrows(SQLException.class, () -> checked.tryGetOrNull("x")); + assertFalse(checked.isLatched("x")); + + Throwing unchecked = new Throwing(new IllegalStateException()); + assertThrows(IllegalStateException.class, () -> unchecked.tryGetOrNull("x")); + assertFalse(unchecked.isLatched("x")); + } + + /** + * A real {@link NoSuchMethodError}: a caller built against an interface that has {@code b()}, run + * against one that does not. + */ + @Test + void latchesARealNoSuchMethodError(@TempDir Path dir) throws Exception { + JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); + assumeTrue(compiler != null, "needs a JDK"); + + Path oldDir = Files.createDirectory(dir.resolve("old")); + Path newDir = Files.createDirectory(dir.resolve("new")); + compile(compiler, oldDir, null, "I", "public interface I { String a(); String b(); }"); + compile( + compiler, + oldDir, + oldDir, + "Caller", + "public class Caller { public static String call(I i) { return i.b(); } }"); + compile(compiler, newDir, null, "I", "public interface I { String a(); }"); + compile( + compiler, + newDir, + newDir, + "Impl", + "public class Impl implements I { public String a() { return \"a\"; } }"); + + try (URLClassLoader loader = + new URLClassLoader( + new URL[] {newDir.toUri().toURL(), oldDir.toUri().toURL()}, + HandleNoSuchOrAbstractMethodTest.class.getClassLoader())) { + Class iface = loader.loadClass("I"); + Object impl = loader.loadClass("Impl").getDeclaredConstructor().newInstance(); + Method call = loader.loadClass("Caller").getMethod("call", iface); + + Handling latch = + new Handling() { + @Override + protected Object invoke(Object target) throws Exception { + try { + return call.invoke(null, target); + } catch (InvocationTargetException e) { + if (e.getCause() instanceof NoSuchMethodError) { + throw (NoSuchMethodError) e.getCause(); + } + throw e; + } + } + }; + + assertNull(latch.tryGetOrNull(impl)); + assertNull(latch.tryGetOrNull(impl)); + + assertEquals(1, latch.calls.get(), "second call should be skipped"); + assertTrue(latch.isLatched(impl)); + } + } + + private static void compile( + JavaCompiler compiler, Path out, Path classpath, String name, String source) + throws IOException { + Path file = out.resolve(name + ".java"); + Files.write(file, singletonList(source), StandardCharsets.UTF_8); + int result = + classpath == null + ? compiler.run(null, null, null, "-d", out.toString(), file.toString()) + : compiler.run( + null, + null, + null, + "-cp", + classpath.toString() + File.pathSeparator, + "-d", + out.toString(), + file.toString()); + assertEquals(0, result, "compiling " + name); + } +} From 424bbf36c05d912f9d7eae11ce5192810536fa48 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 30 Sep 2026 16:12:11 -0400 Subject: [PATCH 10/15] Name the JDBC latch constant CLIENT_INFO_LATCH Co-Authored-By: Claude Sonnet 5.5 --- .../datadog/trace/instrumentation/jdbc/JDBCDecorator.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java index f6d5fe0fd90..4ab4161cc44 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java @@ -50,7 +50,7 @@ public class JDBCDecorator extends DatabaseClientDecorator { private static final Logger log = LoggerFactory.getLogger(JDBCDecorator.class); /** Old drivers and pool proxies may not implement getClientInfo at all. */ - private static final ClassLatch CLIENT_INFO = + private static final ClassLatch CLIENT_INFO_LATCH = new ClassLatch() { @Override protected Properties get(Connection connection) throws SQLException { @@ -257,7 +257,7 @@ public static DBInfo parseDBInfoFromConnection(final Connection connection) { if (metaData != null && (url = metaData.getURL()) != null) { Properties clientInfo = null; try { - clientInfo = CLIENT_INFO.tryGetOrNull(connection); + clientInfo = CLIENT_INFO_LATCH.tryGetOrNull(connection); } catch (final SQLException ex) { // getClientInfo is not allowed, we can still extract info from the url alone log.debug(LogCollector.EXCLUDE_TELEMETRY, "Could not get client info from DB", ex); From d7153408a1a860a22d337e4bc0da6e2c8a9540b0 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 30 Sep 2026 17:10:10 -0400 Subject: [PATCH 11/15] 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 --- .../instrumentation/jdbc/JDBCDecorator.java | 2 +- .../trace/util/ClassLatchBenchmark.java | 8 ++++---- .../java/datadog/trace/util/ClassLatch.java | 20 +++++++++---------- .../datadog/trace/util/ClassLatchTest.java | 16 +++++++-------- .../trace/util/HandleAbstractMethodTest.java | 4 ++-- .../trace/util/HandleNoSuchMethodTest.java | 6 +++--- .../HandleNoSuchOrAbstractMethodTest.java | 4 ++-- 7 files changed, 30 insertions(+), 30 deletions(-) diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java index 4ab4161cc44..b80bcadf056 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java @@ -53,7 +53,7 @@ public class JDBCDecorator extends DatabaseClientDecorator { private static final ClassLatch CLIENT_INFO_LATCH = new ClassLatch() { @Override - protected Properties get(Connection connection) throws SQLException { + protected Properties handle(Connection connection) throws SQLException { return handleAbstractMethod(connection, Connection::getClientInfo); } }; diff --git a/internal-api/src/jmh/java/datadog/trace/util/ClassLatchBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/ClassLatchBenchmark.java index 69ea15a81d2..a57876fb87d 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/ClassLatchBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/ClassLatchBenchmark.java @@ -118,7 +118,7 @@ private static Object invokeHandle(Object target) { private static final ClassLatch MISSING = new ClassLatch() { @Override - protected Object get(Object target) { + protected Object handle(Object target) { return handleAbstractMethod(target, ClassLatchBenchmark::invokeHandle); } }; @@ -126,7 +126,7 @@ protected Object get(Object target) { private static final ClassLatch WRAPPER = new ClassLatch() { @Override - protected Object get(Object target) { + protected Object handle(Object target) { return handleAbstractMethod(target, ClassLatchBenchmark::invokeHandle); } }; @@ -134,7 +134,7 @@ protected Object get(Object target) { private static final ClassLatch PRESENT = new ClassLatch() { @Override - protected Object get(Object target) { + protected Object handle(Object target) { return handleAbstractMethod(target, ClassLatchBenchmark::invokeHandle); } }; @@ -144,7 +144,7 @@ private abstract static class SubclassStyle extends ClassLatchIntended as a {@code static final} anonymous subclass, one per call site and per operation: a * class lacking one method says nothing about another, so latches must not be shared. As a constant - * of a known exact type, the receiver lets the JIT inline {@link #get} and {@link #keyOf}. - * Subclasses decide what counts as a failure in their own {@code try/catch} inside {@link #get}, so - * checked exceptions and a tight {@code try} scope come for free, and latch through the protected - * helpers. Only the declaring subclass can change the state. + * of a known exact type, the receiver lets the JIT inline {@link #handle} and {@link #keyOf}. + * Subclasses decide what counts as a failure in their own {@code try/catch} inside {@link #handle}, + * so checked exceptions and a tight {@code try} scope come for free, and latch through the + * protected helpers. Only the declaring subclass can change the state. * *

{@link #keyOf} chooses the class the latch is keyed on, and is used by every operation, so the * check and the latch cannot disagree. The default is the target's own class. Never key on a @@ -31,7 +31,7 @@ * * @param the type of the value the operation is applied to * @param the type of the result - * @param the checked exception {@link #get} may throw + * @param the checked exception {@link #handle} may throw */ public abstract class ClassLatch { private static final String RECEIVER_PREFIX = "Receiver class "; @@ -55,7 +55,7 @@ protected boolean[] computeValue(Class type) { /** Performs the operation. Latch through the protected helpers when it failed for the class. */ @Nullable - protected abstract R get(T target) throws E; + protected abstract R handle(T target) throws E; /** The class the latch is keyed on. The target's own class unless overridden. */ protected Class keyOf(T target) { @@ -69,7 +69,7 @@ protected Class keyOf(T target) { */ @Nullable public final R tryGetOrNull(@Nullable T target) throws E { - return target == null || isLatched(target) ? null : get(target); + return target == null || isLatched(target) ? null : handle(target); } /** @@ -123,7 +123,7 @@ protected final boolean latchIfNamed(T target, AbstractMethodError error) { * Anything else, checked exceptions included, propagates unchanged. * * 

{@code
-   * protected Properties get(Connection c) throws SQLException {
+   * protected Properties handle(Connection c) throws SQLException {
    *   return handleAbstractMethod(c, Connection::getClientInfo);
    * }
    * }
@@ -167,7 +167,7 @@ protected final R handleAbstractMethod(T target, @Strategy ThrowingFunction{@code - * protected Properties get(Connection c) throws SQLException { + * protected Properties handle(Connection c) throws SQLException { * return handleNoSuchMethod(c, Connection::getClientInfo); * } * } @@ -193,7 +193,7 @@ protected final R handleNoSuchMethod(T target, @Strategy ThrowingFunction{@code - * protected Properties get(Connection c) throws SQLException { + * protected Properties handle(Connection c) throws SQLException { * return handleNoSuchOrAbstractMethod(c, Connection::getClientInfo); * } * } diff --git a/internal-api/src/test/java/datadog/trace/util/ClassLatchTest.java b/internal-api/src/test/java/datadog/trace/util/ClassLatchTest.java index 7af43b9505d..8a9aa2d8253 100644 --- a/internal-api/src/test/java/datadog/trace/util/ClassLatchTest.java +++ b/internal-api/src/test/java/datadog/trace/util/ClassLatchTest.java @@ -25,7 +25,7 @@ private static class Counting extends ClassLatch latch = new ClassLatch() { @Override - protected Boolean get(Object target) { + protected Boolean handle(Object target) { return false; } }; @@ -96,7 +96,7 @@ void aCallThatYieldsNothingAndASkippedCallAgree() { ClassLatch latch = new ClassLatch() { @Override - protected String get(Object target) { + protected String handle(Object target) { latch(target); return null; } @@ -119,7 +119,7 @@ final class Wrapper { ClassLatch latch = new ClassLatch() { @Override - protected String get(Wrapper target) { + protected String handle(Wrapper target) { latch(target); return "called"; } @@ -140,7 +140,7 @@ protected Class keyOf(Wrapper target) { /** A subclass may expose {@code unlatch}, for a policy that retries. */ private static final class Resumable extends ClassLatch { @Override - protected String get(Object target) { + protected String handle(Object target) { latch(target); return "called"; } @@ -171,7 +171,7 @@ void latchIfNamedLatchesOnlyWhenTheErrorNamesTheKey() { ClassLatch latch = new ClassLatch() { @Override - protected String get(Object target) { + protected String handle(Object target) { result[0] = latchIfNamed(target, receiverError(target.getClass())); return "named"; } @@ -183,7 +183,7 @@ protected String get(Object target) { ClassLatch other = new ClassLatch() { @Override - protected String get(Object target) { + protected String handle(Object target) { result[0] = latchIfNamed(target, receiverError(Integer.class)); return "other"; } @@ -229,7 +229,7 @@ void checkedExceptionsPropagateWithoutLatching() { ClassLatch latch = new ClassLatch() { @Override - protected String get(Object target) throws SQLException { + protected String handle(Object target) throws SQLException { throw new SQLException("boom"); } }; diff --git a/internal-api/src/test/java/datadog/trace/util/HandleAbstractMethodTest.java b/internal-api/src/test/java/datadog/trace/util/HandleAbstractMethodTest.java index 1e8a48e444c..f6bf4235978 100644 --- a/internal-api/src/test/java/datadog/trace/util/HandleAbstractMethodTest.java +++ b/internal-api/src/test/java/datadog/trace/util/HandleAbstractMethodTest.java @@ -27,12 +27,12 @@ class HandleAbstractMethodTest { - /** What a call site writes: {@code get} delegating to {@code handleAbstractMethod}. */ + /** What a call site writes: {@code handle} delegating to {@code handleAbstractMethod}. */ private abstract static class Handling extends ClassLatch { protected abstract R invoke(T target) throws E; @Override - protected final R get(T target) throws E { + protected final R handle(T target) throws E { return handleAbstractMethod(target, this::invoke); } } diff --git a/internal-api/src/test/java/datadog/trace/util/HandleNoSuchMethodTest.java b/internal-api/src/test/java/datadog/trace/util/HandleNoSuchMethodTest.java index 5b21e8fb405..1f2153f0bd4 100644 --- a/internal-api/src/test/java/datadog/trace/util/HandleNoSuchMethodTest.java +++ b/internal-api/src/test/java/datadog/trace/util/HandleNoSuchMethodTest.java @@ -12,7 +12,7 @@ class HandleNoSuchMethodTest { - /** What a call site writes: {@code get} delegating to {@code handleNoSuchMethod}. */ + /** What a call site writes: {@code handle} delegating to {@code handleNoSuchMethod}. */ private static final class Throwing extends ClassLatch { final AtomicInteger calls = new AtomicInteger(); final Throwable failure; @@ -22,7 +22,7 @@ private static final class Throwing extends ClassLatch { @@ -40,7 +40,7 @@ void returnsTheResultAndLatchesNothing() throws Exception { ClassLatch latch = new ClassLatch() { @Override - protected String get(Object target) { + protected String handle(Object target) { return handleNoSuchMethod(target, t -> "ok"); } }; diff --git a/internal-api/src/test/java/datadog/trace/util/HandleNoSuchOrAbstractMethodTest.java b/internal-api/src/test/java/datadog/trace/util/HandleNoSuchOrAbstractMethodTest.java index 1be4e1f317b..5c700fdfde0 100644 --- a/internal-api/src/test/java/datadog/trace/util/HandleNoSuchOrAbstractMethodTest.java +++ b/internal-api/src/test/java/datadog/trace/util/HandleNoSuchOrAbstractMethodTest.java @@ -26,14 +26,14 @@ class HandleNoSuchOrAbstractMethodTest { - /** What a call site writes: {@code get} delegating to {@code handleNoSuchOrAbstractMethod}. */ + /** What a call site writes: {@code handle} delegating to {@code handleNoSuchOrAbstractMethod}. */ private abstract static class Handling extends ClassLatch { final AtomicInteger calls = new AtomicInteger(); protected abstract R invoke(T target) throws E; @Override - protected final R get(T target) throws E { + protected final R handle(T target) throws E { return handleNoSuchOrAbstractMethod( target, t -> { From 15102808751015529810f424f0de6019d7c9e836 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 30 Sep 2026 21:36:45 -0400 Subject: [PATCH 12/15] 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 --- .../trace/util/ClassLatchBenchmark.java | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/internal-api/src/jmh/java/datadog/trace/util/ClassLatchBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/ClassLatchBenchmark.java index a57876fb87d..87ce6925c7d 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/ClassLatchBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/ClassLatchBenchmark.java @@ -85,6 +85,41 @@ * about 36M and 32M. So the mean and its error are two modes averaged, and the ranking of the two * forms at depth 50 is not established. Both modes (roughly 85 ns and 32 ns) are far below the * status quo of about 6 us. The cause was not investigated. + * + *

Results, one run: Zulu 17.0.7 (HotSpot), MacBook M1, single thread, 5 forks, on a laptop with + * normal background activity (load about 4). JDK 8 and x86 are not measured. {@code latched} uses + * {@code handleAbstractMethod} with a method reference; {@code subclass} is a benchmark-local + * dedicated subclass, for comparison. + * + * 

+ * Benchmark                (depth)          ops/s     ns/op    err   B/op
+ * unguardedMissing               0        227,738    4391.0   0.4%    896
+ * latchedWrapperMissing          0        232,316    4304.5   0.5%    896
+ * latchedMissing                 0    205,379,219      4.87   0.6%      0
+ * subclassMissing                0    205,736,674      4.86   0.6%      0
+ * unguardedPresent               0    224,381,889      4.46   0.4%      0
+ * latchedPresent                 0    209,902,233      4.76   0.3%      0
+ * subclassPresent                0    211,697,167      4.72   0.1%      0
+ *
+ * unguardedMissing              50        165,183    6053.9   1.2%   2256
+ * latchedWrapperMissing         50        171,300    5837.7   0.5%   2256
+ * latchedMissing                50     11,741,580      85.2   0.8%      0
+ * subclassMissing               50     11,842,809      84.4   0.3%      0
+ * unguardedPresent              50     35,092,870      28.5   3.4%      0
+ * latchedPresent                50     35,079,191      28.5   3.3%      0
+ * subclassPresent               50     36,004,724      27.8   3.0%      0
+ *
+ * + * A latched class costs about 4.9 ns where the status quo costs about 4.4 us (6.1 us at depth 50), + * and allocates nothing where the status quo allocates 896 B (2,256 B): roughly 900 times cheaper + * at depth 0 and 70 times at depth 50. A wrapper, which cannot be latched, performs like the status + * quo. The method-reference form and the dedicated subclass are indistinguishable (within 1%). The + * latch adds about 0.3 ns on the working path at depth 0 and nothing measurable at depth 50. + * + *

Unexplained: at depth 50 a latched skip (about 85 ns) is slower than a call that runs (about + * 28.5 ns), though at depth 0 the skip is as cheap as expected. All five forks agreed (11.7M to + * 11.9M ops/s per fork for the latched arm), so it is not noise. It may be a JIT effect of the + * benchmark's recursion rather than a property of the latch, but that was not tested. */ @Fork(2) @Warmup(iterations = 3) From b630167985236fe832e27faba002d2cbbacb220d Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 30 Sep 2026 23:11:47 -0400 Subject: [PATCH 13/15] 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 --- .../instrumentation/jdbc/JDBCDecorator.java | 4 +- .../trace/util/ClassLatchBenchmark.java | 24 ++++---- .../java/datadog/trace/util/ClassLatch.java | 24 ++++---- .../datadog/trace/util/ClassLatchTest.java | 60 +++++++++---------- .../trace/util/HandleAbstractMethodTest.java | 34 +++++------ .../trace/util/HandleNoSuchMethodTest.java | 18 +++--- .../HandleNoSuchOrAbstractMethodTest.java | 32 +++++----- 7 files changed, 98 insertions(+), 98 deletions(-) diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java index b80bcadf056..100823a5a4e 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java @@ -53,7 +53,7 @@ public class JDBCDecorator extends DatabaseClientDecorator { private static final ClassLatch CLIENT_INFO_LATCH = new ClassLatch() { @Override - protected Properties handle(Connection connection) throws SQLException { + protected Properties apply(Connection connection) throws SQLException { return handleAbstractMethod(connection, Connection::getClientInfo); } }; @@ -257,7 +257,7 @@ public static DBInfo parseDBInfoFromConnection(final Connection connection) { if (metaData != null && (url = metaData.getURL()) != null) { Properties clientInfo = null; try { - clientInfo = CLIENT_INFO_LATCH.tryGetOrNull(connection); + clientInfo = CLIENT_INFO_LATCH.tryApplyOrNull(connection); } catch (final SQLException ex) { // getClientInfo is not allowed, we can still extract info from the url alone log.debug(LogCollector.EXCLUDE_TELEMETRY, "Could not get client info from DB", ex); diff --git a/internal-api/src/jmh/java/datadog/trace/util/ClassLatchBenchmark.java b/internal-api/src/jmh/java/datadog/trace/util/ClassLatchBenchmark.java index 87ce6925c7d..951e957e93d 100644 --- a/internal-api/src/jmh/java/datadog/trace/util/ClassLatchBenchmark.java +++ b/internal-api/src/jmh/java/datadog/trace/util/ClassLatchBenchmark.java @@ -153,7 +153,7 @@ private static Object invokeHandle(Object target) { private static final ClassLatch MISSING = new ClassLatch() { @Override - protected Object handle(Object target) { + protected Object apply(Object target) { return handleAbstractMethod(target, ClassLatchBenchmark::invokeHandle); } }; @@ -161,7 +161,7 @@ protected Object handle(Object target) { private static final ClassLatch WRAPPER = new ClassLatch() { @Override - protected Object handle(Object target) { + protected Object apply(Object target) { return handleAbstractMethod(target, ClassLatchBenchmark::invokeHandle); } }; @@ -169,7 +169,7 @@ protected Object handle(Object target) { private static final ClassLatch PRESENT = new ClassLatch() { @Override - protected Object handle(Object target) { + protected Object apply(Object target) { return handleAbstractMethod(target, ClassLatchBenchmark::invokeHandle); } }; @@ -179,7 +179,7 @@ private abstract static class SubclassStyle extends ClassLatch 0 ? latchedMissing(remaining - 1) : MISSING.tryGetOrNull(impl); + return remaining > 0 ? latchedMissing(remaining - 1) : MISSING.tryApplyOrNull(impl); } private Object latchedWrapperMissing(int remaining) { - return remaining > 0 ? latchedWrapperMissing(remaining - 1) : WRAPPER.tryGetOrNull(wrapper); + return remaining > 0 ? latchedWrapperMissing(remaining - 1) : WRAPPER.tryApplyOrNull(wrapper); } private Object unguardedPresent(int remaining) { @@ -347,15 +347,15 @@ private Object unguardedPresent(int remaining) { } private Object latchedPresent(int remaining) { - return remaining > 0 ? latchedPresent(remaining - 1) : PRESENT.tryGetOrNull(full); + return remaining > 0 ? latchedPresent(remaining - 1) : PRESENT.tryApplyOrNull(full); } private Object subclassMissing(int remaining) { - return remaining > 0 ? subclassMissing(remaining - 1) : SUBCLASS_MISSING.tryGetOrNull(impl); + return remaining > 0 ? subclassMissing(remaining - 1) : SUBCLASS_MISSING.tryApplyOrNull(impl); } private Object subclassPresent(int remaining) { - return remaining > 0 ? subclassPresent(remaining - 1) : SUBCLASS_PRESENT.tryGetOrNull(full); + return remaining > 0 ? subclassPresent(remaining - 1) : SUBCLASS_PRESENT.tryApplyOrNull(full); } private static void compile( diff --git a/internal-api/src/main/java/datadog/trace/util/ClassLatch.java b/internal-api/src/main/java/datadog/trace/util/ClassLatch.java index 1b4137b1d77..ce20efd869b 100644 --- a/internal-api/src/main/java/datadog/trace/util/ClassLatch.java +++ b/internal-api/src/main/java/datadog/trace/util/ClassLatch.java @@ -13,8 +13,8 @@ * *

Intended as a {@code static final} anonymous subclass, one per call site and per operation: a * class lacking one method says nothing about another, so latches must not be shared. As a constant - * of a known exact type, the receiver lets the JIT inline {@link #handle} and {@link #keyOf}. - * Subclasses decide what counts as a failure in their own {@code try/catch} inside {@link #handle}, + * of a known exact type, the receiver lets the JIT inline {@link #apply} and {@link #keyOf}. + * Subclasses decide what counts as a failure in their own {@code try/catch} inside {@link #apply}, * so checked exceptions and a tight {@code try} scope come for free, and latch through the * protected helpers. Only the declaring subclass can change the state. * @@ -31,7 +31,7 @@ * * @param the type of the value the operation is applied to * @param the type of the result - * @param the checked exception {@link #handle} may throw + * @param the checked exception {@link #apply} may throw */ public abstract class ClassLatch { private static final String RECEIVER_PREFIX = "Receiver class "; @@ -55,7 +55,7 @@ protected boolean[] computeValue(Class type) { /** Performs the operation. Latch through the protected helpers when it failed for the class. */ @Nullable - protected abstract R handle(T target) throws E; + protected abstract R apply(T target) throws E; /** The class the latch is keyed on. The target's own class unless overridden. */ protected Class keyOf(T target) { @@ -68,17 +68,17 @@ protected Class keyOf(T target) { * it produced no value. */ @Nullable - public final R tryGetOrNull(@Nullable T target) throws E { - return target == null || isLatched(target) ? null : handle(target); + public final R tryApplyOrNull(@Nullable T target) throws E { + return target == null || isLatched(target) ? null : apply(target); } /** - * Like {@link #tryGetOrNull}, but returns {@code fallback} when there is nothing available. The + * Like {@link #tryApplyOrNull}, but returns {@code fallback} when there is nothing available. The * fallback is also used when the operation itself produced {@code null}, so a call and a skipped * call always agree. */ - public final R tryGetOrDefault(@Nullable T target, R fallback) throws E { - final R result = tryGetOrNull(target); + public final R tryApplyOrDefault(@Nullable T target, R fallback) throws E { + final R result = tryApplyOrNull(target); return result != null ? result : fallback; } @@ -123,7 +123,7 @@ protected final boolean latchIfNamed(T target, AbstractMethodError error) { * Anything else, checked exceptions included, propagates unchanged. * * 

{@code
-   * protected Properties handle(Connection c) throws SQLException {
+   * protected Properties apply(Connection c) throws SQLException {
    *   return handleAbstractMethod(c, Connection::getClientInfo);
    * }
    * }
@@ -167,7 +167,7 @@ protected final R handleAbstractMethod(T target, @Strategy ThrowingFunction{@code - * protected Properties handle(Connection c) throws SQLException { + * protected Properties apply(Connection c) throws SQLException { * return handleNoSuchMethod(c, Connection::getClientInfo); * } * } @@ -193,7 +193,7 @@ protected final R handleNoSuchMethod(T target, @Strategy ThrowingFunction{@code - * protected Properties handle(Connection c) throws SQLException { + * protected Properties apply(Connection c) throws SQLException { * return handleNoSuchOrAbstractMethod(c, Connection::getClientInfo); * } * } diff --git a/internal-api/src/test/java/datadog/trace/util/ClassLatchTest.java b/internal-api/src/test/java/datadog/trace/util/ClassLatchTest.java index 8a9aa2d8253..2058e450293 100644 --- a/internal-api/src/test/java/datadog/trace/util/ClassLatchTest.java +++ b/internal-api/src/test/java/datadog/trace/util/ClassLatchTest.java @@ -25,7 +25,7 @@ private static class Counting extends ClassLatch latch = new ClassLatch() { @Override - protected Boolean handle(Object target) { + protected Boolean apply(Object target) { return false; } }; // a real false must not be replaced by the fallback - assertEquals(false, latch.tryGetOrDefault("x", Boolean.TRUE)); + assertEquals(false, latch.tryApplyOrDefault("x", Boolean.TRUE)); } @Test - void tryGetOrDefaultReturnsTheFallbackWhenSkippedOrNull() { + void tryApplyOrDefaultReturnsTheFallbackWhenSkippedOrNull() { Counting latch = new Counting(); // the first call latches and yields a value; later calls are skipped - assertEquals("failed", latch.tryGetOrDefault("x", "fallback")); - assertEquals("fallback", latch.tryGetOrDefault("x", "fallback")); - assertEquals("fallback", latch.tryGetOrDefault(null, "fallback")); + assertEquals("failed", latch.tryApplyOrDefault("x", "fallback")); + assertEquals("fallback", latch.tryApplyOrDefault("x", "fallback")); + assertEquals("fallback", latch.tryApplyOrDefault(null, "fallback")); } @Test @@ -96,14 +96,14 @@ void aCallThatYieldsNothingAndASkippedCallAgree() { ClassLatch latch = new ClassLatch() { @Override - protected String handle(Object target) { + protected String apply(Object target) { latch(target); return null; } }; - assertEquals("fallback", latch.tryGetOrDefault("x", "fallback")); - assertEquals("fallback", latch.tryGetOrDefault("x", "fallback")); + assertEquals("fallback", latch.tryApplyOrDefault("x", "fallback")); + assertEquals("fallback", latch.tryApplyOrDefault("x", "fallback")); } @Test @@ -119,7 +119,7 @@ final class Wrapper { ClassLatch latch = new ClassLatch() { @Override - protected String handle(Wrapper target) { + protected String apply(Wrapper target) { latch(target); return "called"; } @@ -130,17 +130,17 @@ protected Class keyOf(Wrapper target) { } }; - assertEquals("called", latch.tryGetOrNull(new Wrapper("x"))); + assertEquals("called", latch.tryApplyOrNull(new Wrapper("x"))); assertTrue(latch.isLatched(new Wrapper("another string"))); assertFalse(latch.isLatched(new Wrapper(Integer.valueOf(1)))); - assertEquals("called", latch.tryGetOrNull(new Wrapper(Integer.valueOf(1)))); + assertEquals("called", latch.tryApplyOrNull(new Wrapper(Integer.valueOf(1)))); } /** A subclass may expose {@code unlatch}, for a policy that retries. */ private static final class Resumable extends ClassLatch { @Override - protected String handle(Object target) { + protected String apply(Object target) { latch(target); return "called"; } @@ -153,8 +153,8 @@ void resume(Object target) { @Test void unlatchResumesForThatKeyOnly() { Resumable latch = new Resumable(); - latch.tryGetOrNull("x"); - latch.tryGetOrNull(Integer.valueOf(1)); + latch.tryApplyOrNull("x"); + latch.tryApplyOrNull(Integer.valueOf(1)); assertTrue(latch.isLatched("x")); assertTrue(latch.isLatched(Integer.valueOf(1))); @@ -162,7 +162,7 @@ void unlatchResumesForThatKeyOnly() { assertFalse(latch.isLatched("x")); assertTrue(latch.isLatched(Integer.valueOf(1))); - assertEquals("called", latch.tryGetOrNull("x")); + assertEquals("called", latch.tryApplyOrNull("x")); } @Test @@ -171,24 +171,24 @@ void latchIfNamedLatchesOnlyWhenTheErrorNamesTheKey() { ClassLatch latch = new ClassLatch() { @Override - protected String handle(Object target) { + protected String apply(Object target) { result[0] = latchIfNamed(target, receiverError(target.getClass())); return "named"; } }; - latch.tryGetOrNull("x"); + latch.tryApplyOrNull("x"); assertTrue(result[0]); assertTrue(latch.isLatched("x")); ClassLatch other = new ClassLatch() { @Override - protected String handle(Object target) { + protected String apply(Object target) { result[0] = latchIfNamed(target, receiverError(Integer.class)); return "other"; } }; - other.tryGetOrNull("x"); + other.tryApplyOrNull("x"); assertFalse(result[0]); assertFalse(other.isLatched("x")); } @@ -229,12 +229,12 @@ void checkedExceptionsPropagateWithoutLatching() { ClassLatch latch = new ClassLatch() { @Override - protected String handle(Object target) throws SQLException { + protected String apply(Object target) throws SQLException { throw new SQLException("boom"); } }; - assertThrows(SQLException.class, () -> latch.tryGetOrNull("x")); + assertThrows(SQLException.class, () -> latch.tryApplyOrNull("x")); assertFalse(latch.isLatched("x")); } } diff --git a/internal-api/src/test/java/datadog/trace/util/HandleAbstractMethodTest.java b/internal-api/src/test/java/datadog/trace/util/HandleAbstractMethodTest.java index f6bf4235978..d394f254aa6 100644 --- a/internal-api/src/test/java/datadog/trace/util/HandleAbstractMethodTest.java +++ b/internal-api/src/test/java/datadog/trace/util/HandleAbstractMethodTest.java @@ -27,12 +27,12 @@ class HandleAbstractMethodTest { - /** What a call site writes: {@code handle} delegating to {@code handleAbstractMethod}. */ + /** What a call site writes: {@code apply} delegating to {@code handleAbstractMethod}. */ private abstract static class Handling extends ClassLatch { protected abstract R invoke(T target) throws E; @Override - protected final R handle(T target) throws E { + protected final R apply(T target) throws E { return handleAbstractMethod(target, this::invoke); } } @@ -77,7 +77,7 @@ protected String invoke(Object target) { } }; - assertEquals("ok", latch.tryGetOrNull("x")); + assertEquals("ok", latch.tryApplyOrNull("x")); assertFalse(latch.isLatched("x")); } @@ -85,9 +85,9 @@ protected String invoke(Object target) { void latchesTheReceiverClassAndStopsCalling() throws Exception { Throwing latch = new Throwing(new AbstractMethodError()); - assertNull(latch.tryGetOrNull("x")); - assertNull(latch.tryGetOrNull("y")); - assertNull(latch.tryGetOrNull("z")); + assertNull(latch.tryApplyOrNull("x")); + assertNull(latch.tryApplyOrNull("y")); + assertNull(latch.tryApplyOrNull("z")); assertEquals(1, latch.calls.get()); assertTrue(latch.isLatched("x")); @@ -96,10 +96,10 @@ void latchesTheReceiverClassAndStopsCalling() throws Exception { @Test void otherClassesAreUnaffectedByALatch() throws Exception { Throwing latch = new Throwing(new AbstractMethodError()); - latch.tryGetOrNull("x"); + latch.tryApplyOrNull("x"); assertFalse(latch.isLatched(Integer.valueOf(1))); - assertNull(latch.tryGetOrNull(Integer.valueOf(1))); + assertNull(latch.tryApplyOrNull(Integer.valueOf(1))); assertEquals(2, latch.calls.get()); } @@ -116,8 +116,8 @@ protected String invoke(Object target) { } }; - assertNull(latch.tryGetOrNull("x")); - assertNull(latch.tryGetOrNull("x")); + assertNull(latch.tryApplyOrNull("x")); + assertNull(latch.tryApplyOrNull("x")); assertEquals(2, calls.get()); assertFalse(latch.isLatched("x")); @@ -139,7 +139,7 @@ protected String invoke(Object target) { } }; - assertNull(latch.tryGetOrNull("x")); + assertNull(latch.tryApplyOrNull("x")); assertFalse(latch.isLatched("x")); } assertEquals(2, calls.get()); @@ -149,8 +149,8 @@ protected String invoke(Object target) { void unsupportedOperationYieldsTheDefaultOnEveryCallAndIsNeverLatched() throws Exception { Throwing latch = new Throwing(new UnsupportedOperationException()); - assertNull(latch.tryGetOrNull("x")); - assertNull(latch.tryGetOrNull("x")); + assertNull(latch.tryApplyOrNull("x")); + assertNull(latch.tryApplyOrNull("x")); assertEquals(2, latch.calls.get()); assertFalse(latch.isLatched("x")); @@ -161,7 +161,7 @@ void checkedExceptionsPropagateAndDoNotLatch() { SQLException failure = new SQLException("boom"); Throwing latch = new Throwing(failure); - SQLException thrown = assertThrows(SQLException.class, () -> latch.tryGetOrNull("x")); + SQLException thrown = assertThrows(SQLException.class, () -> latch.tryApplyOrNull("x")); assertSame(failure, thrown); assertFalse(latch.isLatched("x")); @@ -171,7 +171,7 @@ void checkedExceptionsPropagateAndDoNotLatch() { void otherUncheckedExceptionsPropagateAndDoNotLatch() { Throwing latch = new Throwing(new IllegalStateException()); - assertThrows(IllegalStateException.class, () -> latch.tryGetOrNull("x")); + assertThrows(IllegalStateException.class, () -> latch.tryApplyOrNull("x")); assertFalse(latch.isLatched("x")); } @@ -228,8 +228,8 @@ protected Object invoke(Object target) throws Exception { } }; - assertNull(latch.tryGetOrNull(impl)); - assertNull(latch.tryGetOrNull(impl)); + assertNull(latch.tryApplyOrNull(impl)); + assertNull(latch.tryApplyOrNull(impl)); assertEquals(1, calls.get(), "second call should be skipped"); assertTrue(latch.isLatched(impl)); diff --git a/internal-api/src/test/java/datadog/trace/util/HandleNoSuchMethodTest.java b/internal-api/src/test/java/datadog/trace/util/HandleNoSuchMethodTest.java index 1f2153f0bd4..5ade6d31012 100644 --- a/internal-api/src/test/java/datadog/trace/util/HandleNoSuchMethodTest.java +++ b/internal-api/src/test/java/datadog/trace/util/HandleNoSuchMethodTest.java @@ -12,7 +12,7 @@ class HandleNoSuchMethodTest { - /** What a call site writes: {@code handle} delegating to {@code handleNoSuchMethod}. */ + /** What a call site writes: {@code apply} delegating to {@code handleNoSuchMethod}. */ private static final class Throwing extends ClassLatch { final AtomicInteger calls = new AtomicInteger(); final Throwable failure; @@ -22,7 +22,7 @@ private static final class Throwing extends ClassLatch { @@ -40,12 +40,12 @@ void returnsTheResultAndLatchesNothing() throws Exception { ClassLatch latch = new ClassLatch() { @Override - protected String handle(Object target) { + protected String apply(Object target) { return handleNoSuchMethod(target, t -> "ok"); } }; - assertEquals("ok", latch.tryGetOrNull("x")); + assertEquals("ok", latch.tryApplyOrNull("x")); assertFalse(latch.isLatched("x")); } @@ -53,8 +53,8 @@ protected String handle(Object target) { void noSuchMethodYieldsNullAndLatchesTheTargetsClass() throws Exception { Throwing latch = new Throwing(new NoSuchMethodError("I.b()Ljava/lang/String;")); - assertNull(latch.tryGetOrNull("x")); - assertNull(latch.tryGetOrNull("y")); + assertNull(latch.tryApplyOrNull("x")); + assertNull(latch.tryApplyOrNull("y")); assertEquals(1, latch.calls.get(), "later calls should be skipped"); assertTrue(latch.isLatched("x")); @@ -65,18 +65,18 @@ void noSuchMethodYieldsNullAndLatchesTheTargetsClass() throws Exception { @Test void doesNotHandleAbstractMethodErrorOrUnsupportedOperation() { Throwing abstractMethod = new Throwing(new AbstractMethodError("Impl.b()V")); - assertThrows(AbstractMethodError.class, () -> abstractMethod.tryGetOrNull("x")); + assertThrows(AbstractMethodError.class, () -> abstractMethod.tryApplyOrNull("x")); assertFalse(abstractMethod.isLatched("x")); Throwing unsupported = new Throwing(new UnsupportedOperationException()); - assertThrows(UnsupportedOperationException.class, () -> unsupported.tryGetOrNull("x")); + assertThrows(UnsupportedOperationException.class, () -> unsupported.tryApplyOrNull("x")); assertFalse(unsupported.isLatched("x")); } @Test void otherFailuresPropagateWithoutLatching() { Throwing checked = new Throwing(new SQLException("boom")); - assertThrows(SQLException.class, () -> checked.tryGetOrNull("x")); + assertThrows(SQLException.class, () -> checked.tryApplyOrNull("x")); assertFalse(checked.isLatched("x")); } } diff --git a/internal-api/src/test/java/datadog/trace/util/HandleNoSuchOrAbstractMethodTest.java b/internal-api/src/test/java/datadog/trace/util/HandleNoSuchOrAbstractMethodTest.java index 5c700fdfde0..743e1e796bf 100644 --- a/internal-api/src/test/java/datadog/trace/util/HandleNoSuchOrAbstractMethodTest.java +++ b/internal-api/src/test/java/datadog/trace/util/HandleNoSuchOrAbstractMethodTest.java @@ -26,14 +26,14 @@ class HandleNoSuchOrAbstractMethodTest { - /** What a call site writes: {@code handle} delegating to {@code handleNoSuchOrAbstractMethod}. */ + /** What a call site writes: {@code apply} delegating to {@code handleNoSuchOrAbstractMethod}. */ private abstract static class Handling extends ClassLatch { final AtomicInteger calls = new AtomicInteger(); protected abstract R invoke(T target) throws E; @Override - protected final R handle(T target) throws E { + protected final R apply(T target) throws E { return handleNoSuchOrAbstractMethod( target, t -> { @@ -63,9 +63,9 @@ protected String invoke(Object target) throws Exception { void noSuchMethodYieldsNullAndLatchesTheTargetsClass() throws Exception { Throwing latch = new Throwing(new NoSuchMethodError("I.b()Ljava/lang/String;")); - assertNull(latch.tryGetOrNull("x")); - assertNull(latch.tryGetOrNull("y")); - assertNull(latch.tryGetOrNull("z")); + assertNull(latch.tryApplyOrNull("x")); + assertNull(latch.tryApplyOrNull("y")); + assertNull(latch.tryApplyOrNull("z")); assertEquals(1, latch.calls.get(), "later calls should be skipped"); assertTrue(latch.isLatched("x")); @@ -76,10 +76,10 @@ void noSuchMethodNeverLatchesTheWholeSite() throws Exception { // the message names the declared type, not the receiver, so it cannot be attributed to a class; // latching only the target's key means another class still gets its own attempt Throwing latch = new Throwing(new NoSuchMethodError("I.b()Ljava/lang/String;")); - latch.tryGetOrNull("x"); + latch.tryApplyOrNull("x"); assertFalse(latch.isLatched(Integer.valueOf(1))); - assertNull(latch.tryGetOrNull(Integer.valueOf(1))); + assertNull(latch.tryApplyOrNull(Integer.valueOf(1))); assertEquals(2, latch.calls.get()); assertTrue(latch.isLatched(Integer.valueOf(1))); } @@ -92,12 +92,12 @@ void abstractMethodStillLatchesOnlyWhenTheErrorNamesTheKey() throws Exception { "Receiver class " + String.class.getName() + " does not define or inherit an implementation of the resolved method")); - assertNull(named.tryGetOrNull("x")); + assertNull(named.tryApplyOrNull("x")); assertTrue(named.isLatched("x")); Throwing unnamed = new Throwing(new AbstractMethodError("something else entirely")); - assertNull(unnamed.tryGetOrNull("x")); - assertNull(unnamed.tryGetOrNull("x")); + assertNull(unnamed.tryApplyOrNull("x")); + assertNull(unnamed.tryApplyOrNull("x")); assertFalse(unnamed.isLatched("x")); assertEquals(2, unnamed.calls.get()); } @@ -106,8 +106,8 @@ void abstractMethodStillLatchesOnlyWhenTheErrorNamesTheKey() throws Exception { void unsupportedOperationIsSwallowedAndNeverLatched() throws Exception { Throwing latch = new Throwing(new UnsupportedOperationException()); - assertNull(latch.tryGetOrNull("x")); - assertNull(latch.tryGetOrNull("x")); + assertNull(latch.tryApplyOrNull("x")); + assertNull(latch.tryApplyOrNull("x")); assertEquals(2, latch.calls.get()); assertFalse(latch.isLatched("x")); @@ -116,11 +116,11 @@ void unsupportedOperationIsSwallowedAndNeverLatched() throws Exception { @Test void otherFailuresPropagateWithoutLatching() { Throwing checked = new Throwing(new SQLException("boom")); - assertThrows(SQLException.class, () -> checked.tryGetOrNull("x")); + assertThrows(SQLException.class, () -> checked.tryApplyOrNull("x")); assertFalse(checked.isLatched("x")); Throwing unchecked = new Throwing(new IllegalStateException()); - assertThrows(IllegalStateException.class, () -> unchecked.tryGetOrNull("x")); + assertThrows(IllegalStateException.class, () -> unchecked.tryApplyOrNull("x")); assertFalse(unchecked.isLatched("x")); } @@ -173,8 +173,8 @@ protected Object invoke(Object target) throws Exception { } }; - assertNull(latch.tryGetOrNull(impl)); - assertNull(latch.tryGetOrNull(impl)); + assertNull(latch.tryApplyOrNull(impl)); + assertNull(latch.tryApplyOrNull(impl)); assertEquals(1, latch.calls.get(), "second call should be skipped"); assertTrue(latch.isLatched(impl)); From ff0fca706b902b7034b5f02a5b5459b82a40bcbb Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 30 Sep 2026 23:13:38 -0400 Subject: [PATCH 14/15] Remove a stray blank line in JDBCDecorator Co-Authored-By: Claude Sonnet 5.5 --- .../java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java | 1 - 1 file changed, 1 deletion(-) diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java index 100823a5a4e..d1baf7f883a 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java @@ -61,7 +61,6 @@ protected Properties apply(Connection connection) throws SQLException { public static final JDBCDecorator DECORATE = new JDBCDecorator(); public static final CharSequence JAVA_JDBC = UTF8BytesString.create("java-jdbc"); public static final CharSequence DATABASE_QUERY = UTF8BytesString.create("database.query"); - private static final UTF8BytesString DB_QUERY = UTF8BytesString.create("DB Query"); private static final UTF8BytesString JDBC_STATEMENT = UTF8BytesString.create("java-jdbc-statement"); From d176106f76fcefca2218645113b3b108e13bbeb6 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 30 Sep 2026 23:55:34 -0400 Subject: [PATCH 15/15] 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 --- .../instrumentation/jdbc/JDBCDecorator.java | 5 ++-- .../jdbc/ParseDBInfoClientInfoTest.java | 29 ++++++++++++------- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java index d1baf7f883a..ce32bf98b1e 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java @@ -257,8 +257,9 @@ public static DBInfo parseDBInfoFromConnection(final Connection connection) { Properties clientInfo = null; try { clientInfo = CLIENT_INFO_LATCH.tryApplyOrNull(connection); - } catch (final SQLException ex) { - // getClientInfo is not allowed, we can still extract info from the url alone + } catch (final Throwable ex) { + // getClientInfo can fail in many ways (old drivers, pool proxies, test doubles), and we + // can still extract info from the url alone log.debug(LogCollector.EXCLUDE_TELEMETRY, "Could not get client info from DB", ex); } dbInfo = JDBCConnectionUrlParser.extractDBInfo(url, clientInfo); diff --git a/dd-java-agent/instrumentation/jdbc/src/test/java/datadog/trace/instrumentation/jdbc/ParseDBInfoClientInfoTest.java b/dd-java-agent/instrumentation/jdbc/src/test/java/datadog/trace/instrumentation/jdbc/ParseDBInfoClientInfoTest.java index 3d3126fe1c7..bbf0c6a88e7 100644 --- a/dd-java-agent/instrumentation/jdbc/src/test/java/datadog/trace/instrumentation/jdbc/ParseDBInfoClientInfoTest.java +++ b/dd-java-agent/instrumentation/jdbc/src/test/java/datadog/trace/instrumentation/jdbc/ParseDBInfoClientInfoTest.java @@ -1,7 +1,6 @@ package datadog.trace.instrumentation.jdbc; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertSame; import datadog.trace.bootstrap.instrumentation.jdbc.DBInfo; import java.lang.reflect.InvocationHandler; @@ -99,17 +98,25 @@ void urlIsStillParsedWhenGetClientInfoIsMissing() { } @Test - void unexpectedFailuresFallBackToDefaultInsteadOfBeingSwallowed() { + void anyOtherFailureStillYieldsUrlBasedDbInfo() { AtomicInteger calls = new AtomicInteger(); - Connection connection = - connection( - calls, - () -> { - throw new IllegalStateException("unexpected"); - }); - - // not one of the expected "unsupported" shapes, so it reaches the outer handler - assertSame(DBInfo.DEFAULT, JDBCDecorator.parseDBInfoFromConnection(connection)); + for (Throwable failure : + new Throwable[] { + new IllegalStateException("unexpected"), new Throwable("not even an Exception") + }) { + Connection connection = + connection( + calls, + () -> { + throw failure; + }); + + // getClientInfo can fail in any way; the URL alone is still enough for the DB info + DBInfo info = JDBCDecorator.parseDBInfoFromConnection(connection); + + assertEquals("postgresql", info.getType()); + assertEquals("orders", info.getDb()); + } } @Test