crash: Remove crash_sender timing logic.
Per the linked bug, the metrics timing crash_sender's runtime will
expire in a few weeks. We no longer need the metrics, so remove the code
computing them (in preparation for removing the metrics altogether).
BUG=chromium:1156196
TEST=CQ
Change-Id: Ic804c363167851d931ce9ba9781fb74a05c82574
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2605214
Tested-by: Miriam Zimmerman <mutexlox@chromium.org>
Reviewed-by: Ian Barkley-Yeung <iby@chromium.org>
Commit-Queue: Miriam Zimmerman <mutexlox@chromium.org>
diff --git a/crash-reporter/crash_sender.cc b/crash-reporter/crash_sender.cc
index e224553..a83b6f8 100644
--- a/crash-reporter/crash_sender.cc
+++ b/crash-reporter/crash_sender.cc
@@ -31,9 +31,6 @@
namespace {
-const char kSenderRuntimeName[] = "CrashReport.Sender.Runtime";
-const char kSenderActiveRuntimeName[] = "CrashReport.Sender.ActiveRuntime";
-
// Sets up the minijail sandbox.
//
// crash_sender currently needs to run as root:
@@ -70,12 +67,6 @@
util::CommandLineFlags flags;
util::ParseCommandLine(argc, argv, &flags);
- base::Time start;
- if (flags.max_spread_time.is_zero()) {
- // We want a monotonic time, so use SystemTime
- start = base::Time::NowFromSystemTime();
- }
-
if (util::DoesPauseFileExist() && !flags.ignore_pause_file) {
LOG(INFO) << "Exiting early due to " << paths::kPauseCrashSending;
return EXIT_FAILURE;
@@ -142,18 +133,7 @@
lock_file.Close();
util::SortReports(&reports_to_send);
- base::TimeDelta sleeping_time;
- sender.SendCrashes(reports_to_send, &sleeping_time);
-
- if (flags.max_spread_time.is_zero()) {
- base::TimeDelta runtime = base::Time::NowFromSystemTime() - start;
- MetricsLibrary metrics_lib;
- metrics_lib.SendToUMA(kSenderRuntimeName, runtime.InMilliseconds(),
- /*min=*/100, /*max=*/120'000, /*nbuckets=*/50);
- base::TimeDelta active = runtime - sleeping_time;
- metrics_lib.SendToUMA(kSenderActiveRuntimeName, active.InMilliseconds(),
- /*min=*/100, /*max=*/120'000, /*nbuckets=*/50);
- }
+ sender.SendCrashes(reports_to_send);
return EXIT_SUCCESS;
}
diff --git a/crash-reporter/crash_sender_fuzzer.cc b/crash-reporter/crash_sender_fuzzer.cc
index 5067900..44b96b3 100644
--- a/crash-reporter/crash_sender_fuzzer.cc
+++ b/crash-reporter/crash_sender_fuzzer.cc
@@ -121,8 +121,7 @@
sender.RemoveAndPickCrashFiles(test_dir, &reports_to_send);
util::SortReports(&reports_to_send);
- base::TimeDelta sleep;
- sender.SendCrashes(reports_to_send, &sleep);
+ sender.SendCrashes(reports_to_send);
return 0;
}
diff --git a/crash-reporter/crash_sender_util.cc b/crash-reporter/crash_sender_util.cc
index 5b39ad1..1ffa033 100644
--- a/crash-reporter/crash_sender_util.cc
+++ b/crash-reporter/crash_sender_util.cc
@@ -438,17 +438,12 @@
}
}
-void Sender::SendCrashes(const std::vector<MetaFile>& crash_meta_files,
- base::TimeDelta* total_sleep_time) {
+void Sender::SendCrashes(const std::vector<MetaFile>& crash_meta_files) {
if (crash_meta_files.empty())
return;
std::string client_id = GetClientId();
- if (total_sleep_time) {
- *total_sleep_time = base::TimeDelta();
- }
-
base::File lock(AcquireLockFileOrDie());
for (const auto& pair : crash_meta_files) {
const base::FilePath& meta_file = pair.first;
@@ -469,9 +464,7 @@
} else if (!sleep_function_.is_null()) {
sleep_function_.Run(sleep_time);
}
- if (total_sleep_time) {
- *total_sleep_time += sleep_time;
- }
+
lock = AcquireLockFileOrDie();
{
diff --git a/crash-reporter/crash_sender_util.h b/crash-reporter/crash_sender_util.h
index 1504f23..875ed5a 100644
--- a/crash-reporter/crash_sender_util.h
+++ b/crash-reporter/crash_sender_util.h
@@ -180,9 +180,7 @@
// - Checks if the device enters guest mode, and stops if entered.
// - Enforces the rate limit per 24 hours.
// - Removes crash files that are successfully uploaded.
- // |total_sleep_time|: if nonnull, set to the total amount of time sleeping.
- void SendCrashes(const std::vector<MetaFile>& crash_meta_files,
- base::TimeDelta* total_sleep_time);
+ void SendCrashes(const std::vector<MetaFile>& crash_meta_files);
// Given the |details| for a crash, creates a brillo::http::FormData object
// which will have all of the fields for submission to the crash server
diff --git a/crash-reporter/crash_sender_util_test.cc b/crash-reporter/crash_sender_util_test.cc
index a5ca252..f843bb6 100644
--- a/crash-reporter/crash_sender_util_test.cc
+++ b/crash-reporter/crash_sender_util_test.cc
@@ -1609,8 +1609,8 @@
SendEnumToUMA("Platform.CrOS.CrashSenderRemoveReason",
Sender::kFinishedUploading, Sender::kSendReasonCount))
.Times(0);
- base::TimeDelta total_sleep_time;
- sender.SendCrashes(crashes_to_send, &total_sleep_time);
+
+ sender.SendCrashes(crashes_to_send);
testing::Mock::VerifyAndClearExpectations(raw_metrics_lib);
// We shouldn't be processing any crashes still.
@@ -1622,9 +1622,6 @@
// doing uploads.
EXPECT_FALSE(base::PathExists(paths::Get(paths::kChromeCrashLog)));
EXPECT_EQ(1, sleep_times.size());
- EXPECT_EQ(total_sleep_time,
- std::accumulate(sleep_times.begin(), sleep_times.end(),
- base::TimeDelta()));
sleep_times.clear();
// Exit from guest mode/re-enable metrics, and send crashes again.
@@ -1640,7 +1637,7 @@
SendEnumToUMA("Platform.CrOS.CrashSenderRemoveReason",
Sender::kTotalRemoval, Sender::kSendReasonCount))
.Times(2);
- sender.SendCrashes(crashes_to_send, &total_sleep_time);
+ sender.SendCrashes(crashes_to_send);
// We shouldn't be processing any crashes still.
EXPECT_FALSE(base::PathExists(system_processing));
@@ -1657,9 +1654,6 @@
// crash rate.
ASSERT_EQ(2, rows.size());
EXPECT_EQ(3, sleep_times.size());
- EXPECT_EQ(total_sleep_time,
- std::accumulate(sleep_times.begin(), sleep_times.end(),
- base::TimeDelta()));
// Each line of the uploads.log file is "{"upload_time":<value>,"upload_id":
// <value>,"local_id":<value>,"capture_time":<value>,"state":<value>,"source":
@@ -1739,7 +1733,7 @@
Sender sender(std::move(metrics_lib_),
std::make_unique<test_util::AdvancingClock>(), options);
- sender.SendCrashes(crashes_to_send, nullptr);
+ sender.SendCrashes(crashes_to_send);
// We shouldn't be processing the crash still -- sending failed, but didn't
// crash.
@@ -1795,8 +1789,7 @@
ASSERT_TRUE(SetMockCrashSending(true));
sender.SetCrashDuringSendForTesting(true);
- EXPECT_DEATH(sender.SendCrashes(crashes_to_send, nullptr),
- "crashing as requested");
+ EXPECT_DEATH(sender.SendCrashes(crashes_to_send), "crashing as requested");
// We crashed, so the ".processing" file should still exist.
EXPECT_TRUE(base::PathExists(system_processing));
