1131069 - Missing histograms in Nightly payloads

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect)

Description

[Georg Fritzsche [:gfritzsche]]

Per this graph we have a sharp drop in Nightly telemetry payload size:
http://ec2-50-112-66-71.us-west-2.compute.amazonaws.com:4352/#sandboxes/TelemetryChannelMetrics60DaysAggregator/outputs/TelemetryChannelMetrics60DaysAggregator.nightly.cbuf

Roberto found that we lost all histograms except TEST_RELEASE_OPTOUT and all keyed histograms except TELEMETRY_TEST_KEYED_RELEASE_OPTOUT.

Comment 1

[Georg Fritzsche [:gfritzsche]]

From the above it looks like this is bug 1120369.

Apparently i didn't do the renaming of constants properly :(
DATASET_EXTENDED should now be DATASET_RELEASE_CHANNEL_OPTIN:
https://dxr.mozilla.org/mozilla-central/search?q=DATASET_EXTENDED&case=true
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/nsITelemetry.idl#38

It looks like we just end up only sending the _OPTOUT histograms because DATASET_EXTENDED is |undefined| and hence |0|.

Comment 2

[Georg Fritzsche [:gfritzsche]]

It looks like we have not noticed this in tests because bug 1129504 accidentally removed test coverage in the last days :(

Comment 3

[Alessio Placitelli [:Dexter]]

Attachment: part 1 - Replaces the constants

Comment 4

[Alessio Placitelli [:Dexter]]

Attachment: part 2 - Fixes the test coverage.

Comment 5

[Alessio Placitelli [:Dexter]]

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

Comment 6

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8561412 [details] [diff] [review]
part 1 - Replaces the constants

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

r+ if this is green on try.
I think this could use a better commit message though - we could just read the diff to see what exactly was replaced.

Comment 7

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8561413 [details] [diff] [review]
part 2 - Fixes the test coverage.

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

Looks good if this is green on try too.
Can we be a little more descriptive and mention the "missing telemetry payload test coverage" in the commit message instead of just naming the function?

Comment 8

[Alessio Placitelli [:Dexter]]

Attachment: part 1 - Replaces the constants

Fixed the commit message

Comment 9

[Alessio Placitelli [:Dexter]]

Attachment: part 2 - Fixes the test coverage.

Fixed the commit message.

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/5975847e7d0b
https://hg.mozilla.org/integration/fx-team/rev/c2fbc180eeb7

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5975847e7d0b
https://hg.mozilla.org/mozilla-central/rev/c2fbc180eeb7