Repository navigation
✨ [RUM-19007] Attribute automatic click actions with element stacks - #5098
amortemousque wants to merge 12 commits into
Conversation
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 977b623 | Docs | View more details | Give us feedback! |
Bundles Sizes Evolution
|
567a2bf to
65086b8
Compare
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65086b87d4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
More details
Element stacks are stored per SDK instance and snapshotted at pointerdown, preserving attribution for ongoing actions while keeping stacks out of serialized events. The completed static review found no reportable defects.
🤖 Bits Code Review · Commit 65086b8
| bufferedDataObservable: BufferedObservable<BufferedData>, | ||
| telemetry: Telemetry, | ||
| hooks: Hooks, | ||
| elementStacks: WeakMap<Element, string>, |
There was a problem hiding this comment.
💭 thought: elementStacks is passed through startRum / startRumEventCollection only to reach the action domain. What if actionCollection owned it and exposed attachElementStack through the strategy, like addAction? To keep the current guarantees (no buffering, weak references before init), preStartRum could keep its own WeakMap for calls made before init and hand it to actionCollection when RUM starts, instead of replaying the calls through bufferApiCalls.
| callMonitored(() => { | ||
| addTelemetryUsage({ feature: 'attach-element-stack' }) | ||
| if (stack?.includes('\n')) { | ||
| elementStacks.set(element, stack) |
There was a problem hiding this comment.
💬 suggestion: A non-Element argument (e.g. null from a getElementById that ran too early) makes WeakMap.set throw. callMonitored swallows it, so JS/CDN customers get no feedback, clicks stay attributed to the shell, and we receive it as an SDK error in telemetry. Could we warn and ignore, like sanitizeStringOption does?
|
|
||
| let stack | ||
| if ('handlingStack' in domainContext) { | ||
| if (rawRumEvent.type === 'action' && 'elementStack' in domainContext) { |
There was a problem hiding this comment.
🥜 nitpick: rawRumEvent.type === 'action' looks redundant: only RumActionEventDomainContext declares elementStack, so the in check alone already narrows to actions.
Motivation
Microfrontend applications use service and version attribution to identify which MFE generated a RUM event. Existing attribution relies on application stack traces to resolve build-plugin metadata or let customers match bundle URLs in
beforeSend.Automatic click actions lack an application handling stack, so clicks within an MFE cannot use these attribution mechanisms and remain attributed to the shell application. Associating a stack captured from the MFE's bundle with its DOM container lets clicks on that container and its descendants reuse the existing attribution logic, without requiring customers to duplicate service and version values in their application code.
Changes
DD_RUM.attachElementStack(element)to RUM and RUM Slim. Capture the caller's stack at the public API entry point and immediately register it in an instance-ownedWeakMap, including beforeinit(). Registrations survive startup without buffering or retaining detached elements strongly.context.elementStackinbeforeSendfor manual attribution. Keep stacks out of serialized events and preserve shell fallback.Test instructions
The new scenarios can be run after building the SDK and test apps:
yarn build yarn build:apps --app microfrontend --app vanilla yarn test:e2e microfrontend.scenario.ts -g 'with element stacks' --project chromium --project firefox --project webkitChecklist