Skip to content

dgram: skip dns.lookup() for literal IP addresses#64133

Open
BridgeAR wants to merge 1 commit into
nodejs:mainfrom
BridgeAR:BridgeAR/2026-06-25-dgram-skip-ip-default
Open

dgram: skip dns.lookup() for literal IP addresses#64133
BridgeAR wants to merge 1 commit into
nodejs:mainfrom
BridgeAR:BridgeAR/2026-06-25-dgram-skip-ip-default

Conversation

@BridgeAR

@BridgeAR BridgeAR commented Jun 25, 2026

Copy link
Copy Markdown
Member

dgram resolved the destination through dns.lookup() on every unconnected
send() and on the implicit first-send bind, even when the destination 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 default dns.lookup() 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.

Refs: DataDog/dd-trace-js#2984

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/net
  • @nodejs/performance

@nodejs-github-bot nodejs-github-bot added dgram Issues and PRs related to the dgram subsystem / UDP. needs-ci PRs that need a full CI run. labels Jun 25, 2026
@BridgeAR BridgeAR force-pushed the BridgeAR/2026-06-25-dgram-skip-ip-default branch 2 times, most recently from 6486166 to 4f87304 Compare June 25, 2026 16:02
@BridgeAR BridgeAR marked this pull request as ready for review June 25, 2026 16:43
Comment thread doc/api/dgram.md
* `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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's leave it as is to keep it non-major for this PR 👍 .

@BridgeAR BridgeAR added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 25, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 25, 2026
@nodejs-github-bot

Copy link
Copy Markdown
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>
@BridgeAR BridgeAR force-pushed the BridgeAR/2026-06-25-dgram-skip-ip-default branch from 4f87304 to 5d63356 Compare June 25, 2026 18:02
@BridgeAR BridgeAR added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 25, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 25, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. dgram Issues and PRs related to the dgram subsystem / UDP. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants