Closed Bug 1131069 Opened 10 years ago Closed 10 years ago

Missing histograms in Nightly payloads

Categories

(Toolkit :: Telemetry, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla38
Tracking Status
firefox38 --- fixed

People

(Reporter: gfritzsche, Assigned: Dexter)

References

Details

Attachments

(2 files, 2 obsolete files)

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.
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|.
It looks like we have not noticed this in tests because bug 1129504 accidentally removed test coverage in the last days :(
Attached patch part 1 - Replaces the constants (obsolete) (deleted) — Splinter Review
Assignee: gfritzsche → alessio.placitelli
Status: NEW → ASSIGNED
Attachment #8561412 - Flags: review?(gfritzsche)
Attached patch part 2 - Fixes the test coverage. (obsolete) (deleted) — Splinter Review
Attachment #8561413 - Flags: review?(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.
Attachment #8561412 - Flags: review?(gfritzsche) → review+
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?
Attachment #8561413 - Flags: review?(gfritzsche) → review+
Attached patch part 1 - Replaces the constants (deleted) — Splinter Review
Fixed the commit message
Attachment #8561412 - Attachment is obsolete: true
Fixed the commit message.
Attachment #8561413 - Attachment is obsolete: true
Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: