Write local items to the sync log if any logs have "Trace" level

RESOLVED FIXED in Firefox 40

Status

P5
normal
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: markh, Assigned: markh)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 40
Dependency tree / graph
Bug Flags:
firefox-backlog +
qe-verify -

Firefox Tracking Flags

(firefox37 unaffected, firefox38 affected, firefox39 affected, firefox40 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
Created attachment 8590661 [details] [diff] [review]
0008-Bug-XX-have-the-readinglist-sync-engine-log-existing.patch

Bug 1152307 has a sync log that looks successful (ie, does nothing) but the user reports Sync not working. Today I had someone in #taskcontinuity say they were having trouble syncing deleted items without many details.

Which made me think it would be cool if we could arrange for the logs to report the local items before it starts syncing. This patch does this, but only if the log level has been adjusted to "Trace", so it does *not* happen by default - but it allows us to ask a bug reporter to flip a pref to "Trace" and have this local info written.  This patch generates the following for me:

> 1428649495074   readinglist.sync        TRACE   Have 2 local item(s)
> 1428649495074   readinglist.sync        TRACE   {"guid":"3f815d124a7b402cb7d36bb3e3415b20","url":"https://bugzilla.mozilla.org/show_bug.cgi?id=1149023","resolvedURL":"https://bugzilla.mozilla.org/show_bug.cgi?id=1149023","serverLastModified":1427870578619,"syncStatus":0}
> 1428649495074   readinglist.sync        TRACE   {"guid":"47d75830c4024b6a98f994382ea0a296","url":"http://www.abc.net.au/news/2015-04-01/local-surfboard-makers-are-being-wiped-out-foreign-drop-ins/6362240","resolvedURL":"http://www.abc.net.au/news/2015-04-01/local-surfboard-makers-are-being-wiped-out-foreign-drop-ins/6362240","serverLastModified":1427870786575,"syncStatus":0}
> 1428649495077   readinglist.sync        TRACE   Have 1 deleted item(s): ["3b7e0eb83def43a999eba920a0baf3bf"]

Drew, what do you think?
Attachment #8590661 - Flags: feedback?(adw)
Blocks: 1132074
Status: NEW → ASSIGNED
Iteration: --- → 40.1 - 13 Apr
Flags: qe-verify?
Flags: firefox-backlog+
Priority: -- → P5
Hi Mark, can you provide a point value.
Assignee: nobody → mhammond
Flags: needinfo?(mhammond)
Attachment #8590661 - Flags: feedback?(adw) → feedback+
(Assignee)

Updated

4 years ago
Attachment #8590661 - Flags: review?(adw)
(Assignee)

Comment 2

4 years ago
We should consider uplifting this - on one hand it is purely diagnostic so doesn't impact whether the feature works or not (so normally wouldn't be considered), but on the other hand it will give us a powerful tool for people who report bugs on 38.
Points: --- → 1
status-firefox37: --- → unaffected
status-firefox38: --- → affected
status-firefox39: --- → affected
status-firefox40: --- → affected
Flags: needinfo?(mhammond)
Iteration: 40.1 - 13 Apr → 40.2 - 27 Apr
Attachment #8590661 - Flags: review?(adw) → review+
Flags: qe-verify? → qe-verify-
https://hg.mozilla.org/mozilla-central/rev/36b7439df64e
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-firefox40: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 40
(Assignee)

Comment 5

4 years ago
Comment on attachment 8590661 [details] [diff] [review]
0008-Bug-XX-have-the-readinglist-sync-engine-log-existing.patch

This is not important from a functionality POV, but would be very useful for diagnostics when we get reports of "does not sync". The functionality is behind a pref and the patch is simple, so I'm requesting uplift anyway

Approval Request Comment
[Feature/regressing bug #]: readinglist
[User impact if declined]: Impact is more on dev/qa when tracking down unexplained "sync doesn't work" reports.
[Describe test coverage new/current, TreeHerder]: No new tests
[Risks and why]: Low - see above.
[String/UUID change made/needed]: None
Attachment #8590661 - Flags: approval-mozilla-beta?
Attachment #8590661 - Flags: approval-mozilla-aurora?
Mark, do we still need to uplift this given that we don't seem to be planning to ship sync in 39?   Or, will uplifting it now make things easier to understand or disentangle later, in some way?
Flags: needinfo?(mhammond)
(Assignee)

Comment 7

4 years ago
Comment on attachment 8590661 [details] [diff] [review]
0008-Bug-XX-have-the-readinglist-sync-engine-log-existing.patch

No need to uplift this.
Flags: needinfo?(mhammond)
Attachment #8590661 - Flags: approval-mozilla-beta?
Attachment #8590661 - Flags: approval-mozilla-aurora?
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.