Merge heatmaps into develop#3566
Conversation
Merge develop into feature/heatmaps
…eal/PANA-5681/view-identity-resolver
…ntity-resolver PANA-5681: Add `HeatmapIdentifier` infrastructure [1 of 4]
…pdates PANA-5681: Update RUM and Session Replay schemas for heatmaps [2 of 4]
…maps PANA-5681: Integrate HeatmapIdentifierRegistry into RUM for heatmaps [3 of 4]
…replay-heatmaps PANA-5681: Integrate HeatmapIdentifierRegistry into Session Replay for heatmaps [4 of 4]
PANA-5681: Add heatmaps feature flag
…rossplatform-prep [PANA-5681] Preparation for Heatmaps cross-platform integration
Merge develop into feature/heatmaps
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c009ee6c4d
ℹ️ 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".
| override fun setHeatmapIdentifiers(identifiers: Map<Long, HeatmapIdentifier>, screenName: String) { | ||
| delegate()?.setHeatmapIdentifiers(identifiers, screenName) |
There was a problem hiding this comment.
When Session Replay heatmaps start before RUM is enabled, delegate() is null here so the first snapshots are silently dropped, but HeatmapIdentifierResolver.publishIfChanged() still updates lastPublishedScreenName/lastPublishedEntries before calling this method. If RUM registers later and the hierarchy is unchanged, SR will consider the snapshot already published and never call setHeatmapIdentifiers again, leaving the RUM registry empty until some UI change forces a republish; native taps in that common initialization order will therefore miss dd.action.target.permanent_id.
Useful? React with 👍 / 👎.
| ), | ||
| configuration = ActionEvent.Configuration(sessionSampleRate = sampleRate) | ||
| configuration = ActionEvent.Configuration(sessionSampleRate = sampleRate), | ||
| action = heatmapResolver?.resolve(rumContext.viewUrl.orEmpty()) |
There was a problem hiding this comment.
When a tapped View is moved or recycled under the same parent before the action scope is flushed, this late lookup reads the latest Session Replay registry entry for the captured view key rather than the entry that existed at tap time. A RecyclerView row can reuse the same View object and parent with a different path/permanentId, so the RUM action is attributed to the new row instead of the element the user actually tapped; capture the permanentId when the tap is observed or otherwise pin the SR snapshot used for this action.
Useful? React with 👍 / 👎.
| val systemInformation = miscUtils.resolveSystemInformation(context) | ||
| val item = recordedDataQueueHandler.addSnapshotItem(systemInformation) ?: return | ||
|
|
||
| val currentViewUrl = rumContextProvider.getRumContext().viewUrl |
There was a problem hiding this comment.
After addSnapshotItem() succeeds, the item already contains the RUM context that will be used to serialize this snapshot, but this line reads the volatile provider a second time. If RUM navigates between those two reads, the segment is queued for one view while heatmap identifiers are hashed with another view URL, so the wireframe permanentIds cannot match the RUM actions for the segment; use item.recordedQueuedItemContext.newRumContext.viewUrl instead of re-reading the provider.
Useful? React with 👍 / 👎.
| if (pathNeededForChildren || view.isValidTapTarget()) { | ||
| ctx.resolveIdentity(view, nodePath, typeIndex) |
There was a problem hiding this comment.
For clickable views whose mapper returns emptyList() (for example a transparent clickable ViewGroup with no background), this registers a heatmap identifier even though NodeFlattener has no wireframe on which to stamp the same permanentId. A tap on such a container can therefore send dd.action.target.permanent_id from the RUM registry, but the Session Replay segment contains no matching wireframe, so the heatmap correlation is broken; only publish IDs for tap targets that actually produce a wireframe, or emit a placeholder wireframe for them.
Useful? React with 👍 / 👎.
3 participants
What does this PR do?
Merges the heatmaps feature into develop
Motivation
What inspired you to submit this pull request?
Additional Notes
Anything else we should know when reviewing?
Review checklist (to be filled by reviewers)