Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2896863002:
Add UMA histograms for time between policy fetches (Closed)
|
Created:
3 years, 7 months ago by Denis Kuznetsov (DE-MUC) Modified:
3 years, 4 months ago CC:
chromium-reviews, asvitkine+watch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionUMA metrics for tracking the number of days
between policy fetches.
BUG=704091
Review-Url: https://codereview.chromium.org/2896863002
Cr-Commit-Position: refs/heads/master@{#492321}
Committed: https://chromium.googlesource.com/chromium/src/+/659129b468b34cd565df37ae47dd0dbeb5970b2a
Patch Set 1 #
Total comments: 4
Patch Set 2 : Rebase to tot and fix nits #Patch Set 3 : Rebase to ToT #
Messages
Total messages: 25 (13 generated)
Description was changed from
==========
Add UMA histograms for time between policy fetches
==========
to
==========
UMA metrics for tracking the number of days
between policy fetches.
BUG=704091
==========
antrim@chromium.org changed reviewers: + bartfab@chromium.org
https://codereview.chromium.org/2896863002/diff/1/components/policy/core/comm... File components/policy/core/common/cloud/cloud_policy_service.cc (right): https://codereview.chromium.org/2896863002/diff/1/components/policy/core/comm... components/policy/core/common/cloud/cloud_policy_service.cc:125: base::TimeDelta age = policy_timestamp - old_timestamp; Nit: const https://codereview.chromium.org/2896863002/diff/1/tools/metrics/histograms/hi... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2896863002/diff/1/tools/metrics/histograms/hi... tools/metrics/histograms/histograms.xml:15576: +<histogram name="Enterprise.PolicyUpdatePeriod.Device" units="days"> Are you sure days are a good unit here? IIRC, our fallback policy polling happens once every 24 hours, so pretty much all active users will have a 1 in the histogram.
https://codereview.chromium.org/2896863002/diff/1/components/policy/core/comm... File components/policy/core/common/cloud/cloud_policy_service.cc (right): https://codereview.chromium.org/2896863002/diff/1/components/policy/core/comm... components/policy/core/common/cloud/cloud_policy_service.cc:125: base::TimeDelta age = policy_timestamp - old_timestamp; On 2017/06/12 15:01:58, bartfab (slow) wrote: > Nit: const Done. https://codereview.chromium.org/2896863002/diff/1/tools/metrics/histograms/hi... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2896863002/diff/1/tools/metrics/histograms/hi... tools/metrics/histograms/histograms.xml:15576: +<histogram name="Enterprise.PolicyUpdatePeriod.Device" units="days"> On 2017/06/12 15:01:58, bartfab (slow) wrote: > Are you sure days are a good unit here? IIRC, our fallback policy polling > happens once every 24 hours, so pretty much all active users will have a 1 in > the histogram. Yes, as we are trying to estimate the long tail of the of the curve so that we can remove old dmtokens for ephemeral sessions.
lgtm
The CQ bit was checked by antrim@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from bartfab@chromium.org Link to the patchset: https://codereview.chromium.org/2896863002/#ps40001 (title: "Rebase to ToT")
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
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
antrim@chromium.org changed reviewers: + holte@chromium.org
+holte@ as owner of histograms.xml
lgtm
The CQ bit was checked by antrim@chromium.org
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
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by antrim@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1502111533010670,
"parent_rev": "c8bb0188f69127148bca00d77a87a62929de2bc6", "commit_rev":
"4834c25efcc88d7f9942745f769cde68cfbb1514"}
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1502111533010670,
"parent_rev": "f808fa2ee42b46124ed104dd46cd407632a492e0", "commit_rev":
"027fc5193661036b23b4960ee04aa07f48a07a36"}
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1502111533010670,
"parent_rev": "bf1fe350e796481f5731800cbe5e1af638d0158d", "commit_rev":
"659129b468b34cd565df37ae47dd0dbeb5970b2a"}
Message was sent while issue was closed.
Description was changed from
==========
UMA metrics for tracking the number of days
between policy fetches.
BUG=704091
==========
to
==========
UMA metrics for tracking the number of days
between policy fetches.
BUG=704091
Review-Url: https://codereview.chromium.org/2896863002
Cr-Commit-Position: refs/heads/master@{#492321}
Committed:
https://chromium.googlesource.com/chromium/src/+/659129b468b34cd565df37ae47dd...
==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/659129b468b34cd565df37ae47dd... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
