crash-reporter: Use chromeos::KeyValueStore.

Make the crash reporter use libchromeos's KeyValueStore
class to read its log-collection config file and
/etc/lsb-release instead of its own parsing code.

Also update the log-collection config file to split long
commands across multiple lines and to use '=' instead of ':'
to separate executable names from commands.

BUG=chromium:452520
TEST=updated tests; also triggered powerd and chrome crashes
     and checked that logs were attached

Change-Id: I4e2447712869608f32a4ae38f5d5cb9c6046af14
Reviewed-on: https://chromium-review.googlesource.com/244121
Reviewed-by: Dan Erat <derat@chromium.org>
Commit-Queue: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
This commit is contained in:
Daniel Erat 2015-01-28 09:48:10 -07:00 committed by ChromeOS Commit Bot
parent d257ea1cbe
commit 731da3379b
7 changed files with 141 additions and 162 deletions

View file

@ -103,6 +103,7 @@
'chrome_collector_test.cc',
'crash_collector_test.cc',
'crash_collector_test.h',
'crash_reporter_logs_test.cc',
'kernel_collector_test.cc',
'kernel_collector_test.h',
'testrunner.cc',

View file

@ -30,19 +30,26 @@
#include <chromeos/cryptohome.h>
#include <chromeos/dbus/dbus.h>
#include <chromeos/dbus/service_constants.h>
#include <chromeos/key_value_store.h>
#include <chromeos/process.h>
static const char kCollectChromeFile[] =
namespace {
const char kCollectChromeFile[] =
"/mnt/stateful_partition/etc/collect_chrome_crashes";
static const char kCrashTestInProgressPath[] = "/tmp/crash-test-in-progress";
static const char kDefaultLogConfig[] = "/etc/crash_reporter_logs.conf";
static const char kDefaultUserName[] = "chronos";
static const char kLeaveCoreFile[] = "/root/.leave_core";
static const char kLsbRelease[] = "/etc/lsb-release";
static const char kShellPath[] = "/bin/sh";
static const char kSystemCrashPath[] = "/var/spool/crash";
static const char kUploadVarPrefix[] = "upload_var_";
static const char kUploadFilePrefix[] = "upload_file_";
const char kCrashTestInProgressPath[] = "/tmp/crash-test-in-progress";
const char kDefaultLogConfig[] = "/etc/crash_reporter_logs.conf";
const char kDefaultUserName[] = "chronos";
const char kLeaveCoreFile[] = "/root/.leave_core";
const char kLsbRelease[] = "/etc/lsb-release";
const char kShellPath[] = "/bin/sh";
const char kSystemCrashPath[] = "/var/spool/crash";
const char kUploadVarPrefix[] = "upload_var_";
const char kUploadFilePrefix[] = "upload_file_";
// Key of the lsb-release entry containing the OS version.
const char kLsbVersionKey[] = "CHROMEOS_RELEASE_VERSION";
// Normally this path is not used. Unfortunately, there are a few edge cases
// where we need this. Any process that runs as kDefaultUserName that crashes
// is consider a "user crash". That includes the initial Chrome browser that
@ -53,16 +60,18 @@ static const char kUploadFilePrefix[] = "upload_file_";
// This also comes up when running autotests. The GUI is sitting at the login
// screen while tests are sshing in, changing users, and triggering crashes as
// the user (purposefully).
static const char kFallbackUserCrashPath[] = "/home/chronos/crash";
const char kFallbackUserCrashPath[] = "/home/chronos/crash";
// Directory mode of the user crash spool directory.
static const mode_t kUserCrashPathMode = 0755;
const mode_t kUserCrashPathMode = 0755;
// Directory mode of the system crash spool directory.
static const mode_t kSystemCrashPathMode = 01755;
const mode_t kSystemCrashPathMode = 01755;
static const uid_t kRootOwner = 0;
static const uid_t kRootGroup = 0;
const uid_t kRootOwner = 0;
const uid_t kRootGroup = 0;
} // namespace
// Maximum crash reports per crash spool directory. Note that this is
// a separate maximum from the maximum rate at which we upload these
@ -416,65 +425,28 @@ bool CrashCollector::CheckHasCapacity(const FilePath &crash_directory) {
return !full;
}
bool CrashCollector::IsCommentLine(const std::string &line) {
size_t found = line.find_first_not_of(" ");
return found != std::string::npos && line[found] == '#';
}
bool CrashCollector::ReadKeyValueFile(
const FilePath &path,
const char separator,
std::map<std::string, std::string> *dictionary) {
std::string contents;
if (!base::ReadFileToString(path, &contents)) {
return false;
}
typedef std::vector<std::string> StringVector;
StringVector lines;
base::SplitString(contents, '\n', &lines);
bool any_errors = false;
for (StringVector::iterator line = lines.begin(); line != lines.end();
++line) {
// Allow empty strings.
if (line->empty())
continue;
// Allow comment lines.
if (IsCommentLine(*line))
continue;
StringVector sides;
base::SplitString(*line, separator, &sides);
if (sides.size() != 2) {
any_errors = true;
continue;
}
dictionary->insert(std::pair<std::string, std::string>(sides[0], sides[1]));
}
return !any_errors;
}
bool CrashCollector::GetLogContents(const FilePath &config_path,
const std::string &exec_name,
const FilePath &output_file) {
std::map<std::string, std::string> log_commands;
if (!ReadKeyValueFile(config_path, ':', &log_commands)) {
chromeos::KeyValueStore store;
if (!store.Load(config_path)) {
LOG(INFO) << "Unable to read log configuration file "
<< config_path.value();
return false;
}
if (log_commands.find(exec_name) == log_commands.end())
std::string command;
if (!store.GetString(exec_name, &command))
return false;
chromeos::ProcessImpl diag_process;
diag_process.AddArg(kShellPath);
std::string shell_command = log_commands[exec_name];
diag_process.AddStringOption("-c", shell_command);
diag_process.AddStringOption("-c", command);
diag_process.RedirectOutput(output_file.value());
int result = diag_process.Run();
const int result = diag_process.Run();
if (result != 0) {
LOG(INFO) << "Running shell command " << shell_command << "failed with: "
<< result;
LOG(INFO) << "Log command \"" << command << "\" exited with " << result;
return false;
}
return true;
@ -500,15 +472,16 @@ void CrashCollector::AddCrashMetaUploadData(const std::string &key,
void CrashCollector::WriteCrashMetaData(const FilePath &meta_path,
const std::string &exec_name,
const std::string &payload_path) {
std::map<std::string, std::string> contents;
if (!ReadKeyValueFile(FilePath(lsb_release_), '=', &contents)) {
chromeos::KeyValueStore store;
if (!store.Load(FilePath(lsb_release_))) {
LOG(ERROR) << "Problem parsing " << lsb_release_;
// Even though there was some failure, take as much as we could read.
}
std::string version("unknown");
std::map<std::string, std::string>::iterator i;
if ((i = contents.find("CHROMEOS_RELEASE_VERSION")) != contents.end()) {
version = i->second;
if (!store.GetString(kLsbVersionKey, &version)) {
LOG(ERROR) << "Unable to read " << kLsbVersionKey << " from "
<< lsb_release_;
}
int64_t payload_size = -1;
base::GetFileSize(FilePath(payload_path), &payload_size);

View file

@ -43,10 +43,8 @@ class CrashCollector {
FRIEND_TEST(CrashCollectorTest, ForkExecAndPipe);
FRIEND_TEST(CrashCollectorTest, FormatDumpBasename);
FRIEND_TEST(CrashCollectorTest, Initialize);
FRIEND_TEST(CrashCollectorTest, IsCommentLine);
FRIEND_TEST(CrashCollectorTest, IsUserSpecificDirectoryEnabled);
FRIEND_TEST(CrashCollectorTest, MetaData);
FRIEND_TEST(CrashCollectorTest, ReadKeyValueFile);
FRIEND_TEST(CrashCollectorTest, Sanitize);
FRIEND_TEST(CrashCollectorTest, WriteNewFile);
FRIEND_TEST(ForkExecAndPipeTest, Basic);
@ -120,15 +118,6 @@ class CrashCollector {
// crash.
bool CheckHasCapacity(const base::FilePath &crash_directory);
// Checks if the line starts with '#' after optional whitespace.
static bool IsCommentLine(const std::string &line);
// Read the given file of form [<key><separator><value>\n...] and return
// a map of its contents.
bool ReadKeyValueFile(const base::FilePath &file,
char separator,
std::map<std::string, std::string> *dictionary);
// Write a log applicable to |exec_name| to |output_file| based on the
// log configuration file at |config_path|.
bool GetLogContents(const base::FilePath &config_path,

View file

@ -220,59 +220,6 @@ TEST_F(CrashCollectorTest, CheckHasCapacityStrangeNames) {
EXPECT_FALSE(CheckHasCapacity());
}
TEST_F(CrashCollectorTest, IsCommentLine) {
EXPECT_FALSE(CrashCollector::IsCommentLine(""));
EXPECT_TRUE(CrashCollector::IsCommentLine("#"));
EXPECT_TRUE(CrashCollector::IsCommentLine("#real comment"));
EXPECT_TRUE(CrashCollector::IsCommentLine(" # real comment"));
EXPECT_FALSE(CrashCollector::IsCommentLine("not comment"));
EXPECT_FALSE(CrashCollector::IsCommentLine(" not comment"));
}
TEST_F(CrashCollectorTest, ReadKeyValueFile) {
const char *contents = ("a=b\n"
"\n"
" c=d \n");
FilePath path(test_dir_.Append("keyval"));
std::map<std::string, std::string> dictionary;
std::map<std::string, std::string>::iterator i;
base::WriteFile(path, contents, strlen(contents));
EXPECT_TRUE(collector_.ReadKeyValueFile(path, '=', &dictionary));
i = dictionary.find("a");
EXPECT_TRUE(i != dictionary.end() && i->second == "b");
i = dictionary.find("c");
EXPECT_TRUE(i != dictionary.end() && i->second == "d");
dictionary.clear();
contents = ("a=b c d\n"
"e\n"
" f g = h\n"
"i=j\n"
"=k\n"
"#comment=0\n"
"l=\n");
base::WriteFile(path, contents, strlen(contents));
EXPECT_FALSE(collector_.ReadKeyValueFile(path, '=', &dictionary));
EXPECT_EQ(5, dictionary.size());
i = dictionary.find("a");
EXPECT_TRUE(i != dictionary.end() && i->second == "b c d");
i = dictionary.find("e");
EXPECT_TRUE(i == dictionary.end());
i = dictionary.find("f g");
EXPECT_TRUE(i != dictionary.end() && i->second == "h");
i = dictionary.find("i");
EXPECT_TRUE(i != dictionary.end() && i->second == "j");
i = dictionary.find("");
EXPECT_TRUE(i != dictionary.end() && i->second == "k");
i = dictionary.find("l");
EXPECT_TRUE(i != dictionary.end() && i->second == "");
}
TEST_F(CrashCollectorTest, MetaData) {
const char kMetaFileBasename[] = "generated.meta";
FilePath meta_file = test_dir_.Append(kMetaFileBasename);
@ -280,7 +227,10 @@ TEST_F(CrashCollectorTest, MetaData) {
FilePath payload_file = test_dir_.Append("payload-file");
std::string contents;
collector_.lsb_release_ = lsb_release.value();
const char kLsbContents[] = "CHROMEOS_RELEASE_VERSION=version\n";
const char kLsbContents[] =
"CHROMEOS_RELEASE_BOARD=lumpy\n"
"CHROMEOS_RELEASE_VERSION=6727.0.2015_01_26_0853\n"
"CHROMEOS_RELEASE_NAME=Chromium OS\n";
ASSERT_TRUE(base::WriteFile(lsb_release, kLsbContents, strlen(kLsbContents)));
const char kPayload[] = "foo";
ASSERT_TRUE(base::WriteFile(payload_file, kPayload, strlen(kPayload)));
@ -290,7 +240,7 @@ TEST_F(CrashCollectorTest, MetaData) {
const char kExpectedMeta[] =
"foo=bar\n"
"exec_name=kernel\n"
"ver=version\n"
"ver=6727.0.2015_01_26_0853\n"
"payload=test/payload-file\n"
"payload_size=3\n"
"done=1\n";
@ -328,7 +278,7 @@ TEST_F(CrashCollectorTest, GetLogContents) {
FilePath config_file = test_dir_.Append("crash_config");
FilePath output_file = test_dir_.Append("crash_log");
const char kConfigContents[] =
"foobar:echo hello there | sed -e \"s/there/world/\"";
"foobar=echo hello there | \\\n sed -e \"s/there/world/\"";
ASSERT_TRUE(
base::WriteFile(config_file, kConfigContents, strlen(kConfigContents)));
base::DeleteFile(FilePath(output_file), false);

View file

@ -2,64 +2,102 @@
# Use of this source code is governed by a BSD-style license that can
# be found in the LICENSE file.
# This file has the format:
# <basename>:<shell command>\n
# This file is parsed by chromeos::KeyValueStore. It has the format:
#
# Where when any executable with the basename <basename> crashes, the
# given <shell command> is executed and its standard output and
# standard error is sent along with the crash report.
# <basename>=<shell command>\n
#
# Use caution in modifying this file. Only run common unix commands
# here as these commands will be run when a crash has recently
# occurred and we should avoid running anything that might cause
# another crash. Similarly these command block the notification of
# the crash to parent processes, so commands should execute quickly.
# Commands cannot contain colons.
# Commands may be split across multiple lines using trailing backslashes.
#
# When an executable named <basename> crashes, the corresponding command is
# executed and its standard output and standard error are attached to the crash
# report.
#
# Use caution in modifying this file. Only run common Unix commands here, as
# these commands will be run when a crash has recently occurred and we should
# avoid running anything that might cause another crash. Similarly, these
# commands block notification of the crash to parent processes, so commands
# should execute quickly.
update_engine:cat $(ls -1tr /var/log/update_engine | tail -5 | sed s.^./var/log/update_engine/.) | tail -c 50000
update_engine=cat $(ls -1tr /var/log/update_engine | tail -5 | \
sed s.^./var/log/update_engine/.) | tail -c 50000
# The cros_installer output is logged into the update engine log file,
# so it is handled in the same way as update_engine.
cros_installer:cat $(ls -1tr /var/log/update_engine | tail -5 | sed s.^./var/log/update_engine/.) | tail -c 50000
cros_installer=cat $(ls -1tr /var/log/update_engine | tail -5 | \
sed s.^./var/log/update_engine/.) | tail -c 50000
# Dump the last 20 lines of the last two files in Chrome's system and user log
# directories, along with the last 20 messages from the session manager.
chrome:for f in $(ls -1rt /var/log/chrome/chrome_[0-9]* | tail -2) $(ls -1rt /home/chronos/u-*/log/chrome_[0-9]* 2>/dev/null | tail -2); do echo "===$f (tail)==="; tail -20 $f; echo EOF; echo; done; echo "===session_manager (tail)==="; awk '$3 ~ "^session_manager\[" { print }' /var/log/messages | tail -20; echo EOF
chrome=\
for f in $(ls -1rt /var/log/chrome/chrome_[0-9]* | tail -2) \
$(ls -1rt /home/chronos/u-*/log/chrome_[0-9]* 2>/dev/null | tail -2); do \
echo "===$f (tail)==="; \
tail -20 $f; \
echo EOF; \
echo; \
done; \
echo "===session_manager (tail)==="; \
awk '$3 ~ "^session_manager\[" { print }' /var/log/messages | tail -20; \
echo EOF
# The following rule is used for generating additional diagnostics when
# collection of user crashes fails. This output should not be too large
# as it is stored in memory. The output format specified for 'ps' is the
# same as with the "u" ("user-oriented") option, except it doesn't show
# the commands' arguments (i.e. "comm" instead of "command").
crash_reporter-user-collection:echo "===ps output==="; ps axw -o user,pid,%cpu,%mem,vsz,rss,tname,stat,start_time,bsdtime,comm | tail -c 25000; echo "===meminfo==="; cat /proc/meminfo
crash_reporter-user-collection=\
echo "===ps output==="; \
ps axw -o user,pid,%cpu,%mem,vsz,rss,tname,stat,start_time,bsdtime,comm | \
tail -c 25000; \
echo "===meminfo==="; \
cat /proc/meminfo
# This rule is similar to the crash_reporter-user-collection rule, except it is
# run for kernel errors reported through udev events.
crash_reporter-udev-collection-change-card0-drm:for dri in /sys/kernel/debug/dri/*; do echo "===$dri/i915_error_state==="; cat $dri/i915_error_state; done
crash_reporter-udev-collection-change-card0-drm=\
for dri in /sys/kernel/debug/dri/*; do \
echo "===$dri/i915_error_state==="; \
cat $dri/i915_error_state; \
done
# When trackpad driver cyapa detects some abnormal behavior, we collect
# additional logs from kernel messages.
crash_reporter-udev-collection-change--i2c-cyapa:/usr/sbin/kernel_log_collector.sh cyapa 30
# When trackpad/touchscreen driver atmel_mxt_ts detects some abnormal behavior, we collect
# additional logs from kernel messages.
crash_reporter-udev-collection-change--i2c-atmel_mxt_ts:/usr/sbin/kernel_log_collector.sh atmel 30
# When touch device noise are detected, we collect relevant logs. (crosbug.com/p/16788)
crash_reporter-udev-collection---TouchNoise:cat /var/log/touch_noise.log
crash_reporter-udev-collection-change--i2c-cyapa=\
/usr/sbin/kernel_log_collector.sh cyapa 30
# When trackpad/touchscreen driver atmel_mxt_ts detects some abnormal behavior,
# we collect additional logs from kernel messages.
crash_reporter-udev-collection-change--i2c-atmel_mxt_ts=\
/usr/sbin/kernel_log_collector.sh atmel 30
# When touch device noise are detected, we collect relevant logs.
# (crosbug.com/p/16788)
crash_reporter-udev-collection---TouchNoise=cat /var/log/touch_noise.log
# Periodically collect touch event log for debugging (crosbug.com/p/17244)
crash_reporter-udev-collection---TouchEvent:cat /var/log/touch_event.log
crash_reporter-udev-collection---TouchEvent=cat /var/log/touch_event.log
# Dump the last 50 lines of the last two powerd log files -- if the job has
# already restarted, we want to see the end of the previous instance's logs.
powerd:for f in $(ls -1tr /var/log/power_manager/powerd.[0-9]* | tail -2); do echo "===$(basename $f) (tail)==="; tail -50 $f; echo EOF; done
powerd=\
for f in $(ls -1tr /var/log/power_manager/powerd.[0-9]* | tail -2); do \
echo "===$(basename $f) (tail)==="; \
tail -50 $f; \
echo EOF; \
done
# If power_supply_info aborts (due to e.g. a bad battery), its failure message
# could end up in various places depending on which process was running it.
# Attach the end of powerd's log since it might've also logged the underlying
# problem.
power_supply_info:echo "===powerd.LATEST (tail)==="; tail -50 /var/log/power_manager/powerd.LATEST; echo EOF
power_supply_info=\
echo "===powerd.LATEST (tail)==="; \
tail -50 /var/log/power_manager/powerd.LATEST; \
echo EOF
# powerd_setuid_helper gets run by powerd, so its stdout/stderr will be mixed in
# with powerd's stdout/stderr.
powerd_setuid_helper:echo "===powerd.OUT (tail)==="; tail -50 /var/log/powerd.out; echo EOF
powerd_setuid_helper=\
echo "===powerd.OUT (tail)==="; \
tail -50 /var/log/powerd.out; \
echo EOF
# The following rules are only for testing purposes.
crash_log_test:echo hello world
crash_log_recursion_test:sleep 1 && /usr/local/autotest/tests/crash_log_recursion_test
crash_log_test=echo hello world
crash_log_recursion_test=sleep 1 && \
/usr/local/autotest/tests/crash_log_recursion_test

View file

@ -0,0 +1,28 @@
// Copyright 2015 The Chromium OS Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include <string>
#include <base/files/file_path.h>
#include <chromeos/key_value_store.h>
#include <gtest/gtest.h>
namespace {
// Name of the checked-in configuration file containing log-collection commands.
const char kConfigFile[] = "crash_reporter_logs.conf";
// Executable name for Chrome. kConfigFile is expected to contain this entry.
const char kChromeExecName[] = "chrome";
} // namespace
// Tests that the config file is parsable and that Chrome is listed.
TEST(CrashReporterLogsTest, ReadConfig) {
chromeos::KeyValueStore store;
ASSERT_TRUE(store.Load(base::FilePath(kConfigFile)));
std::string command;
EXPECT_TRUE(store.GetString(kChromeExecName, &command));
EXPECT_FALSE(command.empty());
}

View file

@ -19,9 +19,9 @@ const char kLogConfigFileName[] = "log_config_file";
// A bunch of random rules to put into the dummy log config file.
const char kLogConfigFileContents[] =
"crash_reporter-udev-collection-change-card0-drm:echo change card0 drm\n"
"crash_reporter-udev-collection-add-state0-cpu:echo change state0 cpu\n"
"cros_installer:echo not for udev";
"crash_reporter-udev-collection-change-card0-drm=echo change card0 drm\n"
"crash_reporter-udev-collection-add-state0-cpu=echo change state0 cpu\n"
"cros_installer=echo not for udev";
void CountCrash() {}