✨[PANA-5156] Expose better session replay internal API#4018
Conversation
| export * from '../types' | ||
|
|
||
| export { serializeNode, serializeNode as serializeNodeWithId } from '../domain/record' | ||
| export { takeFullSnapshot, takeNodeSnapshot, serializeNode as serializeNodeWithId } from '../domain/record' |
There was a problem hiding this comment.
serializeNode() was just added in a previous PR in this PR stack, so nothing's using it yet. takeNodeSnapshot() is a superior alternative, so I've removed serializeNode() in favor of the new function. I plan to remove serializeNodeWithId(), too, once we've switched all callers over to the new APIs.
| doTakeFullSnapshot( | ||
| timeStampNow(), | ||
| SerializationKind.INITIAL_FULL_SNAPSHOT, | ||
| emitRecord, |
There was a problem hiding this comment.
We must be sure that doTakeFullSnapshot will always be synchronous for this to work.
There was a problem hiding this comment.
Indeed! In the near term, for better or worse, I think we'll be stuck with that limitation anyway, because if we yield, the DOM might mutate out from under us. (It'd be interesting to investigate a full snapshot algorithm that could tolerate that, though...)
Bundles Sizes Evolution
🚀 CPU PerformancePending... 🧠 Memory Performance
|
|
42d52d9 to
7dc5b2e
Compare
There was a problem hiding this comment.
I’m a bit confused. Why is there so much logic tied to internalAPI? FMU, internal.ts was meant to expose the underlying APIs as-is, not to add internal-specific code.
There was a problem hiding this comment.
This is the downside of dependency injection; we need to configure all the dependencies for this code! All of the logic is just calls to factory functions, or direct construction of "noop" versions of dependencies in cases where we don't need the full versions.
Motivation
The session replay code in the browser SDK currently exposes a small internal API. This API's primary use case is to support unit testing of Datadog-internal code that integrates with session replay in some way.
Today, unfortunately, this API is quite unstable and hard to use correctly. The reason is that the function we expose (
serializeNodeWithId) is too low-level. Using this function requires constructing a very complex configuration object, involving constants and functions with types that aren't exposed outside of the SDK code. Worse, the details of this configuration object have been changing quite a bit lately, so keeping up with the changes is becoming harder and harder.Instead of exposing an unstable internal function, let's define an explicit, stable internal API that callers in other repos can depend on. (Of course, stability is relative here; this API is explicitly not subject to semver.)
Changes
This PR exposes two new internal API functions:
takeFullSnapshot()takes a snapshot of the entire document.takeNodeSnapshot()snapshots the DOM subtree of an individual node.Both require no configuration at all, though you can pass in a partial
RumConfigurationto override the defaults if you want.I've added tests for these functions to demonstrate usage and show that they work as expected.
Checklist