Merge the publish branch into main - #26
Merged
Merged
Conversation
…ser-sdk into feat/publish_flashcat
feat: remove dd trace type
…tion chore: restore release safety baseline
…ATH precedence - pageActivationObservable: ignore element-level focus/blur captured at window; only window-targeted events indicate page (de)activation. Fixes spurious view_change full snapshots when moving focus between inputs (broke recorder masking e2e). - ci.yml: scope BUILD_MODE=release to the bundle build step so script tests run in dev mode as their specs expect. - scripts/cli: prepend node_modules/.bin to PATH so repo-pinned tsc wins over globally installed versions on CI runners. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
rumViewEvents[1] is order-dependent: a view emits several update events and custom_timings only appear on later updates, which made the assertion flaky on CI. Resolve views by id and assert on the update with the highest document_version, like the manually-tracked scenario. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
fix: CI baseline follow-up — replay focus misfire, BUILD_MODE leak, PATH precedence
check-release.js verifies the release tag is on HEAD, but actions/checkout@v4 does not fetch tags by default, so the release preflight failed to resolve v0.0.5. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Initial view metrics (FCP/LCP/FID/loading time) are anchored to the page navigation start (`clocksOrigin()`) and only collected on the INITIAL_LOAD view. For pages that load in the background or are pre-warmed and kept hidden (e.g. a hidden Electron BrowserWindow that only renders content much later via IPC), LCP ends up measured from the original navigation and is reported as an abnormally large value. The existing safeguards (`firstHidden`, the 10-minute cap) don't help because such hidden windows don't reliably report `document.visibilityState === 'hidden'`. Add a `trackWebVitals` init option (default `true`). When set to `false`, `trackInitialViewMetrics` is not started, so no FCP/LCP/FID/loading-time is collected for that SDK instance — the field is simply absent rather than a misleading value. Like `profilingSampleRate` and `propagateTraceBaggage`, this fork-specific option is not part of the upstream telemetry schema, so it is intentionally excluded from `serializeRumConfiguration` (the generated telemetry types must not be hand-edited). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
feat(rum): add trackWebVitals config to opt out of initial view metrics
…dge is present Add a `sessionReplayDirectUpload` init option. When the SDK runs inside a host application that injects `DatadogEventBridge` (an Electron renderer process, a mobile WebView), upstream hands Session Replay over to the host, and drops it entirely when the host does not declare the `records` capability. Our Electron SDK never takes it over, so Session Replay is simply lost there. The option makes the page keep the regular recorder and upload the segments itself, over the same intake connection a plain web page uses. The intake needs no change: the payload, the endpoint builder (including `proxy`) and the authentication are the ones a web page already uses. Three condition changes, marked `FLASHCAT FORK (n/3)`: 1. `recorderApi.ts` - the bridge check moved from `makeRecorderApi()` to `onRumStart()`, since it now depends on a configuration option that is not known when `makeRecorderApi()` runs. `makeRecorderApi()` keeps only the browser support check. 2. `startRecording.ts` - records go to `startSegmentCollection` instead of `startRecordBridge`. 3. `rumSessionManager.ts` - the session is `SAMPLED` so views carry `session.has_replay`, which is what the read side keys on. Segments are joined to a session by `(account, session id)`, and nothing overwrites the session id of a segment uploaded from the page. The stub session manager therefore reads the id the host actually uses through two new optional bridge methods, `getSessionId()` and `getAnonymousId()`, falling back to the placeholder id when a host built against an older SDK does not implement them. `getAnonymousId()` also fixes the anonymous id being always empty in a host application, which made unique user counts always zero. `sessionReplaySampleRate` still defaults to 0, so it must be set explicitly for anything to be recorded. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… host bridge Two follow-ups from the first end-to-end round on Electron. Replay segments were all rejected with 401 and nothing reached storage. `overrideInitConfigurationForBridge()` replaces `clientToken` with the literal `'empty'` and `applicationId` with a placeholder, on the assumption that a page hosting a bridge never sends a request of its own. `sessionReplayDirectUpload` breaks that assumption, so both are now kept when the option is set. Verified by comparison during the round: same URL and payload, `dd-api-key=empty` gives 401, the real token gives 400, and a well formed segment with the real token and the real application id gives 202 and lands with an `acct_id`/`session_id` matching its session row. RUM events are unaffected, as they never go over HTTP. Giving the stub session an `anonymousId` also activated the `usr.id` backfill in `userContext.ts`, and every renderer process event started carrying `usr.id = anonymous_id`. That backfill is a Flashcat addition that the web needs, because the web baseline counts only `usr.id` and would otherwise miss every logged-out visitor. A host application does not need it: its baseline already prefers the anonymous id, which is stable across logins, so the copy only imports the web double-counting bug where one device counts as two users around a login. It is now skipped when a bridge provides the anonymous id, keyed on `canUseEventBridge()` rather than on the option, since the id is host-owned whenever a bridge is present. Web behavior is byte for byte unchanged, and a regression test pins it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The sessionReplayDirectUpload markers were numbered out of three while a fourth site already existed, and the userContext and rumSessionManager identity changes were tagged with them even though they belong to the host-provided identifier work and are reachable without the option. Number the four option sites 1/4..4/4 and point the identity comments at DatadogEventBridge instead.
The implementation folds an empty answer into "not implemented" with `||`, and host applications rely on that to say "no session right now" — but the interface did not say so, leaving the next reader free to tighten it to `??` and hand the rest of the SDK an empty session id.
A host application that owns the session id answers `''` through the bridge while it has none — a session that timed out and has not been renewed yet. The bridge wrapper folded that empty answer into `undefined`, the same value it returns for a host that does not implement `getSessionId()` at all, and the session manager stub then fell back to `STUB_SESSION_ID`. RUM events survive that: the host overrides their session id, or drops them. But Session Replay segments are uploaded by this page directly, so they reached the intake carrying a placeholder that every application built on the host shares. Records collected during the gap were both lost to the real session and mixed into a fake one shared across applications. The two answers are now distinct. `undefined` still means "this host does not answer for the session id" and still falls back to the placeholder. `''` means "this host has no session right now", and the stub reports no tracked session for it: `computeSegmentContext` then finds no session and segment creation stops, which is the behaviour segmentCollection already documents for a session that ends. Reusing the id the host held a moment ago would be just as wrong — it would attribute records to a session that has ended — so nothing is cached. The stub also watches the host session. The bridge is pull-only, so reading it again is the only way to notice a transition; that is how the regular session manager works too, polling its own store every STORAGE_POLL_DELAY and turning what it sees into expire/renew notifications. Mirroring it lets everything already subscribed to SESSION_EXPIRED / SESSION_RENEWED behave: the recorder flushes its pending segment and stops instead of holding it open across the end of the host session, and restarts on a fresh view — and so a fresh full snapshot, without which the next segment would be unplayable — once the host has a session again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…-upload-publish feat(rum): keep collecting Session Replay in the page when a host bridge is present
The release preflight runs the TypeScript compatibility check, so this has been blocking every release, whatever its contents, since TypeScript 7 became `latest`. Two things broke, in sequence. Yarn 4.9.1 could not install TypeScript 7 at all: it applies a builtin compatibility patch to the package, and that patch cannot be applied to a TypeScript this new, so `yarn add` failed inside `patchPackage` before any type checking happened. Yarn 4.17.1 installs it cleanly. Past that, the check itself was asking for a configuration that no longer exists. TypeScript 7 removed `target: es5` and `moduleResolution: node10`, the two options the test app's tsconfig is built on — so the three checks that run against `latest` were compiling our type definitions under a setup nobody on a current TypeScript can have. They now override those two options with what such a consumer uses instead, at the oldest target still available, which keeps what the older pinned checks are there to prove. `ignoreDeprecations: '6.0'` goes with them: the options it silenced are gone rather than deprecated, so it no longer has anything to do. The lockfile bump is yarn writing its current format, and the ignore entry is for a file yarn writes into the test apps by itself.
`yarn install --immutable`, which every CI job starts with, refuses an install that would rewrite the lockfile — and the new yarn rewrites this one: it bumps the format version and re-hashes the builtin `resolve` patch. Missed on the first pass because building and testing locally reuses an existing `node_modules` and never re-runs the install the way CI does.
Running the new yarn rewrote `.yarnrc.yml` on its own: it dropped the comment explaining why the global cache is off, and added `approvedGitRepositories: "**"`, `enableScripts: true` and `npmMinimalAgeGate: 0` — the last two of which turn off a supply chain protection and let any git host serve a dependency. None of it belongs in a toolchain upgrade, and none of it is needed: `yarn install --immutable` and all six compatibility checks pass with the file as it was, plus the new yarn path. The ignore entry goes with them. The file it covered is written into the test apps by yarn only when the settings above are set at the root, so with those gone nothing generates it, and ignoring it would only have hidden them coming back.
Yarn 4.17 stopped running them by default, where 4.9 ran them, and the e2e suite depends on one: puppeteer fetches the Chrome it drives from its own install script. Without it the suite fails on every run looking for a browser nothing ever downloaded. Restoring it keeps the upgrade to the toolchain. Leaving scripts off is a worthwhile change on its own, but it needs the browser fetched some other way first, and that is not this.
fix(ci): unblock releases from TypeScript 7
Two things stood between a tag and a release, and both had to be worked around by hand every time. Checkout failed before any job did real work. This workflow is triggered *by* the tag, so the tag is the ref being checked out and is already present — asking for `fetch-tags` on top makes checkout fetch the commit and the tag into the same `refs/tags/*`, which git refuses. That took down every tag-triggered release from v0.0.5 onward; v0.0.5, v0.0.6 and v0.0.7 all went out through the manual workflow instead. `deploy-manual.yml` keeps the flag, and needs it: it runs from a branch, where the tag is not the ref. `yarn release` failed before that, on the machine running it. `lerna version` starts with `git remote update`, which fetches every remote and recurses into their submodules; branches on the upstream fork reference `rum-events-format` commits its own remote does not serve. Nothing in a release reads upstream, so it is now kept out of that update — from the release command itself, so it holds on any clone rather than on whichever machine happened to be configured.
fix(ci): make a pushed tag actually release again
The two lines had drifted far apart: publish carries the releases (npm is at the version it holds, and the release tags are on it) along with the repairs that keep a release working, while main had the newer feature work. Reconciling them makes either branch a place you can release from and stops the next person guessing which one is true. Three files needed a decision. The deploy workflows exist on both sides: publish rebuilt them into a preflight job that validates the tag against the package version and hands a verified bundle to the deploy job, which is strictly better than what main had, so that shape wins. The ES5 compatibility gate main added is put back into it, after the build, where every bundle exists and the check that the modern bundles are rejected still means something. The lockfile is regenerated rather than kept as the text merge left it - a merged lockfile is not a lockfile, and the merged one failed --immutable. One spec still carried a hardcoded internal staging address, which is also why it failed once the two sides met: the host had been scrubbed from the source on one side and not from the fixture on the other. It derives the host from the constant now, so the next change to it cannot leave the fixture behind. The bundle sync script and its documentation followed the old layout, where a release overwrote the directory of its major version. The pipeline this merge adopts publishes each release to its own directory, so a url pins the version it names, and the script defaults to the full version.
Merging the two lines brought a CI workflow main did not have, and it runs checks main never ran. Two of them fail, both because this package was added while nothing was looking: The package metadata check requires an .npmignore per package, and this one had none. It follows the convention the others use — exclude everything, then name what ships — which here is the bundle alone. The deploy and source map specs still described three packages. Adding a fourth changed what those scripts do without changing what the specs expected of them, so every one of them failed. They now expect the legacy bundle to be uploaded, renamed and invalidated alongside the others. This was raised in review earlier and set aside because these specs did not run anywhere. They do now.
The previous attempt inserted the legacy command inside the object that held the slim one, which is two strings in a row and parses as nothing. Each expectation is now copied as a whole and rewritten.
The repository-wide typecheck compiles the specs of every package together, which main's CI never did. Two things only show up there: A spec elsewhere augments the global Window with its own type for the FC_RUM property, so an interface here extending Window with a different type for it is two declarations of one global disagreeing. This one no longer extends Window; it only needs somewhere to put the api. Two event assembly fixtures predate the view start time being part of a view's context and had not been given one. The source map expectations also had the legacy bundle renamed between the slim bundle's script and its source map, rather than after both. The renames happen a package at a time.
The fixture gained a start time, which dates the event rather than travelling inside the view object, so comparing the whole fixture to what was assembled started failing.
The clock was frozen after the view had already opened, so the view read the real wall clock for its start and the test read a frozen one two seconds later - a millisecond between the two lines was enough to fail it. It froze once on a CI runner and would have again.
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
2 participants
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.
mainandpublishhad drifted far apart:publishcarries the releases — npm is at the version it holds and the release tags are on it — along with the repairs that keep a release working and a CI workflowmaindoes not have, whilemaincarried the newer feature work. Reconciling them makes either branch a place you can release from, and stops the next person having to work out which one is true.What this measured
The unit suite on
maintoday ends with 42 failures. On this merge it is green: 2861 specs, no failures. Those failures were not fork drift, which is what they looked like frommain— they are already fixed onpublish.The three things that needed a decision
The deploy workflows were the only conflict, on all three files.
publishrebuilt them into a preflight job that validates the pushed tag against the package version and hands a verified bundle to the deploy job. That shape wins. The ES5 compatibility gate is put back into it after the build, where every bundle exists and the assertion that the modern bundles are rejected as ES5 still means something.The lockfile is regenerated, not kept as the text merge left it. The merged one fails
yarn install --immutable; the regenerated one passes and differs frompublish's by the entries this branch actually adds.One spec still carried a hardcoded internal staging address, which is why it failed once the two sides met: the host had been scrubbed from the source on one side and not from the fixture on the other. It derives the host from the constant now, so the next change cannot leave the fixture behind. That was the last occurrence of the address in this repository.
One correction that rides along
The bundle sync script and its documentation described the old layout, where a release overwrote the directory of its major version. The pipeline this merge adopts publishes each release into its own directory, so a url pins the version it names. Verified against the CDN: both layouts exist there and the major-version directory is stale. The script now defaults to the full version, and the documented urls name a release.