Skip to content

Peel dbUser out of DatabaseClientDecorator into a param-injected TagExtractor#11823

Draft
dougqh wants to merge 2 commits into
dougqh/jdbc-tag-extractorfrom
dougqh/db-decorator-dbuser-peel
Draft

Peel dbUser out of DatabaseClientDecorator into a param-injected TagExtractor#11823
dougqh wants to merge 2 commits into
dougqh/jdbc-tag-extractorfrom
dougqh/db-decorator-dbuser-peel

Conversation

@dougqh

@dougqh dougqh commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

What

First real dissolution of the DB decorator hierarchy: removes the dbUser template method from DatabaseClientDecorator and replaces it with a param-injected connection-tags TagExtractor. Stacked on #11820.

Why dbUser

A null-ratio scan of the DB template methods flagged it as the sparsest: 28/30 leaves returned null — only JDBC and Vertx-SQL provide a real user. That sparsity is the SQL/NoSQL false-generalization showing through: db.user is a SQL-auth concept the NoSQL/cache stores (Redis, Mongo, Cassandra, …) can only answer with null, yet every DB span paid a virtual dbUser() call to set DB_USER = null. (dbInstance 0.65 / dbHostname 0.52 are next, but they're DERIVED — they feed service-name split — so they wait on the declarative-derivation layer.)

The pattern (param-injected extractor)

DatabaseClientDecorator gains onConnection(span, conn, TagExtractor<CONNECTION>); the 2-arg overload delegates with a shared no-op extractor. The extractor is passed as a caller-side static final constant, so at a monomorphic advice site it devirtualizes + inlines when onConnection inlines — the key advantage over a virtual connectionExtractor() provider (which would re-megamorphize). This is also the seam for disentangling pure extraction from side-effects in the harder decorators later.

Changes

  • Base: param-form onConnection + no-op default; remove abstract dbUser and the setTag(DB_USER, …) line.
  • JDBC: fold db.user into ConnectionInfoExtractor, apply via super.onConnection(..., INSTANCE).
  • Vertx-SQL: new VertxSqlConnectionExtractor, applied via the param form at its super.onConnection call (the provider-with-no-override case the param form is for).
  • 26 NoSQL/cache decorators: drop the dead dbUsernull override.
  • DatabaseClientConnectionBenchmark (jmh): the inlining acceptance harness (below).

Net −71 lines. db.user is set unconditionally by the two providers to preserve the prior (present-check-free) behavior exactly.

Inlining verification (the acceptance test — done)

DatabaseClientConnectionBenchmark exercises the real param-form onConnection at a mono site (one static final extractor — the production shape) vs mega (8 rotating extractor types — worst case). -XX:+PrintInlining:

  • mono: DatabaseClientDecorator::onConnection — inline (hot) and the concrete extractor's extract() — inline (hot) (the abstract TagExtractor::extract slot is bypassed). Full devirt + inline. 79.0M ops/s.
  • mega: 67.6M ops/s, only −14% — the JIT polymorphically inlined all 8 tiny bodies. (Honest caveat: mega is optimistic — trivial 11-byte bodies fit HotSpot's polymorphic-inline ceiling; fuller real bodies would fall to a virtual call sooner. Production is mono.)

So the devirt claim holds at a monomorphic site.

Testing

  • agent-bootstrap decorator tests updated + green (incl. new param-form coverage). These are mock interaction-asserts for the old mechanism — flagged as Spock→JUnit migration candidates, not migrated here to keep the PR focused.
  • JDBCInstrumentationV0Test: 87/87; ConnectionInfoExtractor (now incl. db.user) auto-injected as a transitive helper — no helperClassNames() edit.
  • Vertx-SQL / NoSQL integration tests are container-gated (CI); VertxSqlConnectionExtractor rides the same transitive-helper collection (VertxSqlClientDecorator is an explicit helper).

Follow-ups

  • dbInstance/dbHostname peel — gated on the declarative-derivation layer (their values feed setServiceName, a computed side-effect, not just a tag). Until that lands, onConnection (and its 2-arg overload) stays as the wrapper around that derivation.
  • Spock→JUnit migration of the DatabaseClientDecoratorTest cascade.

🤖 Generated with Claude Code

…xtractor

The null-ratio scan showed dbUser was the sparsest template method on
DatabaseClientDecorator: 28/30 leaves returned null (only JDBC and Vertx-SQL
provide a real user). That sparsity is the SQL/NoSQL false-generalization
showing through — db.user is a SQL-auth concept the NoSQL/cache stores can only
answer with null, yet every DB span paid a virtual dbUser() call to set
DB_USER=null.

Replaces the dbUser template method with a param-injected connection-tags
extractor:
- DatabaseClientDecorator gains onConnection(span, conn, TagExtractor<C>); the
  2-arg overload delegates with a shared no-op extractor. The extractor is
  passed as a caller-side constant so it devirtualizes+inlines at a monomorphic
  advice site (rather than a virtual provider method). dbInstance/dbHostname
  stay for now (they are DERIVED — feed service-name split — a later peel).
- The 2 SQL providers set db.user themselves: JDBC folds it into
  ConnectionInfoExtractor; Vertx-SQL gets VertxSqlConnectionExtractor, both
  applied via the param form. db.user is set unconditionally to preserve the
  prior (present-check-free) behavior exactly.
- The 26 NoSQL/cache decorators shed their dead `return null` dbUser override.

Behavior-preserving. agent-bootstrap decorator tests updated (the mock
interaction-asserts for the old mechanism; these are Spock migration candidates)
and green; JDBCInstrumentationV0Test 87/87. Vertx-SQL's extractor is auto-injected
as a transitive helper (its tests are container-gated / CI-only).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dougqh dougqh added the tag: no release notes Changes to exclude from release notes label Jul 1, 2026
@datadog-datadog-us1-prod

datadog-datadog-us1-prod Bot commented Jul 1, 2026

Copy link
Copy Markdown

🎯 Code Coverage (details)
Patch Coverage: 100.00%
Overall Coverage: 54.94% (-0.01%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: f4cad21 | Docs | Datadog PR Page | Give us feedback!

Acceptance harness for the dbUser peel's inlining claim. Exercises the real
DatabaseClientDecorator.onConnection param-form at a mono call site (one
static-final extractor — the production shape) vs mega (8 rotating extractor
types — the worst case of a single shared site).

PrintInlining (mono) confirms the chain collapses: DatabaseClientDecorator
::onConnection inline (hot) and the concrete extractor's extract() devirtualizes
and inlines (the abstract TagExtractor::extract slot is bypassed). mega only
costs ~14% here because the trivial extractor bodies fit HotSpot's polymorphic
inline ceiling; larger real bodies would fall to a virtual call sooner, so mega
is an optimistic worst-case. The production shape is mono.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dd-octo-sts

dd-octo-sts Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

🟢 Java Benchmark SLOs — All performance SLOs passed

Suite Status
Startup 🟢 pass

SLO thresholds are defined here based on automatically generated metrics. A warning is raised when results are within 5% of the threshold.

PR vs. master results
Scenario Candidate master Δ (95% CI of mean)
startup:insecure-bank:iast:Agent 13.73 s 13.94 s [-2.2%; -0.9%] (maybe better)
startup:insecure-bank:tracing:Agent 12.77 s 12.99 s [-2.8%; -0.6%] (maybe better)
startup:petclinic:appsec:Agent 16.44 s 16.65 s [-2.1%; -0.4%] (maybe better)
startup:petclinic:iast:Agent 16.53 s 16.78 s [-2.4%; -0.6%] (maybe better)
startup:petclinic:profiling:Agent 15.84 s 16.79 s [-9.6%; -1.7%] (significantly better)
startup:petclinic:sca:Agent 16.54 s 16.65 s [-1.7%; +0.3%] (no difference)
startup:petclinic:tracing:Agent 15.72 s 15.56 s [-3.3%; +5.3%] (no difference)

Commit: f4cad210 · CI Pipeline · Benchmarking Platform UI


Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tag: no release notes Changes to exclude from release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant