Closed Bug 1386619 Opened 3 years ago Closed 3 years ago

If the automatic updates are disabled, the update ping is not sent for manual downloads

Categories

(Toolkit :: Telemetry, defect, P1)

56 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox57 --- fixed

People

(Reporter: vcarciu, Assigned: Dexter)

References

Details

(Whiteboard: [measurement:client])

Attachments

(1 file)

Steps to reproduce:
1. Open Firefox nightly build from 30.06.2017 with a clean profile.
2. Go to Menu - Options and disable automatic updates
3. Go to Help - About Firefox and observe that first update was already downloaded and  FF is waiting for restart in order to apply it
4. Restart Firefox and observe that it was upgraded
5. Manually check and download another update. Wait for Update ping.

Expected results:
Update ping should be sent in for both steps 3 and 5

Actual results:
Update ping is not sent if automatically updates are disabled

Notes/Issues: 
Build identifier: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0
Please note that there is a typo in Step 1. The build is from 30.07.2017 

Thanks
Severity: major → normal
Whiteboard: [measurement:client]
Matt, given the STR in comment 0, are we supposed to notify "update-downloaded" if the 'app.update.auto' pref is set to false, when going to the "Help-> About Firefox" menu?
Flags: needinfo?(mhowell)
Oh. Right, I suppose we won't be sending update-downloaded if we don't want to show any prompts, because that's all it's really been for prior to this. But there's not really anything else you could be observing, so we would have to rearrange where the notification is sent vs. how we determine what prompts to show...

I wonder if we'd be better off triggering the update ping directly from within the update service, like how we set the app update histograms, instead of refactoring the notifications to accommodate it. Robert, what would you say to that?
Flags: needinfo?(mhowell) → needinfo?(robert.strong.bugs)
(In reply to Matt Howell [:mhowell] from comment #3)
> I wonder if we'd be better off triggering the update ping directly from
> within the update service, like how we set the app update histograms,
> instead of refactoring the notifications to accommodate it. Robert, what
> would you say to that?

There's also another aspect to this: if the percentage of users with the 'app.update.auto' pref is relatively low, we might just add this edge case to the documentation for the update ping.
(In reply to Alessio Placitelli [:Dexter] from comment #4)
> (In reply to Matt Howell [:mhowell] from comment #3)
> > I wonder if we'd be better off triggering the update ping directly from
> > within the update service, like how we set the app update histograms,
> > instead of refactoring the notifications to accommodate it. Robert, what
> > would you say to that?
> 
> There's also another aspect to this: if the percentage of users with the
> 'app.update.auto' pref is relatively low, we might just add this edge case
> to the documentation for the update ping.

This query [1] suggests that only a very low percentage of the users disable automatic updates.
Let's keep the code as it is and just document the edge case in the update ping docs [2].

[1] - https://sql.telemetry.mozilla.org/queries/21667
[2] - https://gecko.readthedocs.io/en/latest/toolkit/components/telemetry/telemetry/data/update-ping.html#expected-behaviours
Flags: needinfo?(robert.strong.bugs)
Assignee: nobody → alessio.placitelli
Priority: P2 → P1
Comment on attachment 8899395 [details]
Bug 1386619 - Add documentation for 'update' pings edge cases.

https://reviewboard.mozilla.org/r/170640/#review175814

::: toolkit/components/telemetry/docs/data/update-ping.rst:55
(Diff revision 1)
>    - *for users who did not see the privacy policy*, the ``update`` ping is saved to disk and after the policy is displayed.
>  - **If the download of the update retries or other fallback occur**: the ``update`` ping will not be generated
>    multiple times, but only one time once the download is complete and verified.
> +- **If automatic updates are disabled**: when the user forces a manual update, no ``update`` ping will be generated.
> +- **If updates fail to apply**: in some cases the client will download the same update blob and generate a new ``update`` ping for the same target version and build id, with a different document id.
> +- **If the build update channel contains the CCK keyword**, the update ping will not report it but rather report a vanilla channel name (e.g. ``mozilla-cck-test-beta`` gets reported as ``beta``).

Does this match with expectations?
I.e. are those "cck" instances loading updates from our "beta" channel?
Attachment #8899395 - Flags: review?(gfritzsche) → review+
(In reply to Georg Fritzsche [:gfritzsche] from comment #7)
> Comment on attachment 8899395 [details]
> Bug 1386619 - Add documentation for 'update' pings edge cases.
> 
> https://reviewboard.mozilla.org/r/170640/#review175814
> 
> ::: toolkit/components/telemetry/docs/data/update-ping.rst:55
> (Diff revision 1)
> >    - *for users who did not see the privacy policy*, the ``update`` ping is saved to disk and after the policy is displayed.
> >  - **If the download of the update retries or other fallback occur**: the ``update`` ping will not be generated
> >    multiple times, but only one time once the download is complete and verified.
> > +- **If automatic updates are disabled**: when the user forces a manual update, no ``update`` ping will be generated.
> > +- **If updates fail to apply**: in some cases the client will download the same update blob and generate a new ``update`` ping for the same target version and build id, with a different document id.
> > +- **If the build update channel contains the CCK keyword**, the update ping will not report it but rather report a vanilla channel name (e.g. ``mozilla-cck-test-beta`` gets reported as ``beta``).
> 
> Does this match with expectations?
> I.e. are those "cck" instances loading updates from our "beta" channel?

I'm confident that that's the case, since the "beta" channel is read from the update blob itself.

@Matt, if a downloaded update blob is reporting "beta" as the current channel (while the environment in the browser reports "mozilla-cck-something-beta" for example), does it mean that it was downloaded from our beta channel?
Flags: needinfo?(mhowell)
I don't really know how CCK affects updates; I usually just start waving my hands whenever it comes up. Redirecting the above question to mkaply.
Flags: needinfo?(mhowell) → needinfo?(mozilla)
The use of the term "cck" there is a misnomer. cck is placed into the update channel whenever a partner build is involved:

http://searchfox.org/mozilla-central/source/toolkit/modules/UpdateUtils.jsm#41

For partner builds, we always get updates from the standard channels unless we explicitly setup a custom channel for some reason (which we have only done once).
Flags: needinfo?(mozilla)
(In reply to Mike Kaply [:mkaply] from comment #10)
> The use of the term "cck" there is a misnomer. cck is placed into the update
> channel whenever a partner build is involved:
> 
> http://searchfox.org/mozilla-central/source/toolkit/modules/UpdateUtils.
> jsm#41
> 
> For partner builds, we always get updates from the standard channels unless
> we explicitly setup a custom channel for some reason (which we have only
> done once).

Perfect, thanks Mike!
Pushed by alessio.placitelli@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/e8a5813bc9b9
Add documentation for 'update' pings edge cases. r=gfritzsche
https://hg.mozilla.org/mozilla-central/rev/e8a5813bc9b9
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.