Add histograms for shill network metrics

Add histograms for shill network metrics

Added histogram Network.Shill.UserInitiatedEvents for sampling
the number of user-initiated events in shill. Currently only
tracks the user-initated WIFI scans.

Added histogram Network.Shill.WiFi.TransmitBitrateMbps for sampling
the transmit bitrate for the wifi device. The bitrate is updated
every minutes when wifi is connected to a network.

Added histogram Network.Shill.WiFi.UserInitiatedConnectionResult
for sampling the outcome of user-initiated connection attempts.
This histogram is updated everytime the wifi connection is initiated
by the user.

Consolidate the histograms for DHCPOptionFailureDetected, where there
was one histogram per technology type. Instead, changed to one only
histogram, with each technology type as a bucket under the histogram.

BUG=chromium:368761, chromium:369135, chromium:369545
TEST=Start chrome, browse to chrome://histograms to make
sure these histograms exist after manually
initiate/establish a wifi network connection.
NOTRY=true

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=271135

[Ilya Sherman, 2014-05-09 23:55:54]

Hi, apologies for being slow to respond.  I haven't had a chance to review this
yet, but it's definitely on my radar.  I'm aiming to get to it early next week.
Thanks for your patience!

[Ilya Sherman, 2014-05-13 01:28:14]

https://codereview.chromium.org/273883003/diff/1/tools/metrics/histograms/his...
File tools/metrics/histograms/histograms.xml (left):

https://codereview.chromium.org/273883003/diff/1/tools/metrics/histograms/his...
tools/metrics/histograms/histograms.xml:14960: -</histogram>
Please mark the old histogram as deprecated, rather than deleteting it.  Ditto
for the other old histograms.

https://codereview.chromium.org/273883003/diff/1/tools/metrics/histograms/his...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/273883003/diff/1/tools/metrics/histograms/his...
tools/metrics/histograms/histograms.xml:15323: +</histogram>
Histograms with just a single bucket are rarely a good design.  It's very hard
to understand whether a raw number like "10,000" is "a lot" or "barely anything"
in context.  That depends on how often you expect these events, which in turn
depends on a bunch of varying factors like the number of users, the news cycle,
etc.  It's generally best to build in a baseline, so that you can understand the
metric as a relative number, i.e. a percentage.  Is there a baseline that you
could add to this histogram to make the data easier to interpret?

https://codereview.chromium.org/273883003/diff/1/tools/metrics/histograms/his...
tools/metrics/histograms/histograms.xml:15692: +    for the wifi device when it
is connected to a network.
How often is it sampled / when is this histogram emitted to?

https://codereview.chromium.org/273883003/diff/1/tools/metrics/histograms/his...
tools/metrics/histograms/histograms.xml:15701: +    wifi connection attempts.
Your use of "sampling" makes me wonder whether this is recorded on every
user-initiated wifi connection attempt, or just on a subset.

https://codereview.chromium.org/273883003/diff/1/tools/metrics/histograms/his...
tools/metrics/histograms/histograms.xml:38085: +  <int value="6"
label="Unknown"/>
It's unfortunate that you've chosen the last bucket to map to unknown, as it
makes it hard to extend the histogram in the future.  Well, not hard, exactly,
but slightly odd -- the "unknown" bucket ends up somewhere in the middle.  For
future reference, the best practice is always to have "Unknown" come first.

[zqiu1, 2014-05-14 17:29:53]

https://codereview.chromium.org/273883003/diff/1/tools/metrics/histograms/his...
File tools/metrics/histograms/histograms.xml (left):

https://codereview.chromium.org/273883003/diff/1/tools/metrics/histograms/his...
tools/metrics/histograms/histograms.xml:14960: -</histogram>
On 2014/05/13 01:28:14, Ilya Sherman wrote:
> Please mark the old histogram as deprecated, rather than deleteting it.  Ditto
> for the other old histograms.

Done.

https://codereview.chromium.org/273883003/diff/1/tools/metrics/histograms/his...
tools/metrics/histograms/histograms.xml:14960: -</histogram>
On 2014/05/13 01:28:14, Ilya Sherman wrote:
> Please mark the old histogram as deprecated, rather than deleteting it.  Ditto
> for the other old histograms.

Done.

