dgram: skip dns.lookup() for literal IP addresses#64133
Open
BridgeAR wants to merge 1 commit into
Open
Conversation
Collaborator
|
Review requested:
|
6486166 to
4f87304
Compare
bengl
reviewed
Jun 25, 2026
| * `recvBufferSize` {number} Sets the `SO_RCVBUF` socket value. | ||
| * `sendBufferSize` {number} Sets the `SO_SNDBUF` socket value. | ||
| * `lookup` {Function} Custom lookup function. **Default:** [`dns.lookup()`][]. | ||
| When the default is used, a literal IP address of the socket's family |
Member
There was a problem hiding this comment.
Why only when the default is used? Is it because the previous behaviour would always call custom function, and not doing so would be breaking?
Member
Author
There was a problem hiding this comment.
I have a stacked PR that does it in general, I was just concerned about it being major if we do it for custom lookup methods, so I decided to split it. I am fine to have it as one PR.
Member
There was a problem hiding this comment.
Let's leave it as is to keep it non-major for this PR 👍 .
bengl
approved these changes
Jun 25, 2026
Collaborator
Every unconnected send(), and the implicit bind on first send, resolved the destination through dns.lookup() even when it was already a literal IP. The address resolves to itself, so the call is redundant, and tools that instrument dns.lookup() record a lookup for every datagram sent to an IP. Skip the resolver for a literal IP of the socket's family and report it on the next tick, keeping dns.lookup()'s asynchronous contract. A custom lookup function is still consulted for every address. Refs: DataDog/dd-trace-js#2984 Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>
4f87304 to
5d63356
Compare
bengl
approved these changes
Jun 25, 2026
Collaborator
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
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.
dgramresolved the destination throughdns.lookup()on every unconnectedsend()and on the implicit first-send bind, even when the destination wasalready a literal IP. The address resolves to itself, so the call is redundant,
and tools that instrument
dns.lookup()record a lookup for every datagramsent to an IP. Skip the default
dns.lookup()for a literal IP of the socket'sfamily and report it on the next tick, keeping
dns.lookup()'s asynchronouscontract. A custom
lookupfunction is still consulted.Refs: DataDog/dd-trace-js#2984