Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
935223002:
Add connection information metrics recorded after successful auth attempts. (Closed)
|
Created:
5 years, 10 months ago by Tim Song Modified:
5 years, 9 months ago Reviewers:
Ilya Sherman CC:
chromium-reviews, asvitkine+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd connection information metrics recorded after successful auth attempts.
BUG=459842
Committed: https://crrev.com/6c6d81a4532b2140dc05cc298995122e9f9948cf
Cr-Commit-Position: refs/heads/master@{#318765}
Patch Set 1 #
Total comments: 10
Patch Set 2 : Update histogram names #Patch Set 3 : Add documentation for invalid values #Patch Set 4 : rebase #
Messages
Total messages: 15 (5 generated)
tengs@chromium.org changed reviewers: + isherman@chromium.org
https://codereview.chromium.org/935223002/diff/1/tools/metrics/histograms/his... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/935223002/diff/1/tools/metrics/histograms/his... tools/metrics/histograms/histograms.xml:6430: + <owner>xiaowenx@google.com</owner> nit: Please use @chromium.org throughout. https://codereview.chromium.org/935223002/diff/1/tools/metrics/histograms/his... tools/metrics/histograms/histograms.xml:6435: +</histogram> Please associate an enum with at least some of the common models that we expect to see. https://codereview.chromium.org/935223002/diff/1/tools/metrics/histograms/his... tools/metrics/histograms/histograms.xml:6455: + attempt to maintain by adjusting their transmit power levels. Please document what is recorded in the case that there was no zero RSSI value recorded. https://codereview.chromium.org/935223002/diff/1/tools/metrics/histograms/his... tools/metrics/histograms/histograms.xml:6464: + successfully unlocks or signs in using Smart Lock. We should probably measure a difference between the current transmit power and the max, since the max varies by hardware model.
https://chromiumcodereview.appspot.com/935223002/diff/1/tools/metrics/histogr... File tools/metrics/histograms/histograms.xml (right): https://chromiumcodereview.appspot.com/935223002/diff/1/tools/metrics/histogr... tools/metrics/histograms/histograms.xml:6428: +<histogram name="EasyUnlock.AuthSuccessRemoteDeviceModelHash"> By the way, I've been trying to group new, related histograms in subcategories, e.g. EasyUnlock.Setup.*. I think it would be nice to do the same for these -- e.g. EasyUnlock.AuthSuccess.* (extra dot after "success") or EasyUnlock.ProximityHeuristics.AuthSuccess.*.
https://codereview.chromium.org/935223002/diff/1/tools/metrics/histograms/his... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/935223002/diff/1/tools/metrics/histograms/his... tools/metrics/histograms/histograms.xml:6428: +<histogram name="EasyUnlock.AuthSuccessRemoteDeviceModelHash"> On 2015/02/19 08:01:28, Ilya Sherman wrote: > By the way, I've been trying to group new, related histograms in subcategories, > e.g. EasyUnlock.Setup.*. I think it would be nice to do the same for these -- > e.g. EasyUnlock.AuthSuccess.* (extra dot after "success") or > EasyUnlock.ProximityHeuristics.AuthSuccess.*. Done. I put them under the EasyUnlock.AuthProximity group. https://codereview.chromium.org/935223002/diff/1/tools/metrics/histograms/his... tools/metrics/histograms/histograms.xml:6430: + <owner>xiaowenx@google.com</owner> On 2015/02/19 07:20:58, Ilya Sherman wrote: > nit: Please use @chromium.org throughout. Done. https://codereview.chromium.org/935223002/diff/1/tools/metrics/histograms/his... tools/metrics/histograms/histograms.xml:6435: +</histogram> On 2015/02/19 07:20:58, Ilya Sherman wrote: > Please associate an enum with at least some of the common models that we expect > to see. Done. https://codereview.chromium.org/935223002/diff/1/tools/metrics/histograms/his... tools/metrics/histograms/histograms.xml:6455: + attempt to maintain by adjusting their transmit power levels. On 2015/02/19 07:20:58, Ilya Sherman wrote: > Please document what is recorded in the case that there was no zero RSSI value > recorded. Done. https://codereview.chromium.org/935223002/diff/1/tools/metrics/histograms/his... tools/metrics/histograms/histograms.xml:6464: + successfully unlocks or signs in using Smart Lock. On 2015/02/19 07:20:58, Ilya Sherman wrote: > We should probably measure a difference between the current transmit power and > the max, since the max varies by hardware model. Done.
LGTM, thanks.
The CQ bit was checked by tengs@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/935223002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios_dbg_simulator on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...) ios_rel_device_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ng...) ios_rel_device_ninja_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
The CQ bit was checked by tengs@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from isherman@chromium.org Link to the patchset: https://codereview.chromium.org/935223002/#ps60001 (title: "rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/935223002/60001
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/6c6d81a4532b2140dc05cc298995122e9f9948cf Cr-Commit-Position: refs/heads/master@{#318765} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
