From 6125b3776c6daf12fccf128a552461be96aa4ee3 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 30 Jun 2026 17:03:16 -0400 Subject: [PATCH 1/8] Introduce TagContributor + TagExtractor span-tag APIs Foundational contract for dissolving Decorators (see decorator-dissolution.md). API types only -- no wiring or consumers yet. - TagContributor (intrinsic): a typed POJO we own projects its own state onto a span via addTo(AgentSpan). DbInfo / git-meta / ExtractedContext are the precedents; addTo is a flat setTag sweep that becomes setTag(KnownTagIds.X, field) once the id-arm lands. - TagExtractor (extrinsic, @FunctionalInterface): extracts tags from a foreign object we don't own (Connection/Request) onto a span. Intended as a static-final non-capturing lambda invoked from the integration's own advice (a monomorphic site the JIT devirtualizes + inlines) -- the opposite of the decorator's shared megamorphic dispatch. Two modes: place tags directly, or build a memoized POJO that is itself a TagContributor. Both target AgentSpan for now so they can drive span-level state (resource name, error, status) during the transition; the sink narrows toward TagMap as those fields migrate into the tag model. Javadoc bakes in the compile-to-~zero litmus + the monomorphic-site / no-shared-sink discipline. Co-Authored-By: Claude Opus 4.8 --- .../instrumentation/api/TagContributor.java | 23 ++++++++++++++ .../instrumentation/api/TagExtractor.java | 30 +++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/TagContributor.java create mode 100644 internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/TagExtractor.java diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/TagContributor.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/TagContributor.java new file mode 100644 index 00000000000..e3f7877e27b --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/TagContributor.java @@ -0,0 +1,23 @@ +package datadog.trace.bootstrap.instrumentation.api; + +/** + * An object that projects its own typed state onto a span — the inverse of a builder that takes + * external {@code setTag} calls. Implemented by typed POJOs we own whose fields map to known tags + * (e.g. {@code DbInfo}, git metadata, extracted context). {@code addTo} is a flat sweep of {@code + * span.setTag(...)}; once the id-arm lands it becomes {@code setTag(KnownTagIds.X, field)}, so it + * compiles to field-loads + positional stores with no {@code keyOf}. + * + *

