Work around exceptions when the 'high' value for a histogram is not present. This happens when the …

Work around exceptions when the 'high' value for a histogram is not present. This happens when the histogram hits the max value.

BUG=455349
Committed: https://crrev.com/b73fe537668cdea6cf1cbbafe734a752785f7721
Cr-Commit-Position: refs/heads/master@{#317377}

[sullivan, 2015-02-19 22:54:15]

sullivan@chromium.org changed reviewers:
+ nednguyen@google.com

[sullivan, 2015-02-19 22:54:16]

Is this too hacky? The buckets come from UMA histograms, so they're not
generated by telemetry code. When the value is above the histogram max, there is
no 'high' value. In this case we just want the mean to be the histogram max,
which is in the 'low' value. UMA Timeline handles it the same way.

[nednguyen, 2015-02-19 23:07:47]

I look at where the HistogramValue are created, and found that there is
histogram_util that does operations on histogram. Doesn't those operation need
to deal with this edge cases too? If so, I think it's more natural to put this
in histogram_util and not construct historgram from raw_values directly but some
factory method from histogram_util

[sullivan, 2015-02-20 16:43:11]

On 2015/02/19 23:07:47, nednguyen wrote:
> I look at where the HistogramValue are created, and found that there is
> histogram_util that does operations on histogram. Doesn't those operation need
> to deal with this edge cases too? If so, I think it's more natural to put this
> in histogram_util and not construct historgram from raw_values directly but
some
> factory method from histogram_util

Yeah, I found an operation there that wasn't handling the edge case, so I moved
the code to that file and use it there as well. Also made sure the code is only
handling the expected error (only the last bucket can have 'high' value
missing). And added unit test. PTAL

[sullivan, 2015-02-20 16:43:20]

https://codereview.chromium.org/941803002/diff/40001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/value/histogram_util.py (right):

https://codereview.chromium.org/941803002/diff/40001/tools/telemetry/telemetr...
tools/telemetry/telemetry/value/histogram_util.py:92: key = (b['low'],
b['high'])
This line would be buggy without a fix in histogram_utils, too.

[nednguyen, 2015-02-20 17:18:33]

lgtm

https://codereview.chromium.org/941803002/diff/40001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/value/histogram.py (right):

https://codereview.chromium.org/941803002/diff/40001/tools/telemetry/telemetr...
tools/telemetry/telemetry/value/histogram.py:10: class
HistogramValueBucket(object):
This is a nice abstraction for telemetry code to deal with rather than the
dictionary. 
If the code was structured so that the conversion from raw_value or
raw_value_json to list of HistogramValueBucket happen as early as possible so
the rest of the stack only deal with this abstraction, the fix would be a 1 line
fix.

[sullivan, 2015-02-20 18:09:32]

The CQ bit was checked by sullivan@chromium.org

[commit-bot: I haz the power, 2015-02-20 18:10:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/941803002/40001

[commit-bot: I haz the power, 2015-02-20 18:40:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-02-20 18:40:46]

Try jobs failed on following builders:
  win8_chromium_rel on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)

[sullivan, 2015-02-20 18:51:37]

The CQ bit was checked by sullivan@chromium.org

[commit-bot: I haz the power, 2015-02-20 18:52:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/941803002/40001

[commit-bot: I haz the power, 2015-02-20 19:46:59]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-02-20 19:47:20]

Patchset 3 (id:??) landed as
https://crrev.com/b73fe537668cdea6cf1cbbafe734a752785f7721
Cr-Commit-Position: refs/heads/master@{#317377}