|
|
Chromium Code Reviews
DescriptionAdd UMA to log the size of the Previews opt out DB
This logs the number of rows in the Previews opt out DB at profile
startup.
BUG=656739
Committed: https://crrev.com/ee93075a7392ec5d933ac5aa89543ca4822f9c8e
Cr-Commit-Position: refs/heads/master@{#427793}
Patch Set 1 #
Total comments: 4
Patch Set 2 : tbansal comments #
Total comments: 2
Patch Set 3 : asvitkine nits #
Messages
Total messages: 24 (15 generated)
The CQ bit was checked by ryansturm@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
ryansturm@chromium.org changed reviewers: + tbansal@chromium.org
tbansal: PTAL
https://codereview.chromium.org/2448213002/diff/1/components/previews/core/pr... File components/previews/core/previews_opt_out_store_sql.cc (right): https://codereview.chromium.org/2448213002/diff/1/components/previews/core/pr... components/previews/core/previews_opt_out_store_sql.cc:183: // TODO(ryansturm): Add UMA to log |count|. crbug.com/656739 remove this line
lgtm % nits. https://codereview.chromium.org/2448213002/diff/1/components/previews/core/pr... File components/previews/core/previews_opt_out_store_sql.cc (right): https://codereview.chromium.org/2448213002/diff/1/components/previews/core/pr... components/previews/core/previews_opt_out_store_sql.cc:181: UMA_HISTOGRAM_COUNTS_10000("Previews.OptOutDBRowCount", count); May be s/Previews.OptOutDBRowCount/Previews.OptOut.DBRowCount/ So all histograms related to opt-out pipeline can have same prefix.
The CQ bit was checked by ryansturm@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by ryansturm@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
ryansturm@chromium.org changed reviewers: + asvitkine@chromium.org
asvitkine: PTAL @ histograms https://codereview.chromium.org/2448213002/diff/1/components/previews/core/pr... File components/previews/core/previews_opt_out_store_sql.cc (right): https://codereview.chromium.org/2448213002/diff/1/components/previews/core/pr... components/previews/core/previews_opt_out_store_sql.cc:181: UMA_HISTOGRAM_COUNTS_10000("Previews.OptOutDBRowCount", count); On 2016/10/25 21:05:54, tbansal1 wrote: > May be s/Previews.OptOutDBRowCount/Previews.OptOut.DBRowCount/ > So all histograms related to opt-out pipeline can have same prefix. Done. https://codereview.chromium.org/2448213002/diff/1/components/previews/core/pr... components/previews/core/previews_opt_out_store_sql.cc:183: // TODO(ryansturm): Add UMA to log |count|. crbug.com/656739 On 2016/10/25 21:01:13, Ryan Sturm wrote: > remove this line Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/2448213002/diff/20001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2448213002/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:47961: +<histogram name="Previews.OptOut.DBRowCount"> Nit: add units="rows"
Thanks for the review! https://codereview.chromium.org/2448213002/diff/20001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2448213002/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:47961: +<histogram name="Previews.OptOut.DBRowCount"> On 2016/10/26 15:16:12, Alexei Svitkine (slow) wrote: > Nit: add units="rows" Done.
The CQ bit was checked by ryansturm@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from tbansal@chromium.org, asvitkine@chromium.org Link to the patchset: https://codereview.chromium.org/2448213002/#ps40001 (title: "asvitkine nits")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Add UMA to log the size of the Previews opt out DB This logs the number of rows in the Previews opt out DB at profile startup. BUG=656739 ========== to ========== Add UMA to log the size of the Previews opt out DB This logs the number of rows in the Previews opt out DB at profile startup. BUG=656739 Committed: https://crrev.com/ee93075a7392ec5d933ac5aa89543ca4822f9c8e Cr-Commit-Position: refs/heads/master@{#427793} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/ee93075a7392ec5d933ac5aa89543ca4822f9c8e Cr-Commit-Position: refs/heads/master@{#427793} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
