1348727 - Write validation summary information to the sync logs.

Status: RESOLVED FIXED

(Firefox :: Sync, enhancement, P3)

Description

[Mark Hammond [:markh] [:mhammond]]

It would be helpful for diagnosis if the bookmark validator wrote summary information to the logs, to help diagnose when certain problems started.

Comment 1

[Jeongkyu Kim]

I'd like to work on this issue. Could someone guide me on where to start?

Comment 2

[Thom Chiovoloni [:tcsc] (ex-moco)]

Great!

Easiest place to do this would be right before the return statement in this function: https://dxr.mozilla.org/mozilla-central/rev/7a9536f89bc75b0672060f16ffbe6eb2c1ff3deb/services/sync/modules/bookmark_validator.js#845 You can log using the engines logger. Specifically, calling `engine._log.debug(msg)` in that function.

You want to loop over the items returned by result.problemData.getSummary() which are records containing string "name" and numeric "count" properties (a count of 0 means "no problem of this type found", but validation is rare enough that I think there's no reason not to log these anyway).

Could you also log the duration? Something along along the lines of engine._log.debug(`Validated bookmarks in ${duration}ms`), before you loop over the summary.

This doesn't need tests, and you should request review from me.

Comment 3

[Jeongkyu Kim]

Attachment: Bug 1348727 - Write validation summary information to the sync logs

Review commit: https://reviewboard.mozilla.org/r/152220/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/152220/

Comment 4

[Jeongkyu Kim]

Thanks for your kind explanation.

The submitted patch will write the validation summary as follows:

1498236933608	Sync.Engine.Bookmarks	DEBUG	Validated bookmarks in 397ms
1498236933608	Sync.Engine.Bookmarks	DEBUG	Problem summary
1498236933608	Sync.Engine.Bookmarks	DEBUG	  clientMissing: 0
1498236933608	Sync.Engine.Bookmarks	DEBUG	  serverMissing: 0
1498236933608	Sync.Engine.Bookmarks	DEBUG	  serverDeleted: 0
1498236933608	Sync.Engine.Bookmarks	DEBUG	  serverUnexpected: 0
1498236933608	Sync.Engine.Bookmarks	DEBUG	  structuralDifferences: 0
1498236933608	Sync.Engine.Bookmarks	DEBUG	  differences: 0
1498236933608	Sync.Engine.Bookmarks	DEBUG	  missingIDs: 0
1498236933608	Sync.Engine.Bookmarks	DEBUG	  rootOnServer: 0
1498236933608	Sync.Engine.Bookmarks	DEBUG	  duplicates: 0
1498236933608	Sync.Engine.Bookmarks	DEBUG	  parentChildMismatches: 0
1498236933608	Sync.Engine.Bookmarks	DEBUG	  cycles: 0
1498236933608	Sync.Engine.Bookmarks	DEBUG	  clientCycles: 0
1498236933608	Sync.Engine.Bookmarks	DEBUG	  badClientRoots: 0
1498236933608	Sync.Engine.Bookmarks	DEBUG	  orphans: 0
1498236933608	Sync.Engine.Bookmarks	DEBUG	  missingChildren: 0
1498236933608	Sync.Engine.Bookmarks	DEBUG	  deletedChildren: 0
1498236933608	Sync.Engine.Bookmarks	DEBUG	  multipleParents: 0
1498236933608	Sync.Engine.Bookmarks	DEBUG	  deletedParents: 0
1498236933608	Sync.Engine.Bookmarks	DEBUG	  childrenOnNonFolder: 0
1498236933608	Sync.Engine.Bookmarks	DEBUG	  duplicateChildren: 0
1498236933608	Sync.Engine.Bookmarks	DEBUG	  parentNotFolder: 0

Comment 5

[Thom Chiovoloni [:tcsc] (ex-moco)]

Comment on attachment 8880863 [details]
Bug 1348727 - Write validation summary information to the sync logs

https://reviewboard.mozilla.org/r/152220/#review157326

Looks great, thanks!

One note: the commit message should have r?reviewer (not r=) before someone gives it an r+, the ? is changed to = when landing (sometimes earlier, but it should only be used if that person has given it r+). It doesn't matter that much, but it's worth keeping in mind for the future.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/70d523c77a20
Write validation summary information to the sync logs r=tcsc

Comment 7

[Jeongkyu Kim]

> One note: the commit message should have r?reviewer (not r=) before someone gives it an r+...

Ah, I see. Thanks for pointing it out!

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/70d523c77a20