Add sampling support in the metrics reporter
The sample rate percentage can be configured via 2 runtime
flags (MetricsReportingMods and MetricsReportingNumMods).
A runtime session will report metrics if and only if its
session id % MetricsReportingNumMods < MetricsReportingMods.
A value of 0 on the mods will effectively disable reporting,
while configuring MetricsReportingMods to be equal to the
MetricsReportingNumMods will enable reporting in all cases.
Test: gtest
Bug: 170149255
Merged-In: Ie1353e4fc0ff695fc627696145d95e9ccd3c6a94
Change-Id: Ie1353e4fc0ff695fc627696145d95e9ccd3c6a94
(cherry picked from commit 17e0b652b59b0f7ebc821c3a86036e86079642bb)
diff --git a/libartbase/base/flags.cc b/libartbase/base/flags.cc
index aaa7661..6cf9ef1 100644
--- a/libartbase/base/flags.cc
+++ b/libartbase/base/flags.cc
@@ -35,28 +35,41 @@
// The various ParseValue functions store the parsed value into *destination. If parsing fails for
// some reason, ParseValue makes no changes to *destination.
-void ParseValue(const std::string_view value, std::optional<bool>* destination) {
+bool ParseValue(const std::string_view value, std::optional<bool>* destination) {
switch (::android::base::ParseBool(value)) {
case ::android::base::ParseBoolResult::kError:
- return;
+ return false;
case ::android::base::ParseBoolResult::kTrue:
*destination = true;
- return;
+ return true;
case ::android::base::ParseBoolResult::kFalse:
*destination = false;
- return;
+ return true;
}
}
-void ParseValue(const std::string_view value, std::optional<int>* destination) {
- int parsed_value = 0;
+bool ParseValue(const std::string_view value, std::optional<int32_t>* destination) {
+ int32_t parsed_value = 0;
if (::android::base::ParseInt(std::string{value}, &parsed_value)) {
*destination = parsed_value;
+ return true;
}
+ return false;
}
-void ParseValue(const std::string_view value, std::optional<std::string>* destination) {
+bool ParseValue(const std::string_view value,
+ std::optional<uint32_t>* destination) {
+ uint32_t parsed_value = 0;
+ if (::android::base::ParseUint(std::string{value}, &parsed_value)) {
+ *destination = parsed_value;
+ return true;
+ }
+ return false;
+}
+
+bool ParseValue(const std::string_view value, std::optional<std::string>* destination) {
*destination = value;
+ return true;
}
} // namespace
@@ -112,7 +125,9 @@
from_system_property_ = std::nullopt;
const std::string sysprop = ::android::base::GetProperty(system_property_name_, kUndefinedValue);
if (sysprop != kUndefinedValue) {
- ParseValue(sysprop, &from_system_property_);
+ if (!ParseValue(sysprop, &from_system_property_)) {
+ LOG(ERROR) << "Failed to parse " << system_property_name_ << "=" << sysprop;
+ }
}
// Load the server-side configuration.
@@ -120,7 +135,9 @@
const std::string server_config =
::android::base::GetProperty(server_setting_name_, kUndefinedValue);
if (server_config != kUndefinedValue) {
- ParseValue(server_config, &from_server_setting_);
+ if (!ParseValue(server_config, &from_server_setting_)) {
+ LOG(ERROR) << "Failed to parse " << server_setting_name_ << "=" << server_config;
+ }
}
}
diff --git a/libartbase/base/flags.h b/libartbase/base/flags.h
index 1afe9a9..0345ebc 100644
--- a/libartbase/base/flags.h
+++ b/libartbase/base/flags.h
@@ -258,15 +258,48 @@
// TODO: can be removed once we add real flags.
Flag<int32_t> MyFeatureTestFlag{"my-feature-test.flag", 42, FlagType::kDeviceConfig};
+
// Metric infra flags.
- Flag<bool> WriteMetricsToLogcat{ "metrics.write-to-logcat", false, FlagType::kCmdlineOnly};
- Flag<std::string> WriteMetricsToFile{"metrics.write-to-file", "", FlagType::kCmdlineOnly};
- Flag<bool> WriteMetricsToStatsd{ "metrics.write-to-statsd", false, FlagType::kDeviceConfig};
-
+ // The reporting spec for regular apps. An example of valid value is "S,1,2,4,*".
+ // See metrics::ReportingPeriodSpec for complete docs.
Flag<std::string> MetricsReportingSpec{"metrics.reporting-spec", "", FlagType::kDeviceConfig};
+
+ // The reporting spec for the system server. See MetricsReportingSpec as well.
Flag<std::string> MetricsReportingSpecSystemServer{"metrics.reporting-spec-server", "",
FlagType::kDeviceConfig};
+
+ // The mods that should report metrics. Together with MetricsReportingNumMods, they
+ // dictate what percentage of the runtime execution will report metrics.
+ // If the `session_id (a random number) % MetricsReportingNumMods < MetricsReportingMods`
+ // then the runtime session will report metrics.
+ //
+ // By default, the mods are 0, which means the reporting is disabled.
+ Flag<uint32_t> MetricsReportingMods{"metrics.reporting-mods", 0,
+ FlagType::kDeviceConfig};
+
+ // See MetricsReportingMods docs.
+ //
+ // By default the number of mods is 100, so MetricsReportingMods will naturally
+ // read as the percent of runtime sessions that will report metrics. If a finer
+ // grain unit is needed (e.g. a tenth of a percent), the num-mods can be increased.
+ Flag<uint32_t> MetricsReportingNumMods{"metrics.reporting-num-mods", 100,
+ FlagType::kDeviceConfig};
+
+ // Whether or not we should write metrics to statsd.
+ // Note that the actual write is still controlled by
+ // MetricsReportingMods and MetricsReportingNumMods.
+ Flag<bool> MetricsWriteToStatsd{ "metrics.write-to-statsd", false, FlagType::kDeviceConfig};
+
+ // Whether or not we should write metrics to logcat.
+ // Note that the actual write is still controlled by
+ // MetricsReportingMods and MetricsReportingNumMods.
+ Flag<bool> MetricsWriteToLogcat{ "metrics.write-to-logcat", false, FlagType::kCmdlineOnly};
+
+ // Whether or not we should write metrics to a file.
+ // Note that the actual write is still controlled by
+ // MetricsReportingMods and MetricsReportingNumMods.
+ Flag<std::string> MetricsWriteToFile{"metrics.write-to-file", "", FlagType::kCmdlineOnly};
};
// This is the actual instance of all the flags.
diff --git a/runtime/metrics/reporter.cc b/runtime/metrics/reporter.cc
index df4a4b3..2398ea9 100644
--- a/runtime/metrics/reporter.cc
+++ b/runtime/metrics/reporter.cc
@@ -52,10 +52,21 @@
config_ = config;
}
+bool MetricsReporter::IsMetricsReportingEnabled(const SessionData& session_data) const {
+ return session_data.session_id % config_.reporting_num_mods < config_.reporting_mods;
+}
+
bool MetricsReporter::MaybeStartBackgroundThread(SessionData session_data) {
CHECK(!thread_.has_value());
+
+ session_data_ = session_data;
+ LOG_STREAM(DEBUG) << "Received session metadata: " << session_data_.session_id;
+
+ if (!IsMetricsReportingEnabled(session_data_)) {
+ return false;
+ }
+
thread_.emplace(&MetricsReporter::BackgroundThreadRun, this);
- messages_.SendMessage(BeginSessionMessage{session_data});
return true;
}
@@ -68,7 +79,7 @@
}
void MetricsReporter::NotifyStartupCompleted() {
- if (thread_.has_value()) {
+ if (ShouldReportAtStartup() && thread_.has_value()) {
messages_.SendMessage(StartupCompletedMessage{});
}
}
@@ -118,10 +129,6 @@
while (running) {
messages_.SwitchReceive(
- [&](BeginSessionMessage message) {
- session_data_ = message.session_data;
- LOG_STREAM(DEBUG) << "Received session metadata: " << session_data_.session_id;
- },
[&]([[maybe_unused]] ShutdownRequestedMessage message) {
LOG_STREAM(DEBUG) << "Shutdown request received " << session_data_.session_id;
running = false;
@@ -187,13 +194,16 @@
}
bool MetricsReporter::ShouldReportAtStartup() const {
- return config_.period_spec.has_value() &&
+ return IsMetricsReportingEnabled(session_data_) &&
+ config_.period_spec.has_value() &&
config_.period_spec->report_startup_first;
}
bool MetricsReporter::ShouldContinueReporting() const {
bool result =
- // Only if we have period spec
+ // Only if the reporting is enabled
+ IsMetricsReportingEnabled(session_data_) &&
+ // and if we have period spec
config_.period_spec.has_value() &&
// and the periods are non empty
!config_.period_spec->periods_seconds.empty() &&
@@ -229,6 +239,7 @@
: gFlags.MetricsReportingSpec.GetValueOptional();
std::optional<ReportingPeriodSpec> period_spec = std::nullopt;
+
if (spec_str.has_value()) {
std::string error;
period_spec = ReportingPeriodSpec::Parse(spec_str.value(), &error);
@@ -237,11 +248,22 @@
<< " with error: " << error;
}
}
+
+ uint32_t reporting_num_mods = gFlags.MetricsReportingNumMods();
+ uint32_t reporting_mods = gFlags.MetricsReportingMods();
+ if (reporting_mods > reporting_num_mods) {
+ LOG(ERROR) << "Invalid metrics reporting mods: " << reporting_mods
+ << " num modes=" << reporting_num_mods;
+ reporting_mods = 0;
+ }
+
return {
- .dump_to_logcat = gFlags.WriteMetricsToLogcat(),
- .dump_to_file = gFlags.WriteMetricsToFile.GetValueOptional(),
- .dump_to_statsd = gFlags.WriteMetricsToStatsd(),
+ .dump_to_logcat = gFlags.MetricsWriteToLogcat(),
+ .dump_to_file = gFlags.MetricsWriteToFile.GetValueOptional(),
+ .dump_to_statsd = gFlags.MetricsWriteToStatsd(),
.period_spec = period_spec,
+ .reporting_num_mods = reporting_num_mods,
+ .reporting_mods = reporting_mods,
};
}
diff --git a/runtime/metrics/reporter.h b/runtime/metrics/reporter.h
index ab3aa3c..4db5df1 100644
--- a/runtime/metrics/reporter.h
+++ b/runtime/metrics/reporter.h
@@ -79,6 +79,13 @@
// The reporting period configuration.
std::optional<ReportingPeriodSpec> period_spec;
+
+ // The mods that should report metrics. Together with reporting_num_mods, they
+ // dictate what percentage of the runtime execution will report metrics.
+ // If the `session_id (a random number) % reporting_num_mods < reporting_mods`
+ // then the runtime session will report metrics.
+ uint32_t reporting_mods{0};
+ uint32_t reporting_num_mods{100};
};
// MetricsReporter handles periodically reporting ART metrics.
@@ -126,6 +133,9 @@
MetricsReporter(const ReportingConfig& config, Runtime* runtime);
private:
+ // Whether or not we should reporting metrics according to the sampling rate.
+ bool IsMetricsReportingEnabled(const SessionData& session_data) const;
+
// The background reporting thread main loop.
void BackgroundThreadRun();
@@ -162,12 +172,6 @@
// A message indicating that app startup has completed.
struct StartupCompletedMessage {};
- // A message marking the beginning of a metrics logging session.
- //
- // The primary purpose of this is to pass the session metadata from the Runtime to the metrics
- // backends.
- struct BeginSessionMessage{ SessionData session_data; };
-
// A message requesting an explicit metrics report.
//
// The synchronous field specifies whether the reporting thread will send a message back when
@@ -183,7 +187,6 @@
MessageQueue<ShutdownRequestedMessage,
StartupCompletedMessage,
- BeginSessionMessage,
RequestMetricsReportMessage,
CompilationInfoMessage>
messages_;
diff --git a/runtime/metrics/reporter_test.cc b/runtime/metrics/reporter_test.cc
index ba8e6a0..8c2a581 100644
--- a/runtime/metrics/reporter_test.cc
+++ b/runtime/metrics/reporter_test.cc
@@ -105,10 +105,12 @@
// Configures the metric reporting.
void SetupReporter(const char* period_spec,
- uint32_t session_id = 1) {
+ uint32_t session_id = 1,
+ uint32_t reporting_mods = 100) {
ReportingConfig config;
if (period_spec != nullptr) {
std::string error;
+ config.reporting_mods = reporting_mods;
config.period_spec = ReportingPeriodSpec::Parse(period_spec, &error);
ASSERT_TRUE(config.period_spec.has_value());
}
@@ -310,6 +312,53 @@
ASSERT_FALSE(ShouldContinueReporting());
}
+// Verify we don't start reporting if the sample rate is set to 0.
+TEST_F(MetricsReporterTest, SampleRateDisable) {
+ SetupReporter("1", /*session_id=*/ 1, /*reporting_mods=*/ 0);
+
+ // The background thread should not start.
+ ASSERT_FALSE(MaybeStartBackgroundThread(/*add_metrics=*/ false));
+
+ ASSERT_FALSE(ShouldReportAtStartup());
+ ASSERT_FALSE(ShouldContinueReporting());
+}
+
+// Verify we don't start reporting if the sample rate is low and the session does
+// not meet conditions.
+TEST_F(MetricsReporterTest, SampleRateDisable24) {
+ SetupReporter("1", /*session_id=*/ 125, /*reporting_mods=*/ 24);
+
+ // The background thread should not start.
+ ASSERT_FALSE(MaybeStartBackgroundThread(/*add_metrics=*/ false));
+
+ ASSERT_FALSE(ShouldReportAtStartup());
+ ASSERT_FALSE(ShouldContinueReporting());
+}
+
+// Verify we start reporting if the sample rate and the session meet
+// reporting conditions
+TEST_F(MetricsReporterTest, SampleRateEnable50) {
+ SetupReporter("1", /*session_id=*/ 125, /*reporting_mods=*/ 50);
+
+ // The background thread should not start.
+ ASSERT_TRUE(MaybeStartBackgroundThread(/*add_metrics=*/ false));
+
+ ASSERT_FALSE(ShouldReportAtStartup());
+ ASSERT_TRUE(ShouldContinueReporting());
+}
+
+// Verify we start reporting if the sample rate and the session meet
+// reporting conditions
+TEST_F(MetricsReporterTest, SampleRateEnableAll) {
+ SetupReporter("1", /*session_id=*/ 1099, /*reporting_mods=*/ 100);
+
+ // The background thread should not start.
+ ASSERT_TRUE(MaybeStartBackgroundThread(/*add_metrics=*/ false));
+
+ ASSERT_FALSE(ShouldReportAtStartup());
+ ASSERT_TRUE(ShouldContinueReporting());
+}
+
// Test class for period spec parsing
class ReportingPeriodSpecTest : public testing::Test {
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index ca6b82a..370ee02 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -1096,7 +1096,9 @@
metrics_reporter_->ReloadConfig(metrics_config);
metrics::SessionData session_data{metrics::SessionData::CreateDefault()};
- session_data.session_id = GetRandomNumber<int64_t>(0, std::numeric_limits<int64_t>::max());
+ // Start the session id from 1 to avoid clashes with the default value.
+ // (better for debugability)
+ session_data.session_id = GetRandomNumber<int64_t>(1, std::numeric_limits<int64_t>::max());
// TODO: set session_data.compilation_reason and session_data.compiler_filter
metrics_reporter_->MaybeStartBackgroundThread(session_data);
}
diff --git a/test/2232-write-metrics-to-log/run b/test/2232-write-metrics-to-log/run
index 07c1e60..d34ec6c 100755
--- a/test/2232-write-metrics-to-log/run
+++ b/test/2232-write-metrics-to-log/run
@@ -15,4 +15,4 @@
# limitations under the License.
export ANDROID_LOG_TAGS="*:i"
-exec ${RUN} $@ --external-log-tags --runtime-option -Xmetrics-write-to-logcat:true
+exec ${RUN} $@ --external-log-tags --runtime-option -Xmetrics-write-to-logcat:true --runtime-option -Xmetrics-reporting-mods:100
diff --git a/test/925-threadgroups/run b/test/925-threadgroups/run
index c6e62ae..8e44166 100755
--- a/test/925-threadgroups/run
+++ b/test/925-threadgroups/run
@@ -14,4 +14,4 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-./default-run "$@" --jvmti
+./default-run "$@" --jvmti --runtime-option -Xmetrics-reporting-mods:100