RUM-15277: Merge native user info to webview events

[kikoveiga]

What does this PR do?

Merges the native SDK's user info (including anonymous_id) into webview RUM and Log events, without overwriting fields already set by the Browser SDK. Native fields are only added when the key doesn't already exist in the event's usr object.

Example of before vs after:

Motivation

RUMS-5561

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: 14549c7219

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

[datadog-datadog-prod-us1-2]

🎯 Code Coverage (details)
• Patch Coverage: 100.00%
• Overall Coverage: 71.41% (-0.12%)

_{This comment will be updated automatically if new data arrives.
🔗 Commit SHA: de7985d | Docs | Datadog PR Page | Was this helpful? React with 👍/👎 or give us feedback!}

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.56%. Comparing base (41ae98f) to head (de7985d).

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3306      +/-   ##
===========================================
+ Coverage    71.49%   71.56%   +0.07%     
===========================================
  Files          946      946              
  Lines        34895    34912      +17     
  Branches      5906     5912       +6     
===========================================
+ Hits         24947    24983      +36     
+ Misses        8283     8277       -6     
+ Partials      1665     1652      -13     

Files with missing lines	Coverage Δ
...kotlin/com/datadog/android/api/context/UserInfo.kt	64.58% <ø> (ø)
...id/webview/internal/log/WebViewLogEventConsumer.kt	82.47% <100.00%> (+1.16%)	⬆️
...id/webview/internal/rum/WebViewRumEventConsumer.kt	92.06% <100.00%> (+0.68%)	⬆️
...roid/webview/internal/rum/WebViewRumEventMapper.kt	100.00% <100.00%> (ø)

... and 44 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ambushwork]

Question: The logic seems like it overrides only a part of the user info if they are absent and keep the rest of them if present, what's the reason of doing that?

[kikoveiga]

The logic here is that we do not override the information that is already present from the browser side, and just add new fields that are not there (anonymousId will be the case for example). I added an image on the pr description for some clarity.

[ambushwork]

This sounds strange for me, in some corner cases when Browser SDK and native SDK both possess partial user info can lead to a fake composed user info if I am not wrong.

// Webview
UserInfo(
    anonymousId = null,
    id = "001" ,
    name = "John" ,
    email = null
) 

// Native
UserInfo(
    anonymousId = "anonymous name",
    id = "002" ,
    name = "Bob" ,
    email = "Bob@datadoghq.com"
) 

// After the logic it becomes
UserInfo(
    anonymousId = "anonymous name",
    id = "001" ,
    name = "John" ,
    email = "Bob@datadoghq.com"
) 

[kikoveiga]

Oh ok I didn't understand previously, thanks! So maybe I should just stick to adding the anonymousId, which is a native field only, wdyt?

[ambushwork]

it can be an option, or the alternative solution is to completely override the user info of Browser SDK. it's better to check with iOS what is their approach.

[kikoveiga]

iOS doesn't do it yet as well, I'm supposed to do the same there: RUM-15279.

I had tried before to override everything, but it didn't work because some fields disappeared (web-side name and email for example). Tested it in Shopist. Also, codex advised against this in the above resolved comments.

[ambushwork]

I think codex missed the whole picture when giving the advise, IMO the correct path should be:

• If we detect that two user info are same, by checking id for example, we are safe to merge them
• If not, we need to make a decision that if we want to keep the native one or browser one.

Also it's better to add a unit test to protect this corner case.

[kikoveiga]

I don't know the exact logic of webviews and browser sdk, but isn't the id set on their side when on a webview? If so, ids will never be the same. I think info from browser and from native is unrelated, and trying to merge them can change behaviour unexpectedly. That's why I was only adding non-existing fields, but the safest is definitely just adding the anonymousId (which is only present on the native side).

[cdn34dd]

The anonymousId field is not a mobile field only, you can also have it the browser SDK, I think the difference is that in the browser SDK is disabled by default and we enable it by default on mobile if I'm not mistaken.

I'd say here we should simply add the anonymousId if not present to ensure we don't make any breaking changes in minor versions as people can be depending on things working like they do now. If we want to review this at some point we should do so in a major version.

[cdn34dd]

I notice that on webview events context also has a usr field, and it seems that we're not setting anonymousId there:

{
  (...)
  "context": {
    "usr": {
      "use_new_checkout_flow": true,
      "name": "harper.harry",
      "id": "1gfdsgsdfg",
      "team": "engineering",
      "use_discount": false,
      "email": "harper.harry@example.com"
    },
    "browser_test": false
  }
 (...)
}

I'm not sure if there's any backend functionality that specifically looks for context.usr, but maybe the safe option would be to add it also there ? As it seems that if browser were to set anonymousId itself, this would also be there.