Skip to content

feat: populate dnslookup timing with actual DNS resolution time#743

Draft
Copilot wants to merge 2 commits intomasterfrom
copilot/fix-timing-dns-query
Draft

feat: populate dnslookup timing with actual DNS resolution time#743
Copilot wants to merge 2 commits intomasterfrom
copilot/fix-timing-dns-query

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 12, 2026

The dnslookup field in the Timing object was defined and initialized to 0 but never populated, making it impossible to distinguish DNS resolution time from overall connection time.

Changes

  • src/symbols.ts: Add kSocketDnsLookupTime symbol for storing DNS resolution timestamp on sockets
  • src/diagnosticsChannel.ts: Capture DNS lookup completion time via Socket.prototype.emit hook on the 'lookup' event (same pattern as the existing Socket.prototype.destroy hook). Compute dnslookup timing relative to request start in the undici:client:sendHeaders handler for first-request-on-socket
  • test/options.timing.test.ts: Assert dnslookup > 0 and dnslookup <= connected on first request; assert dnslookup === 0 on reused socket

Result

{
  "queuing": 2.781,
  "dnslookup": 9.038,
  "connected": 12.655,
  "requestHeadersSent": 13.389,
  "requestSent": 13.571,
  "waiting": 24.036,
  "contentDownload": 25.092
}

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • http://registry.npmjs.org:80/urllib/latest
    • Triggering command: /opt/hostedtoolcache/node/24.14.1/x64/bin/node /opt/hostedtoolcache/node/24.14.1/x64/bin/node --experimental-import-meta-resolve --require /home/REDACTED/work/urllib/urllib/node_modules/.pnpm/@voidzero-dev&#43;vite-plus-test@0.1.15_@arethetypeswrong&#43;core@0.18.2_@types&#43;node@22.19.15__7c6ae394869a5d43fd0cf7dc1d33dc1c/node_modules/@voidzero-dev/vite-plus-test/suppress-warnings.cjs --conditions node --conditions development /home/REDACTED/work/urllib/urllib/node_modules/.pnpm/@voidzero-dev&#43;vite-plus-test@0.1.15_@arethetypeswrong&#43;core@0.18.2_@types&#43;node@22.19.15__7c6ae394869a5d43fd0cf7dc1d33dc1c/node_modules/@voidzero-dev/vite-plus-test/dist/workers/forks.js (http block)
    • Triggering command: /opt/hostedtoolcache/node/24.14.1/x64/bin/node /opt/hostedtoolcache/node/24.14.1/x64/bin/node --experimental-import-meta-resolve --require /home/REDACTED/work/urllib/urllib/node_modules/.pnpm/@voidzero-dev&#43;vite-plus-test@0.1.15_@arethetypeswrong&#43;core@0.18.2_@types&#43;node@22.19.15__7c6ae394869a5d43fd0cf7dc1d33dc1c/node_modules/@voidzero-dev/vite-plus-test/suppress-warnings.cjs --conditions node --conditions development /home/REDACTED/work/urllib/urllib/node_modules/.pnpm/@voidzero-dev&#43;vite-plus-test@0.1.15_@arethetypeswrong&#43;core@0.18.2_@types&#43;node@22.19.15__7c6ae394869a5d43fd0cf7dc1d33dc1c/node_modules/@voidzero-dev/vite-plus-test/dist/workers/forks.js k 3899 on REDACTED-w (http block)
  • registry.npmmirror.com
    • Triggering command: /opt/hostedtoolcache/node/24.14.1/x64/bin/node /opt/hostedtoolcache/node/24.14.1/x64/bin/node --experimental-import-meta-resolve --require /home/REDACTED/work/urllib/urllib/node_modules/.pnpm/@voidzero-dev&#43;vite-plus-test@0.1.15_@arethetypeswrong&#43;core@0.18.2_@types&#43;node@22.19.15__7c6ae394869a5d43fd0cf7dc1d33dc1c/node_modules/@voidzero-dev/vite-plus-test/suppress-warnings.cjs --conditions node --conditions development /home/REDACTED/work/urllib/urllib/node_modules/.pnpm/@voidzero-dev&#43;vite-plus-test@0.1.15_@arethetypeswrong&#43;core@0.18.2_@types&#43;node@22.19.15__7c6ae394869a5d43fd0cf7dc1d33dc1c/node_modules/@voidzero-dev/vite-plus-test/dist/workers/forks.js (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI linked an issue Apr 12, 2026 that may be closed by this pull request
…ockets

Captures DNS lookup time via Socket.prototype.emit monkey-patch for the
'lookup' event. When the first request is sent on a socket, the DNS
lookup time is calculated relative to the request start time and stored
in the timing object.

Closes #577

Agent-Logs-Url: https://github.com/node-modules/urllib/sessions/efde376e-b12f-42f6-9490-2ce39519fe7c

Co-authored-by: fengmk2 <156269+fengmk2@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix timing differentiation for DNS query feat: populate dnslookup timing with actual DNS resolution time Apr 12, 2026
Copilot AI requested a review from fengmk2 April 12, 2026 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

timing 需要区分一下 DNS 查询

2 participants