1410907 - Add canRecordReleaseData/canRecordPrereleaseData to nsITelemetry

Status: RESOLVED FIXED

(Toolkit :: Telemetry, enhancement, P1)

Description

[Alessio Placitelli [:Dexter]]

In order to support the "data preferences" changes, we could should add the following read only properties/methods to nsITelemetry:

- canRecordReleaseData, that reflects the value returned by canRecordBase[1];
- canRecordPrereleaseData, that reflects the value returned by canRecordExtended[1].

Once these land, we should update the docs to mention that they should be used as canRecordBase/canRecordExtended will be deprecated.

[1] - http://searchfox.org/mozilla-central/rev/8a6a6bef7c54425970aa4fb039cc6463a19c0b7f/toolkit/components/telemetry/nsITelemetry.idl#265,275

Comment 1

[Chris H-C :chutten]

Attachment: bug 1410907 - Add CanRecord(Pre)Release data to nsITTelemetry

We're starting a shift towards talking about release/prerelease data instead of
base/extended collection. For now this is just a naming change.

At the same time, we can render these attributes read-only, which is nice.

Review commit: https://reviewboard.mozilla.org/r/196346/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/196346/

Comment 2

[Alessio Placitelli [:Dexter]]

Comment on attachment 8925110 [details]
bug 1410907 - Add CanRecord(Pre)Release data to nsITTelemetry

https://reviewboard.mozilla.org/r/196346/#review201782

Hey Chris, thanks, this looks good. It would be great to have these documented in some guide, marking the usage of canRecordBase/Extended deprecated in favour of these. This is probably something for bug 1413969 et al :)

Comment 3

[Chris H-C :chutten]

Yup. Hoping to handle it as part of bug 1406394

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/58b0f65614be
Add CanRecord(Pre)Release data to nsITTelemetry r=Dexter

Comment 5

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for build bustage at toolkit/components/telemetry/Telemetry.cpp:1591:

https://hg.mozilla.org/integration/autoland/rev/d0c21ca40a543227a1a1f2b1a075d9620abdefd5

Push with bustage: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=58b0f65614bed267d3297baadf93d2f29f0c3886&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Build log: https://treeherder.mozilla.org/logviewer.html#?job_id=142449907&repo=autoland

[task 2017-11-06T15:37:20.798Z] 15:37:20     INFO -  /builds/worker/workspace/build/src/toolkit/components/telemetry/Telemetry.cpp:1591:1: error: 'static bool {anonymous}::TelemetryImpl::CanRecordPrereleaseData()' defined but not used [-Werror=unused-function]
[task 2017-11-06T15:37:20.798Z] 15:37:20     INFO -   TelemetryImpl::CanRecordPrereleaseData()
[task 2017-11-06T15:37:20.798Z] 15:37:20     INFO -   ^~~~~~~~~~~~~
[task 2017-11-06T15:37:20.799Z] 15:37:20     INFO -  /builds/worker/workspace/build/src/toolkit/components/telemetry/Telemetry.cpp:1585:1: error: 'static bool {anonymous}::TelemetryImpl::CanRecordReleaseData()' defined but not used [-Werror=unused-function]
[task 2017-11-06T15:37:20.799Z] 15:37:20     INFO -   TelemetryImpl::CanRecordReleaseData()
[task 2017-11-06T15:37:20.799Z] 15:37:20     INFO -   ^~~~~~~~~~~~~
[task 2017-11-06T15:37:20.799Z] 15:37:20     INFO -  cc1plus: all warnings being treated as errors
[task 2017-11-06T15:37:20.800Z] 15:37:20     INFO -  /builds/worker/workspace/build/src/config/rules.mk:1038: recipe for target 'Telemetry.o' failed
[task 2017-11-06T15:37:20.800Z] 15:37:20     INFO -  gmake[5]: *** [Telemetry.o] Error 1

Comment 6

[Chris H-C :chutten]

I may have missed one step in the call chain which would take care of this without us actually having any consumers of this API yet. I can double-check tomorrow.

Comment 7

[Chris H-C :chutten]

Comment on attachment 8925110 [details]
bug 1410907 - Add CanRecord(Pre)Release data to nsITTelemetry

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/196346/diff/1-2/

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eba518f10477
Add CanRecord(Pre)Release data to nsITTelemetry r=Dexter

Comment 9

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/eba518f10477

Comment 10

[Madalin Cotetiu]

Verified as fixed in Nigtly 59.0a1 and Beta 58.0b3. (this change is not available in a release build so the testing was focused only in nightly and beta builds )

The status of the pref was verified in the following platforms and FF versions:

canRecordReleaseData
Release build not available for testing
go to about:telemetry, open browser console
enter Services.telemetry.canRecordReleaseData

Desktop  (windows 7 x64)
Release - 
Beta - true
Nightly -  true

Desktop (MacOS 10.13)
Release - 
Beta - true 
Nightly -  true

Desktop (Ubuntu 14.04)
Release - 
Beta -  true 
Nightly -  true
	
Android 7.1.2
Release - 
Beta - true
Nightly - true



canRecordPrereleaseData

Release build not available for testing
go to about:telemetry, open browser console
enter Services.telemetry.canRecordPrereleaseData

Desktop  (windows 7 x64)
Release - 
Beta - true
Nightly -  true


Desktop (MacOS 10.13)
Release - 
Beta - true
Nightly -  true


Desktop (Ubuntu 14.04)
Release - 
Beta -  true
Nightly -  true
	

Android 7.1.2
Release - 
Beta - true
Nightly - true