Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(95)

Issue 633373011: Histogram for recording actual log upload interval added. (Closed)

Created:
6 years, 2 months ago by gayane -on leave until 09-2017
Modified:
6 years, 2 months ago
CC:
chromium-reviews, asvitkine+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Histogram for recording actual log upload interval added. Histogram is in minutes. In case logs are uploaded by multiple tasks the interval between those is not recorded. BUG=421543 Committed: https://crrev.com/aa0b30fe93d8dbcf54cb693918c46f95f933d954 Cr-Commit-Position: refs/heads/master@{#299902}

Patch Set 1 #

Total comments: 10

Patch Set 2 : Max value of histogram extended and bucket number reduced + few fixes #

Total comments: 2

Patch Set 3 : nits #

Total comments: 3

Patch Set 4 : space fix #

Unified diffs Side-by-side diffs Delta from patch set Stats (+27 lines, -0 lines) Patch
M components/metrics/metrics_reporting_scheduler.h View 1 1 chunk +4 lines, -0 lines 0 comments Download
M components/metrics/metrics_reporting_scheduler.cc View 1 2 3 3 chunks +15 lines, -0 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 chunk +8 lines, -0 lines 0 comments Download

Messages

Total messages: 16 (4 generated)
gayane -on leave until 09-2017
Please have a look.
6 years, 2 months ago (2014-10-15 19:43:36 UTC) #2
Alexei Svitkine (slow)
https://codereview.chromium.org/633373011/diff/1/components/metrics/metrics_reporting_scheduler.cc File components/metrics/metrics_reporting_scheduler.cc (right): https://codereview.chromium.org/633373011/diff/1/components/metrics/metrics_reporting_scheduler.cc#newcode69 components/metrics/metrics_reporting_scheduler.cc:69: base::TimeDelta::FromHours(4).InMinutes(), Let's make this longer to ensure we catch ...
6 years, 2 months ago (2014-10-15 20:44:19 UTC) #3
gayane -on leave until 09-2017
Fixed. https://codereview.chromium.org/633373011/diff/1/components/metrics/metrics_reporting_scheduler.cc File components/metrics/metrics_reporting_scheduler.cc (right): https://codereview.chromium.org/633373011/diff/1/components/metrics/metrics_reporting_scheduler.cc#newcode69 components/metrics/metrics_reporting_scheduler.cc:69: base::TimeDelta::FromHours(4).InMinutes(), On 2014/10/15 20:44:19, Alexei Svitkine wrote: > ...
6 years, 2 months ago (2014-10-15 21:02:00 UTC) #4
Alexei Svitkine (slow)
LGTM with 2 more comments for you to address. https://codereview.chromium.org/633373011/diff/20001/components/metrics/metrics_reporting_scheduler.cc File components/metrics/metrics_reporting_scheduler.cc (right): https://codereview.chromium.org/633373011/diff/20001/components/metrics/metrics_reporting_scheduler.cc#newcode66 components/metrics/metrics_reporting_scheduler.cc:66: ...
6 years, 2 months ago (2014-10-15 21:14:45 UTC) #5
gayane -on leave until 09-2017
Fixed. Committing now.
6 years, 2 months ago (2014-10-16 14:25:19 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/633373011/40001
6 years, 2 months ago (2014-10-16 14:26:32 UTC) #8
Alexei Svitkine (slow)
https://codereview.chromium.org/633373011/diff/40001/components/metrics/metrics_reporting_scheduler.cc File components/metrics/metrics_reporting_scheduler.cc (right): https://codereview.chromium.org/633373011/diff/40001/components/metrics/metrics_reporting_scheduler.cc#newcode66 components/metrics/metrics_reporting_scheduler.cc:66: UMA_HISTOGRAM_CUSTOM_COUNTS("UMA.ActualLogUploadInterval", This still looks too indented, please fix.
6 years, 2 months ago (2014-10-16 14:50:17 UTC) #9
gayane -on leave until 09-2017
https://codereview.chromium.org/633373011/diff/40001/components/metrics/metrics_reporting_scheduler.cc File components/metrics/metrics_reporting_scheduler.cc (right): https://codereview.chromium.org/633373011/diff/40001/components/metrics/metrics_reporting_scheduler.cc#newcode66 components/metrics/metrics_reporting_scheduler.cc:66: UMA_HISTOGRAM_CUSTOM_COUNTS("UMA.ActualLogUploadInterval", On 2014/10/16 14:50:17, Alexei Svitkine wrote: > This ...
6 years, 2 months ago (2014-10-16 15:05:42 UTC) #11
gayane -on leave until 09-2017
https://codereview.chromium.org/633373011/diff/40001/components/metrics/metrics_reporting_scheduler.cc File components/metrics/metrics_reporting_scheduler.cc (right): https://codereview.chromium.org/633373011/diff/40001/components/metrics/metrics_reporting_scheduler.cc#newcode66 components/metrics/metrics_reporting_scheduler.cc:66: UMA_HISTOGRAM_CUSTOM_COUNTS("UMA.ActualLogUploadInterval", On 2014/10/16 14:50:17, Alexei Svitkine wrote: > This ...
6 years, 2 months ago (2014-10-16 15:28:25 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/633373011/60001
6 years, 2 months ago (2014-10-16 15:35:35 UTC) #14
commit-bot: I haz the power
Committed patchset #4 (id:60001)
6 years, 2 months ago (2014-10-16 16:35:44 UTC) #15
commit-bot: I haz the power
6 years, 2 months ago (2014-10-16 16:36:25 UTC) #16
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/aa0b30fe93d8dbcf54cb693918c46f95f933d954
Cr-Commit-Position: refs/heads/master@{#299902}

Powered by Google App Engine
This is Rietveld 408576698