✨ [RUM-3902] Add privacy control for action names

[cy-moi]

Motivation

To be consistent with session Replay and ease the customer configuration, we could use the same privacy options as Replay. To avoid breaking changes, we need to add an extra init option that indicates the action names should take into account the defaultPrivacyLevel and the HTML overrides.

Desired behavior

When the user opt-in for enablePrivacyForActionName, we mask action names when no action name attributes given and no html override.

When the user does not opt-in for this feature, we mask action names when html override is set.

Changes

Add an opt-in field enablePrivacyForActionName: boolean in configuration. Once enablePrivacyForActionName: true, the action name computation would behave (taking into account of all privacy configs) as follow:

Header	Header
allow	The action names are collected
mask	The action names are not collected
mask-user-input	The action names are collected since we don’t rely on the user input value to compute the action name
hidden	The actions are not collected

This also works for html override data-dd-privacy accordingly.

Testing

• Local
• Staging
• Unit
• End to end

---

I have gone over the contributing documentation.

[amortemousque]

💭 thought: ‏This check actionName === ACTION_NAME_PLACEHOLDER makes me wondering if the privacy logic could not be collocated to the action name computation. Something like

function getActionNameFromElement(element: Element, configuration: RumConfiguration): { value: string, masked: boolean }

[cit-pr-commenter]

Bundles Sizes Evolution

📦 Bundle Name	Base Size	Local Size	𝚫	𝚫%	Status
Rum	157.18 KiB	157.91 KiB	747 B	+0.46%	✅
Logs	56.42 KiB	56.49 KiB	66 B	+0.11%	✅
Rum Slim	103.74 KiB	106.60 KiB	2.85 KiB	+2.75%	✅
Worker	25.21 KiB	25.21 KiB	0 B	0.00%	✅

🚀 CPU Performance

Action Name	Base Average Cpu Time (ms)	Local Average Cpu Time (ms)	𝚫
addglobalcontext	0.001	0.002	0.000
addaction	0.015	0.016	0.001
adderror	0.031	0.033	0.003
addtiming	0.001	0.001	0.000
startview	0.812	0.859	0.048
startstopsessionreplayrecording	0.731	0.720	-0.011
logmessage	0.005	0.005	0.000

🧠 Memory Performance

Action Name	Base Consumption Memory (bytes)	Local Consumption Memory (bytes)	𝚫
addglobalcontext	NaN KiB	27.20 KiB	NaN KiB
addaction	NaN KiB	48.22 KiB	NaN KiB
adderror	NaN KiB	50.76 KiB	NaN KiB
addtiming	NaN KiB	9.22 KiB	NaN KiB
startview	NaN KiB	2055.34 KiB	NaN KiB
startstopsessionreplayrecording	NaN KiB	10.35 KiB	NaN KiB
logmessage	NaN KiB	60.93 KiB	NaN KiB

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 96.61017% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 93.27%. Comparing base (d1b0048) to head (9a054d1).
Report is 23 commits behind head on main.

Files	Patch %	Lines
packages/rum-core/src/browser/polyfills.ts	80.00%	1 Missing ⚠️
...core/src/domain/action/getActionNameFromElement.ts	96.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2707   +/-   ##
=======================================
  Coverage   93.26%   93.27%           
=======================================
  Files         241      240    -1     
  Lines        7028     7051   +23     
  Branches     1553     1564   +11     
=======================================
+ Hits         6555     6577   +22     
- Misses        473      474    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[amortemousque]

💬 suggestion: ‏I think this condition could be done earlier in processPointerDown. The privacy level could be passed to computeClickActionBase. This way no need for this check:

browser-sdk/packages/rum-core/src/domain/action/trackClickActions.ts

Line 91 in f0dacd9

if (clickActionBase) {

[cy-moi]

I moved it to processPointerDown but I do not see how we could get rid of the check of clickActionBase because if would still be undefined when we skip hidden override.

Edit: should be fixed

[amortemousque]

What about action names relying on innerText (cf Privacy control options for action names RFC)?

[cy-moi]

This logic should be added when there are html privacy overriding attributes.

[bits-bot]

All committers have signed the CLA.

[amortemousque]

💬 suggestion: ‏We probably want the same treatment for "mask" | "hidden" and "mask-user-input".
Also customers can use class name to define privacy level. cf doc

[cy-moi]

Should mask-user-input not be masked when it is not an input area?

[amortemousque]

Yes in fact there is nothing to do for "mask-user-input" has the name does not take into account input value

[BenoitZugmeyer]

suggestion 1: ‏Don't compute nodePrivacyLevel if enablePrivacyForActionName is false. Not only it will improve performance, it will also be easier to understand the code

Suggestion 2: pass nodePrivacyLevel to computeClickActionBase so we don't have to re-compute it there

  let nodePrivacyLevel: NodePrivacyLevel
  
  if (configuration.enablePrivacyForActionName) {
    nodePrivacyLevel = getNodePrivacyLevel(pointerDownEvent.target, configuration.defaultPrivacyLevel)

    // When the node is set to hidden, we do not track click action
    // Make sure everything has been done before return
    if (nodePrivacyLevel === NodePrivacyLevel.HIDDEN) {
      return undefined
    }
  } else {
   nodePrivacyLevel = NodePrivacyLevel.ALLOW
  }

  const clickActionBase = computeClickActionBase(
    pointerDownEvent,
    nodePrivacyLevel,
    configuration.actionNameAttribute
  )

[amortemousque]

💬 suggestion: ‏Maybe even simpler ;)

  const nodePrivacyLevel = configuration.enablePrivacyForActionName
    ? getNodePrivacyLevel(pointerDownEvent.target, configuration.defaultPrivacyLevel)
    : NodePrivacyLevel.ALLOW
  
  if (nodePrivacyLevel === NodePrivacyLevel.HIDDEN) {
    return undefined
  }
  
  const clickActionBase = computeClickActionBase(
    pointerDownEvent,
    nodePrivacyLevel,
    configuration.actionNameAttribute
  )

[amortemousque]

💬 suggestion: ‏The file name "constants.ts" is too generic. All constants in that file are related to privacy. Maybe it could merged into the "privacy.ts"

[cy-moi]

Good point. I merged it in privacy.ts.

[amortemousque]

💬 suggestion: ‏I think we could extract the logic of having to check both the class and the attribute. Something like:

  removeTextFromElements(
    getPrivacySelector(NodePrivacyLevel.MASK),
    getPrivacySelector(NodePrivacyLevel.HIDDEN)
  )

It could even be used in getNodeSelfPrivacyLevel

// instead of 
const privAttr = node.getAttribute(PRIVACY_ATTR_NAME)
if (privAttr === PRIVACY_ATTR_VALUE_MASK || elementClasslistContains(node, PRIVACY_CLASS_MASK))

// we could use 
if(node.matches(getPrivacySelector(NodePrivacyLevel.MASK))

@BenoitZugmeyer do you see any downside of using matches, maybe the perf?

[amortemousque]

💬 suggestion: ‏Could be nice to add a comment saying // TODO next major: remove this option and make privacy for action name the default behaviour

[dd-devflow]

⚠️ Branch Integration: This commit was already integrated

Commit 82036b1ee4 had already been merged into staging-21

If you need support, contact us on Slack #devflow!