PANA-5681: Add `ViewIdentityResolver` infrastructure by gonzalezreal · Pull Request #3202 · DataDog/dd-sdk-android

gonzalezreal · 2026-02-25T11:56:14Z

What does this PR do?

Adds ViewIdentityResolver interface and ViewIdentityResolverImpl to dd-sdk-android-internal. This component assigns stable, globally unique identifiers to Android Views by hashing their canonical paths with MD5. These identifiers will be used to correlate RUM actions with Session Replay wireframes for heatmap visualization.

Motivation

Support mobile heatmaps. This is PR 1 of 4, the foundational infrastructure that RUM and Session Replay will build on.

Additional Notes

Ported from #3164. Self-contained with no dependency on schema changes.

Review checklist (to be filled by reviewers)

Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
Make sure you discussed the feature or bugfix with the maintaining team in an Issue
Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cebbd7bc94

ℹ️ 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".

...oid-internal/src/main/java/com/datadog/android/internal/identity/ViewIdentityResolverImpl.kt

ambushwork · 2026-02-27T16:05:33Z

...android-internal/src/main/java/com/datadog/android/internal/identity/ViewIdentityResolver.kt

+    /**
+     * Sets the current screen identifier. Takes precedence over Activity-based detection.
+     * @param identifier the screen identifier (typically RUM view URL), or null to clear
+     */


Do we really clear it? if so what's the case?

ambushwork · 2026-02-27T16:17:54Z

...oid-internal/src/test/java/com/datadog/android/internal/identity/ViewIdentityResolverTest.kt

