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

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

Created:
11 months, 1 week ago by benjhayden
Modified:
11 months 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
11 months, 1 week 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 ...
11 months 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
11 months 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 ...
11 months 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)
11 months 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
11 months ago (2017-08-19 17:52:09 UTC) #12
commit-bot: I haz the power
11 months 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 408576698