Closed Bug 1356281 Opened 5 years ago Closed 5 years ago

Remove bookmark contents from sync debug logs.


(Firefox :: Sync, enhancement, P3)




Firefox 55
Tracking Status
firefox55 --- fixed


(Reporter: tcsc, Assigned: tiago, Mentored)


(Keywords: good-first-bug)


(1 file)

We end up logging out all bookmarks records we apply as we apply them.

IIRC, in theory things like this should only be present for trace logs.

The downside here is it makes users more reluctant to attach their logs, and means we need to mark bugs as confidential sometimes when we otherwise wouldn't need to.

To be specific, BookmarkStore.prototype.update, and BookmarkStore.prototype.create both need fixing (removing the `, item` is probably the right choice, although making them _log.trace and not _log.debug would also be fine).

I don't see any others cases looking at my logs, but I could be missing some.
Mentor: tchiovoloni
Keywords: good-first-bug
Priority: -- → P3

I just submitted a patch for review, in that patch I changed the log from debug to trace.

I was wondering if I should also remove the bookmark information (removing the `, item`). In that case I was thinking the log to be something like "Created a bookmark" and "Updated a bookmark" respectively. Is that ok?

Thanks in advance.
Flags: needinfo?(tchiovoloni)
Comment on attachment 8869346 [details]
Bug 1356281 - Remove bookmark contents from sync debug logs.

Looks good to me. Some changes around serialization should land in the near future that will allow us to only log partial records so I don't think removing the item is necessary any more.
Attachment #8869346 - Flags: review?(tchiovoloni) → review+
Pushed by
Remove bookmark contents from sync debug logs. r=tcsc
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Flags: needinfo?(tchiovoloni)
Assignee: nobody → tiago.paez11
You need to log in before you can comment on or make changes to this bug.