Randomize variations setting and record UMA |
|||||||||
Issue descriptionWith https://crrev.com/c/1050931, variations is enabled 100% in m68. We should break off some small percentage into a study with enabled/disabled groups. We can't use variations itself to enabled/disable variations, so we should hard-code the random assignment and record a UMA histogram with the result. We can then split other metrics by the new histogram as described under go/uma-dremel, "Histogram Filtered by a Secondary Histogram".
,
Jun 12 2018
,
Jun 16 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/074de24df77eee54efbb6fc3646528b008bdb513 commit 074de24df77eee54efbb6fc3646528b008bdb513 Author: Paul Miller <paulmiller@google.com> Date: Sat Jun 16 00:26:37 2018 WebView: Add study for variations rollout With https://crrev.com/c/1050931, variations is enabled 100% in m68. We want to keep some small percentage disabled in order to compare metrics, but can't use variations to do so. So hard-code the random group assignment instead, and add a UMA histogram, Android.WebView.VariationsEnableState, with the result. The logic about whether variations is enabled is now pretty complex, but we'll remove most of it after 100% rollout. The current variables are: - mEnabledByCmd Controlled by the --enable-webview-variations switch. - mEnabledByExperiment Controlled by AGSA's experiment. - sVariationsAlwaysEnabled Hard-coded to true in m68, intended for easy revert in case we decided to completely disable variations in m68. - sEnableState The newly-added tri-state: default enabled, control-group enabled, and experiment-group enabled. Before this change, variations would be enabled if any 1 of the first 3 variables were true. The new sEnableState can disable variations, overriding sVariationsAlwaysEnabled. This means that "sVariationsAlwaysEnabled" is no longer an accurate name, since it can be overridden, but I'm keeping it as-is to allow easy reverts. Bug: 851045 Change-Id: I5b96d5636d1a1d2b0d662a121cd8b285cdb37a48 Reviewed-on: https://chromium-review.googlesource.com/1094139 Commit-Queue: Paul Miller <paulmiller@chromium.org> Reviewed-by: Changwan Ryu <changwan@chromium.org> Reviewed-by: Bo <boliu@chromium.org> Reviewed-by: Ilya Sherman <isherman@chromium.org> Cr-Commit-Position: refs/heads/master@{#567838} [modify] https://crrev.com/074de24df77eee54efbb6fc3646528b008bdb513/android_webview/java/src/org/chromium/android_webview/VariationsSeedLoader.java [modify] https://crrev.com/074de24df77eee54efbb6fc3646528b008bdb513/tools/metrics/histograms/enums.xml [modify] https://crrev.com/074de24df77eee54efbb6fc3646528b008bdb513/tools/metrics/histograms/histograms.xml
,
Jun 16 2018
,
Jun 16 2018
The bug is marked as P3 or Feature. It should not be merged as M68 is in beta. Please contact the approriate milestone owner if you have questions. Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jun 16 2018
,
Jun 16 2018
^ yeah, well, ya know that's just like, uh, your opinion, man
,
Jun 16 2018
This bug requires manual review: M68 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jun 18 2018
is it possible to verify this in canary?
,
Jun 18 2018
We discussed this in the meeting this morning. Testing manually would be difficult, because you'll get the existing behavior (variations is enabled) exactly 99.9% of the time. The only thing to test would be that variations is disabled .1% of the time. I tested this manually myself by changing the probabilities to 1/3 default, 1/3 control, 1/3 experiment (as opposed to 99.8% default, .1% control, .1% experiment), and verified that all 3 states worked. But we can't change the probabilities in canary. Variations, the feature which this commit is disables .1% of the time, has already gotten manual testing: https://crbug.com/848910.
,
Jun 18 2018
Hmm... I agree that it is hard to manually test this. How about automated testing, then? Could you try increasing the probability to 100% to 'disabled' on trybot? Does this break anything? Also, we may be able to verify this through UMAs after some time: does this increase crash rates on experiment?
,
Jun 18 2018
> Could you try increasing the probability to 100% to 'disabled' on trybot? Like so? https://chromium-review.googlesource.com/c/chromium/src/+/1105282 > Also, we may be able to verify this through UMAs after some time: does this increase crash rates on experiment? There is already some data for Android.WebView.VariationsEnableState on the dashboard, though not enough to make any conclusions. But I don't know how to split crashes by UMA data.
,
Jun 19 2018
All good! Thanks!
,
Jun 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/55a2f913aa0eb3668bdef455b5ed3b71d230fb31 commit 55a2f913aa0eb3668bdef455b5ed3b71d230fb31 Author: Paul Miller <paulmiller@google.com> Date: Tue Jun 19 21:07:57 2018 WebView: Add study for variations rollout With https://crrev.com/c/1050931, variations is enabled 100% in m68. We want to keep some small percentage disabled in order to compare metrics, but can't use variations to do so. So hard-code the random group assignment instead, and add a UMA histogram, Android.WebView.VariationsEnableState, with the result. The logic about whether variations is enabled is now pretty complex, but we'll remove most of it after 100% rollout. The current variables are: - mEnabledByCmd Controlled by the --enable-webview-variations switch. - mEnabledByExperiment Controlled by AGSA's experiment. - sVariationsAlwaysEnabled Hard-coded to true in m68, intended for easy revert in case we decided to completely disable variations in m68. - sEnableState The newly-added tri-state: default enabled, control-group enabled, and experiment-group enabled. Before this change, variations would be enabled if any 1 of the first 3 variables were true. The new sEnableState can disable variations, overriding sVariationsAlwaysEnabled. This means that "sVariationsAlwaysEnabled" is no longer an accurate name, since it can be overridden, but I'm keeping it as-is to allow easy reverts. Bug: 851045 Change-Id: I5b96d5636d1a1d2b0d662a121cd8b285cdb37a48 Reviewed-on: https://chromium-review.googlesource.com/1094139 Commit-Queue: Paul Miller <paulmiller@chromium.org> Reviewed-by: Changwan Ryu <changwan@chromium.org> Reviewed-by: Bo <boliu@chromium.org> Reviewed-by: Ilya Sherman <isherman@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#567838}(cherry picked from commit 074de24df77eee54efbb6fc3646528b008bdb513) Reviewed-on: https://chromium-review.googlesource.com/1106917 Reviewed-by: Paul Miller <paulmiller@chromium.org> Cr-Commit-Position: refs/branch-heads/3440@{#450} Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733} [modify] https://crrev.com/55a2f913aa0eb3668bdef455b5ed3b71d230fb31/android_webview/java/src/org/chromium/android_webview/VariationsSeedLoader.java [modify] https://crrev.com/55a2f913aa0eb3668bdef455b5ed3b71d230fb31/tools/metrics/histograms/enums.xml [modify] https://crrev.com/55a2f913aa0eb3668bdef455b5ed3b71d230fb31/tools/metrics/histograms/histograms.xml
,
Jun 26 2018
Please close if this is fixed and verified.
,
Jun 26 2018
,
Jul 18
Please mark them verified if possible as we cannot do manual verification as per #10
,
Jul 26
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/57e6c106954b12f71a6af72811eaa70ac81455b7 commit 57e6c106954b12f71a6af72811eaa70ac81455b7 Author: Paul Miller <paulmiller@google.com> Date: Thu Jul 26 00:08:32 2018 WebView: Remove variations experiment code There were 4 mechanisms for enabling/disabling variations in WebView: - a command-line flag (mEnabledByCmd) - the AGSA experiment in m67 (mEnabledByExperiment) - the hard-coded setting (sVariationsAlwaysEnabled) - the hold-back experiment in m68 & m69 (sEnableState) Remove all of them, so that variations is always enabled. Set the hold-back experiment histogram, VariationsEnableState, to expire after m69. Don't mark it as obsolete yet, because we'll want to watch it for the lifetime of m68 & m69. BUG= 851045 Change-Id: I44bd36789019074f422ec0c25be2afadba95b8c4 Reviewed-on: https://chromium-review.googlesource.com/1150772 Reviewed-by: Steven Holte <holte@chromium.org> Reviewed-by: Bo <boliu@chromium.org> Commit-Queue: Paul Miller <paulmiller@chromium.org> Cr-Commit-Position: refs/heads/master@{#578136} [modify] https://crrev.com/57e6c106954b12f71a6af72811eaa70ac81455b7/android_webview/browser/aw_browser_main_parts.cc [modify] https://crrev.com/57e6c106954b12f71a6af72811eaa70ac81455b7/android_webview/browser/aw_metrics_service_client.cc [modify] https://crrev.com/57e6c106954b12f71a6af72811eaa70ac81455b7/android_webview/common/aw_switches.cc [modify] https://crrev.com/57e6c106954b12f71a6af72811eaa70ac81455b7/android_webview/common/aw_switches.h [modify] https://crrev.com/57e6c106954b12f71a6af72811eaa70ac81455b7/android_webview/java/src/org/chromium/android_webview/AwSwitches.java [modify] https://crrev.com/57e6c106954b12f71a6af72811eaa70ac81455b7/android_webview/java/src/org/chromium/android_webview/VariationsSeedLoader.java [modify] https://crrev.com/57e6c106954b12f71a6af72811eaa70ac81455b7/android_webview/javatests/src/org/chromium/android_webview/test/VariationsSeedLoaderTest.java [modify] https://crrev.com/57e6c106954b12f71a6af72811eaa70ac81455b7/tools/metrics/histograms/histograms.xml
,
Jul 30
No manual verification required. |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by sheriffbot@chromium.org
, Jun 12 2018