✨ Support { includeDefaults: true } in trackResourceHeaders#4825
Conversation
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 0cd0389 | Docs | Datadog PR Page | Give us feedback! |
Allow including the default tracked resource headers within a custom
trackResourceHeaders array via a `{ includeDefaults: true }` entry,
removing the need to spread DEFAULT_TRACKED_RESOURCE_HEADERS manually.
Deprecate the DEFAULT_TRACKED_RESOURCE_HEADERS field on the DD_RUM
public
API in favor of the new sentinel (kept to avoid a breaking change).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2e08107 to
0cd0389
Compare
Bundles Sizes Evolution
|
| * trackResourceHeaders: [{ url: /\/api\//, name: 'server-timing', extractor: /dur=(\d+)/, location: 'response' }] | ||
| */ | ||
| trackResourceHeaders?: boolean | MatchHeader[] | undefined | ||
| trackResourceHeaders?: boolean | Array<MatchHeader | { includeDefaults: true }> | undefined |
There was a problem hiding this comment.
suggestion: make includeDefaults a MatchHeader option?
There was a problem hiding this comment.
I don't think it's a good idea, if we want to do this we either have to:
A)
Add includeDefaults field on type MatchHeader, then we may end up with non-sensical entries such as { name: 'x-my-header', includeDefaults: true}, we could make name optional, but we will have to handle possibly undefined matchHeader.name
What's nice in the current design is that validateAndBuildTrackResourceHeaders always resolves entries of the configuration option to a MatchHeader, which the rest of the SDK uses.
B)
We do something like type MatchHeader = HeaderMatcher | IncludeDefaultsOption, SDK uses internally the HeaderMatcher type. It adds a type whose name will be hard to not be confusing.
There was a problem hiding this comment.
I like the optional MatchHeader.name because:
-
it is more powerful, i.e. the user can include default headers for a specific URL only, or specific "location". Solution B only allows to include the default headers on all URLs and locations
-
it doesn't add a new data model/concept into the mix
What's nice in the current design is that validateAndBuildTrackResourceHeaders always resolves entries of the configuration option to a MatchHeader, which the rest of the SDK uses.
Having an optional name is trivial to handle, I would argue that it makes things even simpler. See this commit for illustration.
Motivation
Enabling
trackResourceHeaderswith custom matchers while still collecting the built-in defaults previously required spreadingDEFAULT_TRACKED_RESOURCE_HEADERSmanually:This is verbose and forces customers to import (NPM) or reference (CDN) the
DEFAULT_TRACKED_RESOURCE_HEADERSconstant. This change adds an ergonomic sentinel so the defaults can be pulled in inline.Changes
configuration.ts):validateAndBuildTrackResourceHeadersnow expands a{ includeDefaults: true }array entry intoDEFAULT_TRACKED_RESOURCE_HEADERSin place, preserving array order so it can be combined with custom matchers. A typedshouldIncludeDefaultHeaderspredicate keeps the union narrowing intact.{ includeDefaults: true }entry and updated the example to use it.DEFAULT_TRACKED_RESOURCE_HEADERSfield on theDD_RUMpublic API as@deprecatedin favor of the new sentinel. The field/value is kept to avoid a breaking change.New usage:
Test instructions
Automated
All
trackResourceHeaderstests pass (89/89), typecheck and lint are clean.Manual (end-to-end)
Start the dev server and create a temporary sandbox page that combines the sentinel with a custom matcher:
Then exercise it (dev-server URL from
yarn dev-server status):Expected output — the custom matcher (
x-custom-header) is captured on the request and the expanded defaults (content-type,content-length, …) are captured on the response:{"request_headers":{"x-custom-header":"hello"},"response_headers":{"content-type":"text/javascript; charset=utf-8","content-length":"2429864"}}Cleanup:
Checklist