This is an authoring aid meant to compile to ~zero — the opposite of a Decorator. Apply it at + * a CONCRETE-type, monomorphic call site (the integration's own advice) so {@code addTo} + * devirtualizes and inlines. Do not route many contributors through one shared {@code + * List} sink — that re-megamorphizes and recreates the decorator problem. Prefer + * stateless implementations. + * + *

Targets {@link AgentSpan} for now (so it can also drive span-level state — resource name, + * error, measured — that is not yet expressed as tags). As those span fields migrate into the tag + * model, this surface narrows toward a future {@code addTo(TagMap)}. See {@link TagExtractor} for + * the extrinsic counterpart (foreign objects we do not own). + */ +public interface TagContributor { + void addTo(AgentSpan span); +} diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/TagExtractor.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/TagExtractor.java new file mode 100644 index 00000000000..4881308a231 --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/TagExtractor.java @@ -0,0 +1,30 @@ +package datadog.trace.bootstrap.instrumentation.api; + +/** + * Extracts tags from a foreign object we do not own (a framework {@code Connection}, request, + * response, etc.) onto a span — the extrinsic counterpart to {@link TagContributor}. This is the + * irreducible "reach into version-specific framework objects" that cannot be expressed as data; it + * stays imperative, but contained in a narrow, single-purpose place. + * + *

Two usage modes, chosen by the source's lifecycle vs. the span: + * + *

    + *
  • read {@code source} and place tags directly — when {@code source} is per-span (a request / + * statement); + *
  • build a memoized typed POJO that is itself a {@link TagContributor} — when {@code source} + * outlives the span (e.g. {@code Connection -> DbInfo}, extracted once and cached). + *
+ * + *

This is an authoring aid meant to compile to ~zero — the opposite of a Decorator. Intended as + * a {@code static final}, non-capturing lambda invoked from the integration's own advice: a + * monomorphic call site the JIT devirtualizes and inlines (strictly better than dispatching through + * the decorator hierarchy). Never route many extractors through one shared site. Targets {@link + * AgentSpan} so it can also drive span-level state (resource name, error, status) during the + * transition; that surface narrows as those fields migrate into the tag model. + * + * @param the foreign source type to extract from + */ +@FunctionalInterface +public interface TagExtractor { + void extract(T source, AgentSpan span); +} From 114e15115833309ab31462d1f332f97a0518c490 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 30 Jun 2026 17:23:23 -0400 Subject: [PATCH 2/8] Add TagProjectionBenchmark: decorator vs extractor/contributor dispatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Representative benchmark for the decorator-dissolution thesis: project a typed source's fields onto a span three ways (decorator template-method pattern / TagExtractor-shaped lambda / TagContributor-shaped), to show what the JIT compiles away and what survives. Synthetic structural equivalents + a light DCE-safe sink isolate *dispatch* cost. @Param mode {mono, mega}: mono = one type (charitable to the decorator); mega = 8 equivalent loaded types exercised so the call site / shared-base template calls go megamorphic. Methodology: @Threads(8), @Fork(3), Throughput + -prof gc (dougqh's standard — multi-thread reveals alloc pressure, 3+ forks expose bimodal JIT/devirt). Results (ops/s, 3f x 8t): mono ~ties (1.26-1.66B, abstraction free); mega decorator 426M (-72%) vs extractor 914M (-27%) / contributor 1026M (-38%). Zero alloc in all arms (pure dispatch). PrintInlining confirms the mechanism: decorator-mono inlines all 4 template calls; decorator- mega leaves all 4 as `virtual call` (megamorphic); extractor-mega has ONE virtual call + inlined body. The template-method pattern pays one megamorphic dispatch PER template method; extractor/contributor pay one total -> the decorator's cost scales with field count (worse for the 7+ HttpServer hooks), while extractor/contributor can stay monomorphic per-site (~3-4x faster in reality). Co-Authored-By: Claude Opus 4.8 --- .../trace/api/TagProjectionBenchmark.java | 373 ++++++++++++++++++ 1 file changed, 373 insertions(+) create mode 100644 internal-api/src/jmh/java/datadog/trace/api/TagProjectionBenchmark.java diff --git a/internal-api/src/jmh/java/datadog/trace/api/TagProjectionBenchmark.java b/internal-api/src/jmh/java/datadog/trace/api/TagProjectionBenchmark.java new file mode 100644 index 00000000000..b37eb514b4f --- /dev/null +++ b/internal-api/src/jmh/java/datadog/trace/api/TagProjectionBenchmark.java @@ -0,0 +1,373 @@ +package datadog.trace.api; + +import java.util.concurrent.TimeUnit; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Level; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +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.Threads; +import org.openjdk.jmh.annotations.Warmup; + +/** + * Models projecting a typed source's fields onto a span three ways — the decorator template-method + * pattern vs. the {@code TagExtractor} / {@code TagContributor} shape — to show what compiles away + * and what doesn't. Uses synthetic structural equivalents (a light DCE-safe {@link Sink} stands in + * for the span) so it isolates *dispatch* cost, not span/tag-storage cost; the real interfaces + * target {@code AgentSpan}, but the call-site shape is what this measures. + * + *

{@code mode}: + * + *

    + *
  • mono — one concrete type at the call site. The JIT devirtualizes + inlines all three + * → they should ~tie (the abstraction is free at the decorator's BEST case — charitable, + * since a real program loads many decorators). + *
  • mega — {@link #TYPES} equivalent loaded types, exercised so the call site (and, for + * the decorator, the shared base's template-method calls) go megamorphic. The decorator is + * FORCED here in reality (its template dispatch is structural). Extractor/contributor are + * shown in mega too to prove they are not magic: routed through a shared megamorphic site + * they degrade identically. The real win is that they CAN stay mono (per-integration site) + * and the decorator cannot. + *
+ * + *

Run with {@code -prof gc} (captures should stay flat) and {@code -XX:+PrintInlining} to + * confirm the mechanism: decorator template calls megamorphic/not-inlined under mega; + * extractor/contributor inlined under mono. Throughput is the headline; per your methodology, + * multi-thread reveals alloc pressure and 3+ forks expose bimodal JIT/devirt behavior. + */ +@State(Scope.Thread) +@BenchmarkMode(Mode.Throughput) +@OutputTimeUnit(TimeUnit.SECONDS) +@Warmup(iterations = 5, time = 2) +@Measurement(iterations = 5, time = 2) +@Fork(3) +@Threads(8) +public class TagProjectionBenchmark { + static final int TYPES = 8; + + @Param({"mono", "mega"}) + String mode; + + /** Light DCE-safe stand-in for the span — identical trivial work per set across all arms. */ + static final class Sink { + long acc; + + void set(String key, Object value) { + this.acc += key.length() + (value == null ? 0 : value.hashCode()); + } + } + + /** The typed source whose fields are projected (a stand-in for DbInfo). */ + static final class DbPojo { + final String dbType, dbUser, dbInstance, dbHostname; + + DbPojo(String t, String u, String i, String h) { + this.dbType = t; + this.dbUser = u; + this.dbInstance = i; + this.dbHostname = h; + } + } + + // ---- (1) decorator template-method pattern: shared base onConnection() calls abstract hooks + // ---- + abstract static class TemplateDecorator { + abstract String dbType(DbPojo p); + + abstract String dbUser(DbPojo p); + + abstract String dbInstance(DbPojo p); + + abstract String dbHostname(DbPojo p); + + // shared base: calls the abstract template methods -> megamorphic when many subclasses loaded + final void onConnection(Sink sink, DbPojo p) { + String type = dbType(p); + if (type != null) sink.set("db.type", type); + String user = dbUser(p); + if (user != null) sink.set("db.user", user); + String instance = dbInstance(p); + if (instance != null) sink.set("db.instance", instance); + String host = dbHostname(p); + if (host != null) sink.set("peer.hostname", host); + } + } + + // ---- (2) extractor shape: stateless lambda-style, reads source -> sets directly ---- + interface Extractor { + void extract(DbPojo p, Sink sink); + } + + // ---- (3) contributor shape: the source projects itself ---- + interface Contributor { + void addTo(Sink sink); + } + + private DbPojo[] pojos; + private TemplateDecorator[] decorators; + private Extractor[] extractors; + private Contributor[] contributors; + private Sink sink; + private int idx; + + @Setup(Level.Trial) + public void setup() { + this.sink = new Sink(); + this.pojos = new DbPojo[TYPES]; + this.decorators = new TemplateDecorator[TYPES]; + this.extractors = new Extractor[TYPES]; + this.contributors = new Contributor[TYPES]; + for (int i = 0; i < TYPES; i++) { + DbPojo pojo = new DbPojo("h2", "sa", "db" + i, "localhost"); + this.pojos[i] = pojo; + this.decorators[i] = newDecorator(i); + this.extractors[i] = newExtractor(i); + this.contributors[i] = newContributor(pojo, i); + } + } + + // distinct concrete types per index so 'mega' has TYPES loaded implementations to megamorphize + private static TemplateDecorator newDecorator(int i) { + switch (i) { + case 0: + return new TemplateDecorator() { + String dbType(DbPojo p) { + return p.dbType; + } + + String dbUser(DbPojo p) { + return p.dbUser; + } + + String dbInstance(DbPojo p) { + return p.dbInstance; + } + + String dbHostname(DbPojo p) { + return p.dbHostname; + } + }; + case 1: + return new TemplateDecorator() { + String dbType(DbPojo p) { + return p.dbType; + } + + String dbUser(DbPojo p) { + return p.dbUser; + } + + String dbInstance(DbPojo p) { + return p.dbInstance; + } + + String dbHostname(DbPojo p) { + return p.dbHostname; + } + }; + case 2: + return new TemplateDecorator() { + String dbType(DbPojo p) { + return p.dbType; + } + + String dbUser(DbPojo p) { + return p.dbUser; + } + + String dbInstance(DbPojo p) { + return p.dbInstance; + } + + String dbHostname(DbPojo p) { + return p.dbHostname; + } + }; + case 3: + return new TemplateDecorator() { + String dbType(DbPojo p) { + return p.dbType; + } + + String dbUser(DbPojo p) { + return p.dbUser; + } + + String dbInstance(DbPojo p) { + return p.dbInstance; + } + + String dbHostname(DbPojo p) { + return p.dbHostname; + } + }; + case 4: + return new TemplateDecorator() { + String dbType(DbPojo p) { + return p.dbType; + } + + String dbUser(DbPojo p) { + return p.dbUser; + } + + String dbInstance(DbPojo p) { + return p.dbInstance; + } + + String dbHostname(DbPojo p) { + return p.dbHostname; + } + }; + case 5: + return new TemplateDecorator() { + String dbType(DbPojo p) { + return p.dbType; + } + + String dbUser(DbPojo p) { + return p.dbUser; + } + + String dbInstance(DbPojo p) { + return p.dbInstance; + } + + String dbHostname(DbPojo p) { + return p.dbHostname; + } + }; + case 6: + return new TemplateDecorator() { + String dbType(DbPojo p) { + return p.dbType; + } + + String dbUser(DbPojo p) { + return p.dbUser; + } + + String dbInstance(DbPojo p) { + return p.dbInstance; + } + + String dbHostname(DbPojo p) { + return p.dbHostname; + } + }; + default: + return new TemplateDecorator() { + String dbType(DbPojo p) { + return p.dbType; + } + + String dbUser(DbPojo p) { + return p.dbUser; + } + + String dbInstance(DbPojo p) { + return p.dbInstance; + } + + String dbHostname(DbPojo p) { + return p.dbHostname; + } + }; + } + } + + // distinct Extractor lambda expressions -> distinct synthetic types (megamorphic when rotated) + private static Extractor newExtractor(int i) { + switch (i) { + case 0: + return TagProjectionBenchmark::extract; + case 1: + return (p, s) -> extract(p, s); + case 2: + return (DbPojo p, Sink s) -> extract(p, s); + case 3: + return (p, s) -> { + extract(p, s); + }; + case 4: + return (final DbPojo p, final Sink s) -> extract(p, s); + case 5: + return (p, s) -> extract(p, s); + case 6: + return (DbPojo p, Sink s) -> { + extract(p, s); + }; + default: + return (p, s) -> extract(p, s); + } + } + + // the actual projection, shared so all extractor variants do identical work + private static void extract(DbPojo p, Sink sink) { + if (p.dbType != null) sink.set("db.type", p.dbType); + if (p.dbUser != null) sink.set("db.user", p.dbUser); + if (p.dbInstance != null) sink.set("db.instance", p.dbInstance); + if (p.dbHostname != null) sink.set("peer.hostname", p.dbHostname); + } + + private static Contributor newContributor(DbPojo p, int i) { + switch (i) { + case 0: + return s -> extract(p, s); + case 1: + return (Sink s) -> extract(p, s); + case 2: + return s -> { + extract(p, s); + }; + case 3: + return (final Sink s) -> extract(p, s); + case 4: + return s -> extract(p, s); + case 5: + return (Sink s) -> { + extract(p, s); + }; + case 6: + return s -> extract(p, s); + default: + return (Sink s) -> extract(p, s); + } + } + + // mono: always index 0 (one type at the site). mega: rotate -> the site sees all TYPES. + private int next() { + if ("mono".equals(this.mode)) return 0; + int i = this.idx + 1; + if (i >= TYPES) i = 0; + this.idx = i; + return i; + } + + @Benchmark + public long decorator() { + int i = next(); + this.decorators[i].onConnection(this.sink, this.pojos[i]); + return this.sink.acc; + } + + @Benchmark + public long extractor() { + int i = next(); + this.extractors[i].extract(this.pojos[i], this.sink); + return this.sink.acc; + } + + @Benchmark + public long contributor() { + int i = next(); + this.contributors[i].addTo(this.sink); + return this.sink.acc; + } +} From f2878b2f17a424e45267065b47e399ef47fde637 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 30 Jun 2026 20:50:57 -0400 Subject: [PATCH 3/8] Add span-first AgentSpan.setTags(source, extractor) The span-first, product-dev-facing form of extractor.extract(source, this) (matches setTag / setAllTags). The indirection inlines away at a monomorphic call site, and it's the seam where an implementer can coarse-lock the whole extraction. Part of the TagExtractor API; consumed by the JDBC canary and subsequent extractor migrations. Co-Authored-By: Claude Opus 4.8 --- .../bootstrap/instrumentation/api/AgentSpan.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java index 0eb67f3ad52..4a92241097f 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java @@ -88,6 +88,18 @@ default boolean isValid() { AgentSpan setAllTags(Map map); + /** + * Sets tags on this span by applying a {@link TagExtractor} to a foreign source — the span-first, + * product-dev-facing form of {@code extractor.extract(source, this)} (matches {@code + * setTag}/{@code setAllTags}). The extra indirection inlines away at a monomorphic call site. + * This is also the seam where the whole extraction can be coarse-locked (one critical section) — + * an implementer may override to do so. + */ + default AgentSpan setTags(final T source, final TagExtractor extractor) { + extractor.extract(source, this); + return this; + } + @Override AgentSpan setTag(String key, Number value); From e9b4e88243c607010623835406d93f3c01ac20a5 Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 30 Jun 2026 20:58:34 -0400 Subject: [PATCH 4/8] Extract peer-connection tagging into a TagExtractor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lifts the pure peer-connection field->tag logic (hostname / IPv4 / IPv6 / port) out of BaseDecorator into a static-final, non-capturing PEER_CONNECTION_EXTRACTOR. This is the extrinsic TagExtractor case — extraction from a JDK type we don't own (InetSocketAddress) — complementing the JDBC canary's intrinsic (owned DBInfo) case. Behavior-preserving: onPeerConnection(span, InetSocketAddress) now invokes the extractor (the shared setPeerAddress helper + port), and the hostName resolver cache is unchanged. Internal plumbing calls extract() directly so it runs on test doubles; the span-first span.setTags(...) sugar is for hand-written integration call sites. Co-Authored-By: Claude Opus 4.8 --- .../decorator/BaseDecorator.java | 35 ++++++++++++++++--- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java index 2628e9416cf..c114077b17d 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java @@ -12,6 +12,7 @@ import datadog.trace.bootstrap.instrumentation.api.AgentScope; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities; +import datadog.trace.bootstrap.instrumentation.api.TagExtractor; import datadog.trace.bootstrap.instrumentation.api.Tags; import java.lang.reflect.Method; import java.net.Inet4Address; @@ -148,12 +149,31 @@ public ContextScope onError(final ContextScope scope, final Throwable throwable) return scope; } + /** + * Extracts peer-connection tags (hostname / IPv4 / IPv6 / port) from a remote {@link + * InetSocketAddress}. This is the extrinsic {@link TagExtractor} form of the peer-connection + * tagging that used to live only in {@link #onPeerConnection(AgentSpan, InetSocketAddress)} — a + * static-final, non-capturing lambda over a JDK type we don't own, so it can be applied at a + * monomorphic call site ({@code span.setTags(addr, PEER_CONNECTION_EXTRACTOR)}) and inlined. The + * logic (and the {@code hostName} resolver cache it uses) is unchanged. + */ + public static final TagExtractor PEER_CONNECTION_EXTRACTOR = + (remoteConnection, span) -> { + if (remoteConnection != null) { + setPeerAddress(span, remoteConnection.getAddress(), !remoteConnection.isUnresolved()); + final int port = remoteConnection.getPort(); + if (port > UNSET_PORT) { + span.setTag(Tags.PEER_PORT, port); + } + } + }; + public AgentSpan onPeerConnection( final AgentSpan span, final InetSocketAddress remoteConnection) { - if (remoteConnection != null) { - onPeerConnection(span, remoteConnection.getAddress(), !remoteConnection.isUnresolved()); - setPeerPort(span, remoteConnection.getPort()); - } + // Invoke the extractor directly rather than via span.setTags(...): this legacy plumbing path + // is behavior-identical to the old inline code, and a direct call runs on test doubles too + // (the span-first span.setTags(...) sugar is for hand-written integration call sites). + PEER_CONNECTION_EXTRACTOR.extract(remoteConnection, span); return span; } @@ -162,6 +182,12 @@ public AgentSpan onPeerConnection(final AgentSpan span, final InetAddress remote } public AgentSpan onPeerConnection(AgentSpan span, InetAddress remoteAddress, boolean resolved) { + setPeerAddress(span, remoteAddress, resolved); + return span; + } + + private static void setPeerAddress( + final AgentSpan span, final InetAddress remoteAddress, final boolean resolved) { if (remoteAddress != null) { String ip = remoteAddress.getHostAddress(); if (resolved && Config.get().isPeerHostNameEnabled()) { @@ -173,7 +199,6 @@ public AgentSpan onPeerConnection(AgentSpan span, InetAddress remoteAddress, boo span.setTag(Tags.PEER_HOST_IPV6, ip); } } - return span; } public AgentSpan setPeerPort(AgentSpan span, String port) { From 07dc157661799c8911a6573a0f1c98f9d36d709f Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Tue, 30 Jun 2026 21:07:08 -0400 Subject: [PATCH 5/8] Promote PEER_CONNECTION_EXTRACTOR to a named singleton class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Turns the inline lambda into PeerConnectionExtractor (a proper TagExtractor implementation with a private-ctor INSTANCE singleton). This gives it a name at call sites (span.setTags(addr, PeerConnectionExtractor.INSTANCE)), makes it composable with other extractors — the axis the Decorator inheritance chain lacked — and gives the pure statics + the hostName resolver cache a clean home (a class static, not lambda-captured state). Behavior unchanged; BaseDecorator delegates. Co-Authored-By: Claude Opus 4.8 --- .../decorator/BaseDecorator.java | 43 +------------- .../decorator/PeerConnectionExtractor.java | 57 +++++++++++++++++++ 2 files changed, 59 insertions(+), 41 deletions(-) create mode 100644 dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/PeerConnectionExtractor.java diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java index c114077b17d..9e68a833f09 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java @@ -1,7 +1,5 @@ package datadog.trace.bootstrap.instrumentation.decorator; -import static datadog.trace.bootstrap.instrumentation.java.net.HostNameResolver.hostName; - import datadog.context.Context; import datadog.context.ContextScope; import datadog.trace.api.Config; @@ -12,11 +10,8 @@ import datadog.trace.bootstrap.instrumentation.api.AgentScope; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities; -import datadog.trace.bootstrap.instrumentation.api.TagExtractor; import datadog.trace.bootstrap.instrumentation.api.Tags; import java.lang.reflect.Method; -import java.net.Inet4Address; -import java.net.Inet6Address; import java.net.InetAddress; import java.net.InetSocketAddress; import java.util.concurrent.ExecutionException; @@ -149,31 +144,12 @@ public ContextScope onError(final ContextScope scope, final Throwable throwable) return scope; } - /** - * Extracts peer-connection tags (hostname / IPv4 / IPv6 / port) from a remote {@link - * InetSocketAddress}. This is the extrinsic {@link TagExtractor} form of the peer-connection - * tagging that used to live only in {@link #onPeerConnection(AgentSpan, InetSocketAddress)} — a - * static-final, non-capturing lambda over a JDK type we don't own, so it can be applied at a - * monomorphic call site ({@code span.setTags(addr, PEER_CONNECTION_EXTRACTOR)}) and inlined. The - * logic (and the {@code hostName} resolver cache it uses) is unchanged. - */ - public static final TagExtractor PEER_CONNECTION_EXTRACTOR = - (remoteConnection, span) -> { - if (remoteConnection != null) { - setPeerAddress(span, remoteConnection.getAddress(), !remoteConnection.isUnresolved()); - final int port = remoteConnection.getPort(); - if (port > UNSET_PORT) { - span.setTag(Tags.PEER_PORT, port); - } - } - }; - public AgentSpan onPeerConnection( final AgentSpan span, final InetSocketAddress remoteConnection) { // Invoke the extractor directly rather than via span.setTags(...): this legacy plumbing path // is behavior-identical to the old inline code, and a direct call runs on test doubles too // (the span-first span.setTags(...) sugar is for hand-written integration call sites). - PEER_CONNECTION_EXTRACTOR.extract(remoteConnection, span); + PeerConnectionExtractor.INSTANCE.extract(remoteConnection, span); return span; } @@ -182,25 +158,10 @@ public AgentSpan onPeerConnection(final AgentSpan span, final InetAddress remote } public AgentSpan onPeerConnection(AgentSpan span, InetAddress remoteAddress, boolean resolved) { - setPeerAddress(span, remoteAddress, resolved); + PeerConnectionExtractor.setPeerAddress(span, remoteAddress, resolved); return span; } - private static void setPeerAddress( - final AgentSpan span, final InetAddress remoteAddress, final boolean resolved) { - if (remoteAddress != null) { - String ip = remoteAddress.getHostAddress(); - if (resolved && Config.get().isPeerHostNameEnabled()) { - span.setTag(Tags.PEER_HOSTNAME, hostName(remoteAddress, ip)); - } - if (remoteAddress instanceof Inet4Address) { - span.setTag(Tags.PEER_HOST_IPV4, ip); - } else if (remoteAddress instanceof Inet6Address) { - span.setTag(Tags.PEER_HOST_IPV6, ip); - } - } - } - public AgentSpan setPeerPort(AgentSpan span, String port) { span.setTag(Tags.PEER_PORT, port); diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/PeerConnectionExtractor.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/PeerConnectionExtractor.java new file mode 100644 index 00000000000..7592961789e --- /dev/null +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/PeerConnectionExtractor.java @@ -0,0 +1,57 @@ +package datadog.trace.bootstrap.instrumentation.decorator; + +import static datadog.trace.bootstrap.instrumentation.java.net.HostNameResolver.hostName; + +import datadog.trace.api.Config; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.TagExtractor; +import datadog.trace.bootstrap.instrumentation.api.Tags; +import java.net.Inet4Address; +import java.net.Inet6Address; +import java.net.InetAddress; +import java.net.InetSocketAddress; + +/** + * Named singleton {@link TagExtractor} for peer-connection tags (hostname / IPv4 / IPv6 / port) + * from a remote {@link InetSocketAddress}. + * + *

Promoted from an inline lambda in {@link BaseDecorator} to a named class so it can be: + * referenced by name at call sites ({@code span.setTags(addr, PeerConnectionExtractor.INSTANCE)}), + * composed with other extractors (the axis the Decorator inheritance chain lacked), and + * given a home for the pure statics and the {@code hostName} resolver cache it consults. + * Non-capturing and stateless — the single {@link #INSTANCE} is effectively a static function + * object, so a monomorphic call site inlines it away. + */ +public final class PeerConnectionExtractor implements TagExtractor { + public static final PeerConnectionExtractor INSTANCE = new PeerConnectionExtractor(); + + private static final int UNSET_PORT = 0; + + private PeerConnectionExtractor() {} + + @Override + public void extract(final InetSocketAddress remoteConnection, final AgentSpan span) { + if (remoteConnection != null) { + setPeerAddress(span, remoteConnection.getAddress(), !remoteConnection.isUnresolved()); + final int port = remoteConnection.getPort(); + if (port > UNSET_PORT) { + span.setTag(Tags.PEER_PORT, port); + } + } + } + + static void setPeerAddress( + final AgentSpan span, final InetAddress remoteAddress, final boolean resolved) { + if (remoteAddress != null) { + String ip = remoteAddress.getHostAddress(); + if (resolved && Config.get().isPeerHostNameEnabled()) { + span.setTag(Tags.PEER_HOSTNAME, hostName(remoteAddress, ip)); + } + if (remoteAddress instanceof Inet4Address) { + span.setTag(Tags.PEER_HOST_IPV4, ip); + } else if (remoteAddress instanceof Inet6Address) { + span.setTag(Tags.PEER_HOST_IPV6, ip); + } + } + } +} From 175086bea0c99490c29500e49499d98e8340014e Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 1 Jul 2026 22:07:42 -0400 Subject: [PATCH 6/8] Trim onPeerConnection comment (review feedback) Co-Authored-By: Claude Opus 4.8 --- .../bootstrap/instrumentation/decorator/BaseDecorator.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java index 9e68a833f09..ea9efe59cf7 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java @@ -146,9 +146,7 @@ public ContextScope onError(final ContextScope scope, final Throwable throwable) public AgentSpan onPeerConnection( final AgentSpan span, final InetSocketAddress remoteConnection) { - // Invoke the extractor directly rather than via span.setTags(...): this legacy plumbing path - // is behavior-identical to the old inline code, and a direct call runs on test doubles too - // (the span-first span.setTags(...) sugar is for hand-written integration call sites). + // Direct extract (not span.setTags) so this legacy path also runs on mocked spans. PeerConnectionExtractor.INSTANCE.extract(remoteConnection, span); return span; } From 28b356adc9f8f525c8edfa9a3d466604d12faa9d Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Wed, 8 Jul 2026 10:11:39 -0400 Subject: [PATCH 7/8] Guard AgentSpan.setTags against a null source A null source is now a no-op, so a TagExtractor can assume a non-null source and integrations don't each repeat the null-check. Co-Authored-By: Claude Opus 4.8 --- .../trace/bootstrap/instrumentation/api/AgentSpan.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java index 4a92241097f..46257f93aeb 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java @@ -96,7 +96,9 @@ default boolean isValid() { * an implementer may override to do so. */ default AgentSpan setTags(final T source, final TagExtractor extractor) { - extractor.extract(source, this); + if (source != null) { + extractor.extract(source, this); + } return this; } From 162e185e1ed39a3d2df68693b8bb17b035d8036b Mon Sep 17 00:00:00 2001 From: Douglas Q Hawkins Date: Thu, 9 Jul 2026 06:54:27 -0400 Subject: [PATCH 8/8] Route peer-connection tagging through AgentSpan.setTags; drop extractor null-check onPeerConnection(InetSocketAddress) now calls span.setTags(remoteConnection, PeerConnectionExtractor.INSTANCE) instead of extracting directly, so the setTags default-method null-guard is the sole null check and the extractor assumes a non-null source (matching the TagExtractor contract, and slide 15's 'handles null automatically'). The mock-based 'test onPeerConnection' relied on the direct extract call (a Spock mock won't dispatch the setTags default method); migrated its coverage to a JUnit 5 PeerConnectionExtractorTest using literal IPs (no DNS). Co-Authored-By: Claude Opus 4.8 --- .../decorator/BaseDecorator.java | 3 +- .../decorator/PeerConnectionExtractor.java | 12 ++-- .../decorator/BaseDecoratorTest.groovy | 25 +------- .../PeerConnectionExtractorTest.java | 62 +++++++++++++++++++ 4 files changed, 71 insertions(+), 31 deletions(-) create mode 100644 dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/decorator/PeerConnectionExtractorTest.java diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java index ea9efe59cf7..96c70a91de4 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/BaseDecorator.java @@ -146,8 +146,7 @@ public ContextScope onError(final ContextScope scope, final Throwable throwable) public AgentSpan onPeerConnection( final AgentSpan span, final InetSocketAddress remoteConnection) { - // Direct extract (not span.setTags) so this legacy path also runs on mocked spans. - PeerConnectionExtractor.INSTANCE.extract(remoteConnection, span); + span.setTags(remoteConnection, PeerConnectionExtractor.INSTANCE); return span; } diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/PeerConnectionExtractor.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/PeerConnectionExtractor.java index 7592961789e..f7767054fe6 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/PeerConnectionExtractor.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/PeerConnectionExtractor.java @@ -31,12 +31,12 @@ private PeerConnectionExtractor() {} @Override public void extract(final InetSocketAddress remoteConnection, final AgentSpan span) { - if (remoteConnection != null) { - setPeerAddress(span, remoteConnection.getAddress(), !remoteConnection.isUnresolved()); - final int port = remoteConnection.getPort(); - if (port > UNSET_PORT) { - span.setTag(Tags.PEER_PORT, port); - } + // Non-null guaranteed by AgentSpan.setTags (the sole entry point); extractors skip + // null-handling. + setPeerAddress(span, remoteConnection.getAddress(), !remoteConnection.isUnresolved()); + final int port = remoteConnection.getPort(); + if (port > UNSET_PORT) { + span.setTag(Tags.PEER_PORT, port); } } diff --git a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/BaseDecoratorTest.groovy b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/BaseDecoratorTest.groovy index 5b70cba2085..1a8b0c3d66b 100644 --- a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/BaseDecoratorTest.groovy +++ b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/BaseDecoratorTest.groovy @@ -46,29 +46,8 @@ class BaseDecoratorTest extends DDSpecification { 0 * _ } - def "test onPeerConnection"() { - when: - decorator.onPeerConnection(span, connection) - - then: - if (!connection.isUnresolved()) { - 1 * span.setTag(Tags.PEER_HOSTNAME, connection.hostName) - } - 1 * span.setTag(Tags.PEER_PORT, connection.port) - if (connection.address instanceof Inet4Address) { - 1 * span.setTag(Tags.PEER_HOST_IPV4, connection.address.hostAddress) - } - if (connection.address instanceof Inet6Address) { - 1 * span.setTag(Tags.PEER_HOST_IPV6, connection.address.hostAddress) - } - 0 * _ - - where: - connection | _ - new InetSocketAddress("localhost", 888) | _ - new InetSocketAddress("ipv6.google.com", 999) | _ - InetSocketAddress.createUnresolved("bad.address.local", 999) | _ - } + // "test onPeerConnection" migrated to PeerConnectionExtractorTest (JUnit 5): onPeerConnection now + // routes through AgentSpan.setTags, a default method a Spock mock won't dispatch to the extractor. def "test onError"() { when: diff --git a/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/decorator/PeerConnectionExtractorTest.java b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/decorator/PeerConnectionExtractorTest.java new file mode 100644 index 00000000000..ac0ccd09255 --- /dev/null +++ b/dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/instrumentation/decorator/PeerConnectionExtractorTest.java @@ -0,0 +1,62 @@ +package datadog.trace.bootstrap.instrumentation.decorator; + +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.Tags; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.UnknownHostException; +import org.junit.jupiter.api.Test; + +/** + * Unit tests for {@link PeerConnectionExtractor#extract}. Covers the deterministic IP-family and + * port routing; the config/resolver-dependent {@code PEER_HOSTNAME} tag is intentionally not + * asserted (that behaviour belongs to the host-name resolver, and peer-hostname is on by default, + * so it would couple this test to reverse-DNS). Null source handling is the responsibility of + * {@code AgentSpan.setTags} (the extractor's sole entry point), not the extractor. + * + *

Literal IPs are used so no DNS lookup happens. + */ +class PeerConnectionExtractorTest { + + @Test + void ipv4AddressSetsIpv4TagAndPort() throws UnknownHostException { + final AgentSpan span = mock(AgentSpan.class); + final InetAddress addr = InetAddress.getByName("1.2.3.4"); // literal -> no DNS + + PeerConnectionExtractor.INSTANCE.extract(new InetSocketAddress(addr, 8080), span); + + verify(span).setTag(Tags.PEER_HOST_IPV4, addr.getHostAddress()); + verify(span).setTag(Tags.PEER_PORT, 8080); + verify(span, never()).setTag(eq(Tags.PEER_HOST_IPV6), anyString()); + } + + @Test + void ipv6AddressSetsIpv6TagAndPort() throws UnknownHostException { + final AgentSpan span = mock(AgentSpan.class); + final InetAddress addr = InetAddress.getByName("2001:db8::1"); // literal -> no DNS + + PeerConnectionExtractor.INSTANCE.extract(new InetSocketAddress(addr, 9090), span); + + verify(span).setTag(Tags.PEER_HOST_IPV6, addr.getHostAddress()); + verify(span).setTag(Tags.PEER_PORT, 9090); + verify(span, never()).setTag(eq(Tags.PEER_HOST_IPV4), anyString()); + } + + @Test + void unresolvedAddressSetsOnlyPort() { + final AgentSpan span = mock(AgentSpan.class); + + PeerConnectionExtractor.INSTANCE.extract( + InetSocketAddress.createUnresolved("no.dns.local", 999), span); + + verify(span).setTag(Tags.PEER_PORT, 999); + verify(span, never()).setTag(eq(Tags.PEER_HOST_IPV4), anyString()); + verify(span, never()).setTag(eq(Tags.PEER_HOST_IPV6), anyString()); + } +}