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

1 files changed, 35 insertions, 104 deletions

diff --git a/odrefresh/odr_metrics_test.cc b/odrefresh/odr_metrics_test.cc
index f222caaf2e..7bc6f6becd 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 @@ class OdrMetricsTest : public CommonArtTest {
     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 @@ class OdrMetricsTest : public CommonArtTest {
   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 @@ TEST_F(OdrMetricsTest, NoMetricsFileIsCreatedIfTriggered) {
   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 @@ TEST_F(OdrMetricsTest, TimeValuesAreRecorded) {
   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