crash: Add --upload_old_reports flag to crash_sender.
Sometimes Jetstream products will not have a software update for more
than 6 months because of which crash_sender would skip uploading crashes
to go/crash dashboard.
Adding a flag (upload_old_reports) to crash_sender, by default this flag
is set to false, only Jetstream products this flag would be set to true.
BUG=b:153814539
TEST=Compiled and verified crash_sender doesn't check timestamp when
upload_old_reports flag is set.
Change-Id: I420e1e7c2cfaa82c45ba1fdfe601bdd06db4c433
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2321730
Reviewed-by: Miriam Zimmerman <mutexlox@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Tested-by: Raju Konduru <rkonduru@google.com>
Commit-Queue: Raju Konduru <rkonduru@google.com>
diff --git a/crash-reporter/crash_sender.cc b/crash-reporter/crash_sender.cc
index 318ff8f..6d35d6a 100644
--- a/crash-reporter/crash_sender.cc
+++ b/crash-reporter/crash_sender.cc
@@ -98,10 +98,12 @@
return EXIT_FAILURE;
}
- if (util::IsOsTimestampTooOldForUploads(util::GetOsTimestamp(),
- clock.get())) {
- LOG(INFO) << "Version is too old, will not upload crash reports";
- return EXIT_FAILURE;
+ if (!flags.upload_old_reports) {
+ if (util::IsOsTimestampTooOldForUploads(util::GetOsTimestamp(),
+ clock.get())) {
+ LOG(INFO) << "Version is too old, will not upload crash reports";
+ return EXIT_FAILURE;
+ }
}
}
diff --git a/crash-reporter/crash_sender_util.cc b/crash-reporter/crash_sender_util.cc
index 71d0df5..c63384e 100644
--- a/crash-reporter/crash_sender_util.cc
+++ b/crash-reporter/crash_sender_util.cc
@@ -182,6 +182,8 @@
DEFINE_bool(ignore_test_image, false,
"Upload crashes to the crash server even if running on a test "
"image, but do NOT ignore official image check.");
+ DEFINE_bool(upload_old_reports, false,
+ "If set, ignore the timestamp check and upload older reports.");
brillo::FlagHelper::Init(argc, argv, "Chromium OS Crash Sender");
if (FLAGS_max_spread_time < 0) {
LOG(ERROR) << "Invalid value for max spread time: "
@@ -197,6 +199,7 @@
flags->test_mode = FLAGS_test_mode;
flags->delete_crashes = FLAGS_delete_crashes;
flags->ignore_test_image = FLAGS_ignore_test_image;
+ flags->upload_old_reports = FLAGS_upload_old_reports;
if (flags->test_mode) {
// The pause file is intended to pause the cronjob crash_sender during
// tests, not the crash_sender invoked by the test code.
@@ -589,6 +592,7 @@
allow_dev_sending_(options.allow_dev_sending),
test_mode_(options.test_mode),
delete_crashes_(options.delete_crashes),
+ upload_old_reports_(options.upload_old_reports),
clock_(std::move(clock)) {}
bool Sender::Init() {
@@ -737,12 +741,13 @@
return kRemove;
}
- // If we have an OS timestamp in the metadata and it's too old to upload then
- // remove the report. We wouldn't have gotten here if the current OS version
- // is too old, so this is an old report from before an OS update.
+ // If we have an OS timestamp in the metadata and it's too old to upload and
+ // upload_old_reports flag is not set then remove the report. We wouldn't have
+ // gotten here if the current OS version is too old, so this is an old report
+ // from before an OS update.
std::string os_timestamp_str;
int64_t os_millis;
- if (!allow_dev_sending_ && !test_mode_ &&
+ if (!allow_dev_sending_ && !test_mode_ && !upload_old_reports_ &&
info->metadata.GetString(kOsTimestamp, &os_timestamp_str) &&
base::StringToInt64(os_timestamp_str, &os_millis) &&
util::IsOsTimestampTooOldForUploads(
diff --git a/crash-reporter/crash_sender_util.h b/crash-reporter/crash_sender_util.h
index 7c8aaae..0f330c3 100644
--- a/crash-reporter/crash_sender_util.h
+++ b/crash-reporter/crash_sender_util.h
@@ -54,6 +54,7 @@
bool test_mode = false;
bool delete_crashes = true;
bool ignore_test_image = false;
+ bool upload_old_reports = false;
};
// Crash information obtained in ChooseAction().
@@ -245,6 +246,9 @@
// Else, create a new file in the spool directory with the same basename
// and a special extension to indicate that the crash was already uploaded.
bool delete_crashes = true;
+
+ // If true, ignore timestamp check and upload old reports.
+ bool upload_old_reports = false;
};
Sender(std::unique_ptr<MetricsLibraryInterface> metrics_lib,
@@ -349,6 +353,7 @@
bool allow_dev_sending_;
const bool test_mode_;
const bool delete_crashes_;
+ const bool upload_old_reports_;
std::unique_ptr<base::Clock> clock_;
scoped_refptr<dbus::Bus> bus_;
std::unique_ptr<brillo::OsReleaseReader> os_release_reader_;
diff --git a/crash-reporter/crash_sender_util_test.cc b/crash-reporter/crash_sender_util_test.cc
index b2fe952..0993d3b 100644
--- a/crash-reporter/crash_sender_util_test.cc
+++ b/crash-reporter/crash_sender_util_test.cc
@@ -507,6 +507,7 @@
EXPECT_FALSE(flags.ignore_hold_off_time);
EXPECT_FALSE(flags.allow_dev_sending);
EXPECT_FALSE(flags.ignore_pause_file);
+ EXPECT_FALSE(flags.upload_old_reports);
}
TEST_F(CrashSenderUtilDeathTest, ParseCommandLine_InvalidMaxSpreadTime) {
@@ -532,6 +533,7 @@
EXPECT_FALSE(flags.ignore_hold_off_time);
EXPECT_FALSE(flags.allow_dev_sending);
EXPECT_FALSE(flags.ignore_pause_file);
+ EXPECT_FALSE(flags.upload_old_reports);
}
TEST_F(CrashSenderUtilTest, ParseCommandLine_IgnoreRateLimits) {
@@ -547,6 +549,7 @@
EXPECT_FALSE(flags.ignore_hold_off_time);
EXPECT_FALSE(flags.allow_dev_sending);
EXPECT_FALSE(flags.ignore_pause_file);
+ EXPECT_FALSE(flags.upload_old_reports);
}
TEST_F(CrashSenderUtilTest, ParseCommandLine_IgnoreHoldOffTime) {
@@ -562,6 +565,7 @@
EXPECT_TRUE(flags.ignore_hold_off_time);
EXPECT_FALSE(flags.allow_dev_sending);
EXPECT_FALSE(flags.ignore_pause_file);
+ EXPECT_FALSE(flags.upload_old_reports);
}
TEST_F(CrashSenderUtilTest, ParseCommandLine_CrashDirectory) {
@@ -577,6 +581,7 @@
EXPECT_FALSE(flags.ignore_hold_off_time);
EXPECT_FALSE(flags.allow_dev_sending);
EXPECT_FALSE(flags.ignore_pause_file);
+ EXPECT_FALSE(flags.upload_old_reports);
}
TEST_F(CrashSenderUtilTest, ParseCommandLine_Dev) {
@@ -592,6 +597,7 @@
EXPECT_FALSE(flags.ignore_hold_off_time);
EXPECT_TRUE(flags.allow_dev_sending);
EXPECT_FALSE(flags.ignore_pause_file);
+ EXPECT_FALSE(flags.upload_old_reports);
}
TEST_F(CrashSenderUtilTest, ParseCommandLine_IgnorePauseFile) {
@@ -607,6 +613,23 @@
EXPECT_FALSE(flags.ignore_hold_off_time);
EXPECT_FALSE(flags.allow_dev_sending);
EXPECT_TRUE(flags.ignore_pause_file);
+ EXPECT_FALSE(flags.upload_old_reports);
+}
+
+TEST_F(CrashSenderUtilTest, ParseCommandLine_UploadOldReports) {
+ const char* argv[] = {"crash_sender", "--upload_old_reports"};
+ base::CommandLine command_line(base::size(argv), argv);
+ brillo::FlagHelper::GetInstance()->set_command_line_for_testing(
+ &command_line);
+ CommandLineFlags flags;
+ ParseCommandLine(base::size(argv), argv, &flags);
+ EXPECT_EQ(flags.max_spread_time.InSeconds(), kMaxSpreadTimeInSeconds);
+ EXPECT_TRUE(flags.crash_directory.empty());
+ EXPECT_FALSE(flags.ignore_rate_limits);
+ EXPECT_FALSE(flags.ignore_hold_off_time);
+ EXPECT_FALSE(flags.allow_dev_sending);
+ EXPECT_FALSE(flags.ignore_pause_file);
+ EXPECT_TRUE(flags.upload_old_reports);
}
TEST_F(CrashSenderUtilTest, IsMock) {
@@ -868,6 +891,26 @@
EXPECT_EQ(Sender::kSend, sender.ChooseAction(old_os_meta_, &reason, &info));
}
+TEST_F(CrashSenderUtilTest, ChooseActionUploadOldReports) {
+ ASSERT_TRUE(SetConditions(kOfficialBuild, kSignInMode, kMetricsEnabled));
+
+ const base::FilePath crash_directory =
+ paths::Get(paths::kSystemCrashDirectory);
+ ASSERT_TRUE(CreateDirectory(crash_directory));
+ ASSERT_TRUE(CreateTestCrashFiles(crash_directory));
+
+ Sender::Options options;
+ options.upload_old_reports = true;
+ Sender sender(std::move(metrics_lib_),
+ std::make_unique<test_util::AdvancingClock>(), options);
+ ASSERT_TRUE(sender.Init());
+
+ std::string reason;
+ CrashInfo info;
+
+ EXPECT_EQ(Sender::kSend, sender.ChooseAction(old_os_meta_, &reason, &info));
+}
+
TEST_F(CrashSenderUtilTest, RemoveAndPickCrashFiles) {
auto mock =
std::make_unique<org::chromium::SessionManagerInterfaceProxyMock>();
