Add period reporting to the metrics infra
The metric reporting can now be condifured to report at
different periods, according to a predefined spec.
The period spec is given as a string (e.g. "S,1,2,4,8,*")
and can specify:
- startup reporting
- fixed period reporting
- continuous reporting
For example "S,1,2,4,*" means that we will report at
startup, then after 1 seconds, then after 2 and 4,
and finally, that we will continue to report every other 4s.
The specs for system server and the apps are given by different
flags, so that we can have different reporting configurations
for each.
Also, this CL adds much more robust testing to the metrics
reporter, something that was missing before.
Test: gtest
Bug: 170149255
Merged-In: Ifbe32b3877d2e1cdf43d30b67672f6ebfb42dadf
Change-Id: Ifbe32b3877d2e1cdf43d30b67672f6ebfb42dadf
(cherry picked from commit ec11cf1e4a7400af1befae3a5194206f16a58047)
diff --git a/libartbase/base/flags.h b/libartbase/base/flags.h
index e71bb4a..1afe9a9 100644
--- a/libartbase/base/flags.h
+++ b/libartbase/base/flags.h
@@ -138,7 +138,7 @@
FlagType type_;
};
-using FlagBase = FlagMetaBase<bool, int, std::string>;
+using FlagBase = FlagMetaBase<bool, int32_t, uint32_t, std::string>;
template <>
std::forward_list<FlagBase*> FlagBase::ALL_FLAGS;
@@ -256,13 +256,17 @@
struct Flags {
// Flag used to test the infra.
// TODO: can be removed once we add real flags.
- Flag<int> MyFeatureTestFlag{"my-feature-test.flag", 42, FlagType::kDeviceConfig};
+ Flag<int32_t> MyFeatureTestFlag{"my-feature-test.flag", 42, FlagType::kDeviceConfig};
// Metric infra flags.
- Flag<bool> WriteMetricsToStatsd{ "metrics.write-to-statsd", false, FlagType::kDeviceConfig};
+
Flag<bool> WriteMetricsToLogcat{ "metrics.write-to-logcat", false, FlagType::kCmdlineOnly};
- Flag<int> MetricsReportingPeriod{"metrics.reporting-period", 0, FlagType::kCmdlineOnly};
Flag<std::string> WriteMetricsToFile{"metrics.write-to-file", "", FlagType::kCmdlineOnly};
+ Flag<bool> WriteMetricsToStatsd{ "metrics.write-to-statsd", false, FlagType::kDeviceConfig};
+
+ Flag<std::string> MetricsReportingSpec{"metrics.reporting-spec", "", FlagType::kDeviceConfig};
+ Flag<std::string> MetricsReportingSpecSystemServer{"metrics.reporting-spec-server", "",
+ FlagType::kDeviceConfig};
};
// This is the actual instance of all the flags.
diff --git a/runtime/Android.bp b/runtime/Android.bp
index c85d75a..1434fc8 100644
--- a/runtime/Android.bp
+++ b/runtime/Android.bp
@@ -725,6 +725,7 @@
"jni/java_vm_ext_test.cc",
"jni/jni_internal_test.cc",
"method_handles_test.cc",
+ "metrics/reporter_test.cc",
"mirror/dex_cache_test.cc",
"mirror/method_type_test.cc",
"mirror/object_test.cc",
diff --git a/runtime/metrics/reporter.cc b/runtime/metrics/reporter.cc
index 26b55b8..df4a4b3 100644
--- a/runtime/metrics/reporter.cc
+++ b/runtime/metrics/reporter.cc
@@ -16,6 +16,10 @@
#include "reporter.h"
+#include <algorithm>
+
+#include <android-base/parseint.h>
+
#include "base/flags.h"
#include "runtime.h"
#include "runtime_options.h"
@@ -28,25 +32,24 @@
namespace art {
namespace metrics {
-std::unique_ptr<MetricsReporter> MetricsReporter::Create(ReportingConfig config, Runtime* runtime) {
+std::unique_ptr<MetricsReporter> MetricsReporter::Create(
+ const ReportingConfig& config, Runtime* runtime) {
// We can't use std::make_unique here because the MetricsReporter constructor is private.
return std::unique_ptr<MetricsReporter>{new MetricsReporter{std::move(config), runtime}};
}
-MetricsReporter::MetricsReporter(ReportingConfig config, Runtime* runtime)
- : config_{std::move(config)}, runtime_{runtime} {}
+MetricsReporter::MetricsReporter(const ReportingConfig& config, Runtime* runtime)
+ : config_{config},
+ runtime_{runtime},
+ startup_reported_{false},
+ report_interval_index_{0} {}
MetricsReporter::~MetricsReporter() { MaybeStopBackgroundThread(); }
-bool MetricsReporter::IsPeriodicReportingEnabled() const {
- return config_.periodic_report_seconds.has_value();
-}
-
-void MetricsReporter::SetReportingPeriod(unsigned int period_seconds) {
- DCHECK(!thread_.has_value()) << "The reporting period should not be changed after the background "
+void MetricsReporter::ReloadConfig(const ReportingConfig& config) {
+ DCHECK(!thread_.has_value()) << "The config cannot be reloaded after the background "
"reporting thread is started.";
-
- config_.periodic_report_seconds = period_seconds;
+ config_ = config;
}
bool MetricsReporter::MaybeStartBackgroundThread(SessionData session_data) {
@@ -60,6 +63,7 @@
if (thread_.has_value()) {
messages_.SendMessage(ShutdownRequestedMessage{});
thread_->join();
+ thread_.reset();
}
}
@@ -115,35 +119,37 @@
while (running) {
messages_.SwitchReceive(
[&](BeginSessionMessage message) {
- LOG_STREAM(DEBUG) << "Received session metadata";
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";
+ LOG_STREAM(DEBUG) << "Shutdown request received " << session_data_.session_id;
running = false;
ReportMetrics();
},
[&](RequestMetricsReportMessage message) {
- LOG_STREAM(DEBUG) << "Explicit report request received";
+ LOG_STREAM(DEBUG) << "Explicit report request received " << session_data_.session_id;
ReportMetrics();
if (message.synchronous) {
thread_to_host_messages_.SendMessage(ReportCompletedMessage{});
}
},
[&]([[maybe_unused]] TimeoutExpiredMessage message) {
- LOG_STREAM(DEBUG) << "Timer expired, reporting metrics";
+ LOG_STREAM(DEBUG) << "Timer expired, reporting metrics " << session_data_.session_id;
ReportMetrics();
-
MaybeResetTimeout();
},
[&]([[maybe_unused]] StartupCompletedMessage message) {
- LOG_STREAM(DEBUG) << "App startup completed, reporting metrics";
+ LOG_STREAM(DEBUG) << "App startup completed, reporting metrics "
+ << session_data_.session_id;
ReportMetrics();
+ startup_reported_ = true;
+ MaybeResetTimeout();
},
[&](CompilationInfoMessage message) {
- LOG_STREAM(DEBUG) << "Compilation info received";
+ LOG_STREAM(DEBUG) << "Compilation info received " << session_data_.session_id;
session_data_.compilation_reason = message.compilation_reason;
session_data_.compiler_filter = message.compiler_filter;
});
@@ -152,17 +158,21 @@
if (attached) {
runtime_->DetachCurrentThread();
}
- LOG_STREAM(DEBUG) << "Metrics reporting thread terminating";
+ LOG_STREAM(DEBUG) << "Metrics reporting thread terminating " << session_data_.session_id;
}
void MetricsReporter::MaybeResetTimeout() {
- if (config_.periodic_report_seconds.has_value()) {
- messages_.SetTimeout(SecondsToMs(config_.periodic_report_seconds.value()));
+ if (ShouldContinueReporting()) {
+ messages_.SetTimeout(SecondsToMs(GetNextPeriodSeconds()));
}
}
+const ArtMetrics* MetricsReporter::GetMetrics() {
+ return runtime_->GetMetrics();
+}
+
void MetricsReporter::ReportMetrics() {
- ArtMetrics* metrics{runtime_->GetMetrics()};
+ const ArtMetrics* metrics = GetMetrics();
if (!session_started_) {
for (auto& backend : backends_) {
@@ -176,15 +186,107 @@
}
}
-ReportingConfig ReportingConfig::FromFlags() {
+bool MetricsReporter::ShouldReportAtStartup() const {
+ return config_.period_spec.has_value() &&
+ config_.period_spec->report_startup_first;
+}
+
+bool MetricsReporter::ShouldContinueReporting() const {
+ bool result =
+ // Only if we have period spec
+ config_.period_spec.has_value() &&
+ // and the periods are non empty
+ !config_.period_spec->periods_seconds.empty() &&
+ // and we already reported startup or not required to report startup
+ (startup_reported_ || !config_.period_spec->report_startup_first) &&
+ // and we still have unreported intervals or we are asked to report continuously.
+ (config_.period_spec->continuous_reporting ||
+ (report_interval_index_ < config_.period_spec->periods_seconds.size()));
+ return result;
+}
+
+uint32_t MetricsReporter::GetNextPeriodSeconds() {
+ DCHECK(ShouldContinueReporting());
+
+ // The index is either the current report_interval_index or the last index
+ // if we are in continuous mode and reached the end.
+ uint32_t index = std::min(
+ report_interval_index_,
+ static_cast<uint32_t>(config_.period_spec->periods_seconds.size() - 1));
+
+ uint32_t result = config_.period_spec->periods_seconds[index];
+
+ // Advance the index if we didn't get to the end.
+ if (report_interval_index_ < config_.period_spec->periods_seconds.size()) {
+ report_interval_index_++;
+ }
+ return result;
+}
+
+ReportingConfig ReportingConfig::FromFlags(bool is_system_server) {
+ std::optional<std::string> spec_str = is_system_server
+ ? gFlags.MetricsReportingSpecSystemServer.GetValueOptional()
+ : 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);
+ if (!period_spec.has_value()) {
+ LOG(ERROR) << "Failed to create metrics reporting spec from: " << spec_str.value()
+ << " with error: " << error;
+ }
+ }
return {
.dump_to_logcat = gFlags.WriteMetricsToLogcat(),
.dump_to_file = gFlags.WriteMetricsToFile.GetValueOptional(),
.dump_to_statsd = gFlags.WriteMetricsToStatsd(),
- .periodic_report_seconds = gFlags.MetricsReportingPeriod.GetValueOptional(),
+ .period_spec = period_spec,
};
}
+std::optional<ReportingPeriodSpec> ReportingPeriodSpec::Parse(
+ const std::string& spec_str, std::string* error_msg) {
+ *error_msg = "";
+ if (spec_str.empty()) {
+ *error_msg = "Invalid empty spec.";
+ return std::nullopt;
+ }
+
+ // Split the string. Each element is separated by comma.
+ std::vector<std::string> elems;
+ Split(spec_str, ',', &elems);
+
+ // Check the startup marker (front) and the continuous one (back).
+ std::optional<ReportingPeriodSpec> spec = std::make_optional(ReportingPeriodSpec());
+ spec->spec = spec_str;
+ spec->report_startup_first = elems.front() == "S";
+ spec->continuous_reporting = elems.back() == "*";
+
+ // Compute the indices for the period values.
+ size_t start_interval_idx = spec->report_startup_first ? 1 : 0;
+ size_t end_interval_idx = spec->continuous_reporting ? (elems.size() - 1) : elems.size();
+
+ // '*' needs a numeric interval before in order to be valid.
+ if (spec->continuous_reporting &&
+ end_interval_idx == start_interval_idx) {
+ *error_msg = "Invalid period value in spec: " + spec_str;
+ return std::nullopt;
+ }
+
+ // Parse the periods.
+ for (size_t i = start_interval_idx; i < end_interval_idx; i++) {
+ uint32_t period;
+ if (!android::base::ParseUint(elems[i], &period)) {
+ *error_msg = "Invalid period value in spec: " + spec_str;
+ return std::nullopt;
+ }
+ spec->periods_seconds.push_back(period);
+ }
+
+ return spec;
+}
+
} // namespace metrics
} // namespace art
diff --git a/runtime/metrics/reporter.h b/runtime/metrics/reporter.h
index 3e8056b..ab3aa3c 100644
--- a/runtime/metrics/reporter.h
+++ b/runtime/metrics/reporter.h
@@ -26,9 +26,47 @@
namespace art {
namespace metrics {
+/**
+ * Encapsulates the specification of the metric reporting periods.
+ *
+ * The period spec follows the following regex: "(S,)?(\d+,)*\*?"
+ * with the following semantics:
+ * "S" - will only report at startup.
+ *
+ * "S,1,1" - will report startup, than 1 second later, then another
+ * second later.
+ *
+ * "S,1,2,4 " - will report at Startup time, then 1 seconds later,
+ * then 2, then finally 4 seconds later. After that, the
+ * reporting will stop.
+ *
+ * "S,1,2,4,*" - same as above, but after the final 4s period, the
+ * reporting will continue every other 4s.
+ * '*' is an indication we should report continuously
+ * every N seconds, where N is the last period.
+ *
+ * "2,*" - will report every 2 seconds
+ *
+ * Note that "", "*", or "S,*" are not valid specs, and 'S' can only occur
+ * in the beginning.
+ */
+struct ReportingPeriodSpec {
+ static std::optional<ReportingPeriodSpec> Parse(
+ const std::string& spec_str, std::string* error_msg);
+
+ // The original spec.
+ std::string spec;
+ // The intervals when we should report.
+ std::vector<uint32_t> periods_seconds;
+ // Whether or not the reporting is continuous (contains a '*').
+ bool continuous_reporting{false};
+ // Whether or not the reporting should start after startup event (starts with an 'S').
+ bool report_startup_first{false};
+};
+
// Defines the set of options for how metrics reporting happens.
struct ReportingConfig {
- static ReportingConfig FromFlags();
+ static ReportingConfig FromFlags(bool is_system_server = false);
// Causes metrics to be written to the log, which makes them show up in logcat.
bool dump_to_logcat{false};
@@ -39,17 +77,17 @@
// If set, provides a file name to enable metrics logging to a file.
std::optional<std::string> dump_to_file;
- // If set, metrics will be reported every time this many seconds elapses.
- std::optional<unsigned int> periodic_report_seconds;
+ // The reporting period configuration.
+ std::optional<ReportingPeriodSpec> period_spec;
};
// MetricsReporter handles periodically reporting ART metrics.
class MetricsReporter {
public:
// Creates a MetricsReporter instance that matches the options selected in ReportingConfig.
- static std::unique_ptr<MetricsReporter> Create(ReportingConfig config, Runtime* runtime);
+ static std::unique_ptr<MetricsReporter> Create(const ReportingConfig& config, Runtime* runtime);
- ~MetricsReporter();
+ virtual ~MetricsReporter();
// Creates and runs the background reporting thread.
//
@@ -65,27 +103,29 @@
// completes.
void NotifyStartupCompleted();
- bool IsPeriodicReportingEnabled() const;
-
- // Changes the reporting period.
- //
- // This function is not thread safe and may only be called before the background reporting thread
- // has been started.
- void SetReportingPeriod(unsigned int period_seconds);
-
// Requests a metrics report
//
// If synchronous is set to true, this function will block until the report has completed.
void RequestMetricsReport(bool synchronous = true);
+ // Reloads the metrics config from the given value.
+ // Can only be called before starting the background thread.
+ void ReloadConfig(const ReportingConfig& config);
+
void SetCompilationInfo(CompilationReason compilation_reason,
CompilerFilter::Filter compiler_filter);
static constexpr const char* kBackgroundThreadName = "Metrics Background Reporting Thread";
- private:
- MetricsReporter(ReportingConfig config, Runtime* runtime);
+ protected:
+ // Returns the metrics to be reported.
+ // This exists only for testing purposes so that we can verify reporting with minimum
+ // runtime interference.
+ virtual const ArtMetrics* GetMetrics();
+ MetricsReporter(const ReportingConfig& config, Runtime* runtime);
+
+ private:
// The background reporting thread main loop.
void BackgroundThreadRun();
@@ -95,10 +135,26 @@
// Outputs the current state of the metrics to the destination set by config_.
void ReportMetrics();
+ // Whether or not we should wait for startup before reporting for the first time.
+ bool ShouldReportAtStartup() const;
+
+ // Whether or not we should continue reporting (either because we still
+ // have periods to report, or because we are in continuous mode).
+ bool ShouldContinueReporting() const;
+
+ // Returns the next reporting period.
+ // Must be called only if ShouldContinueReporting() is true.
+ uint32_t GetNextPeriodSeconds();
+
ReportingConfig config_;
Runtime* runtime_;
std::vector<std::unique_ptr<MetricsBackend>> backends_;
std::optional<std::thread> thread_;
+ // Whether or not we reported the startup event.
+ bool startup_reported_;
+ // The index into period_spec.periods_seconds which tells the next delay in
+ // seconds for the next reporting.
+ uint32_t report_interval_index_;
// A message indicating that the reporting thread should shut down.
struct ShutdownRequestedMessage {};
@@ -139,6 +195,8 @@
SessionData session_data_{};
bool session_started_{false};
+
+ friend class MetricsReporterTest;
};
} // namespace metrics
diff --git a/runtime/metrics/reporter_test.cc b/runtime/metrics/reporter_test.cc
new file mode 100644
index 0000000..ba8e6a0
--- /dev/null
+++ b/runtime/metrics/reporter_test.cc
@@ -0,0 +1,377 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "reporter.h"
+
+#include "gtest/gtest.h"
+
+#include "common_runtime_test.h"
+#include "base/metrics/metrics.h"
+
+#pragma clang diagnostic push
+#pragma clang diagnostic error "-Wconversion"
+
+namespace art {
+namespace metrics {
+
+// Helper class to verify the metrics reporter.
+// The functionality is identical to the MetricsReporter with the exception of
+// the metrics source. Instead of taking its metrics from the current Runtime,
+// this class will keep its own copy so that it does not get interference from
+// other runtime setup logic.
+class MockMetricsReporter : public MetricsReporter {
+ protected:
+ MockMetricsReporter(const ReportingConfig& config, Runtime* runtime) :
+ MetricsReporter(config, runtime),
+ art_metrics_(new ArtMetrics()) {}
+
+ const ArtMetrics* GetMetrics() override {
+ return art_metrics_.get();
+ }
+
+ std::unique_ptr<ArtMetrics> art_metrics_;
+
+ friend class MetricsReporterTest;
+};
+
+// A test backend which keeps track of all metrics reporting.
+class TestBackend : public MetricsBackend {
+ public:
+ struct Report {
+ uint64_t timestamp_millis;
+ SafeMap<DatumId, uint64_t> data;
+ };
+
+ void BeginSession(const SessionData& session_data) override {
+ session_data_ = session_data;
+ }
+
+ void BeginReport(uint64_t timestamp_millis) override {
+ current_report_.reset(new Report());
+ current_report_->timestamp_millis = timestamp_millis;
+ }
+
+ void ReportCounter(DatumId counter_type, uint64_t value) override {
+ current_report_->data.Put(counter_type, value);
+ }
+
+ void ReportHistogram(DatumId histogram_type ATTRIBUTE_UNUSED,
+ int64_t low_value ATTRIBUTE_UNUSED,
+ int64_t high_value ATTRIBUTE_UNUSED,
+ const std::vector<uint32_t>& buckets ATTRIBUTE_UNUSED) override {
+ // TODO: nothing yet. We should implement and test histograms as well.
+ }
+
+ void EndReport() override {
+ reports_.push_back(*current_report_);
+ current_report_ = nullptr;
+ }
+
+ const std::vector<Report>& GetReports() {
+ return reports_;
+ }
+
+ private:
+ SessionData session_data_;
+ std::vector<Report> reports_;
+ std::unique_ptr<Report> current_report_;
+};
+
+// The actual metrics test class
+class MetricsReporterTest : public CommonRuntimeTest {
+ protected:
+ void SetUp() override {
+ // Do the normal setup.
+ CommonRuntimeTest::SetUp();
+
+ // We need to start the runtime in order to run threads.
+ Thread::Current()->TransitionFromSuspendedToRunnable();
+ bool started = runtime_->Start();
+ CHECK(started);
+ }
+
+ // Configures the metric reporting.
+ void SetupReporter(const char* period_spec,
+ uint32_t session_id = 1) {
+ ReportingConfig config;
+ if (period_spec != nullptr) {
+ std::string error;
+ config.period_spec = ReportingPeriodSpec::Parse(period_spec, &error);
+ ASSERT_TRUE(config.period_spec.has_value());
+ }
+
+ reporter_.reset(new MockMetricsReporter(std::move(config), Runtime::Current()));
+ backend_ = new TestBackend();
+ reporter_->backends_.emplace_back(backend_);
+
+ session_data_ = metrics::SessionData::CreateDefault();
+ session_data_.session_id = session_id;
+ }
+
+ void TearDown() override {
+ reporter_ = nullptr;
+ backend_ = nullptr;
+ }
+
+ bool ShouldReportAtStartup() {
+ return reporter_->ShouldReportAtStartup();
+ }
+
+ bool ShouldContinueReporting() {
+ return reporter_->ShouldContinueReporting();
+ }
+
+ uint32_t GetNextPeriodSeconds() {
+ return reporter_->GetNextPeriodSeconds();
+ }
+
+ void ReportMetrics() {
+ reporter_->ReportMetrics();
+ }
+
+ void NotifyStartupCompleted() {
+ reporter_->NotifyStartupCompleted();
+ }
+
+ // Starts the reporting thread and adds some metrics if necessary.
+ bool MaybeStartBackgroundThread(bool add_metrics) {
+ // TODO: set session_data.compilation_reason and session_data.compiler_filter
+ bool result = reporter_->MaybeStartBackgroundThread(session_data_);
+ if (add_metrics) {
+ reporter_->art_metrics_->JitMethodCompileCount()->Add(1);
+ reporter_->art_metrics_->ClassVerificationCount()->Add(2);
+ }
+ return result;
+ }
+
+ // Right now we either:
+ // 1) don't add metrics (with_metrics = false)
+ // 2) or always add the same metrics (see MaybeStartBackgroundThread)
+ // So we can write a global verify method.
+ void VerifyReports(uint32_t size, bool with_metrics) {
+ // TODO: we should iterate through all the other metrics to make sure they were not
+ // reported. However, we don't have an easy to use iteration mechanism over metrics yet.
+ // We should ads one
+ ASSERT_EQ(backend_->GetReports().size(), size);
+ for (auto report : backend_->GetReports()) {
+ ASSERT_EQ(report.data.Get(DatumId::kClassVerificationCount), with_metrics ? 2u : 0u);
+ ASSERT_EQ(report.data.Get(DatumId::kJitMethodCompileCount), with_metrics ? 1u : 0u);
+ }
+ }
+
+ // Sleeps until the backend received the give number of reports.
+ void WaitForReport(uint32_t report_count, uint32_t sleep_period_ms) {
+ while (true) {
+ if (backend_->GetReports().size() == report_count) {
+ return;
+ }
+ usleep(sleep_period_ms * 1000);
+ }
+ }
+
+ private:
+ std::unique_ptr<MockMetricsReporter> reporter_;
+ TestBackend* backend_;
+ metrics::SessionData session_data_;
+};
+
+// Verifies startup reporting.
+TEST_F(MetricsReporterTest, StartupOnly) {
+ SetupReporter("S");
+
+ // Verify startup conditions
+ ASSERT_TRUE(ShouldReportAtStartup());
+ ASSERT_FALSE(ShouldContinueReporting());
+
+ // Start the thread and notify the startup. This will advance the state.
+ MaybeStartBackgroundThread(/*add_metrics=*/ true);
+
+ NotifyStartupCompleted();
+ WaitForReport(/*report_count=*/ 1, /*sleep_period_ms=*/ 50);
+ VerifyReports(/*size=*/ 1, /*with_metrics*/ true);
+
+ // We still should not report at period.
+ ASSERT_FALSE(ShouldContinueReporting());
+}
+
+// LARGE TEST: This test takes 1s to run.
+// Verifies startup reporting, followed by a fixed, one time only reporting.
+TEST_F(MetricsReporterTest, StartupAndPeriod) {
+ SetupReporter("S,1");
+
+ // Verify startup conditions
+ ASSERT_TRUE(ShouldReportAtStartup());
+ ASSERT_FALSE(ShouldContinueReporting());
+
+ // Start the thread and notify the startup. This will advance the state.
+ MaybeStartBackgroundThread(/*add_metrics=*/ true);
+ NotifyStartupCompleted();
+
+ // We're waiting for 2 reports: the startup one, and the 1s one.
+ WaitForReport(/*report_count=*/ 2, /*sleep_period_ms=*/ 500);
+ VerifyReports(/*size=*/ 2, /*with_metrics*/ true);
+
+ // We should not longer report at period.
+ ASSERT_FALSE(ShouldContinueReporting());
+}
+
+// LARGE TEST: This takes take 2s to run.
+// Verifies startup reporting, followed by continuous reporting.
+TEST_F(MetricsReporterTest, StartupAndPeriodContinuous) {
+ SetupReporter("S,1,*");
+
+ // Verify startup conditions
+ ASSERT_TRUE(ShouldReportAtStartup());
+ ASSERT_FALSE(ShouldContinueReporting());
+
+ // Start the thread and notify the startup. This will advance the state.
+ MaybeStartBackgroundThread(/*add_metrics=*/ true);
+ NotifyStartupCompleted();
+
+ // We're waiting for 3 reports: the startup one, and the 1s one.
+ WaitForReport(/*report_count=*/ 3, /*sleep_period_ms=*/ 500);
+ VerifyReports(/*size=*/ 3, /*with_metrics*/ true);
+
+ // We should keep reporting at period.
+ ASSERT_TRUE(ShouldContinueReporting());
+}
+
+// LARGE TEST: This test takes 1s to run.
+// Verifies a fixed, one time only reporting.
+TEST_F(MetricsReporterTest, OneTime) {
+ SetupReporter("1");
+
+ // Verify startup conditions
+ ASSERT_FALSE(ShouldReportAtStartup());
+ ASSERT_TRUE(ShouldContinueReporting());
+
+ // Start the thread and notify the startup. This will advance the state.
+ MaybeStartBackgroundThread(/*add_metrics=*/ true);
+
+ // We're waiting for 1 report
+ WaitForReport(/*report_count=*/ 1, /*sleep_period_ms=*/ 500);
+ VerifyReports(/*size=*/ 1, /*with_metrics*/ true);
+
+ // We should not longer report at period.
+ ASSERT_FALSE(ShouldContinueReporting());
+}
+
+// LARGE TEST: This takes take 5s to run.
+// Verifies a sequence of reporting, at different interval of times.
+TEST_F(MetricsReporterTest, PeriodContinuous) {
+ SetupReporter("1,2,*");
+
+ // Verify startup conditions
+ ASSERT_FALSE(ShouldReportAtStartup());
+ ASSERT_TRUE(ShouldContinueReporting());
+
+ // Start the thread and notify the startup. This will advance the state.
+ MaybeStartBackgroundThread(/*add_metrics=*/ true);
+ NotifyStartupCompleted();
+
+ // We're waiting for 2 reports: the startup one, and the 1s one.
+ WaitForReport(/*report_count=*/ 3, /*sleep_period_ms=*/ 500);
+ VerifyReports(/*size=*/ 3, /*with_metrics*/ true);
+
+ // We should keep reporting at period.
+ ASSERT_TRUE(ShouldContinueReporting());
+}
+
+// LARGE TEST: This test takes 1s to run.
+// Verifies reporting when no metrics where recorded.
+TEST_F(MetricsReporterTest, NoMetrics) {
+ SetupReporter("1");
+
+ // Verify startup conditions
+ ASSERT_FALSE(ShouldReportAtStartup());
+ ASSERT_TRUE(ShouldContinueReporting());
+
+ // Start the thread and notify the startup. This will advance the state.
+ MaybeStartBackgroundThread(/*add_metrics=*/ false);
+
+ // We're waiting for 1 report
+ WaitForReport(/*report_count=*/ 1, /*sleep_period_ms=*/ 500);
+ VerifyReports(/*size=*/ 1, /*with_metrics*/ false);
+
+ // We should not longer report at period.
+ ASSERT_FALSE(ShouldContinueReporting());
+}
+
+
+// Test class for period spec parsing
+class ReportingPeriodSpecTest : public testing::Test {
+ public:
+ void VerifyFalse(const std::string& spec_str) {
+ Verify(spec_str, false, false, false, {});
+ }
+
+ void VerifyTrue(
+ const std::string& spec_str,
+ bool startup_first,
+ bool continuous,
+ std::vector<uint32_t> periods) {
+ Verify(spec_str, true, startup_first, continuous, periods);
+ }
+
+ void Verify(
+ const std::string& spec_str,
+ bool valid,
+ bool startup_first,
+ bool continuous,
+ std::vector<uint32_t> periods) {
+ std::string error_msg;
+ std::optional<ReportingPeriodSpec> spec = ReportingPeriodSpec::Parse(spec_str, &error_msg);
+
+ ASSERT_EQ(valid, spec.has_value()) << spec_str;
+ if (valid) {
+ ASSERT_EQ(spec->spec, spec_str) << spec_str;
+ ASSERT_EQ(spec->report_startup_first, startup_first) << spec_str;
+ ASSERT_EQ(spec->continuous_reporting, continuous) << spec_str;
+ ASSERT_EQ(spec->periods_seconds, periods) << spec_str;
+ }
+ }
+};
+
+TEST_F(ReportingPeriodSpecTest, ParseTestsInvalid) {
+ VerifyFalse("");
+ VerifyFalse("*");
+ VerifyFalse("S,*");
+ VerifyFalse("foo");
+ VerifyFalse("-1");
+ VerifyFalse("1,S");
+ VerifyFalse("*,1");
+ VerifyFalse("1,2,3,-1,3");
+ VerifyFalse("1,*,2");
+ VerifyFalse("1,S,2");
+}
+
+TEST_F(ReportingPeriodSpecTest, ParseTestsValid) {
+ VerifyTrue("S", true, false, {});
+ VerifyTrue("S,1", true, false, {1});
+ VerifyTrue("S,1,2,3,4", true, false, {1, 2, 3, 4});
+ VerifyTrue("S,1,*", true, true, {1});
+ VerifyTrue("S,1,2,3,4,*", true, true, {1, 2, 3, 4});
+
+ VerifyTrue("1", false, false, {1});
+ VerifyTrue("1,2,3,4", false, false, {1, 2, 3, 4});
+ VerifyTrue("1,*", false, true, {1});
+ VerifyTrue("1,2,3,4,*", false, true, {1, 2, 3, 4});
+}
+
+} // namespace metrics
+} // namespace art
+
+#pragma clang diagnostic pop // -Wconversion
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index ff504e3..ca6b82a 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -1088,13 +1088,12 @@
GetMetrics()->Reset();
if (metrics_reporter_ != nullptr) {
- if (IsSystemServer() && !metrics_reporter_->IsPeriodicReportingEnabled()) {
- // For system server, we don't get startup metrics, so make sure we have periodic reporting
- // enabled.
- //
- // Note that this does not override the command line argument if one is given.
- metrics_reporter_->SetReportingPeriod(kOneHourInSeconds);
- }
+ // Now that we know if we are an app or system server, reload the metrics reporter config
+ // in case there are any difference.
+ metrics::ReportingConfig metrics_config =
+ metrics::ReportingConfig::FromFlags(is_system_server);
+
+ 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());
@@ -1939,7 +1938,7 @@
}
void Runtime::InitMetrics() {
- auto metrics_config = metrics::ReportingConfig::FromFlags();
+ metrics::ReportingConfig metrics_config = metrics::ReportingConfig::FromFlags();
metrics_reporter_ = metrics::MetricsReporter::Create(metrics_config, this);
}