1225522 - [Metrics] Histograms need packing before shipping

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Metrics, defect)

Description

[Tamara Hills [:thills]]

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.

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] tamarahills:bugfix/1225522-Pack-AdvancedTelemetry-Before-Shipping > mozilla-b2g:master

Comment 2

[Tamara Hills [:thills]]

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

Comment 3

[Russ Nicoletti [:russn]]

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

I left some comments in the PR.

Comment 4

[Tamara Hills [:thills]]

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

Comment 5

[Marshall Culpepper [:marshall_law]]

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 :)

Comment 6

[Tamara Hills [:thills]]

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

Comment 7

[Tamara Hills [:thills]]

https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=8821da19b3decc014a1ac50c494d7ddde9f585f9

Comment 8

[Marshall Culpepper [:marshall_law]]

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

Looks good!

Comment 9

[Tamara Hills [:thills]]

https://github.com/mozilla-b2g/gaia/commit/bafe9e9004a44dc4f27edec2ee3412b39be1b0c7