+@MockitoSettings(strictness = Strictness.LENIENT)
+internal class ViewIdentityResolverTest {
+
+    private lateinit var testedManager: ViewIdentityResolverImpl


Suggested change

private lateinit var testedManager: ViewIdentityResolverImpl

private lateinit var testedViewIdentityResolver: ViewIdentityResolverImpl

ambushwork · 2026-02-27T20:01:52Z

...oid-internal/src/main/java/com/datadog/android/internal/identity/ViewIdentityResolverImpl.kt

+     * Cleared when screen changes (namespace depends on currentRumViewIdentifier).
+     */
+    @Suppress("UnsafeThirdPartyFunctionCall") // WeakHashMap() is never null
+    private val rootScreenNamespaceCache: MutableMap<View, String> =


I feel the name is a bit confusing here, "screen" is a concept of heatmaps which represents the whole visible interface according to the RFC, "rootView" is a Android term meaning the root of the view tree hireacy. I think here is more like "screenNamespaceCache indexed by rootView"

ambushwork · 2026-02-27T20:03:43Z

...oid-internal/src/main/java/com/datadog/android/internal/identity/ViewIdentityResolverImpl.kt

+
+    /** Priority 1: Use RUM view identifier if available (set via RumMonitor.startView). */
+    private fun getNamespaceFromRumView(): String? {
+        return currentRumViewIdentifier.get()?.let { viewName ->


Suggested change

return currentRumViewIdentifier.get()?.let { viewName ->

return currentRumViewIdentifier.get()?.let { viewUrl ->

ambushwork · 2026-02-27T20:03:58Z

...oid-internal/src/main/java/com/datadog/android/internal/identity/ViewIdentityResolverImpl.kt

+    /** Priority 1: Use RUM view identifier if available (set via RumMonitor.startView). */
+    private fun getNamespaceFromRumView(): String? {
+        return currentRumViewIdentifier.get()?.let { viewName ->
+            "$NAMESPACE_VIEW_PREFIX${escapePathComponent(viewName)}"


Suggested change

"$NAMESPACE_VIEW_PREFIX${escapePathComponent(viewName)}"

"$NAMESPACE_VIEW_PREFIX${escapePathComponent(viewUrl)}"

ambushwork · 2026-02-27T20:10:03Z

...oid-internal/src/main/java/com/datadog/android/internal/identity/ViewIdentityResolverImpl.kt

+    return input.replace("%", "%25").replace("/", "%2F")
+}
+
+private fun md5Hex(input: String): String? {


we have the exact same logic in MD5HashGenerator, maybe it's time to move it into internal and reuse it here.

0xnm

I went only through the production source files so far and left some comments.

0xnm · 2026-03-02T09:08:21Z

dd-sdk-android-internal/api/apiSurface

+  fun resolveViewIdentity(android.view.View): String?
+  companion object 
+    const val FEATURE_CONTEXT_KEY: String
+class com.datadog.android.internal.identity.ViewIdentityResolverImpl : ViewIdentityResolver


a small trick we use to not expose Impl classes is to create a property (in this case it will be a function create(arg)) in the companion object of ViewIdentityResolver.

0xnm · 2026-03-02T09:09:50Z

...android-internal/src/main/java/com/datadog/android/internal/identity/ViewIdentityResolver.kt

+    fun onWindowRefreshed(root: View)
+
+    /**
+     * Resolves the stable identity for a view (32 hex chars), or null if the view is detached.


nit: probably we can omit 32 hex chars from the description, it is implementation detail

0xnm · 2026-03-02T09:11:30Z

...android-internal/src/main/java/com/datadog/android/internal/identity/ViewIdentityResolver.kt

+        /**
+         * Key used to store the ViewIdentityResolver instance in the feature context.
+         */
+        const val FEATURE_CONTEXT_KEY: String = "_dd.view_identity_resolver"


I think this is quite dangerous. What is the purpose to store such object in the feature context given that we have direct access to the type?

0xnm · 2026-03-02T09:13:50Z

...oid-internal/src/main/java/com/datadog/android/internal/identity/ViewIdentityResolverImpl.kt

+     * resources.getResourceName() which is expensive.
+     */
+    @Suppress("UnsafeThirdPartyFunctionCall") // LinkedHashMap constructor doesn't throw
+    private val resourceNameCache: MutableMap<Int, String> = Collections.synchronizedMap(


I don't think synchronizedMap is a good choice here, maybe ConcurrentHashMap is better, given that after some time the number of reads will be much bigger than number of writes.

0xnm · 2026-03-02T09:15:09Z

...oid-internal/src/main/java/com/datadog/android/internal/identity/ViewIdentityResolverImpl.kt

+    @Suppress("UnsafeThirdPartyFunctionCall") // LinkedHashMap constructor doesn't throw
+    private val resourceNameCache: MutableMap<Int, String> = Collections.synchronizedMap(
+        object : LinkedHashMap<Int, String>(RESOURCE_NAME_CACHE_SIZE, DEFAULT_LOAD_FACTOR, true) {
+            override fun removeEldestEntry(eldest: MutableMap.MutableEntry<Int, String>?): Boolean {


there is androidx.cache.LruCache class though

0xnm · 2026-03-02T09:19:57Z

...oid-internal/src/main/java/com/datadog/android/internal/identity/ViewIdentityResolverImpl.kt

+
+    @Synchronized
+    override fun setCurrentScreen(identifier: String?) {
+        @Suppress("UnsafeThirdPartyFunctionCall") // type-safe: generics prevent VarHandle type mismatches


We should declare this call as safe instead, there is already java.util.concurrent.atomic.AtomicBoolean.getAndSet(kotlin.Boolean) declared.

0xnm · 2026-03-02T09:21:07Z

...oid-internal/src/main/java/com/datadog/android/internal/identity/ViewIdentityResolverImpl.kt

+    /** The current RUM view identifier, set via setCurrentScreen(). */
+    private val currentRumViewIdentifier = AtomicReference<String?>(null)
+
+    @Synchronized


there is no need for the @Synchronized, AtomicReference.getAndSet already does atomic check guaranteeing the absence of concurency.

0xnm · 2026-03-02T09:23:09Z

...oid-internal/src/main/java/com/datadog/android/internal/identity/ViewIdentityResolverImpl.kt

+    @Synchronized
+    override fun onWindowRefreshed(root: View) {
+        indexTree(root)
+    }
+
+    @Synchronized
+    override fun resolveViewIdentity(view: View): String? {


given these will be called on the main thread, we should avoid using Synchronized and use more granular synchronization primitives

0xnm · 2026-03-02T09:24:02Z

...oid-internal/src/main/java/com/datadog/android/internal/identity/ViewIdentityResolverImpl.kt

+    /** Depth-first traversal of view hierarchy, computing and caching identity for each view. */
+    private fun traverseAndIndexViews(root: View, rootCanonicalPath: String) {
+        // Index the root view (all cache insertions happen here for consistency)
+        md5Hex(rootCanonicalPath)?.let { hash ->


There was a point about SHA-1 vs MD5 in the RFC, did we expore it?

0xnm · 2026-03-02T09:25:29Z

...oid-internal/src/main/java/com/datadog/android/internal/identity/ViewIdentityResolverImpl.kt

+        }
+    }
+
+    /** Priority 2: Fall back to Activity class name if root view has Activity context. */


nit: probably we need to remove such comments, they don't really add any value

PANA-5681: Add ViewIdentityResolver infrastructure

cebbd7b

This was referenced Feb 27, 2026

PANA-5681: Update RUM and Session Replay schemas for heatmaps #3203

Open

PANA-5681: Integrate ViewIdentityResolver into RUM for heatmaps #3205

Open

gonzalezreal marked this pull request as ready for review February 27, 2026 10:39

gonzalezreal requested review from a team as code owners February 27, 2026 10:39

chatgpt-codex-connector bot reviewed Feb 27, 2026

View reviewed changes

...oid-internal/src/main/java/com/datadog/android/internal/identity/ViewIdentityResolverImpl.kt Show resolved Hide resolved

...oid-internal/src/main/java/com/datadog/android/internal/identity/ViewIdentityResolverImpl.kt Show resolved Hide resolved

ambushwork reviewed Feb 27, 2026

View reviewed changes

0xnm requested changes Mar 2, 2026

View reviewed changes

	private lateinit var testedManager: ViewIdentityResolverImpl
	private lateinit var testedViewIdentityResolver: ViewIdentityResolverImpl

	return currentRumViewIdentifier.get()?.let { viewName ->
	return currentRumViewIdentifier.get()?.let { viewUrl ->

	"$NAMESPACE_VIEW_PREFIX${escapePathComponent(viewName)}"
	"$NAMESPACE_VIEW_PREFIX${escapePathComponent(viewUrl)}"

Conversation

gonzalezreal commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Motivation

Additional Notes

Review checklist (to be filled by reviewers)

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xnm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gonzalezreal commented Feb 25, 2026 •

edited

Loading