From 5b55fe30f9ba34b81a96e04a23052d4b1d1210e4 Mon Sep 17 00:00:00 2001 From: Jordan Wong Date: Tue, 1 Sep 2026 06:03:11 -0400 Subject: [PATCH 1/3] skill(apm-integrations): check for an interception/listener SPI before hand-writing method advice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a general heuristic to instrumenter-module.md: before instrumenting a client library via manual method advice, check whether the library (or an official companion, e.g. r2dbc-proxy for R2DBC, ClientInterceptor for gRPC, Producer/ConsumerInterceptor for Kafka) exposes a first-class interception/listener SPI, and prefer it. Rationale, aimed at reactive/async clients: a listener SPI delivers one well-defined start/end/error/cancel callback per operation with metadata already assembled, whereas hand-written advice on a reactive method must re-implement the lifecycle by wrapping the returned Publisher/Mono/Flux — a wrapper that finishes the span only on onComplete/onError leaks every cancelled span (reactive pipelines routinely cancel: take(1), timeouts, DiscardOnCancel), so spans are created but never finished/exported, and it's easy to wrap at the wrong granularity. Notes the no-forced-dependency approach: declare the interception library compileOnly and inject the listener via helperClassNames() (or shade it), so it does not land on the user's runtime classpath. Cross-links the existing R2DBC connection-metadata guidance to this section. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../references/instrumenter-module.md | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/.agents/skills/apm-integrations/references/instrumenter-module.md b/.agents/skills/apm-integrations/references/instrumenter-module.md index 48059ed8f1d..f1c8b38e5d6 100644 --- a/.agents/skills/apm-integrations/references/instrumenter-module.md +++ b/.agents/skills/apm-integrations/references/instrumenter-module.md @@ -41,6 +41,26 @@ ✅ `public String[] triggerClasses() { return new String[]{"com.example.Foo"}; }` +### Before hand-writing method advice, check for an official interception SPI + +Before instrumenting a client library by matching and advising its methods directly, check whether the library — or an official companion library in its ecosystem — exposes a first-class **interception / listener SPI**: a listener interface, an interceptor-registration hook, or a proxy/wrapper factory built for observability. Many client libraries provide one, and it is almost always the better hook point than hand-written method advice. + +Examples of such SPIs: + +- **R2DBC** → `r2dbc-proxy` (an official R2DBC project) exposes `ProxyExecutionListener` / `ProxyMethodExecutionListener`, invoked around every query via `beforeQuery(QueryExecutionInfo)` / `afterQuery(QueryExecutionInfo)`. You install it by wrapping the `ConnectionFactory`. +- **gRPC** → `ClientInterceptor` / `ServerInterceptor`. +- **Kafka** → `ProducerInterceptor` / `ConsumerInterceptor` (`interceptor.classes`). +- **JAX-RS / JDBC / many others** → filter, interceptor, or proxy-driver registration hooks. + +**Why prefer the SPI over hand-written advice, especially for reactive/async clients:** a listener SPI is designed around the library's own execution lifecycle and delivers a single well-defined start/end (and error/**cancel**) callback per logical operation, with the operation's metadata (query text, connection info, status, timing) already assembled for you. Hand-written advice on a reactive method must instead re-implement that lifecycle by wrapping the returned `Publisher`/`Mono`/`Flux` and tracking subscribe/complete/error/**cancel** by hand — this is easy to get subtly wrong. A wrapper that finishes the span only on `onComplete`/`onError` will **leak every span that gets cancelled** (reactive pipelines routinely cancel upstreams — e.g. `take(1)`, timeouts, `DiscardOnCancel` operators), so the span is created but never finished and never exported. It is also easy to wrap at the wrong granularity and emit many spans per logical query. The library's listener already handles all of this. + +**Do this without forcing a new runtime dependency on the user.** Using a library's listener SPI does NOT require adding it as a normal (`implementation`/`api`) dependency: + +- Declare the interception library **`compileOnly`** so it is not put on the application's runtime classpath, and inject your listener implementation and any glue via the module's `helperClassNames()` (the same mechanism used for decorators and other injected helpers). The listener classes travel inside the agent, not the user's app. +- If helper injection is impractical for a given SPI, the alternative is to **shade/bundle** the interception library's classes into the instrumentation rather than depend on it at runtime. + +Reach for hand-written method advice when no such SPI exists, or when the SPI cannot express what you need to capture. When one does exist and fits, prefer it — and note the choice (and the `compileOnly`/injection approach) in the PR so a reviewer sees the dependency was considered. + ### Before writing a new module, scan for an existing one Before creating `dd-java-agent/instrumentation/$framework/$framework-$version/`, check whether `dd-java-agent/instrumentation/$framework/` already exists and what's in it. @@ -123,7 +143,7 @@ A helper class is appropriate when multiple instrumentation classes share the sa For database-client integrations (`DatabaseClientDecorator` / `DBTypeProcessingDatabaseClientDecorator`), capture connection metadata (host, port, db name, user) at **connection-establishment** time and cache it in a `ContextStore` keyed on the connection object — not lazily on the first query. The canonical pattern is a dedicated instrumentation on the connect/factory method: - **JDBC** — `dd-java-agent/instrumentation/jdbc/DriverInstrumentation.java` hooks `Driver.connect(url, props)` and populates `InstrumentationContext.get(Connection.class, DBInfo.class)` at open time. Statement advice then reads the already-cached `DBInfo`. -- **Reactive drivers with an async connect** — the equivalent connect point is the connection FACTORY, not the connection object. For R2DBC, `io.r2dbc.spi.ConnectionFactoryOptions` is the only place host/port/database/user are exposed as structured data; `io.r2dbc.spi.ConnectionMetadata` (on the live `Connection`) exposes ONLY product name/version. **But `ConnectionFactory.create()` is a zero-argument SPI method returning a `Publisher` — the options are NOT available at `create()`.** Capture them earlier, where the factory is built: hook `ConnectionFactories.get(ConnectionFactoryOptions)` (or the provider-construction path) and store the options in a `ContextStore`; then, in advice on `create()`, read the stored options for that factory and thread them onto the asynchronously-emitted `Connection` (a second context store keyed on the returned `Connection`). Hooking only `Connection.createStatement()` + `ConnectionMetadata` CANNOT populate `db.name`/`peer.hostname`/`db.user`/port. (OpenTelemetry's R2DBC instrumentation does exactly this options→factory→connection threading; it is a good reference.) +- **Reactive drivers with an async connect** — the equivalent connect point is the connection FACTORY, not the connection object. For R2DBC, `io.r2dbc.spi.ConnectionFactoryOptions` is the only place host/port/database/user are exposed as structured data; `io.r2dbc.spi.ConnectionMetadata` (on the live `Connection`) exposes ONLY product name/version. **But `ConnectionFactory.create()` is a zero-argument SPI method returning a `Publisher` — the options are NOT available at `create()`.** Capture them earlier, where the factory is built: hook `ConnectionFactories.get(ConnectionFactoryOptions)` (or the provider-construction path) and store the options in a `ContextStore`; then, in advice on `create()`, read the stored options for that factory and thread them onto the asynchronously-emitted `Connection` (a second context store keyed on the returned `Connection`). Hooking only `Connection.createStatement()` + `ConnectionMetadata` CANNOT populate `db.name`/`peer.hostname`/`db.user`/port. (OpenTelemetry's R2DBC instrumentation does exactly this options→factory→connection threading; it is a good reference.) Note also — per "Before hand-writing method advice, check for an official interception SPI" above — that R2DBC has `r2dbc-proxy`, whose `ProxyMethodExecutionListener` surfaces this connection/query metadata through a listener and handles the reactive lifecycle for you; prefer it over hand-wrapping the `create()`/`execute()` publishers where it fits. Why eager-at-connect beats lazy-per-query: lazy extraction (e.g. `statement.getConnection().getMetaData().getURL()` on first execute) works for plain JDBC but (a) pays the extraction cost on every connection's first query instead of amortizing at pool-open, and (b) silently yields nothing when the metadata is not reachable from the object the query advice happens to hold — which is exactly what happens for reactive drivers whose statement/connection objects don't carry the factory options. From 241817d02f9ca36937867f99a642353271126f71 Mon Sep 17 00:00:00 2001 From: Jordan Wong Date: Tue, 1 Sep 2026 09:01:55 -0400 Subject: [PATCH 2/3] skill(apm-integrations): add worked example for listener-SPI hooking Concrete R2DBC/r2dbc-proxy example showing the factory-hook + AssignReturned + wrap-helper + listener shape, so the existing "check for an interception SPI" guidance is operationalizable, not just prose. --- .../references/instrumenter-module.md | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/.agents/skills/apm-integrations/references/instrumenter-module.md b/.agents/skills/apm-integrations/references/instrumenter-module.md index f1c8b38e5d6..d138970f626 100644 --- a/.agents/skills/apm-integrations/references/instrumenter-module.md +++ b/.agents/skills/apm-integrations/references/instrumenter-module.md @@ -61,6 +61,58 @@ Examples of such SPIs: Reach for hand-written method advice when no such SPI exists, or when the SPI cannot express what you need to capture. When one does exist and fits, prefer it — and note the choice (and the `compileOnly`/injection approach) in the PR so a reviewer sees the dependency was considered. +**Worked example — hooking a registration/factory point instead of the query methods (R2DBC + `r2dbc-proxy`):** + +1. **Advice target is the factory, not the query.** Hook `io.r2dbc.spi.ConnectionFactories.find(ConnectionFactoryOptions)` — NOT `Statement.execute()` / `Batch.execute()`. Replace the returned `ConnectionFactory` using `@Advice.AssignReturned.ToReturned` on method exit: + + ```java + @Advice.OnMethodExit(suppress = Throwable.class, inline = false) + @Advice.AssignReturned.ToReturned + public static ConnectionFactory wrap( + @Advice.Return ConnectionFactory factory, + @Advice.Argument(0) ConnectionFactoryOptions options) { + return R2dbcTracingSupport.wrapConnectionFactory(factory, options); + } + ``` + + Do NOT use a writable `@Advice.Return(readOnly = false) Publisher<...> ...` here — binding a reactive-streams interface to a writable return against a concrete `Mono`/`Flux` result fails Byte Buddy transformation. `@Advice.AssignReturned.ToReturned` sidesteps this because it substitutes the value rather than mutating a typed slot. + +2. **Wrap helper installs the listener** (injected via `helperClassNames()`, `r2dbc-proxy` declared `compileOnly`): + + ```java + public static ConnectionFactory wrapConnectionFactory( + ConnectionFactory factory, ConnectionFactoryOptions options) { + ProxyConfig cfg = new ProxyConfig(); + cfg.addListener(new TraceProxyListener(options)); + return ProxyConnectionFactory.builder(factory, cfg).build(); + } + ``` + +3. **Listener drives the span lifecycle** — implements `ProxyMethodExecutionListener`; stash the span on the query's own value store (the interception library already carries one per call — do not add a separate `ContextStore` keyed on the driver object): + + ```java + public class TraceProxyListener implements ProxyMethodExecutionListener { + @Override + public void beforeQuery(QueryExecutionInfo qei) { + AgentSpan span = startSpan(...); + DECORATE.afterStart(span); + DECORATE.onStatement(span, qei); + qei.getValueStore().put(SPAN_KEY, span); + } + @Override + public void afterQuery(QueryExecutionInfo qei) { + AgentSpan span = qei.getValueStore().get(SPAN_KEY, AgentSpan.class); + if (qei.getThrowable() != null) DECORATE.onError(span, qei.getThrowable()); + DECORATE.beforeFinish(span); + span.finish(); + } + } + ``` + + `beforeQuery`/`afterQuery` fire once per logical operation and `r2dbc-proxy` itself owns completion/error/**cancel** — you do not hand-roll a `Publisher` wrapper to catch those. + +Net result: 1 advice (factory hook) + 1 wrap helper + 1 listener class — smaller than the method-advice alternative (which needs per-method advice on both `Statement.execute()` and `Batch.execute()`, plus a hand-rolled cancel-safe `Publisher` wrapper) and correct by construction on cancellation. The same shape applies to any other SPI in the examples list above: hook the registration/interceptor-installation point, not the client's own operational methods. + ### Before writing a new module, scan for an existing one Before creating `dd-java-agent/instrumentation/$framework/$framework-$version/`, check whether `dd-java-agent/instrumentation/$framework/` already exists and what's in it. From de3dab5328ddc80e8a28d6840b6351f3dc839238 Mon Sep 17 00:00:00 2001 From: Jordan Wong Date: Fri, 4 Sep 2026 09:34:24 -0400 Subject: [PATCH 3/3] skill(apm-integrations): document muzzle extraDependency for listener-SPI Muzzle only puts a module's pinned primary dependency on its validation classpath by default. When an injected wrap-helper or listener class references a compileOnly interception library's own types directly, muzzle reports them as missing unless that library is added via extraDependency. Verified against a live R2DBC generation where muzzle failed on 8 "missing class" errors until this was added. --- .../references/instrumenter-module.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/.agents/skills/apm-integrations/references/instrumenter-module.md b/.agents/skills/apm-integrations/references/instrumenter-module.md index d138970f626..4e1819bebc9 100644 --- a/.agents/skills/apm-integrations/references/instrumenter-module.md +++ b/.agents/skills/apm-integrations/references/instrumenter-module.md @@ -58,6 +58,7 @@ Examples of such SPIs: - Declare the interception library **`compileOnly`** so it is not put on the application's runtime classpath, and inject your listener implementation and any glue via the module's `helperClassNames()` (the same mechanism used for decorators and other injected helpers). The listener classes travel inside the agent, not the user's app. - If helper injection is impractical for a given SPI, the alternative is to **shade/bundle** the interception library's classes into the instrumentation rather than depend on it at runtime. +- The `compileOnly` interception library must also be added to muzzle's classpath via `extraDependency` (see the worked example below) — otherwise muzzle validation fails with "missing class" for every type the listener/wrap-helper classes reference from it. Reach for hand-written method advice when no such SPI exists, or when the SPI cannot express what you need to capture. When one does exist and fits, prefer it — and note the choice (and the `compileOnly`/injection approach) in the PR so a reviewer sees the dependency was considered. @@ -111,6 +112,21 @@ Reach for hand-written method advice when no such SPI exists, or when the SPI ca `beforeQuery`/`afterQuery` fire once per logical operation and `r2dbc-proxy` itself owns completion/error/**cancel** — you do not hand-roll a `Publisher` wrapper to catch those. +4. **Muzzle needs an `extraDependency` for the `compileOnly` interception library.** Muzzle only puts the module's pinned primary dependency (here, `r2dbc-spi`) on its validation classpath by default. Since the wrap helper and listener classes reference the interception library's own types directly (`ProxyConnectionFactory`, `ProxyMethodExecutionListener`, `QueryExecutionInfo`, ...), muzzle reports them as "missing class" unless you add the interception library explicitly: + + ```groovy + muzzle { + pass { + group = "io.r2dbc" + module = "r2dbc-spi" + versions = "[1.0.0.RELEASE,)" + extraDependency 'io.r2dbc:r2dbc-proxy:1.1.0.RELEASE' + } + } + ``` + + This is the same `extraDependency` directive used elsewhere for a module's secondary compile-time dependency — it is not R2DBC-specific. + Net result: 1 advice (factory hook) + 1 wrap helper + 1 listener class — smaller than the method-advice alternative (which needs per-method advice on both `Statement.execute()` and `Batch.execute()`, plus a hand-rolled cancel-safe `Publisher` wrapper) and correct by construction on cancellation. The same shape applies to any other SPI in the examples list above: hook the registration/interceptor-installation point, not the client's own operational methods. ### Before writing a new module, scan for an existing one