Commit 8c9c9019 authored by Brian White's avatar Brian White Committed by Commit Bot

Allow default UKM sampling rate of 0 to fully disable it.

Bug: 766909
Change-Id: Ib220095f8830f04e7b28476af3b6c1e0a493425b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1935047
Commit-Queue: Brian White <bcwhite@chromium.org>
Auto-Submit: Brian White <bcwhite@chromium.org>
Reviewed-by: default avatarRobert Kaplow <rkaplow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719326}
parent 58b1870b
...@@ -655,7 +655,7 @@ void UkmRecorderImpl::AddEntry(mojom::UkmEntryPtr entry) { ...@@ -655,7 +655,7 @@ void UkmRecorderImpl::AddEntry(mojom::UkmEntryPtr entry) {
} }
if (IsSamplingEnabled()) { if (IsSamplingEnabled()) {
if (default_sampling_rate_ == 0) { if (default_sampling_rate_ < 0) {
LoadExperimentSamplingInfo(); LoadExperimentSamplingInfo();
} }
...@@ -687,9 +687,10 @@ void UkmRecorderImpl::AddEntry(mojom::UkmEntryPtr entry) { ...@@ -687,9 +687,10 @@ void UkmRecorderImpl::AddEntry(mojom::UkmEntryPtr entry) {
} }
void UkmRecorderImpl::LoadExperimentSamplingInfo() { void UkmRecorderImpl::LoadExperimentSamplingInfo() {
DCHECK_EQ(0, default_sampling_rate_); // This should be called only if a sampling rate hasn't been loaded.
DCHECK_LT(default_sampling_rate_, 0);
// Default rate must be > 0 to indicate that load is complete. // Default rate must be >= 0 to indicate that load is complete.
default_sampling_rate_ = 1; default_sampling_rate_ = 1;
// If we don't have the feature, no parameters to load. // If we don't have the feature, no parameters to load.
...@@ -709,9 +710,8 @@ void UkmRecorderImpl::LoadExperimentSamplingInfo() { ...@@ -709,9 +710,8 @@ void UkmRecorderImpl::LoadExperimentSamplingInfo() {
if (key.at(0) == '_') { if (key.at(0) == '_') {
if (key == "_default_sampling") { if (key == "_default_sampling") {
int sampling; int sampling;
// We only load positive global sampling rates. If the global sampling // We only load non-negative global sampling rates.
// is 0, then we stick to the default rate of '1' (unsampled). if (base::StringToInt(kv.second, &sampling) && sampling >= 0)
if (base::StringToInt(kv.second, &sampling) && sampling > 0)
default_sampling_rate_ = sampling; default_sampling_rate_ = sampling;
} }
continue; continue;
...@@ -730,8 +730,9 @@ bool UkmRecorderImpl::IsSampledIn(int64_t source_id, ...@@ -730,8 +730,9 @@ bool UkmRecorderImpl::IsSampledIn(int64_t source_id,
int sampling_rate) { int sampling_rate) {
// A sampling rate of 0 is "never"; everything else is 1-in-N but calculated // A sampling rate of 0 is "never"; everything else is 1-in-N but calculated
// deterministically based on a seed, the source-id, and the event-id. Skip // deterministically based on a seed, the source-id, and the event-id. Skip
// the calculation, though, if N==1 because it will always be true. // the calculation, though, if N==1 because it will always be true. A negative
if (sampling_rate == 0) // rate means "unset"; treat it like "never".
if (sampling_rate <= 0)
return false; return false;
if (sampling_rate == 1) if (sampling_rate == 1)
return true; return true;
......
...@@ -193,7 +193,7 @@ class UkmRecorderImpl : public UkmRecorder { ...@@ -193,7 +193,7 @@ class UkmRecorderImpl : public UkmRecorder {
std::set<uint64_t> whitelisted_entry_hashes_; std::set<uint64_t> whitelisted_entry_hashes_;
// Sampling configurations, loaded from a field-trial. // Sampling configurations, loaded from a field-trial.
int default_sampling_rate_ = 0; int default_sampling_rate_ = -1; // -1 == not yet loaded
base::flat_map<uint64_t, int> event_sampling_rates_; base::flat_map<uint64_t, int> event_sampling_rates_;
// Contains data from various recordings which periodically get serialized // Contains data from various recordings which periodically get serialized
......
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