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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 week, 3 days ago by benjhayden
Modified:
3 days, 15 hours 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
Commit queue not available (can’t edit this change).

Messages

Total messages: 15 (8 generated)
benjhayden
PTAL
1 week, 3 days 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 ...
4 days, 10 hours 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 days, 16 hours 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 days, 16 hours 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 days, 16 hours 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 days, 15 hours ago (2017-08-19 17:52:09 UTC) #12
commit-bot: I haz the power
3 days, 15 hours 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...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld b40b6558b