Georg Fritzsche [:gfritzsche] Reporter
	Description • 9 years ago

With the system changes for unified Telemetry, we won't reset the client id anymore.
This change is reasonable privacy-wise as we already persist historical ping data with the client id on disk.

Now, if/while we hold back unified Telemetry from release, we should avoid corner-case and data inconsistencies by stopping FHR/DRS to reset the saved client id.

	Alessio Placitelli [:Dexter] Assignee
	Comment 1 • 9 years ago

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3c512b904238

	Georg Fritzsche [:gfritzsche] Reporter
	Comment 2 • 9 years ago

Comment on attachment 8631524 [details] [diff] [review]
bug1180790.patch

Review of attachment 8631524 [details] [diff] [review]:
-----------------------------------------------------------------

I see some more - now unused - code related to this:
https://dxr.mozilla.org/mozilla-central/search?q=resetClientID

::: services/healthreport/tests/xpcshell/test_healthreporter.js
@@ +757,5 @@
>      do_check_null(reporter.lastSubmitID);
>      do_check_false(reporter.haveRemoteData());
>      do_check_false(server.hasDocument(reporter.serverNamespace, id));
>  
> +    // Client ID should be updated.

Nit: Just "Client ID should stay the same" and drop the other comment?

	Alessio Placitelli [:Dexter] Assignee
	Comment 3 • 9 years ago

Ouch, thanks.

	Alessio Placitelli [:Dexter] Assignee
	Comment 4 • 9 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=526fc4185bb6

	Pulsebot
	Comment 5 • 9 years ago

https://hg.mozilla.org/integration/fx-team/rev/24ea36d26155

	Alessio Placitelli [:Dexter] Assignee
	Comment 6 • 9 years ago

https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=24ea36d26155

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 7 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/24ea36d26155

	Georg Fritzsche [:gfritzsche] Reporter
	Comment 8 • 9 years ago

Comment on attachment 8631535 [details] [diff] [review]
bug1180790.patch - v2

Approval Request Comment
[Feature/regressing bug #]:
This is part of the uplift3 batch for the unified Telemetry project: http://bit.ly/1TCl4r8
This is targetting 41 now and these are the last minor "feature" patches blocking shipping - any further patches will be fixes for data quality etc. with very limited scope & impact.
[User impact if declined]:
Unified Telemetry can't ship.
This is a change needed for data consistency.
[Describe test coverage new/current, TreeHerder]:
This has good automated test-coverage.
[Risks and why]:
Low-risk, well understood change that only removes one action.
[String/UUID change made/needed]:
No string changes.

	Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)
	Comment 9 • 9 years ago

Tracked. Also, requesting QE team to verify this fix along with all other Unified Telemetry related fixes. Thanks!

	Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)
	Comment 10 • 9 years ago

Comment on attachment 8631535 [details] [diff] [review]
bug1180790.patch - v2

Should be safe to uplift as the patch has been in m-c for a few days and the patch is mainly removing the code where clientID was being reset.

	Ryan VanderMeulen [:RyanVM]
	Comment 11 • 9 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/a047db3341e4

	Ada [:adalucinet]
	Comment 12 • 9 years ago

Verified fixed on 41.0b8, across platforms [1] - no clientID reset encountered during our testing.

[1] Mac OS X 10.10.4, Ubuntu 14.04 32-bit and Windows 7 64-bit