Remove bookmark contents from sync debug logs.

RESOLVED FIXED in Firefox 55

Status

()

enhancement
P3
normal
RESOLVED FIXED
2 years ago
9 months ago

People

(Reporter: tcsc, Assigned: tiago, Mentored)

Tracking

({good-first-bug})

unspecified
Firefox 55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment)

Reporter

Description

2 years ago
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
Comment hidden (mozreview-request)
Assignee

Comment 2

2 years ago
Hi!

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)
Reporter

Comment 3

2 years ago
mozreview-review
Comment on attachment 8869346 [details]
Bug 1356281 - Remove bookmark contents from sync debug logs.

https://reviewboard.mozilla.org/r/140980/#review144702

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+

Comment 4

2 years ago
Pushed by tchiovoloni@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0529a8cb549c
Remove bookmark contents from sync debug logs. r=tcsc

Comment 5

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0529a8cb549c
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Reporter

Updated

2 years ago
Flags: needinfo?(tchiovoloni)
Assignee: nobody → tiago.paez11
You need to log in before you can comment on or make changes to this bug.