-
Notifications
You must be signed in to change notification settings - Fork 19
chore: review sync logging - WPB-16000 #2965
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
Conversation
wire-ios-sync-engine/Source/Synchronization/Transcoders/ZMLastUpdateEventIDTranscoder.m
Show resolved
Hide resolved
Test Results 18 files 1 096 suites 10m 20s ⏱️ For more details on these failures, see this check. Results for commit 213d2e6. ♻️ This comment has been updated with latest results. |
Datadog ReportBranch report: ✅ 0 Failed, 1774 Passed, 26 Skipped, 1m 56.31s Total Time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, left couple of minor questions
WireDomain/Sources/WireDomain/Synchronization/IncrementalSync.swift
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall looks good, but I think some information should be removed from log message as it is contained in the attributes and we log attributes anyway
WireDomain/Sources/WireDomain/Extensions/LogAttributes/SyncLogAttributes.swift
Show resolved
Hide resolved
WireDomain/Sources/WireDomain/Extensions/LogAttributes/SyncLogAttributes.swift
Show resolved
Hide resolved
WireDomain/Sources/WireDomain/Synchronization/IncrementalSync.swift
Outdated
Show resolved
Hide resolved
WireDomain/Sources/WireDomain/Synchronization/IncrementalSync.swift
Outdated
Show resolved
Hide resolved
WireDomain/Sources/WireDomain/Synchronization/PullResourcesSync.swift
Outdated
Show resolved
Hide resolved
Removing myself as there are already 2 reviewers. Feel free to add me back |
…w/legacy mention from sync phase message
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good
Issue
This PR improves logging in the new sync mechanism (V2, V3) to measure time each sync phase takes to complete with dedicated tags:
Similar logging was introduced for legacy sync (V1) so we can compare durations between sync versions.
Also added some new logs for the new NSE.
NEW SYNC LOGS (V2)
LEGACY SYNC LOGS
Testing
Describe how to test.
Optional: attachments like images, videos, etc.
Checklist
[WPB-XXX]
.