Skip to content

RUM-7205 Double full snapshot bug #2154

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

Merged
merged 3 commits into from
Dec 18, 2024

Conversation

maciejburda
Copy link
Member

@maciejburda maciejburda commented Dec 18, 2024

What and why?

This PR is essentially reverting #1567. Assumption made was wrong. We can expect view that from the wireframe perspective have a different root view (it can be done by fully covering the root with nontransparent view, which will remove the root during flattening process).

The reverted code was fixing some minor edge cases in React Native, although it has a serious penalty of creating double fullsnapshots in others.

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 requested review from a team as code owners December 18, 2024 14:43
@datadog-datadog-prod-us1
Copy link

datadog-datadog-prod-us1 bot commented Dec 18, 2024

Datadog Report

Branch report: maciey/RUM-7205/double-full-snapshot-bug
Commit report: f1afae5
Test service: dd-sdk-ios

✅ 0 Failed, 2 Passed, 3591 Skipped, 16.62s Total duration (2m 15.09s time saved)

@maciejburda
Copy link
Member Author

/merge

@dd-devflow
Copy link

dd-devflow bot commented Dec 18, 2024

Devflow running: /merge

View all feedbacks in Devflow UI.


2024-12-18 15:13:20 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.


2024-12-18 16:33:46 UTC ℹ️ MergeQueue: merge request added to the queue

The median merge time in develop is 28m.


2024-12-18 17:30:45 UTC 🚨 MergeQueue: This merge request is in error

mergequeue build completed successfully, but the github api returned an error while merging the pr.
It's probably because:

  • target branch of PR is restricted to only allow up-to-date branches, but the pr is now outdated
Details

Error: PUT https://api.github.com/repos/DataDog/dd-sdk-ios/pulls/2154/merge: 405 Required status check "dd-gitlab/Sync GH Checks" is expected. []

FullStacktrace:
activity error (type: github.GithubService_MergePullRequest, scheduledEventID: 41, startedEventID: 42, identity: 1@github-worker-7c5557687b-pw9fq@): PUT https://api.github.com/repos/DataDog/dd-sdk-ios/pulls/2154/merge: 405 Required status check "dd-gitlab/Sync GH Checks" is expected. [] (type: GitFailure, retryable: false): PUT https://api.github.com/repos/DataDog/dd-sdk-ios/pulls/2154/merge: 405 Required status check "dd-gitlab/Sync GH Checks" is expected. [] (type: ErrorResponse, retryable: true)

Copy link
Member

@ncreated ncreated left a comment

Choose a reason for hiding this comment

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

Good finding, good explanation, good trade-off 💪

@maxep maxep force-pushed the maciey/RUM-7205/double-full-snapshot-bug branch from f1afae5 to fd28491 Compare December 18, 2024 17:32
@maxep
Copy link
Member

maxep commented Dec 18, 2024

/merge

@dd-devflow
Copy link

dd-devflow bot commented Dec 18, 2024

Devflow running: /merge

View all feedbacks in Devflow UI.


2024-12-18 17:32:44 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.


2024-12-18 18:03:11 UTC ℹ️ MergeQueue: merge request added to the queue

The median merge time in develop is 28m.


2024-12-18 18:29:56 UTC ℹ️ MergeQueue: This merge request was merged

@mariedm mariedm deleted the maciey/RUM-7205/double-full-snapshot-bug branch December 19, 2024 07:57
@maciejburda maciejburda mentioned this pull request Jan 2, 2025
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants