Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RUM-917 Fix type mistmatch error #2092

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

maciejburda
Copy link
Contributor

What and why?

Fixes the issues of type mismatch error which was causing full snapshot enforcement in some cases.

How?

We've been wrongly passing identifier of the parent recorder to it's children which was causing in some cases issue where two views would be recorded with different type but the same id. It's because NodeIDGenerator depends on the identifier of the recorder.

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 RUM-917 Enforce unique uuid for all sub recorders RUM-917 Fix type mistmatch error Oct 24, 2024
@maciejburda maciejburda marked this pull request as ready for review October 24, 2024 16:28
@maciejburda maciejburda requested review from a team as code owners October 24, 2024 16:28
@maxep
Copy link
Member

maxep commented Oct 25, 2024

Seems like it's reverting #1967, wouldn't this introduce a regression?

@datadog-datadog-prod-us1
Copy link

Datadog Report

Branch report: RUM-917/fix-typemismatch-bug
Commit report: cbf9ef9
Test service: dd-sdk-ios

✅ 0 Failed, 3347 Passed, 188 Skipped, 2m 4.18s Total duration (46.67s time saved)

@maxep
Copy link
Member

maxep commented Oct 25, 2024

Also, I think we should target the release branch, WDYT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants