[Metrics] Histograms need packing before shipping

RESOLVED FIXED

Status

Firefox OS
Metrics
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: thills, Assigned: thills)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
Histograms need to be packed before shipping.  This will make sure they deliver the .values as opposed to the .counts and .ranges attributes.  The server seems to understand only the .values when looking at histograms.
(Assignee)

Updated

2 years ago
Blocks: 1152000
(Assignee)

Updated

2 years ago
Assignee: nobody → thills
No longer blocks: 1152000
Status: NEW → ASSIGNED
(Assignee)

Updated

2 years ago
Blocks: 1152000

Comment 1

2 years ago
Created attachment 8688629 [details] [review]
[gaia] tamarahills:bugfix/1225522-Pack-AdvancedTelemetry-Before-Shipping > mozilla-b2g:master
(Assignee)

Comment 2

2 years ago
Comment on attachment 8688629 [details] [review]
[gaia] tamarahills:bugfix/1225522-Pack-AdvancedTelemetry-Before-Shipping > mozilla-b2g:master

I've prepared a patch to put things into the dict format.  Can you have a look?

Thanks,
-tamara
Attachment #8688629 - Flags: feedback?(rnicoletti)
Comment on attachment 8688629 [details] [review]
[gaia] tamarahills:bugfix/1225522-Pack-AdvancedTelemetry-Before-Shipping > mozilla-b2g:master

I left some comments in the PR.
Attachment #8688629 - Flags: feedback?(rnicoletti) → feedback+
(Assignee)

Comment 4

2 years ago
Comment on attachment 8688629 [details] [review]
[gaia] tamarahills:bugfix/1225522-Pack-AdvancedTelemetry-Before-Shipping > mozilla-b2g:master

Hi Marshall,

This patch makes a needed change to the histogram format... changing it from the raw format to the 'dict' format.  Apparently, spark server needs dict format.  

What does this mean?  It should convert a histogram that looks like this:
{
                min: 1,
                max: 10000,
                histogram_type: 1,
                sum: 99,
                sum_squares_lo: 9801,
                sum_squares_hi: 0,
                ranges: [0, 1, 1251, 2501, 3751, 5001, 6250, 7500, 8750, 10000],
                counts: [0, 1, 0, 0, 0, 0, 0, 0, 0, 0]
              }

to this:

{
                range: [1, 10000],
                bucket_count:10,
                histogram_type:1,
                values:{'0':0, '1':1, '1251':0},
                sum: 99,
                sum_squares_lo:9801,
                sum_squares_hi:0
              }

It saves us some bytes as well as it converts the .ranges and .counts to .values and only includes the non-zeros.

Thanks,
-tamara
Attachment #8688629 - Flags: review?(marshall)
Comment on attachment 8688629 [details] [review]
[gaia] tamarahills:bugfix/1225522-Pack-AdvancedTelemetry-Before-Shipping > mozilla-b2g:master

This looks good, but check some of my questions/nits in the PR before landing :)
Attachment #8688629 - Flags: review?(marshall) → review+
(Assignee)

Comment 6

2 years ago
Comment on attachment 8688629 [details] [review]
[gaia] tamarahills:bugfix/1225522-Pack-AdvancedTelemetry-Before-Shipping > mozilla-b2g:master

Hi Marshall,

Sorry, this took me so long.  I did address the nits/questions in the PR.  In addition, I made a change based on an error I found in the addonHistograms.  I discovered this while trying to graph the data.

You'll see I removed one level out of the addonHistograms.  This will show up in the advanced_telemetry_tests.js as well as advanced_telemetry.js.  I decided to fix this in a follow up patch (both gecko and gaia)... but for now, it needs that level removed as gecko is not sending two levels currently, it's just sending one.  I'm going to make the gecko change to make it send both levels.

Hope this makes sense.  I know you already reviewed, but thought I would give you another chance to look over the change I made.

Thanks,

-tamara
Attachment #8688629 - Flags: review+ → review?(marshall)
(Assignee)

Comment 7

2 years ago
https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=8821da19b3decc014a1ac50c494d7ddde9f585f9
Comment on attachment 8688629 [details] [review]
[gaia] tamarahills:bugfix/1225522-Pack-AdvancedTelemetry-Before-Shipping > mozilla-b2g:master

Looks good!
Attachment #8688629 - Flags: review?(marshall) → review+
(Assignee)

Comment 9

2 years ago
https://github.com/mozilla-b2g/gaia/commit/bafe9e9004a44dc4f27edec2ee3412b39be1b0c7
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.