Extract peer-connection tagging into a TagExtractor<InetSocketAddress>#11822
Draft
dougqh wants to merge 3 commits into
Draft
Extract peer-connection tagging into a TagExtractor<InetSocketAddress>#11822dougqh wants to merge 3 commits into
dougqh wants to merge 3 commits into
Conversation
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
dougqh
force-pushed
the
dougqh/peer-connection-extractor
branch
from
4601b3b to
07dc157
Compare
Contributor
|
🎯 Code Coverage (details) 🔗 Commit SHA: 175086b | Docs | Datadog PR Page | Give us feedback! |
Contributor
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
dougqh
commented
Jul 2, 2026
| onPeerConnection(span, remoteConnection.getAddress(), !remoteConnection.isUnresolved()); | ||
| setPeerPort(span, remoteConnection.getPort()); | ||
| } | ||
| // Invoke the extractor directly rather than via span.setTags(...): this legacy plumbing path |
Contributor
Author
There was a problem hiding this comment.
We probably don't need this much commenting here
dougqh
commented
Jul 2, 2026
| @@ -150,10 +146,10 @@ public ContextScope onError(final ContextScope scope, final Throwable throwable) | |||
|
|
|||
| public AgentSpan onPeerConnection( | |||
Contributor
Author
There was a problem hiding this comment.
Eventually, I think we'll want onPeerConnection to simply go away, but maybe not in this PR.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Second
TagExtractorcanary (stacked on #11817). Lifts the pure peer-connection field→tag logic —peer.hostname/peer.ipv4/peer.ipv6/peer.port— out ofBaseDecorator.onPeerConnection(...)intoPeerConnectionExtractor, a named singletonTagExtractor<InetSocketAddress>.Why this one
TagExtractorcase — extraction from a JDK type we don't own (InetSocketAddress) — complementing the JDBC canary's intrinsic case (our ownDBInfo). Together they exercise both shapes of the API.hostName(...)resolver cache becomes a class static the extractor consults (not lambda-captured state) — which is exactly why it's promoted to a named class.Named singleton class
Promoted from an inline lambda to
PeerConnectionExtractor implements TagExtractor<InetSocketAddress>with a private-ctorINSTANCE. Buys a name at call sites (span.setTags(addr, PeerConnectionExtractor.INSTANCE)), a home for the pure statics + the resolver cache, and composability — the axis the Decorator inheritance chain lacked. This is the convention the rest of the dissolution follows.Scope / honesty
onPeerConnectionis a concrete shared base method (52 call sites), not a megamorphic template method — so this is a structural dissolution step and a demonstrator of the extrinsic pattern, largely perf-neutral (it's not one of the per-leaf overridden dispatch sites where the megamorphic cost lives). The dispatch win comes later, from the overridden template methods (see #11823).Behavior preservation
Identical tag output. Internal plumbing invokes
extract()directly (so it runs on Spock test doubles too — a default-methodspan.setTagswouldn't); the span-firstspan.setTags(source, extractor)sugar is reserved for hand-written integration call sites.BaseDecoratorTest/ClientDecoratorTest/ServerDecoratorTestpass.🤖 Generated with Claude Code