Skip to content

Commit bb2e83e

Browse files
⚗ [RUMF-902] implement a new mutation observer (#810)
* 🏗 [RUMF-902] introduce serializationUtils This commit introduces a few utility functions to store and retrieve serialized node ids stored in DOM nodes. The equivalent tools (`mirror`, `INode`...) that are only used by the 'mutation' module are intentionally left as-is, and will be removed in a future PR when we'll enable the new mutation observer. Before this commit, some 'observer' strategies didn't check if `mirror.getId` returned a valid `id` before emitting a record. In practice it shouldn't happen, but in unit tests it did happen because we artificially emitted 'input' events on a DOM node that wasn't in the document. This is why some tests were slightly adjusted to reflect the reality more accurately. * ⚗ [RUMF-902] implement a new mutation observer * 🚩 [RUMF-902] use the new mutation observer via a FF * 👌 apply some suggestions from code review Co-authored-by: Bastien Caudan <bastien.caudan@datadoghq.com> * rename utils.spec to serializationUtils.spec * 👌 remove undefined properties when using existing nodes more thoroughly * 👌 remove the "handleMutations" function * 👌 make sure 'nodeIsIgnored' is only called with serialized nodes * 👌 rename a few things in the mutationObserver.spec * 👌 clarify some mutationObserver tests * 👌 remove useless uses of [0] for unused snapshot results * 👌 move and rename sortNodesByTopologicalOrder * 👌 remove unneeded record.spec unit test We chose to remove this test because its intent is a bit obscure and the new mutation observer has a good enough test coverage. I kept the test for the old mutation observer implementation since its coverage is still lacking. It will be removed when we enable the new mutation observer. * Update record spec test title Co-authored-by: Bastien Caudan <bastien.caudan@datadoghq.com> Co-authored-by: Bastien Caudan <bastien.caudan@datadoghq.com>
1 parent 270cff1 commit bb2e83e

File tree

18 files changed

+1274
-93
lines changed

18 files changed

+1274
-93
lines changed

packages/rum-recorder/src/boot/recorder.spec.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ describe('startRecording', () => {
1717
expectedCallsCount: number,
1818
callback: (calls: jasmine.Calls<HttpRequest['send']>) => void
1919
) => void
20+
let sandbox: HTMLElement
21+
let textField: HTMLInputElement
2022
let expectNoExtraRequestSendCalls: (done: () => void) => void
2123

2224
beforeEach(() => {
@@ -25,6 +27,12 @@ describe('startRecording', () => {
2527
}
2628
sessionId = 'session-id'
2729
viewId = 'view-id'
30+
31+
sandbox = document.createElement('div')
32+
document.body.appendChild(sandbox)
33+
textField = document.createElement('input')
34+
sandbox.appendChild(textField)
35+
2836
setupBuilder = setup()
2937
.withParentContexts({
3038
findView() {
@@ -57,6 +65,7 @@ describe('startRecording', () => {
5765
})
5866

5967
afterEach(() => {
68+
sandbox.remove()
6069
setMaxSegmentSize()
6170
setupBuilder.cleanup()
6271
})
@@ -85,7 +94,6 @@ describe('startRecording', () => {
8594
it('flushes the segment when its compressed data is getting too large', (done) => {
8695
setupBuilder.build()
8796
const inputCount = 150
88-
const textField = document.createElement('input')
8997
const inputEvent = createNewEvent('input', { target: textField })
9098
for (let i = 0; i < inputCount; i += 1) {
9199
// Create a random value harder to deflate, so we don't have to send too many events to reach

packages/rum-recorder/src/boot/recorder.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ export function startRecording(
2727

2828
const { stop: stopRecording, takeFullSnapshot } = record({
2929
emit: addRawRecord,
30+
useNewMutationObserver: configuration.isEnabled('new-mutation-observer'),
3031
})
3132

3233
lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, takeFullSnapshot)
Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { serializeNodeWithId, transformAttribute, IGNORED_NODE, snapshot } from './snapshot'
1+
import { serializeNodeWithId, transformAttribute, snapshot } from './snapshot'
22
export * from './types'
3+
export * from './serializationUtils'
34

4-
export { snapshot, serializeNodeWithId, transformAttribute, IGNORED_NODE }
5+
export { snapshot, serializeNodeWithId, transformAttribute }
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import { getSerializedNodeId, hasSerializedNode, setSerializedNode } from './serializationUtils'
2+
3+
describe('serialized Node storage in DOM Nodes', () => {
4+
describe('hasSerializedNode', () => {
5+
it('returns false for DOM Nodes that are not yet serialized', () => {
6+
expect(hasSerializedNode(document.createElement('div'))).toBe(false)
7+
})
8+
9+
it('returns true for DOM Nodes that have been serialized', () => {
10+
const node = document.createElement('div')
11+
setSerializedNode(node, {} as any)
12+
13+
expect(hasSerializedNode(node)).toBe(true)
14+
})
15+
})
16+
17+
describe('getSerializedNodeId', () => {
18+
it('returns undefined for DOM Nodes that are not yet serialized', () => {
19+
expect(getSerializedNodeId(document.createElement('div'))).toBe(undefined)
20+
})
21+
22+
it('returns the serialized Node id', () => {
23+
const node = document.createElement('div')
24+
setSerializedNode(node, { id: 42 } as any)
25+
26+
expect(getSerializedNodeId(node)).toBe(42)
27+
})
28+
})
29+
})
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import { SerializedNodeWithId } from './types'
2+
3+
export const IGNORED_NODE_ID = -2
4+
5+
export interface NodeWithSerializedNode extends Node {
6+
__sn: SerializedNodeWithId
7+
}
8+
9+
export function hasSerializedNode(n: Node): n is NodeWithSerializedNode {
10+
return '__sn' in n
11+
}
12+
13+
export function getSerializedNodeId(n: NodeWithSerializedNode): number
14+
export function getSerializedNodeId(n: Node): number | undefined
15+
export function getSerializedNodeId(n: Node) {
16+
return hasSerializedNode(n) ? n.__sn.id : undefined
17+
}
18+
19+
export function setSerializedNode(n: Node, serializeNode: SerializedNodeWithId) {
20+
;(n as Partial<NodeWithSerializedNode>).__sn = serializeNode
21+
}
22+
23+
export function nodeIsIgnored(n: NodeWithSerializedNode): boolean {
24+
return getSerializedNodeId(n) === IGNORED_NODE_ID
25+
}
26+
27+
export function nodeOrAncestorsIsIgnored(n: NodeWithSerializedNode) {
28+
let current: NodeWithSerializedNode | null = n
29+
while (current) {
30+
if (nodeIsIgnored(current)) {
31+
return true
32+
}
33+
// Since we serialize the document from the root, and any new node is only serialized if they
34+
// are added in a serialized node, we are guaranteed to have a serialized parent node here.
35+
current = current.parentNode as NodeWithSerializedNode | null
36+
}
37+
return false
38+
}

packages/rum-recorder/src/domain/rrweb-snapshot/snapshot.spec.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
/* eslint-disable max-len */
22
import { isIE } from '@datadog/browser-core'
3-
import { absoluteToStylesheet, IGNORED_NODE, serializeNodeWithId } from './snapshot'
3+
import { IGNORED_NODE_ID } from './serializationUtils'
4+
import { absoluteToStylesheet, serializeNodeWithId } from './snapshot'
45

56
describe('absolute url to stylesheet', () => {
67
const href = 'http://localhost/css/style.css'
@@ -95,10 +96,10 @@ describe('serializeNodeWithId', () => {
9596
expect(map).toEqual({})
9697
})
9798

98-
it('sets ignored serialized node id to IGNORED_NODE', () => {
99+
it('sets ignored serialized node id to IGNORED_NODE_ID', () => {
99100
const scriptElement = document.createElement('script')
100101
serializeNodeWithId(scriptElement, defaultOptions)
101-
expect((scriptElement as any).__sn).toEqual(jasmine.objectContaining({ id: IGNORED_NODE }))
102+
expect((scriptElement as any).__sn).toEqual(jasmine.objectContaining({ id: IGNORED_NODE_ID }))
102103
})
103104

104105
it('ignores script tags', () => {

packages/rum-recorder/src/domain/rrweb-snapshot/snapshot.ts

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
import { nodeShouldBeHidden } from '../privacy'
22
import { PRIVACY_ATTR_NAME, PRIVACY_ATTR_VALUE_HIDDEN } from '../../constants'
33
import { SerializedNode, SerializedNodeWithId, NodeType, Attributes, INode, IdNodeMap } from './types'
4+
import { getSerializedNodeId, hasSerializedNode, IGNORED_NODE_ID, setSerializedNode } from './serializationUtils'
45

56
const tagNameRegex = /[^a-z1-6-_]/
67

7-
export const IGNORED_NODE = -2
8-
98
let nextId = 1
109
function genId(): number {
1110
return nextId++
@@ -288,7 +287,7 @@ function lowerIfExists(maybeAttr: string | number | boolean): string {
288287
return (maybeAttr as string).toLowerCase()
289288
}
290289

291-
function isNodeIgnored(sn: SerializedNode): boolean {
290+
function nodeShouldBeIgnored(sn: SerializedNode): boolean {
292291
if (sn.type === NodeType.Comment) {
293292
// TODO: convert IE conditional comments to real nodes
294293
return true
@@ -369,33 +368,34 @@ export function serializeNodeWithId(
369368
): SerializedNodeWithId | null {
370369
const { doc, map, skipChild = false } = options
371370
let { preserveWhiteSpace = true } = options
372-
const _serializedNode = serializeNode(n, {
371+
const serializedNode = serializeNode(n, {
373372
doc,
374373
})
375-
if (!_serializedNode) {
374+
if (!serializedNode) {
376375
// TODO: dev only
377376
console.warn(n, 'not serialized')
378377
return null
379378
}
380379

381380
let id
382381
// Try to reuse the previous id
383-
if ('__sn' in n) {
384-
id = n.__sn.id
382+
if (hasSerializedNode(n)) {
383+
id = getSerializedNodeId(n)
385384
} else if (
386-
isNodeIgnored(_serializedNode) ||
385+
nodeShouldBeIgnored(serializedNode) ||
387386
(!preserveWhiteSpace &&
388-
_serializedNode.type === NodeType.Text &&
389-
!_serializedNode.isStyle &&
390-
!_serializedNode.textContent.replace(/^\s+|\s+$/gm, '').length)
387+
serializedNode.type === NodeType.Text &&
388+
!serializedNode.isStyle &&
389+
!serializedNode.textContent.replace(/^\s+|\s+$/gm, '').length)
391390
) {
392-
id = IGNORED_NODE
391+
id = IGNORED_NODE_ID
393392
} else {
394393
id = genId()
395394
}
396-
const serializedNode = Object.assign(_serializedNode, { id })
397-
;(n as INode).__sn = serializedNode
398-
if (id === IGNORED_NODE) {
395+
const serializedNodeWithId = serializedNode as SerializedNodeWithId
396+
serializedNodeWithId.id = id
397+
setSerializedNode(n, serializedNodeWithId)
398+
if (id === IGNORED_NODE_ID) {
399399
return null
400400
}
401401
map[id] = n as INode
@@ -407,8 +407,8 @@ export function serializeNodeWithId(
407407
}
408408
if ((serializedNode.type === NodeType.Document || serializedNode.type === NodeType.Element) && recordChild) {
409409
if (
410-
_serializedNode.type === NodeType.Element &&
411-
_serializedNode.tagName === 'head'
410+
serializedNode.type === NodeType.Element &&
411+
serializedNode.tagName === 'head'
412412
// would impede performance: || getComputedStyle(n)['white-space'] === 'normal'
413413
) {
414414
preserveWhiteSpace = false
@@ -425,7 +425,7 @@ export function serializeNodeWithId(
425425
}
426426
}
427427
}
428-
return serializedNode
428+
return serializedNodeWithId
429429
}
430430

431431
export function snapshot(n: Document): [SerializedNodeWithId | null, IdNodeMap] {

packages/rum-recorder/src/domain/rrweb/mutation.ts

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
11
import { monitor } from '@datadog/browser-core'
2-
import { IGNORED_NODE, INode, serializeNodeWithId, transformAttribute } from '../rrweb-snapshot'
2+
import {
3+
hasSerializedNode,
4+
IGNORED_NODE_ID,
5+
INode,
6+
nodeIsIgnored,
7+
serializeNodeWithId,
8+
transformAttribute,
9+
} from '../rrweb-snapshot'
310
import { nodeOrAncestorsShouldBeHidden } from '../privacy'
411
import {
512
AddedNodeMutation,
@@ -9,7 +16,7 @@ import {
916
RemovedNodeMutation,
1017
TextCursor,
1118
} from './types'
12-
import { forEach, isAncestorRemoved, isIgnored, mirror } from './utils'
19+
import { forEach, isAncestorRemoved, mirror } from './utils'
1320

1421
interface DoubleLinkedListNode {
1522
previous: DoubleLinkedListNode | null
@@ -184,8 +191,8 @@ export class MutationObserverWrapper {
184191
const addList = new DoubleLinkedList()
185192
const getNextId = (n: Node): number | null => {
186193
let ns: Node | null = n
187-
let nextId: number | null = IGNORED_NODE
188-
while (nextId === IGNORED_NODE) {
194+
let nextId: number | null = IGNORED_NODE_ID
195+
while (nextId === IGNORED_NODE_ID) {
189196
ns = ns && ns.nextSibling
190197
nextId = ns && mirror.getId((ns as unknown) as INode)
191198
}
@@ -308,7 +315,7 @@ export class MutationObserverWrapper {
308315
}
309316

310317
private processMutation = (m: MutationRecord) => {
311-
if (isIgnored(m.target)) {
318+
if (hasSerializedNode(m.target) && nodeIsIgnored(m.target)) {
312319
return
313320
}
314321
switch (m.type) {
@@ -344,7 +351,11 @@ export class MutationObserverWrapper {
344351
forEach(m.removedNodes, (n: Node) => {
345352
const nodeId = mirror.getId(n as INode)
346353
const parentId = mirror.getId(m.target as INode)
347-
if (nodeOrAncestorsShouldBeHidden(n) || nodeOrAncestorsShouldBeHidden(m.target) || isIgnored(n)) {
354+
if (
355+
nodeOrAncestorsShouldBeHidden(n) ||
356+
nodeOrAncestorsShouldBeHidden(m.target) ||
357+
(hasSerializedNode(n) && nodeIsIgnored(n))
358+
) {
348359
return
349360
}
350361
// removed node has not been serialized yet, just remove it from the Set
@@ -388,7 +399,7 @@ export class MutationObserverWrapper {
388399
return
389400
}
390401
if (isINode(n)) {
391-
if (isIgnored(n)) {
402+
if (nodeIsIgnored(n)) {
392403
return
393404
}
394405
this.movedSet.add(n)

0 commit comments

Comments
 (0)