1506892 - Add logging to ServiceWorker implementation exposed via about:serviceworkers

Status: ASSIGNED

(Core :: DOM: Service Workers, enhancement, P2)

Description

[Andrew Sutherland [:asuth] (he/him)]

We're in agreement to add MOZ_LOG support to our ServiceWorker and Cache API implementations to aid in debugging.

We don't expect to get this all done at once, hence this meta bug.  Any bug that adds logging or plans to add logging should be made to block this bug.  Bugs should ideally be short-lived with any additional logging landing quickly.  Any bug that wishes we had this logging should perhaps 'see also' this bug.

Good prior art for the initial implementation is IDB's abstractions at https://searchfox.org/mozilla-central/source/dom/indexedDB/ProfilerHelpers.h that covers both profiler markers as well as general MOZ_LOG-ing.  Note that its implementation is more complex due to its desire to log rich types (keys, key ranges) that don't trivially stringify on their own.  It's okay/desired for most logging to go directly to MOZ_LOG.

NB: There's a cool log analyzer tool that can consume the logs that lives at https://github.com/mayhemer/logan that can be augmented to understand the logs we add.

Comment 1

[Jens Stutte [:jstutte]]

I'd classify this as a task as it does not add value to the end-user.

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

I'm re-purposing this to cover an in-memory circular-ish log of structured JSON log entries for ServiceWorker related subsystems that can be exposed to about:serviceworkers. The end goal will be to make these available for synchronous logging via the existing MOZ_LOG subsystem as well for CI/automated testing, but the goal is to meet the debugging workflow of:

1. Someone experiences a problem that may involve ServiceWorkers.
2. I tell them to reproduce the problem, then go to about:serviceworkers, hit ctrl-f to find the origin in question where we're seeing problems, then either look at the results there or click a "copy" button and then paste that into the bug as an attachment or email the contents to me.
   • A critical point here is that this provides a means of limiting any data provided by the person reproducing the problem to the origin of question. A general concern I have about MOZ_LOG, especially for http logging/etc. is that it captures the entire fire-hose which provides an uncomfortable amount of information. For this specific use case we generally expect the bug to already be discussing a very specific site and so there ideally wouldn't be any additional private data revealed other than what would be present in the URLs for this specific origin.

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

Attachment: Bug 1506892 - Add logging to ServiceWorker implementation exposed via about:serviceworkers.

Comment 4

[Andrew Sutherland [:asuth] (he/him)]

Attachment: p1: Actually a further revised version of what was in phabricator

Comment 5

[Andrew Sutherland [:asuth] (he/him)]

Attachment: p2: patch enhancements

Comment 6

[Andrew Sutherland [:asuth] (he/him)]

Attachment: p3 patch enhancements

Comment 7

[Andrew Sutherland [:asuth] (he/him)]

Attachment: p4 patch enhancements

Comment 8

[Andrew Sutherland [:asuth] (he/him)]

My empty try push also seemed to go through, so the revisions should be available in mercurial for a while: https://treeherder.mozilla.org/jobs?repo=try&revision=8539d60813f41b677efd1dc06e7dd3b6f30406a4

Comment 9

[Randell Jesup [:jesup] (needinfo me)]

Attachment: swlog_p1.diff

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

Attachment: swlog_p2.diff

Comment 11

[Randell Jesup [:jesup] (needinfo me)]

Attachment: swlog_p3.diff

Comment 12

[Randell Jesup [:jesup] (needinfo me)]

Attachment: WIP: Bug 1506892 - sw_log_p1

Comment 13

[Randell Jesup [:jesup] (needinfo me)]

Attachment: WIP: Bug 1506892 - sw_log_p2

Depends on D136491

Comment 14

[Randell Jesup [:jesup] (needinfo me)]

Attachment: WIP: Bug 1506892 - sw_log_p3

Depends on D136492

Comment 15

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 9259820 [details] [diff] [review]
p1: Actually a further revised version of what was in phabricator

(obsoleting these patches in favor of :jesup's de-bitrotted versions. The patch layering for these was not intentional, just an attempt to do append-only periodic commits that would later be cleaned up into a semantic stack.)