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(),