Improve odrefresh metrics collection.
- Add an explicit switch to OdrMetrics to make the logic more
straightforward, and only enable it when compilation was done or an
error occurred.
- Initialize status to kUnknown instead of kOK so that the status
doesn't show "OK" when an unexpected error occurs.
- Explicitly set status to kOK if compilation succeeds.
- Add a new status: kDalvikCachePermissionDenied, to indicate that
odrefresh Failed to access the dalvik-cache directory due to lack of
permission.
- Set the status in many places where the status is missing.
- Before this change, odrefresh fails on the "kCheck" stage when trying
to clean up the dalvik-cache directory if it doesn't have the
permission to create the directory. This is weird. After this change,
odrefresh will fail on the "kPreparation" stage instead.
Bug: 246534524
Test: atest ArtGtestsTargetChroot:OdrMetricsTest
Test: -
1. Run `odrefresh --compile`.
2. See `<stage_reached>60</stage_reached><status>1</status>`.
Test: -
1. Make the SELinux context of the dalvik-cache directory wrong.
2. Run `odrefresh --compile`.
3. See `<stage_reached>20</stage_reached><status>8</status>`.
Test: -
1. Run `odrefresh --compile`.
2. Kill dex2oat.
3. See `<stage_reached>40</stage_reached><status>4</status>`.
Change-Id: I1a239f7d59668fa763934c074e4bfc5ebcc3ee42
diff --git a/odrefresh/odr_metrics.cc b/odrefresh/odr_metrics.cc
index 57a0241..b59ed71 100644
--- a/odrefresh/odr_metrics.cc
+++ b/odrefresh/odr_metrics.cc
@@ -36,7 +36,7 @@
namespace odrefresh {
OdrMetrics::OdrMetrics(const std::string& cache_directory, const std::string& metrics_file)
- : cache_directory_(cache_directory), metrics_file_(metrics_file), status_(Status::kOK) {
+ : cache_directory_(cache_directory), metrics_file_(metrics_file) {
DCHECK(StartsWith(metrics_file_, "/"));
// Remove existing metrics file if it exists.
@@ -56,14 +56,19 @@
}
OdrMetrics::~OdrMetrics() {
- cache_space_free_end_mib_ = GetFreeSpaceMiB(cache_directory_);
+ CaptureSpaceFreeEnd();
- // Log metrics only if odrefresh detected a reason to compile.
- if (trigger_.has_value()) {
+ // Log metrics only if this is explicitly enabled (typically when compilation was done or an error
+ // occurred).
+ if (enabled_) {
WriteToFile(metrics_file_, this);
}
}
+void OdrMetrics::CaptureSpaceFreeEnd() {
+ cache_space_free_end_mib_ = GetFreeSpaceMiB(cache_directory_);
+}
+
void OdrMetrics::SetCompilationTime(int32_t millis) {
switch (stage_) {
case Stage::kPrimaryBootClasspath:
@@ -102,12 +107,6 @@
}
}
-void OdrMetrics::SetStage(Stage stage) {
- if (status_ == Status::kOK) {
- stage_ = stage;
- }
-}
-
int32_t OdrMetrics::GetFreeSpaceMiB(const std::string& path) {
static constexpr uint32_t kBytesPerMiB = 1024 * 1024;
static constexpr uint64_t kNominalMaximumCacheBytes = 1024 * kBytesPerMiB;
@@ -134,29 +133,23 @@
return static_cast<int32_t>(free_space_mib);
}
-bool OdrMetrics::ToRecord(/*out*/OdrMetricsRecord* record) const {
- if (!trigger_.has_value()) {
- return false;
- }
- record->odrefresh_metrics_version = kOdrefreshMetricsVersion;
- record->art_apex_version = art_apex_version_;
- record->trigger = static_cast<uint32_t>(trigger_.value());
- record->stage_reached = static_cast<uint32_t>(stage_);
- record->status = static_cast<uint32_t>(status_);
- record->cache_space_free_start_mib = cache_space_free_start_mib_;
- record->cache_space_free_end_mib = cache_space_free_end_mib_;
- record->primary_bcp_compilation_millis = primary_bcp_compilation_millis_;
- record->secondary_bcp_compilation_millis = secondary_bcp_compilation_millis_;
- record->system_server_compilation_millis = system_server_compilation_millis_;
- return true;
+OdrMetricsRecord OdrMetrics::ToRecord() const {
+ return {
+ .odrefresh_metrics_version = kOdrefreshMetricsVersion,
+ .art_apex_version = art_apex_version_,
+ .trigger = static_cast<int32_t>(trigger_),
+ .stage_reached = static_cast<int32_t>(stage_),
+ .status = static_cast<int32_t>(status_),
+ .cache_space_free_start_mib = cache_space_free_start_mib_,
+ .cache_space_free_end_mib = cache_space_free_end_mib_,
+ .primary_bcp_compilation_millis = primary_bcp_compilation_millis_,
+ .secondary_bcp_compilation_millis = secondary_bcp_compilation_millis_,
+ .system_server_compilation_millis = system_server_compilation_millis_,
+ };
}
void OdrMetrics::WriteToFile(const std::string& path, const OdrMetrics* metrics) {
- OdrMetricsRecord record{};
- if (!metrics->ToRecord(&record)) {
- LOG(ERROR) << "Attempting to report metrics without a compilation trigger.";
- return;
- }
+ OdrMetricsRecord record = metrics->ToRecord();
const android::base::Result<void>& result = record.WriteToFile(path);
if (!result.ok()) {
diff --git a/odrefresh/odr_metrics.h b/odrefresh/odr_metrics.h
index afe3efd..4a29ede 100644
--- a/odrefresh/odr_metrics.h
+++ b/odrefresh/odr_metrics.h
@@ -58,6 +58,8 @@
kTimeLimitExceeded = 5,
kStagingFailed = 6,
kInstallFailed = 7,
+ // Failed to access the dalvik-cache directory due to lack of permission.
+ kDalvikCachePermissionDenied = 8,
};
// Enumeration describing the cause of compilation (if any) in odrefresh.
@@ -75,20 +77,17 @@
const std::string& metrics_file = kOdrefreshMetricsFile);
~OdrMetrics();
+ // Enables/disables metrics writing.
+ void SetEnabled(bool value) { enabled_ = value; }
+
// Gets the ART APEX that metrics are being collected on behalf of.
- int64_t GetArtApexVersion() const {
- return art_apex_version_;
- }
+ int64_t GetArtApexVersion() const { return art_apex_version_; }
// Sets the ART APEX that metrics are being collected on behalf of.
- void SetArtApexVersion(int64_t version) {
- art_apex_version_ = version;
- }
+ void SetArtApexVersion(int64_t version) { art_apex_version_ = version; }
// Gets the ART APEX last update time in milliseconds.
- int64_t GetArtApexLastUpdateMillis() const {
- return art_apex_last_update_millis_;
- }
+ int64_t GetArtApexLastUpdateMillis() const { return art_apex_last_update_millis_; }
// Sets the ART APEX last update time in milliseconds.
void SetArtApexLastUpdateMillis(int64_t last_update_millis) {
@@ -97,31 +96,27 @@
// Gets the trigger for metrics collection. The trigger is the reason why odrefresh considers
// compilation necessary.
- Trigger GetTrigger() const {
- return trigger_.has_value() ? trigger_.value() : Trigger::kUnknown;
- }
+ Trigger GetTrigger() const { return trigger_; }
// Sets the trigger for metrics collection. The trigger is the reason why odrefresh considers
// compilation necessary. Only call this method if compilation is necessary as the presence
// of a trigger means we will try to record and upload metrics.
- void SetTrigger(const Trigger trigger) {
- trigger_ = trigger;
- }
+ void SetTrigger(const Trigger trigger) { trigger_ = trigger; }
// Sets the execution status of the current odrefresh processing stage.
- void SetStatus(const Status status) {
- status_ = status;
- }
+ void SetStatus(const Status status) { status_ = status; }
// Sets the current odrefresh processing stage.
- void SetStage(Stage stage);
+ void SetStage(Stage stage) { stage_ = stage; }
// Sets the result of the current dex2oat invocation.
void SetDex2OatResult(const ExecResult& dex2oat_result);
- // Record metrics into an OdrMetricsRecord.
- // returns true on success, false if instance is not valid (because the trigger value is not set).
- bool ToRecord(/*out*/OdrMetricsRecord* record) const;
+ // Captures the current free space as the end free space.
+ void CaptureSpaceFreeEnd();
+
+ // Records metrics into an OdrMetricsRecord.
+ OdrMetricsRecord ToRecord() const;
private:
OdrMetrics(const OdrMetrics&) = delete;
@@ -135,9 +130,11 @@
const std::string cache_directory_;
const std::string metrics_file_;
+ bool enabled_ = false;
+
int64_t art_apex_version_ = 0;
int64_t art_apex_last_update_millis_ = 0;
- std::optional<Trigger> trigger_ = {}; // metrics are only logged if compilation is triggered.
+ Trigger trigger_ = Trigger::kUnknown;
Stage stage_ = Stage::kUnknown;
Status status_ = Status::kUnknown;
diff --git a/odrefresh/odr_metrics_test.cc b/odrefresh/odr_metrics_test.cc
index f222caa..7bc6f6b 100644
--- a/odrefresh/odr_metrics_test.cc
+++ b/odrefresh/odr_metrics_test.cc
@@ -15,21 +15,24 @@
*/
#include "odr_metrics.h"
-#include "base/casts.h"
-#include "odr_metrics_record.h"
#include <unistd.h>
+#include <chrono>
#include <fstream>
#include <memory>
#include <string>
+#include <thread>
-#include "android-base/result-gmock.h"
+#include "base/casts.h"
#include "base/common_art_test.h"
+#include "odr_metrics_record.h"
namespace art {
namespace odrefresh {
+using std::chrono_literals::operator""ms; // NOLINT
+
class OdrMetricsTest : public CommonArtTest {
public:
void SetUp() override {
@@ -50,14 +53,6 @@
return OS::FileExists(path);
}
- bool RemoveMetricsFile() const {
- const char* path = metrics_file_path_.c_str();
- if (OS::FileExists(path)) {
- return unlink(path) == 0;
- }
- return true;
- }
-
const std::string GetCacheDirectory() const { return cache_directory_; }
const std::string GetMetricsFilePath() const { return metrics_file_path_; }
@@ -67,59 +62,23 @@
std::string cache_directory_;
};
-TEST_F(OdrMetricsTest, ToRecordFailsIfNotTriggered) {
- {
- OdrMetrics metrics(GetCacheDirectory(), GetMetricsFilePath());
- OdrMetricsRecord record {};
- EXPECT_FALSE(metrics.ToRecord(&record));
- }
-
- {
- OdrMetrics metrics(GetCacheDirectory(), GetMetricsFilePath());
- metrics.SetArtApexVersion(99);
- metrics.SetStage(OdrMetrics::Stage::kCheck);
- metrics.SetStatus(OdrMetrics::Status::kNoSpace);
- OdrMetricsRecord record {};
- EXPECT_FALSE(metrics.ToRecord(&record));
- }
-}
-
-TEST_F(OdrMetricsTest, ToRecordSucceedsIfTriggered) {
- OdrMetrics metrics(GetCacheDirectory(), GetMetricsFilePath());
- metrics.SetArtApexVersion(99);
- metrics.SetTrigger(OdrMetrics::Trigger::kApexVersionMismatch);
- metrics.SetStage(OdrMetrics::Stage::kCheck);
- metrics.SetStatus(OdrMetrics::Status::kNoSpace);
-
- OdrMetricsRecord record{};
- EXPECT_TRUE(metrics.ToRecord(&record));
-
- EXPECT_EQ(99, record.art_apex_version);
- EXPECT_EQ(OdrMetrics::Trigger::kApexVersionMismatch,
- enum_cast<OdrMetrics::Trigger>(record.trigger));
- EXPECT_EQ(OdrMetrics::Stage::kCheck, enum_cast<OdrMetrics::Stage>(record.stage_reached));
- EXPECT_EQ(OdrMetrics::Status::kNoSpace, enum_cast<OdrMetrics::Status>(record.status));
-}
-
-TEST_F(OdrMetricsTest, MetricsFileIsNotCreatedIfNotTriggered) {
- EXPECT_TRUE(RemoveMetricsFile());
-
+TEST_F(OdrMetricsTest, MetricsFileIsNotCreatedIfNotEnabled) {
// Metrics file is (potentially) written in OdrMetrics destructor.
{
OdrMetrics metrics(GetCacheDirectory(), GetMetricsFilePath());
metrics.SetArtApexVersion(99);
+ metrics.SetTrigger(OdrMetrics::Trigger::kApexVersionMismatch);
metrics.SetStage(OdrMetrics::Stage::kCheck);
metrics.SetStatus(OdrMetrics::Status::kNoSpace);
}
EXPECT_FALSE(MetricsFileExists());
}
-TEST_F(OdrMetricsTest, NoMetricsFileIsCreatedIfTriggered) {
- EXPECT_TRUE(RemoveMetricsFile());
-
+TEST_F(OdrMetricsTest, MetricsFileIsCreatedIfEnabled) {
// Metrics file is (potentially) written in OdrMetrics destructor.
{
OdrMetrics metrics(GetCacheDirectory(), GetMetricsFilePath());
+ metrics.SetEnabled(true);
metrics.SetArtApexVersion(101);
metrics.SetTrigger(OdrMetrics::Trigger::kDexFilesChanged);
metrics.SetStage(OdrMetrics::Stage::kCheck);
@@ -128,20 +87,6 @@
EXPECT_TRUE(MetricsFileExists());
}
-TEST_F(OdrMetricsTest, StageDoesNotAdvancedAfterFailure) {
- OdrMetrics metrics(GetCacheDirectory(), GetMetricsFilePath());
- metrics.SetArtApexVersion(1999);
- metrics.SetTrigger(OdrMetrics::Trigger::kMissingArtifacts);
- metrics.SetStage(OdrMetrics::Stage::kCheck);
- metrics.SetStatus(OdrMetrics::Status::kNoSpace);
- metrics.SetStage(OdrMetrics::Stage::kComplete);
-
- OdrMetricsRecord record{};
- EXPECT_TRUE(metrics.ToRecord(&record));
-
- EXPECT_EQ(OdrMetrics::Stage::kCheck, enum_cast<OdrMetrics::Stage>(record.stage_reached));
-}
-
TEST_F(OdrMetricsTest, TimeValuesAreRecorded) {
OdrMetrics metrics(GetCacheDirectory(), GetMetricsFilePath());
metrics.SetArtApexVersion(1999);
@@ -150,68 +95,54 @@
metrics.SetStatus(OdrMetrics::Status::kOK);
// Primary boot classpath compilation time.
- OdrMetricsRecord record{};
{
metrics.SetStage(OdrMetrics::Stage::kPrimaryBootClasspath);
ScopedOdrCompilationTimer timer(metrics);
- sleep(2u);
+ std::this_thread::sleep_for(100ms);
}
- EXPECT_TRUE(metrics.ToRecord(&record));
- EXPECT_EQ(OdrMetrics::Stage::kPrimaryBootClasspath,
- enum_cast<OdrMetrics::Stage>(record.stage_reached));
- EXPECT_NE(0, record.primary_bcp_compilation_millis);
- EXPECT_GT(10'000, record.primary_bcp_compilation_millis);
- EXPECT_EQ(0, record.secondary_bcp_compilation_millis);
- EXPECT_EQ(0, record.system_server_compilation_millis);
+ OdrMetricsRecord record = metrics.ToRecord();
+ EXPECT_EQ(enum_cast<OdrMetrics::Stage>(record.stage_reached),
+ OdrMetrics::Stage::kPrimaryBootClasspath);
+ EXPECT_GT(record.primary_bcp_compilation_millis, 0);
+ EXPECT_LT(record.primary_bcp_compilation_millis, 300);
+ EXPECT_EQ(record.secondary_bcp_compilation_millis, 0);
+ EXPECT_EQ(record.system_server_compilation_millis, 0);
// Secondary boot classpath compilation time.
{
metrics.SetStage(OdrMetrics::Stage::kSecondaryBootClasspath);
ScopedOdrCompilationTimer timer(metrics);
- sleep(2u);
+ std::this_thread::sleep_for(100ms);
}
- EXPECT_TRUE(metrics.ToRecord(&record));
+ record = metrics.ToRecord();
EXPECT_EQ(OdrMetrics::Stage::kSecondaryBootClasspath,
enum_cast<OdrMetrics::Stage>(record.stage_reached));
- EXPECT_NE(0, record.primary_bcp_compilation_millis);
- EXPECT_NE(0, record.secondary_bcp_compilation_millis);
- EXPECT_GT(10'000, record.secondary_bcp_compilation_millis);
- EXPECT_EQ(0, record.system_server_compilation_millis);
+ EXPECT_GT(record.primary_bcp_compilation_millis, 0);
+ EXPECT_GT(record.secondary_bcp_compilation_millis, 0);
+ EXPECT_LT(record.secondary_bcp_compilation_millis, 300);
+ EXPECT_EQ(record.system_server_compilation_millis, 0);
// system_server classpath compilation time.
{
metrics.SetStage(OdrMetrics::Stage::kSystemServerClasspath);
ScopedOdrCompilationTimer timer(metrics);
- sleep(2u);
+ std::this_thread::sleep_for(100ms);
}
- EXPECT_TRUE(metrics.ToRecord(&record));
+ record = metrics.ToRecord();
EXPECT_EQ(OdrMetrics::Stage::kSystemServerClasspath,
enum_cast<OdrMetrics::Stage>(record.stage_reached));
- EXPECT_NE(0, record.primary_bcp_compilation_millis);
- EXPECT_NE(0, record.secondary_bcp_compilation_millis);
- EXPECT_NE(0, record.system_server_compilation_millis);
- EXPECT_GT(10'000, record.system_server_compilation_millis);
+ EXPECT_GT(record.primary_bcp_compilation_millis, 0);
+ EXPECT_GT(record.secondary_bcp_compilation_millis, 0);
+ EXPECT_GT(record.system_server_compilation_millis, 0);
+ EXPECT_LT(record.system_server_compilation_millis, 300);
}
TEST_F(OdrMetricsTest, CacheSpaceValuesAreUpdated) {
- OdrMetricsRecord snap {};
- snap.cache_space_free_start_mib = -1;
- snap.cache_space_free_end_mib = -1;
- {
- OdrMetrics metrics(GetCacheDirectory(), GetMetricsFilePath());
- metrics.SetArtApexVersion(1999);
- metrics.SetTrigger(OdrMetrics::Trigger::kMissingArtifacts);
- metrics.SetStage(OdrMetrics::Stage::kCheck);
- metrics.SetStatus(OdrMetrics::Status::kOK);
- EXPECT_TRUE(metrics.ToRecord(&snap));
- EXPECT_NE(0, snap.cache_space_free_start_mib);
- EXPECT_EQ(0, snap.cache_space_free_end_mib);
- }
-
- OdrMetricsRecord on_disk{};
- EXPECT_THAT(on_disk.ReadFromFile(GetMetricsFilePath()), android::base::testing::Ok());
- EXPECT_EQ(snap.cache_space_free_start_mib, on_disk.cache_space_free_start_mib);
- EXPECT_NE(0, on_disk.cache_space_free_end_mib);
+ OdrMetrics metrics(GetCacheDirectory(), GetMetricsFilePath());
+ metrics.CaptureSpaceFreeEnd();
+ OdrMetricsRecord record = metrics.ToRecord();
+ EXPECT_GT(record.cache_space_free_start_mib, 0);
+ EXPECT_GT(record.cache_space_free_end_mib, 0);
}
} // namespace odrefresh
diff --git a/odrefresh/odr_statslog_android.cc b/odrefresh/odr_statslog_android.cc
index ee173d4..f8ed4f8 100644
--- a/odrefresh/odr_statslog_android.cc
+++ b/odrefresh/odr_statslog_android.cc
@@ -77,6 +77,8 @@
return metrics::statsd::ODREFRESH_REPORTED__STATUS__STATUS_STAGING_FAILED;
case OdrMetrics::Status::kInstallFailed:
return metrics::statsd::ODREFRESH_REPORTED__STATUS__STATUS_INSTALL_FAILED;
+ case OdrMetrics::Status::kDalvikCachePermissionDenied:
+ return metrics::statsd::ODREFRESH_REPORTED__STATUS__STATUS_DALVIK_CACHE_PERMISSION_DENIED;
}
LOG(ERROR) << "Unknown status value: " << art_metrics_status;
diff --git a/odrefresh/odrefresh.cc b/odrefresh/odrefresh.cc
index bed694c..cf68aa2 100644
--- a/odrefresh/odrefresh.cc
+++ b/odrefresh/odrefresh.cc
@@ -1241,7 +1241,7 @@
}
Result<void> OnDeviceRefresh::CleanupArtifactDirectory(
- const std::vector<std::string>& artifacts_to_keep) const {
+ OdrMetrics& metrics, const std::vector<std::string>& artifacts_to_keep) const {
const std::string& artifact_dir = config_.GetArtifactDirectory();
std::unordered_set<std::string> artifact_set{artifacts_to_keep.begin(), artifacts_to_keep.end()};
@@ -1259,7 +1259,9 @@
// undefined behavior;
entries.push_back(entry);
}
- if (ec) {
+ if (ec && ec.value() != ENOENT) {
+ metrics.SetStatus(ec.value() == EPERM ? OdrMetrics::Status::kDalvikCachePermissionDenied :
+ OdrMetrics::Status::kIoError);
return Errorf("Failed to iterate over entries in the artifact directory: {}", ec.message());
}
@@ -1269,6 +1271,7 @@
if (!ContainsElement(artifact_set, path)) {
LOG(INFO) << "Removing " << path;
if (unlink(path.c_str()) != 0) {
+ metrics.SetStatus(OdrMetrics::Status::kIoError);
return ErrnoErrorf("Failed to remove file {}", QuotePath(path));
}
}
@@ -1276,6 +1279,7 @@
// Neither a regular file nor a directory. Unexpected file type.
LOG(INFO) << "Removing " << path;
if (unlink(path.c_str()) != 0) {
+ metrics.SetStatus(OdrMetrics::Status::kIoError);
return ErrnoErrorf("Failed to remove file {}", QuotePath(path));
}
}
@@ -1336,7 +1340,11 @@
auto cleanup_and_compile_all = [&, this]() {
compilation_options->compile_boot_classpath_for_isas = config_.GetBootClasspathIsas();
compilation_options->system_server_jars_to_compile = AllSystemServerJars();
- return RemoveArtifactsDirectory() ? ExitCode::kCompilationRequired : ExitCode::kCleanupFailed;
+ if (!RemoveArtifactsDirectory()) {
+ metrics.SetStatus(OdrMetrics::Status::kIoError);
+ return ExitCode::kCleanupFailed;
+ }
+ return ExitCode::kCompilationRequired;
};
std::optional<std::vector<apex::ApexInfo>> apex_info_list = GetApexInfoList();
@@ -1410,7 +1418,7 @@
checked_artifacts.push_back(cache_info_filename_);
}
- Result<void> result = CleanupArtifactDirectory(checked_artifacts);
+ Result<void> result = CleanupArtifactDirectory(metrics, checked_artifacts);
if (!result.ok()) {
LOG(ERROR) << result.error();
return ExitCode::kCleanupFailed;
@@ -1497,6 +1505,7 @@
args.emplace_back("--runtime-arg");
args.emplace_back(Concatenate({"-Xbootclasspath:", android::base::Join(jars_to_compile, ":")}));
if (!AddBootClasspathFds(args, readonly_files_raii, jars_to_compile)) {
+ metrics.SetStatus(OdrMetrics::Status::kIoError);
return false;
}
@@ -1654,6 +1663,7 @@
auto bcp_jars = android::base::Split(config_.GetBootClasspath(), ":");
if (!AddBootClasspathFds(args, readonly_files_raii, bcp_jars)) {
+ metrics.SetStatus(OdrMetrics::Status::kIoError);
return false;
}
std::string unused_error_msg;
@@ -1729,10 +1739,18 @@
const char* staging_dir = nullptr;
metrics.SetStage(OdrMetrics::Stage::kPreparation);
+ if (!EnsureDirectoryExists(config_.GetArtifactDirectory())) {
+ LOG(ERROR) << "Failed to prepare artifact directory";
+ metrics.SetStatus(errno == EPERM ? OdrMetrics::Status::kDalvikCachePermissionDenied :
+ OdrMetrics::Status::kIoError);
+ return ExitCode::kCleanupFailed;
+ }
+
if (config_.GetRefresh()) {
Result<void> result = RefreshExistingArtifacts();
if (!result.ok()) {
LOG(ERROR) << "Failed to refresh existing artifacts: " << result.error();
+ metrics.SetStatus(OdrMetrics::Status::kIoError);
return ExitCode::kCleanupFailed;
}
}
@@ -1741,6 +1759,7 @@
Result<void> result = WriteCacheInfo();
if (!result.ok()) {
LOG(ERROR) << result.error();
+ metrics.SetStatus(OdrMetrics::Status::kIoError);
return ExitCode::kCleanupFailed;
}
@@ -1850,6 +1869,7 @@
}
metrics.SetStage(OdrMetrics::Stage::kComplete);
+ metrics.SetStatus(OdrMetrics::Status::kOK);
return ExitCode::kCompilationSuccess;
}
diff --git a/odrefresh/odrefresh.h b/odrefresh/odrefresh.h
index e48567f..b14aa41 100644
--- a/odrefresh/odrefresh.h
+++ b/odrefresh/odrefresh.h
@@ -112,7 +112,7 @@
// Removes files that are not in the list.
android::base::Result<void> CleanupArtifactDirectory(
- const std::vector<std::string>& artifacts_to_keep) const;
+ OdrMetrics& metrics, const std::vector<std::string>& artifacts_to_keep) const;
// Loads artifacts to memory and writes them back. This is a workaround for old versions of
// odsign, which encounters "file exists" error when it adds existing artifacts to fs-verity. This
diff --git a/odrefresh/odrefresh_main.cc b/odrefresh/odrefresh_main.cc
index c85115c..09f8f56 100644
--- a/odrefresh/odrefresh_main.cc
+++ b/odrefresh/odrefresh_main.cc
@@ -274,10 +274,17 @@
CompilationOptions compilation_options;
if (action == "--check") {
// Fast determination of whether artifacts are up to date.
- return odr.CheckArtifactsAreUpToDate(metrics, &compilation_options);
+ ExitCode exit_code = odr.CheckArtifactsAreUpToDate(metrics, &compilation_options);
+ // Normally, `--check` should not write metrics. If compilation is not required, there's no need
+ // to write metrics; if compilation is required, `--compile` will write metrics. Therefore,
+ // `--check` should only write metrics when things went wrong.
+ metrics.SetEnabled(exit_code != ExitCode::kOkay && exit_code != ExitCode::kCompilationRequired);
+ return exit_code;
} else if (action == "--compile") {
- const ExitCode exit_code = odr.CheckArtifactsAreUpToDate(metrics, &compilation_options);
+ ExitCode exit_code = odr.CheckArtifactsAreUpToDate(metrics, &compilation_options);
if (exit_code != ExitCode::kCompilationRequired) {
+ // No compilation required, so only write metrics when things went wrong.
+ metrics.SetEnabled(exit_code != ExitCode::kOkay);
return exit_code;
}
OdrCompilationLog compilation_log;
@@ -285,6 +292,8 @@
LOG(INFO) << "Compilation skipped because it was attempted recently";
return ExitCode::kOkay;
}
+ // Compilation required, so always write metrics.
+ metrics.SetEnabled(true);
ExitCode compile_result = odr.Compile(metrics, compilation_options);
compilation_log.Log(metrics.GetArtApexVersion(),
metrics.GetArtApexLastUpdateMillis(),