-
-
Notifications
You must be signed in to change notification settings - Fork 675
Feat dns interceptor storage #4589
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 7 commits
d35fb9e
2c156e1
128125f
667f956
3689532
5b1d923
5a1fc07
238f0b3
47d5739
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -1043,6 +1043,7 @@ The `dns` interceptor enables you to cache DNS lookups for a given duration, per | |||||||
| - The function should return a single record from the records array. | ||||||||
| - By default a simplified version of Round Robin is used. | ||||||||
| - The `records` property can be mutated to store the state of the balancing algorithm. | ||||||||
| - `storage: { size: number, get(origin: string): DNSInterceptorOriginRecords | null, set(origin: string, records: DNSInterceptorOriginRecords | null, options?: { ttl: number }): void, delete(origin: string): void, full(): boolean }` - Custom storage for resolved DNS records | ||||||||
|
|
||||||||
| > The `Dispatcher#options` also gets extended with the options `dns.affinity`, `dns.dualStack`, `dns.lookup` and `dns.pick` which can be used to configure the interceptor at a request-per-request basis. | ||||||||
|
|
||||||||
|
|
@@ -1073,6 +1074,45 @@ const response = await client.request({ | |||||||
| }) | ||||||||
| ``` | ||||||||
|
|
||||||||
| **Example - DNS Interceptor and LRU cache as a storage** | ||||||||
|
|
||||||||
| ```js | ||||||||
| const { Client, interceptors } = require("undici"); | ||||||||
| const QuickLRU = require("quick-lru"); | ||||||||
| const { dns } = interceptors; | ||||||||
|
|
||||||||
| const lru = new QuickLRU({ maxSize: 100 }); | ||||||||
|
|
||||||||
| const lruAdapter = { | ||||||||
| get size() { | ||||||||
| return lru.size; | ||||||||
| }, | ||||||||
| get(origin) { | ||||||||
| return lru.get(origin); | ||||||||
| }, | ||||||||
| set(origin, records, { ttl }) { | ||||||||
|
||||||||
| set(origin, records, { ttl }) { | |
| set(origin, records, options = {}) { | |
| const { ttl } = options; |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,14 +5,44 @@ const DecoratorHandler = require('../handler/decorator-handler') | |||||||||||||
| const { InvalidArgumentError, InformationalError } = require('../core/errors') | ||||||||||||||
| const maxInt = Math.pow(2, 31) - 1 | ||||||||||||||
|
|
||||||||||||||
| class DNSStorage { | ||||||||||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
const cache = new LRU({ max: maxItems })
const adapter = {
set: (hostname: string, records: any, opts: { ttl: number }): void => {
cache.set(hostname, records, opts);
},
get: (hostname: string): any => {
return cache.get(hostname);
},
delete: (hostname: string) => {
cache.delete(hostname);
},
full: (): boolean => {
return false;
},
};
const interceptor = interceptors.dns({ maxItems: Infinity, maxTTL: 60000, cache: adapter })
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, if you will be interesting about lru-cache with integrated prometheus metrics, here is our implementation:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cache metrics was a veeeery useful feature for us. But mostly for HTTP requests ofc) |
||||||||||||||
| #maxItems = 0 | ||||||||||||||
| #records = new Map() | ||||||||||||||
|
|
||||||||||||||
| constructor (opts) { | ||||||||||||||
| this.#maxItems = opts.maxItems | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| get size () { | ||||||||||||||
| return this.#records.size | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| get (hostname) { | ||||||||||||||
| return this.#records.get(hostname) ?? null | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| set (hostname, records) { | ||||||||||||||
| this.#records.set(hostname, records) | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| delete (hostname) { | ||||||||||||||
| this.#records.delete(hostname) | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // Delegate to storage decide can we do more lookups or not | ||||||||||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For example, lru-cache adapter can always return |
||||||||||||||
| full () { | ||||||||||||||
| return this.size >= this.#maxItems | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| class DNSInstance { | ||||||||||||||
| #maxTTL = 0 | ||||||||||||||
| #maxItems = 0 | ||||||||||||||
| #records = new Map() | ||||||||||||||
| dualStack = true | ||||||||||||||
| affinity = null | ||||||||||||||
| lookup = null | ||||||||||||||
| pick = null | ||||||||||||||
| storage = null | ||||||||||||||
|
|
||||||||||||||
| constructor (opts) { | ||||||||||||||
| this.#maxTTL = opts.maxTTL | ||||||||||||||
|
|
@@ -21,17 +51,14 @@ class DNSInstance { | |||||||||||||
| this.affinity = opts.affinity | ||||||||||||||
| this.lookup = opts.lookup ?? this.#defaultLookup | ||||||||||||||
| this.pick = opts.pick ?? this.#defaultPick | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| get full () { | ||||||||||||||
| return this.#records.size === this.#maxItems | ||||||||||||||
| this.storage = opts.storage ?? new DNSStorage(opts) | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| runLookup (origin, opts, cb) { | ||||||||||||||
| const ips = this.#records.get(origin.hostname) | ||||||||||||||
| const ips = this.storage.get(origin.hostname) | ||||||||||||||
|
|
||||||||||||||
| // If full, we just return the origin | ||||||||||||||
| if (ips == null && this.full) { | ||||||||||||||
| if (ips == null && this.storage.full()) { | ||||||||||||||
| cb(null, origin) | ||||||||||||||
| return | ||||||||||||||
| } | ||||||||||||||
|
|
@@ -55,7 +82,7 @@ class DNSInstance { | |||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| this.setRecords(origin, addresses) | ||||||||||||||
| const records = this.#records.get(origin.hostname) | ||||||||||||||
| const records = this.storage.get(origin.hostname) | ||||||||||||||
|
|
||||||||||||||
| const ip = this.pick( | ||||||||||||||
| origin, | ||||||||||||||
|
|
@@ -89,7 +116,7 @@ class DNSInstance { | |||||||||||||
|
|
||||||||||||||
| // If no IPs we lookup - deleting old records | ||||||||||||||
| if (ip == null) { | ||||||||||||||
| this.#records.delete(origin.hostname) | ||||||||||||||
| this.storage.delete(origin.hostname) | ||||||||||||||
| this.runLookup(origin, opts, cb) | ||||||||||||||
| return | ||||||||||||||
| } | ||||||||||||||
|
|
@@ -193,7 +220,7 @@ class DNSInstance { | |||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| pickFamily (origin, ipFamily) { | ||||||||||||||
| const records = this.#records.get(origin.hostname)?.records | ||||||||||||||
| const records = this.storage.get(origin.hostname)?.records | ||||||||||||||
| if (!records) { | ||||||||||||||
| return null | ||||||||||||||
| } | ||||||||||||||
|
|
@@ -227,11 +254,13 @@ class DNSInstance { | |||||||||||||
| setRecords (origin, addresses) { | ||||||||||||||
| const timestamp = Date.now() | ||||||||||||||
| const records = { records: { 4: null, 6: null } } | ||||||||||||||
| let minTTL = this.#maxTTL | ||||||||||||||
|
||||||||||||||
| let minTTL = this.#maxTTL | |
| let minTTL = Infinity |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For LRU or any other caches, which will not be able to respect ttl per record, just pass the minimum value from records.
Can't find better solution because records is a mutable object and we need to store the same for every origin,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be fine to keep it as is, this just duplicates it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is to find minimum TTL for all resolved records for origin, and pass it to external storage, I don't quite understand about duplication
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah ok see it now, missed that bit
Copilot
AI
Sep 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The storage interface documentation shows that the ttl parameter is optional, but this code always passes it. Consider checking if the storage implementation supports TTL before passing the options parameter to avoid potential issues with storage implementations that don't expect additional parameters.
| this.storage.set(origin.hostname, records, { ttl: minTTL }) | |
| if (typeof this.storage.set === 'function' && this.storage.set.length >= 3) { | |
| this.storage.set(origin.hostname, records, { ttl: minTTL }) | |
| } else { | |
| this.storage.set(origin.hostname, records) | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The third argument is not used isn't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can be used in external storages, if lru-cache or quick-lru will be used under the hood
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,14 +19,22 @@ declare namespace Interceptors { | |
|
|
||
| // DNS interceptor | ||
| export type DNSInterceptorRecord = { address: string, ttl: number, family: 4 | 6 } | ||
| export type DNSInterceptorOriginRecords = { 4: { ips: DNSInterceptorRecord[] } | null, 6: { ips: DNSInterceptorRecord[] } | null } | ||
| export type DNSInterceptorOriginRecords = { records: { 4: { ips: DNSInterceptorRecord[] } | null, 6: { ips: DNSInterceptorRecord[] } | null } } | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here was incorrect typings - cached records shape is |
||
| export type DNSStorage = { | ||
| size: number | ||
| get(origin: string): DNSInterceptorOriginRecords | null | ||
| set(origin: string, records: DNSInterceptorOriginRecords | null, options?: { ttl: number }): void | ||
| delete(origin: string): void | ||
| full(): boolean | ||
| } | ||
| export type DNSInterceptorOpts = { | ||
| maxTTL?: number | ||
| maxItems?: number | ||
| lookup?: (hostname: string, options: LookupOptions, callback: (err: NodeJS.ErrnoException | null, addresses: DNSInterceptorRecord[]) => void) => void | ||
| lookup?: (origin: URL, options: LookupOptions, callback: (err: NodeJS.ErrnoException | null, addresses: DNSInterceptorRecord[]) => void) => void | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here also was incorrect typings - before |
||
| pick?: (origin: URL, records: DNSInterceptorOriginRecords, affinity: 4 | 6) => DNSInterceptorRecord | ||
| dualStack?: boolean | ||
| affinity?: 4 | 6 | ||
| storage?: DNSStorage | ||
| } | ||
|
|
||
| export function dump (opts?: DumpInterceptorOpts): Dispatcher.DispatcherComposeInterceptor | ||
|
|
||

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's decompose it into its own section within the
DNSinterceptor itself