https://codereview.chromium.org/273883003/diff/1/tools/metrics/histograms/his...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/273883003/diff/1/tools/metrics/histograms/his...
tools/metrics/histograms/histograms.xml:15323: +</histogram>
For this histogram, I don't think there is a good baseline, we'll use the other
histograms to interpret the result. And this histogram will be expanded in the
future to include other events (more buckets).
On 2014/05/13 01:28:14, Ilya Sherman wrote:
> Histograms with just a single bucket are rarely a good design.  It's very hard
> to understand whether a raw number like "10,000" is "a lot" or "barely
anything"
> in context.  That depends on how often you expect these events, which in turn
> depends on a bunch of varying factors like the number of users, the news
cycle,
> etc.  It's generally best to build in a baseline, so that you can understand
the
> metric as a relative number, i.e. a percentage.  Is there a baseline that you
> could add to this histogram to make the data easier to interpret?

https://codereview.chromium.org/273883003/diff/1/tools/metrics/histograms/his...
tools/metrics/histograms/histograms.xml:15692: +    for the wifi device when it
is connected to a network.
The transmit bitrate histogram is sampled/reported once a minute when the wifi
device is connected to a network.
On 2014/05/13 01:28:14, Ilya Sherman wrote:
> How often is it sampled / when is this histogram emitted to?

https://codereview.chromium.org/273883003/diff/1/tools/metrics/histograms/his...
tools/metrics/histograms/histograms.xml:15701: +    wifi connection attempts.
Sorry, bad wording. This histogram will report the result of every
user-initiated wifi connection attempt. I will update it.
On 2014/05/13 01:28:14, Ilya Sherman wrote:
> Your use of "sampling" makes me wonder whether this is recorded on every
> user-initiated wifi connection attempt, or just on a subset.

https://codereview.chromium.org/273883003/diff/1/tools/metrics/histograms/his...
tools/metrics/histograms/histograms.xml:38085: +  <int value="6"
label="Unknown"/>
Thanks for the tip, I will do that in the future.
On 2014/05/13 01:28:14, Ilya Sherman wrote:
> It's unfortunate that you've chosen the last bucket to map to unknown, as it
> makes it hard to extend the histogram in the future.  Well, not hard, exactly,
> but slightly odd -- the "unknown" bucket ends up somewhere in the middle.  For
> future reference, the best practice is always to have "Unknown" come first.

https://codereview.chromium.org/273883003/diff/1/tools/metrics/histograms/his...
tools/metrics/histograms/histograms.xml:38085: +  <int value="6"
label="Unknown"/>
Thanks for the tip, I will do that next time.
On 2014/05/13 01:28:14, Ilya Sherman wrote:
> It's unfortunate that you've chosen the last bucket to map to unknown, as it
> makes it hard to extend the histogram in the future.  Well, not hard, exactly,
> but slightly odd -- the "unknown" bucket ends up somewhere in the middle.  For
> future reference, the best practice is always to have "Unknown" come first.

[zqiu1, 2014-05-14 18:48:45]

Patch updated.

[Ilya Sherman, 2014-05-14 22:10:51]

LGTM with the remaining comments addressed.  Thanks.

https://codereview.chromium.org/273883003/diff/20001/tools/metrics/histograms...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/273883003/diff/20001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:15207: +   
Network.Shill.DHCPOptionFailureDetected.
Rather than updating the <summary> to indicate that the metric is deprecated,
please add an <obsolete> section.  (There are plenty of examples in the file you
can crib from.)  This will hide the deprecated histograms from the dashboard UI,
but still allow them to be surfaced by checking a checkbox, in case anyone wants
to do some historical analysis.

https://codereview.chromium.org/273883003/diff/20001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:15976: +    Mbps for the wifi device
when it is connected to a network.
Please document within the <summary> how often this is sampled.

[zqiu1, 2014-05-14 22:40:43]

Thanks for the comments. Patch is updated according to the comments.

[Ilya Sherman, 2014-05-14 23:13:20]

(Still LGTM.)

[zqiu1, 2014-05-16 22:34:37]

Hi Ilya,

Can you set the CQ bit for this CL? 

Thanks,
Peter,

[Ilya Sherman, 2014-05-16 22:35:10]

The CQ bit was checked by isherman@chromium.org

[Ilya Sherman, 2014-05-16 22:35:17]

On 2014/05/16 22:34:37, zqiu1 wrote:
> Hi Ilya,
> 
> Can you set the CQ bit for this CL? 
> 
> Thanks,
> Peter,

Done.

[commit-bot: I haz the power, 2014-05-16 23:34:00]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/zqiu@chromium.org/273883003/40001

[commit-bot: I haz the power, 2014-05-17 01:04:57]

Change committed as 271135