Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(338)

Issue 2999863002: Results.html: record maximum Histogram.numValues in google analytics. (Closed)

Created:
4 months ago by benjhayden
Modified:
3 months, 3 weeks ago
Reviewers:
eakuefner
CC:
catapult-reviews_chromium.org, tracing-review_chromium.org
Target Ref:
refs/heads/master
Project:
catapult
Visibility:
Public.

Description

Results.html: record maximum Histogram.numValues in google analytics. This CL records an instant event 'HistogramParameterCollector.maxSampleCount' containing the maximum numValues from the raw Histograms that were produced directly by metrics, before the Histograms are merged. We know that this number is always 1 for some metrics. If this number is always 1 across *all* metrics, then we may want to simplify the metrics API from Histograms to Scalars. In that case, Histograms would still be necessary as an implementation detail for results.html and the dashboard Histogram Pipeline, but the metrics API could be simplified. If this number is not always 1, then it will be easier to explain why the metrics API requires Histograms instead of the seemingly simpler Scalars. Review-Url: https://codereview.chromium.org/2999863002 Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/d768f57acb8966c50c10edfe9050c1caa6a2003f

Patch Set 1 #

Patch Set 2 : fix vinn tests #

Unified diffs Side-by-side diffs Delta from patch set Stats (+22 lines, -9 lines) Patch
M tracing/tracing/base/timing.html View 1 1 chunk +14 lines, -9 lines 0 comments Download
M tracing/tracing/value/histogram_parameter_collector.html View 3 chunks +6 lines, -0 lines 0 comments Download
M tracing/tracing/value/ui/timings.md View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 15 (8 generated)
benjhayden
PTAL
4 months ago (2017-08-12 20:25:36 UTC) #3
eakuefner
lgtm How long do we want to evaluate this for? This seems like it could ...
3 months, 3 weeks ago (2017-08-18 23:27:28 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2999863002/1
3 months, 3 weeks ago (2017-08-19 17:09:34 UTC) #6
benjhayden
On 2017/08/18 at 23:27:28, eakuefner wrote: > lgtm > > How long do we want ...
3 months, 3 weeks ago (2017-08-19 17:16:31 UTC) #7
commit-bot: I haz the power
Try jobs failed on following builders: Catapult Linux Tryserver on master.tryserver.client.catapult (JOB_FAILED, https://build.chromium.org/p/tryserver.client.catapult/builders/Catapult%20Linux%20Tryserver/builds/8908)
3 months, 3 weeks ago (2017-08-19 17:37:04 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2999863002/20001
3 months, 3 weeks ago (2017-08-19 17:52:09 UTC) #12
commit-bot: I haz the power
3 months, 3 weeks ago (2017-08-19 18:28:24 UTC) #15
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...

Powered by Google App Engine
This is Rietveld 0eb02b776