Skip to content
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

⚡️ [RUM-2917] optimize getNodePrivacyLevel by adding a cache #2579

Merged
merged 2 commits into from
Feb 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 25 additions & 10 deletions packages/rum/src/domain/record/observers/mutationObserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import type {
RemovedNodeMutation,
TextMutation,
} from '../../../types'
import type { NodePrivacyLevelCache } from '../privacy'
import { getNodePrivacyLevel, getTextContent } from '../privacy'
import type { NodeWithSerializedNode } from '../serialization'
import {
Expand Down Expand Up @@ -109,6 +110,8 @@ function processMutations(
configuration: RumConfiguration,
shadowRootsController: ShadowRootsController
) {
const nodePrivacyLevelCache: NodePrivacyLevelCache = new Map()

mutations
.filter((mutation): mutation is RumChildListMutationRecord => mutation.type === 'childList')
.forEach((mutation) => {
Expand All @@ -125,31 +128,35 @@ function processMutations(
(mutation): mutation is WithSerializedTarget<RumMutationRecord> =>
mutation.target.isConnected &&
nodeAndAncestorsHaveSerializedNode(mutation.target) &&
getNodePrivacyLevel(mutation.target, configuration.defaultPrivacyLevel) !== NodePrivacyLevel.HIDDEN
getNodePrivacyLevel(mutation.target, configuration.defaultPrivacyLevel, nodePrivacyLevelCache) !==
NodePrivacyLevel.HIDDEN
)

const { adds, removes, hasBeenSerialized } = processChildListMutations(
filteredMutations.filter(
(mutation): mutation is WithSerializedTarget<RumChildListMutationRecord> => mutation.type === 'childList'
),
configuration,
shadowRootsController
shadowRootsController,
nodePrivacyLevelCache
)

const texts = processCharacterDataMutations(
filteredMutations.filter(
(mutation): mutation is WithSerializedTarget<RumCharacterDataMutationRecord> =>
mutation.type === 'characterData' && !hasBeenSerialized(mutation.target)
),
configuration
configuration,
nodePrivacyLevelCache
)

const attributes = processAttributesMutations(
filteredMutations.filter(
(mutation): mutation is WithSerializedTarget<RumAttributesMutationRecord> =>
mutation.type === 'attributes' && !hasBeenSerialized(mutation.target)
),
configuration
configuration,
nodePrivacyLevelCache
)

if (!texts.length && !attributes.length && !removes.length && !adds.length) {
Expand All @@ -167,7 +174,8 @@ function processMutations(
function processChildListMutations(
mutations: Array<WithSerializedTarget<RumChildListMutationRecord>>,
configuration: RumConfiguration,
shadowRootsController: ShadowRootsController
shadowRootsController: ShadowRootsController,
nodePrivacyLevelCache: NodePrivacyLevelCache
) {
// First, we iterate over mutations to collect:
//
Expand Down Expand Up @@ -217,7 +225,11 @@ function processChildListMutations(
continue
}

const parentNodePrivacyLevel = getNodePrivacyLevel(node.parentNode!, configuration.defaultPrivacyLevel)
const parentNodePrivacyLevel = getNodePrivacyLevel(
node.parentNode!,
configuration.defaultPrivacyLevel,
nodePrivacyLevelCache
)
if (parentNodePrivacyLevel === NodePrivacyLevel.HIDDEN || parentNodePrivacyLevel === NodePrivacyLevel.IGNORE) {
continue
}
Expand Down Expand Up @@ -271,7 +283,8 @@ function processChildListMutations(

function processCharacterDataMutations(
mutations: Array<WithSerializedTarget<RumCharacterDataMutationRecord>>,
configuration: RumConfiguration
configuration: RumConfiguration,
nodePrivacyLevelCache: NodePrivacyLevelCache
) {
const textMutations: TextMutation[] = []

Expand All @@ -294,7 +307,8 @@ function processCharacterDataMutations(

const parentNodePrivacyLevel = getNodePrivacyLevel(
getParentNode(mutation.target)!,
configuration.defaultPrivacyLevel
configuration.defaultPrivacyLevel,
nodePrivacyLevelCache
)
if (parentNodePrivacyLevel === NodePrivacyLevel.HIDDEN || parentNodePrivacyLevel === NodePrivacyLevel.IGNORE) {
continue
Expand All @@ -312,7 +326,8 @@ function processCharacterDataMutations(

function processAttributesMutations(
mutations: Array<WithSerializedTarget<RumAttributesMutationRecord>>,
configuration: RumConfiguration
configuration: RumConfiguration,
nodePrivacyLevelCache: NodePrivacyLevelCache
) {
const attributeMutations: AttributeMutation[] = []

Expand All @@ -338,7 +353,7 @@ function processAttributesMutations(
if (uncensoredValue === mutation.oldValue) {
continue
}
const privacyLevel = getNodePrivacyLevel(mutation.target, configuration.defaultPrivacyLevel)
const privacyLevel = getNodePrivacyLevel(mutation.target, configuration.defaultPrivacyLevel, nodePrivacyLevelCache)
const attributeValue = serializeAttribute(mutation.target, privacyLevel, mutation.attributeName!, configuration)

let transformedValue: string | null
Expand Down
40 changes: 40 additions & 0 deletions packages/rum/src/domain/record/privacy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,46 @@ describe('getNodePrivacyLevel', () => {
expect(getNodePrivacyLevel(node, NodePrivacyLevel.ALLOW)).toBe(NodePrivacyLevel.MASK)
})
})

describe('cache', () => {
it('fills the cache', () => {
const ancestor = document.createElement('div')
const node = document.createElement('div')
ancestor.setAttribute(PRIVACY_ATTR_NAME, PRIVACY_ATTR_VALUE_MASK)
ancestor.appendChild(node)

const cache = new Map()
getNodePrivacyLevel(node, NodePrivacyLevel.ALLOW, cache)

expect(cache.get(node)).toBe(NodePrivacyLevel.MASK)
})

it('uses the cache', () => {
const ancestor = document.createElement('div')
const node = document.createElement('div')
ancestor.appendChild(node)

const cache = new Map()
cache.set(node, NodePrivacyLevel.MASK_USER_INPUT)

expect(getNodePrivacyLevel(node, NodePrivacyLevel.ALLOW, cache)).toBe(NodePrivacyLevel.MASK_USER_INPUT)
})

it('does not recurse on ancestors if the node is already in the cache', () => {
const ancestor = document.createElement('div')
const node = document.createElement('div')
ancestor.appendChild(node)

const parentNodeGetterSpy = spyOnProperty(node, 'parentNode').and.returnValue(ancestor)

const cache = new Map()
cache.set(node, NodePrivacyLevel.MASK_USER_INPUT)

getNodePrivacyLevel(node, NodePrivacyLevel.ALLOW, cache)

expect(parentNodeGetterSpy).not.toHaveBeenCalled()
})
})
})

describe('getNodeSelfPrivacyLevel', () => {
Expand Down
21 changes: 18 additions & 3 deletions packages/rum/src/domain/record/privacy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,32 @@ export const MAX_ATTRIBUTE_VALUE_CHAR_LENGTH = 100_000

const TEXT_MASKING_CHAR = 'x'

export type NodePrivacyLevelCache = Map<Node, NodePrivacyLevel>

/**
* Get node privacy level by iterating over its ancestors. When the direct parent privacy level is
* know, it is best to use something like:
*
* derivePrivacyLevelGivenParent(getNodeSelfPrivacyLevel(node), parentNodePrivacyLevel)
*/
export function getNodePrivacyLevel(node: Node, defaultPrivacyLevel: NodePrivacyLevel): NodePrivacyLevel {
export function getNodePrivacyLevel(
node: Node,
defaultPrivacyLevel: NodePrivacyLevel,
cache?: NodePrivacyLevelCache
): NodePrivacyLevel {
if (cache && cache.has(node)) {
return cache.get(node)!
}
const parentNode = getParentNode(node)
const parentNodePrivacyLevel = parentNode ? getNodePrivacyLevel(parentNode, defaultPrivacyLevel) : defaultPrivacyLevel
const parentNodePrivacyLevel = parentNode
? getNodePrivacyLevel(parentNode, defaultPrivacyLevel, cache)
: defaultPrivacyLevel
const selfNodePrivacyLevel = getNodeSelfPrivacyLevel(node)
return reducePrivacyLevel(selfNodePrivacyLevel, parentNodePrivacyLevel)
const nodePrivacyLevel = reducePrivacyLevel(selfNodePrivacyLevel, parentNodePrivacyLevel)
if (cache) {
cache.set(node, nodePrivacyLevel)
}
return nodePrivacyLevel
}

/**
Expand Down