Commit 7ab21d83 authored by Reid Kleckner's avatar Reid Kleckner Committed by Commit Bot

Revert "Create fallback config for UKM sampling."

This reverts commit 1e874ff9.

Reason for revert: This appears to be causing many UKM related tests to fail in official builds:
https://uberchromegw.corp.google.com/i/official.desktop.continuous/builders/win%20trunk/builds/86186

Original change's description:
> Create fallback config for UKM sampling.
> 
> Some clients do not have the sampling configuration for UKM records
> and the large number of entries can cause significant bias when weighted
> against those clients that are sampling.  Setting a large default
> sampling value will at least bias the other way.
> 
> Bug: 766909
> Change-Id: Ib6d656bd2ce4d82efb35281d09dc7b94128fadb2
> Reviewed-on: https://chromium-review.googlesource.com/1095596
> Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
> Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
> Commit-Queue: Brian White <bcwhite@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#566831}

TBR=rkaplow@chromium.org,asvitkine@chromium.org,bcwhite@chromium.org

Change-Id: I85b86053a3a41add9a5c93e31006b885b3b2f3d3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 766909
Reviewed-on: https://chromium-review.googlesource.com/1099617Reviewed-by: default avatarReid Kleckner <rnk@chromium.org>
Commit-Queue: Reid Kleckner <rnk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566968}
parent 01cc957d
......@@ -28,9 +28,7 @@
#include "chrome/common/chrome_switches.h"
#include "components/metrics/metrics_pref_names.h"
#include "components/metrics/persistent_system_profile.h"
#include "components/ukm/ukm_recorder_impl.h"
#include "components/variations/variations_associated_data.h"
#include "components/version_info/version_info.h"
#if defined(OS_ANDROID)
#include "chrome/browser/chrome_browser_field_trials_mobile.h"
......@@ -200,20 +198,22 @@ void InstantiatePersistentHistograms() {
// Create a field trial to control metrics/crash sampling for Stable on
// Windows/Android if no variations seed was applied.
void CreateFallbackSamplingTrialIfNeeded(base::FeatureList* feature_list) {
void CreateFallbackSamplingTrialIfNeeded(bool has_seed,
base::FeatureList* feature_list) {
#if defined(OS_WIN) || defined(OS_ANDROID)
// Only create the fallback trial if there isn't already a variations seed
// being applied. This should occur during first run when first-run variations
// isn't supported. It's assumed that, if there is a seed, then it either
// contains the relavent study, or is intentionally omitted, so no fallback is
// needed.
if (has_seed)
return;
ChromeMetricsServicesManagerClient::CreateFallbackSamplingTrial(
chrome::GetChannel(), feature_list);
#endif // defined(OS_WIN) || defined(OS_ANDROID)
}
// Create a field trial to control UKM sampling for Stable if no variations
// seed was applied.
void CreateFallbackUkmSamplingTrialIfNeeded(base::FeatureList* feature_list) {
ukm::UkmRecorderImpl::CreateFallbackSamplingTrial(
chrome::GetChannel() == version_info::Channel::STABLE, feature_list);
}
} // namespace
ChromeBrowserFieldTrials::ChromeBrowserFieldTrials() {}
......@@ -235,15 +235,7 @@ void ChromeBrowserFieldTrials::SetupFieldTrials() {
void ChromeBrowserFieldTrials::SetupFeatureControllingFieldTrials(
bool has_seed,
base::FeatureList* feature_list) {
// Only create the fallback trials if there isn't already a variations seed
// being applied. This should occur during first run when first-run variations
// isn't supported. It's assumed that, if there is a seed, then it either
// contains the relavent studies, or is intentionally omitted, so no fallback
// is needed.
if (!has_seed) {
CreateFallbackSamplingTrialIfNeeded(feature_list);
CreateFallbackUkmSamplingTrialIfNeeded(feature_list);
}
CreateFallbackSamplingTrialIfNeeded(has_seed, feature_list);
}
void ChromeBrowserFieldTrials::InstantiateDynamicTrials() {
......
......@@ -18,7 +18,6 @@
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_split.h"
#include "components/ukm/ukm_source.h"
#include "components/variations/variations_associated_data.h"
#include "services/metrics/public/cpp/ukm_decode.h"
#include "services/metrics/public/cpp/ukm_source_id.h"
#include "third_party/metrics_proto/ukm/entry.pb.h"
......@@ -170,43 +169,6 @@ bool HasUnknownMetrics(const ukm::builders::DecodeMap& decode_map,
UkmRecorderImpl::UkmRecorderImpl() : recording_enabled_(false) {}
UkmRecorderImpl::~UkmRecorderImpl() = default;
// static
void UkmRecorderImpl::CreateFallbackSamplingTrial(
bool is_stable_channel,
base::FeatureList* feature_list) {
static const char kSampledGroup_Stable[] = "Sampled_NoSeed_Stable";
static const char kSampledGroup_Other[] = "Sampled_NoSeed_Other";
const char* sampled_group = kSampledGroup_Other;
int default_sampling = 1; // Sampling is 1-in-N; this is N.
// Nothing is sampled out except for "stable" which omits almost everything
// in this configuration. This is done so that clients that fail to receive
// a configuration from the server do not bias aggregated results because
// of a relatively large number of records from them.
if (is_stable_channel) {
sampled_group = kSampledGroup_Stable;
default_sampling = 1000000;
}
scoped_refptr<base::FieldTrial> trial(
base::FieldTrialList::FactoryGetFieldTrial(
kUkmSamplingRateFeature.name, 100, sampled_group,
base::FieldTrialList::kNoExpirationYear, 1, 1,
base::FieldTrial::ONE_TIME_RANDOMIZED, nullptr));
// Everybody (100%) should have a sampling configuration.
std::map<std::string, std::string> params = {
{"_default_sampling", base::IntToString(default_sampling)}};
variations::AssociateVariationParams(trial->trial_name(), sampled_group,
params);
trial->AppendGroup(sampled_group, 100);
// Setup the feature.
feature_list->RegisterFieldTrialOverride(
kUkmSamplingRateFeature.name, base::FeatureList::OVERRIDE_ENABLE_FEATURE,
trial.get());
}
void UkmRecorderImpl::SourceCounts::Reset() {
*this = SourceCounts();
}
......
......@@ -42,14 +42,6 @@ class UkmRecorderImpl : public UkmRecorder {
UkmRecorderImpl();
~UkmRecorderImpl() override;
// Unconditionally attempts to create a field trial to control client side
// metrics/crash sampling to use as a fallback when one hasn't been
// provided. This is expected to occur on first-run on platforms that don't
// have first-run variations support. This should only be called when there is
// no existing field trial controlling the sampling feature.
static void CreateFallbackSamplingTrial(bool is_stable_channel,
base::FeatureList* feature_list);
// Enables/disables recording control if data is allowed to be collected. The
// |extensions| flag separately controls recording of chrome-extension://
// URLs; this flag should reflect the "sync extensions" user setting.
......
......@@ -4185,6 +4185,29 @@
]
}
],
"UkmSamplingRate": [
{
"platforms": [
"android",
"chromeos",
"ios",
"linux",
"mac",
"win"
],
"experiments": [
{
"name": "Sampled",
"params": {
"_default_sampling": "1"
},
"enable_features": [
"UkmSamplingRate"
]
}
]
}
],
"UpdateMenuItem": [
{
"platforms": [
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment