Closed Bug 1157581 Opened 4 years ago Closed 4 years ago

Add releaseChannelCollection field to app update histograms

Categories

(Toolkit :: Application Update, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox37 --- unaffected
firefox38 --- unaffected
firefox38.0.5 --- unaffected
firefox39 --- fixed
firefox40 --- fixed
firefox-esr31 --- unaffected
firefox-esr38 --- unaffected

People

(Reporter: rstrong, Assigned: rstrong)

Details

Attachments

(2 files, 3 obsolete files)

No description provided.
Attached patch patch in progress (obsolete) — Splinter Review
Vladan, can I get feedback from you on the use of the releaseChannelCollection field. On the release channel several of these histograms could record once daily which seems like too much for the release channel so I've been somewhat conservative. For other histograms they will record only when attempting to apply an update which happens approximately every 6 weeks unless there is a security release. Thanks!
Assignee: nobody → robert.strong.bugs
Status: NEW → ASSIGNED
Attachment #8596373 - Flags: feedback?(vdjeric)
Note: the *_EXTERNAL histograms should only report when the date calculated using the build id is 63 or more days older than the current date so the number of submissions should be low.
Comment on attachment 8596373 [details] [diff] [review]
patch in progress

The updater histograms can record as often as they like, there is no privacy or performance-impact issues with recording frequently. Each measurement just increments a counter. As an extreme example, one of our other histograms records a measurement every time a frame is painted.

I am a bit uncomfortable with making these histograms opt-out indefinitely. I'd be ok if we added an expiry date for the histograms and reviewed whether we still need to be histograms to be opt-out near the expiry date.

I'm about to go on PTO for a week, so maybe you could continue the discussion with another "privacy peer" (Benjamin or Ally): https://wiki.mozilla.org/Firefox/Data_Collection
Attachment #8596373 - Flags: feedback?(vdjeric)
Thanks and will do
Attached patch patch (obsolete) — Splinter Review
Talked with bsmedberg about these and we'll change the values after we've made progress with lessening the error cases.
Attachment #8596373 - Attachment is obsolete: true
Attachment #8597435 - Flags: review?(benjamin)
To be clear, there are two bits here:

* We intend to monitor update metrics permanently as an ongoing quality metric. The histograms necessary for that will be permanent.
* We also need extra metrics to diagnose update orphaning. We don't have a clear end-date for that project and so setting an expiration for those is premature, but Robert will keep that as a task as we wrap up that project.
Comment on attachment 8597435 [details] [diff] [review]
patch

Why are the following metrics not on for everyone? 

UPDATE_CHECK_CODE_NOTIFY: That seems important to monitor AUS, even for release users.
UPDATE_CANNOT_STAGE_NOTIFY: Will another measurement also measure failure here? That there is a failure is something we will probably want to surface via self-support.

Since opt-in is the default/current setting, would it be better to just include the "opt-out" value? Georg, do we have a recommendation for that?
Flags: needinfo?(robert.strong.bugs)
Flags: needinfo?(gfritzsche)
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #7)
> Since opt-in is the default/current setting, would it be better to just
> include the "opt-out" value? Georg, do we have a recommendation for that?

You mean whether it's better to explicitly specify "opt-in" or just leave it to the default?
I recommend leaving it out / to the default, that should always be the "safe" option.
Flags: needinfo?(gfritzsche)
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #7)
> Comment on attachment 8597435 [details] [diff] [review]
> patch
> 
> Why are the following metrics not on for everyone? 
> 
> UPDATE_CHECK_CODE_NOTIFY: That seems important to monitor AUS, even for
> release users.
Will change

> UPDATE_CANNOT_STAGE_NOTIFY: Will another measurement also measure failure
> here? That there is a failure is something we will probably want to surface
> via self-support.
Yes. There is a separate histogram to indicate success and failure code.

This indicates whether staging will be attempted and does not indicate a failure. This indicates that the update can't be staged due to any of the following: app.update.staging.enabled pref is false along with the user doesn't have write access, on Windows the service isn't installed along with the user doesn't have write access, on Windows the service is installed but the app.update.service.enabled pref is false along with the user doesn't have write access, on Windows there is another instance that has an update mutex, and on non-Windows the user doesn't have write access.

> Since opt-in is the default/current setting, would it be better to just
> include the "opt-out" value? Georg, do we have a recommendation for that?
I'll remove those per comment #8
Flags: needinfo?(robert.strong.bugs)
Attached patch patch rev2 (obsolete) — Splinter Review
Attachment #8597435 - Attachment is obsolete: true
Attachment #8597435 - Flags: review?(benjamin)
Attachment #8598051 - Flags: review?(benjamin)
I changed it so it just makes them all opt-out. After this hits release for a cycle I'll back that off to opt-in.
Attachment #8598051 - Attachment is obsolete: true
Attachment #8598051 - Flags: review?(benjamin)
Attachment #8599070 - Flags: review?(benjamin)
Attachment #8599070 - Flags: review?(benjamin) → review+
https://hg.mozilla.org/mozilla-central/rev/e9cf26c0e790
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Comment on attachment 8599070 [details] [diff] [review]
patch - remove bitrot

Approval Request Comment
[Feature/regressing bug #]: This is in support of the update orphaning project
[User impact if declined]: We won't be able to analyze update orphaning as well as we would like and hence might not be able to find issues that prevent users from updating.
[Describe test coverage new/current, TreeHerder]: Tested locally and landed on m-c.
[Risks and why]: Minimal. This sets the condition in which we collect app update telemetry data on release channels (e.g. we collect data unless the user explicitly opts out).
[String/UUID change made/needed]: None
Attachment #8599070 - Flags: approval-mozilla-aurora?
Comment on attachment 8599070 [details] [diff] [review]
patch - remove bitrot

Approved for uplift to aurora.
Attachment #8599070 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.