[crash] added new Inspect metrics

* Added two new metrics for reports:
    1) Number of upload attempts
    2) The final state of the report
* The final state of the report is handled by the database.
* More metrics will be coming soon!

TESTED=`fx run-test crashpad_agent_tests`

Change-Id: Iee140487df481ab5d06f80e7d77d5ea280308f2e

src/developer/feedback/crashpad_agent/database.cc

@@ -23,7 +23,8 @@ using OperationStatus = crashpad::CrashReportDatabase::OperationStatus;
 
 constexpr char kCrashpadDatabasePath[] = "/tmp/crashes";
 
-std::unique_ptr<Database> Database::TryCreate(CrashpadDatabaseConfig config) {
+std::unique_ptr<Database> Database::TryCreate(CrashpadDatabaseConfig config,
+                                              InspectManager* inspect_manager) {
   if (!files::IsDirectory(kCrashpadDatabasePath)) {
     files::CreateDirectory(kCrashpadDatabasePath);
   }
@@ -36,12 +37,14 @@ std::unique_ptr<Database> Database::TryCreate(CrashpadDatabaseConfig config) {
     return nullptr;
   }
 
-  return std::unique_ptr<Database>(new Database(config, std::move(crashpad_database)));
+  return std::unique_ptr<Database>(
+      new Database(config, std::move(crashpad_database), inspect_manager));
 }
 
 Database::Database(CrashpadDatabaseConfig config,
-                   std::unique_ptr<crashpad::CrashReportDatabase> database)
-    : config_(config), database_(std::move(database)) {
+                   std::unique_ptr<crashpad::CrashReportDatabase> database,
+                   InspectManager* inspect_manager)
+    : config_(config), database_(std::move(database)), inspect_manager_(inspect_manager) {
   FXL_DCHECK(database_);
 }
 
@@ -110,6 +113,8 @@ bool Database::MarkAsUploaded(std::unique_ptr<UploadReport> upload_report,
   }
 
   const UUID local_report_id = upload_report->GetUUID();
+
+  inspect_manager_->MarkReportAsUploaded(local_report_id.ToString(), server_report_id);
   if (const auto status =
           database_->RecordUploadComplete(upload_report->TransferUploadReport(), server_report_id);
       status != OperationStatus::kNoError) {
@@ -127,6 +132,8 @@ bool Database::Archive(const crashpad::UUID& local_report_id) {
   FX_LOGS(INFO) << fxl::StringPrintf("Archiving local crash report, ID %s, under %s",
                                      local_report_id.ToString().c_str(), kCrashpadDatabasePath);
 
+  inspect_manager_->MarkReportAsArchived(local_report_id.ToString());
+
   if (const auto status =
           database_->SkipReportUpload(local_report_id, CrashSkippedReason::kUploadFailed);
       status != OperationStatus::kNoError) {
@@ -173,6 +180,7 @@ size_t Database::GarbageCollect() {
     }
 
     for (const auto& uuid : clean_up) {
+      inspect_manager_->MarkReportAsGarbageCollected(uuid.ToString());
       CleanUp(uuid);
     }
   }

src/developer/feedback/crashpad_agent/database.h

@@ -12,6 +12,7 @@
 #include <unordered_map>
 
 #include "src/developer/feedback/crashpad_agent/config.h"
+#include "src/developer/feedback/crashpad_agent/inspect_manager.h"
 #include "src/developer/feedback/crashpad_agent/upload_report.h"
 #include "src/lib/fxl/macros.h"
 #include "third_party/crashpad/client/crash_report_database.h"
@@ -22,7 +23,8 @@ namespace feedback {
 // Wrapper around the Crashpad database that also stores annotations.
 class Database {
  public:
-  static std::unique_ptr<Database> TryCreate(CrashpadDatabaseConfig config);
+  static std::unique_ptr<Database> TryCreate(CrashpadDatabaseConfig config,
+                                             InspectManager* inspect_manager);
 
   // Make a new report in |database_|.
   //
@@ -72,13 +74,15 @@ class Database {
     bool has_minidump;
   };
 
-  Database(CrashpadDatabaseConfig config, std::unique_ptr<crashpad::CrashReportDatabase> database);
+  Database(CrashpadDatabaseConfig config, std::unique_ptr<crashpad::CrashReportDatabase> database,
+           InspectManager* inspect_manager);
 
   // Removes |local_report_id| from |additional_data_|.
   void CleanUp(const crashpad::UUID& local_report_id);
 
   const CrashpadDatabaseConfig config_;
   std::unique_ptr<crashpad::CrashReportDatabase> database_;
+  InspectManager* inspect_manager_;
   std::unordered_map<crashpad::UUID, AdditionalData, UUIDHasher> additional_data_;
 
   FXL_DISALLOW_COPY_AND_ASSIGN(Database);

src/developer/feedback/crashpad_agent/inspect_manager.cc

@@ -51,6 +51,23 @@ bool InspectManager::AddReport(const std::string& program_name,
   return true;
 }
 
+bool InspectManager::IncrementUploadAttempt(const std::string& local_report_id) {
+  if (!Contains(local_report_id)) {
+    FX_LOGS(ERROR) << "Failed to find local crash report, ID " << local_report_id;
+    return false;
+  }
+
+  Report& report = reports_.at(local_report_id);
+
+  if (!report.upload_attempts_) {
+    report.upload_attempts_ = node_manager_.Get(report.Path())->CreateUint("upload_attempts", 1u);
+  } else {
+    report.upload_attempts_.Add(1u);
+  }
+
+  return true;
+}
+
 bool InspectManager::MarkReportAsUploaded(const std::string& local_report_id,
                                           const std::string& server_properties_report_id) {
   if (!Contains(local_report_id)) {
@@ -59,6 +76,8 @@ bool InspectManager::MarkReportAsUploaded(const std::string& local_report_id,
   }
 
   Report& report = reports_.at(local_report_id);
+  report.final_state_ = node_manager_.Get(report.Path())->CreateString("final_state", "uploaded");
+
   const std::string server_path = JoinPath(report.Path(), "crash_server");
 
   inspect::Node* server = node_manager_.Get(server_path);
@@ -69,6 +88,31 @@ bool InspectManager::MarkReportAsUploaded(const std::string& local_report_id,
   return true;
 }
 
+bool InspectManager::MarkReportAsArchived(const std::string& local_report_id) {
+  if (!Contains(local_report_id)) {
+    FX_LOGS(ERROR) << "Failed to find local crash report, ID " << local_report_id;
+    return false;
+  }
+
+  Report& report = reports_.at(local_report_id);
+  report.final_state_ = node_manager_.Get(report.Path())->CreateString("final_state", "archived");
+
+  return true;
+}
+
+bool InspectManager::MarkReportAsGarbageCollected(const std::string& local_report_id) {
+  if (!Contains(local_report_id)) {
+    FX_LOGS(ERROR) << "Failed to find local crash report, ID " << local_report_id;
+    return false;
+  }
+
+  Report& report = reports_.at(local_report_id);
+  report.final_state_ =
+      node_manager_.Get(report.Path())->CreateString("final_state", "garbage_collected");
+
+  return true;
+}
+
 void InspectManager::ExposeConfig(const feedback::Config& config) {
   auto* crashpad_database = &config_.crashpad_database;
   auto* crash_server = &config_.crash_server;

src/developer/feedback/crashpad_agent/inspect_manager.h

@@ -36,12 +36,27 @@ class InspectManager {
   // or another).
   bool AddReport(const std::string& program_name, const std::string& local_report_id);
 
+  // Increments the number of upload attempts for an existing report.
+  //
+  // Returns false if there are no reports with |local_report_id| as ID.
+  bool IncrementUploadAttempt(const std::string& local_report_id);
+
   // Marks an existing report as uploaded, storing its server report ID.
   //
   // Returns false if there are no reports with |local_report_id| as ID.
   bool MarkReportAsUploaded(const std::string& local_report_id,
                             const std::string& server_report_id);
 
+  // Mark an existing report as archived.
+  //
+  // Returns false if there are no reports with |local_report_id| as ID.
+  bool MarkReportAsArchived(const std::string& local_report_id);
+
+  // Mark an existing report as garbage collected.
+  //
+  // Returns false if there are no report with |local_report_id| as ID.
+  bool MarkReportAsGarbageCollected(const std::string& local_report_id);
+
  private:
   bool Contains(const std::string& local_report_id);
 
@@ -81,6 +96,8 @@ class InspectManager {
     const std::string& Path() { return path_; }
 
     inspect::StringProperty creation_time_;
+    inspect::UintProperty upload_attempts_;
+    inspect::StringProperty final_state_;
 
     inspect::StringProperty server_id_;
     inspect::StringProperty server_creation_time_;

src/developer/feedback/crashpad_agent/queue.cc

@@ -23,7 +23,7 @@ std::unique_ptr<Queue> Queue::TryCreate(async_dispatcher_t* dispatcher,
                                         CrashpadDatabaseConfig database_config,
                                         CrashServer* crash_server,
                                         InspectManager* inspect_manager) {
-  auto database = Database::TryCreate(database_config);
+  auto database = Database::TryCreate(database_config, inspect_manager);
   if (!database) {
     return nullptr;
   }
@@ -104,13 +104,14 @@ bool Queue::Upload(const UUID& local_report_id) {
     return true;
   }
 
+  inspect_manager_->IncrementUploadAttempt(local_report_id.ToString());
+
   std::string server_report_id;
   if (crash_server_->MakeRequest(report->GetAnnotations(), report->GetAttachments(),
                                  &server_report_id)) {
     FX_LOGS(INFO) << "Successfully uploaded crash report at https://crash.corp.google.com/"
                   << server_report_id;
     database_->MarkAsUploaded(std::move(report), server_report_id);
-    inspect_manager_->MarkReportAsUploaded(local_report_id.ToString(), server_report_id);
     return true;
   }
 

src/developer/feedback/crashpad_agent/tests/BUILD.gn

@@ -224,6 +224,8 @@ executable("database_unittest") {
   ]
 
   deps = [
+    "//garnet/public/lib/timekeeper:testing",
+    "//sdk/lib/inspect/testing/cpp",
     "//src/developer/feedback/crashpad_agent:src",
     "//src/lib/fsl:fsl",
     "//src/lib/fxl/test:test_settings",

src/developer/feedback/crashpad_agent/tests/crashpad_agent_unittest.cc

@@ -782,7 +782,11 @@ TEST_F(CrashpadAgentTest, Check_InspectTreeAfterSuccessfulUpload) {
           ChildrenMatch(ElementsAre(AllOf(
               NodeMatches(NameMatches(kProgramName)),
               ChildrenMatch(ElementsAre(AllOf(
-                  NodeMatches(PropertyList(ElementsAre(StringIs("creation_time", Not(IsEmpty()))))),
+                  NodeMatches(PropertyList(UnorderedElementsAreArray({
+                      StringIs("creation_time", Not(IsEmpty())),
+                      StringIs("final_state", "uploaded"),
+                      UintIs("upload_attempts", 1u),
+                  }))),
                   ChildrenMatch(ElementsAre(NodeMatches(AllOf(
                       NameMatches("crash_server"), PropertyList(UnorderedElementsAreArray({
                                                        StringIs("creation_time", Not(IsEmpty())),