Skip to content

RUM-7795 Anonymous RUM Identifier#2172

Merged
dd-mergequeue[bot] merged 22 commits into
developfrom
maciey/RUM-7795/anonymous-id
Feb 10, 2025
Merged

RUM-7795 Anonymous RUM Identifier#2172
dd-mergequeue[bot] merged 22 commits into
developfrom
maciey/RUM-7795/anonymous-id

Conversation

@maciejburda

@maciejburda maciejburda commented Jan 10, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

What and why?

Adds capability of tracking anonymous id (enabled by default). This data allows linking sessions coming from the same device.

How?

Following proposal from this RFC (internal) it reuses data store mechanism, and based on configuration it generates and reuses stored identifier.

This identifier is attached to UserInfo object (schema already updated), which is enriching other SDK events.

NOTE: The identifier is read asynchronously from a file, which means RUM events created immediately after calling RUM.enable() will not include it. However, this is not an issue since the anonymous_id belongs to the Session entity. As long as the anonymous_id is present in at least in one event, it is sufficient for linking.

Here is corresponding PR for Android:
DataDog/dd-sdk-android#2487

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)
  • Make sure each commit and the PR mention the Issue number or JIRA reference
  • Add CHANGELOG entry for user facing changes
  • Add Objective-C interface for public APIs (see our guidelines [internal]) and run make api-surface)

@maciejburda maciejburda changed the title Maciey/rum 7795/anonymous RUM-7795 Anonymous RUM Identifier Jan 10, 2025
@datadog-datadog-prod-us1

datadog-datadog-prod-us1 Bot commented Jan 10, 2025 •
edited
Loading

Copy link
Copy Markdown

Datadog Report

Branch report: maciey/RUM-7795/anonymous-id
Commit report: 777da2e
Test service: dd-sdk-ios

✅ 0 Failed, 1013 Passed, 2754 Skipped, 47.38s Total duration (1m 45.5s time saved)

@maciejburda
maciejburda marked this pull request as ready for review January 10, 2025 16:29
@maciejburda
maciejburda requested review from a team as code owners January 10, 2025 16:29
mariedm
mariedm previously approved these changes Jan 10, 2025
View reviewed changes

@mariedm mariedm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGMT! Only a few minor comments.

Comment thread DatadogRUM/Sources/Integrations/AnonymousIdManager.swift Outdated
Comment thread DatadogRUM/Sources/RUMConfiguration.swift
Comment thread DatadogRUM/Tests/Integrations/AnonymousIdentifierManagerTests.swift Outdated
simaoseica-dd
simaoseica-dd previously approved these changes Jan 10, 2025
View reviewed changes
ncreated
ncreated reviewed Jan 13, 2025
View reviewed changes
Comment thread DatadogRUM/Sources/RUMConfiguration.swift
ncreated
ncreated reviewed Jan 13, 2025
View reviewed changes
Comment thread DatadogRUM/Sources/RUMConfiguration.swift Outdated
ncreated
ncreated requested changes Jan 13, 2025
View reviewed changes

@ncreated ncreated left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks great overall 👌 - I only left suggestions on naming and documenting the public API.

Requesting a change tho, to add integration unit tests for this feature. Using real instance of the SDK + proxy and asserting that anonymous ID is generated and inserted into events of two RUM sessions started in two instances of the SDK. I can help and pair on this 🙌

@maciejburda
maciejburda dismissed stale reviews from mariedm and simaoseica-dd via 7ca3267 January 14, 2025 14:02
ncreated
ncreated reviewed Feb 4, 2025
View reviewed changes
Comment thread Datadog/IntegrationUnitTests/RUM/AnonymousIdentifierTests.swift
mariedm
mariedm previously approved these changes Feb 7, 2025
View reviewed changes
Comment thread DatadogRUM/Sources/Integrations/AnonymousIdentifierManager.swift
Comment thread DatadogRUM/Tests/RUMConfigurationTests.swift Outdated
simaoseica-dd
simaoseica-dd previously approved these changes Feb 7, 2025
View reviewed changes

@simaoseica-dd simaoseica-dd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some small nits
👍

Comment thread Datadog/IntegrationUnitTests/RUM/AnonymousIdentifierTests.swift Outdated
Comment thread DatadogCore/Tests/Datadog/DatadogCore/DatadogCoreTests.swift
Comment thread DatadogRUM/Sources/Integrations/AnonymousIdentifierManager.swift
@maciejburda
maciejburda dismissed stale reviews from simaoseica-dd and mariedm via c68095c February 10, 2025 10:39
@maciejburda

maciejburda commented Feb 10, 2025

Copy link
Copy Markdown
Member Author

/merge

@dd-devflow

dd-devflow Bot commented Feb 10, 2025 •
edited
Loading

Copy link
Copy Markdown

Devflow running: /merge

View all feedbacks in Devflow UI.

2025-02-10 10:42:25 UTC ℹ️ MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

2025-02-10 14:04:14 UTC ℹ️ MergeQueue: merge request added to the queue

The median merge time in develop is 27m.

2025-02-10 14:45:06 UTC ℹ️ MergeQueue: This merge request was merged

@dd-mergequeue
dd-mergequeue Bot merged commit 2f63329 into develop Feb 10, 2025
@maciejburda
maciejburda deleted the maciey/RUM-7795/anonymous-id branch February 11, 2025 09:21
@ncreated ncreated mentioned this pull request Mar 6, 2025
Merged
4 tasks
@sbarrio sbarrio mentioned this pull request Mar 11, 2025
Merged
4 tasks
@maciejburda maciejburda mentioned this pull request Mar 19, 2025
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ncreated ncreated ncreated approved these changes
@mariedm mariedm mariedm approved these changes
@simaoseica-dd simaoseica-dd simaoseica-dd approved these changes

Assignees

No one assigned

Labels

mergequeue-status: done

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@maciejburda @ncreated @mariedm @simaoseica-dd