Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(170)

Unified Diff: components/browser_watcher/postmortem_report_collector.cc

Issue 2715903003: Bound the impact of system instability on chrome instability. (Closed)
Patch Set: record start timestamp, analysis metric, make log scrape lazy Created 3 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/browser_watcher/postmortem_report_collector.cc
diff --git a/components/browser_watcher/postmortem_report_collector.cc b/components/browser_watcher/postmortem_report_collector.cc
index 7f5bf1457b81517991d604bfc1a754b9ef75f950..a8e9316454fbd07bbd0b4eede0ba836fff5f1e20 100644
--- a/components/browser_watcher/postmortem_report_collector.cc
+++ b/components/browser_watcher/postmortem_report_collector.cc
@@ -36,6 +36,16 @@ namespace {
const char kFieldTrialKeyPrefix[] = "FieldTrial.";
+// DO NOT CHANGE VALUES. This is logged persistently in a histogram.
+enum SystemSessionAnalysisStatus {
+ SYSTEM_SESSION_ANALYSIS_SUCCESS = 0,
+ SYSTEM_SESSION_ANALYSIS_NO_TIMESTAMP = 1,
+ SYSTEM_SESSION_ANALYSIS_NO_ANALYZER = 2,
+ SYSTEM_SESSION_ANALYSIS_FAILED = 3,
+ SYSTEM_SESSION_ANALYSIS_OUTSIDE_RANGE = 4,
+ SYSTEM_SESSION_ANALYSIS_STATUS_MAX = 5
+};
+
// Collects stability user data from the recorded format to the collected
// format.
void CollectUserData(
@@ -115,6 +125,23 @@ void CollectUserData(
}
}
+bool GetStartTimestamp(
+ const google::protobuf::Map<std::string, TypedValue>& global_data,
+ base::Time* time) {
+ DCHECK(time);
+
+ const auto& it = global_data.find(kStabilityStartTimestamp);
+ if (it == global_data.end())
+ return false;
+
+ const TypedValue& value = it->second;
+ if (value.value_case() != TypedValue::kSignedValue)
+ return false;
+
+ *time = base::Time::FromInternalValue(value.signed_value());
+ return true;
+}
+
void CollectModuleInformation(
const std::vector<GlobalActivityTracker::ModuleInfo>& modules,
ProcessState* process_state) {
@@ -153,10 +180,12 @@ void CollectModuleInformation(
PostmortemReportCollector::PostmortemReportCollector(
const std::string& product_name,
const std::string& version_number,
- const std::string& channel_name)
+ const std::string& channel_name,
+ std::unique_ptr<SystemSessionAnalyzer> analyzer)
: product_name_(product_name),
version_number_(version_number),
- channel_name_(channel_name) {}
+ channel_name_(channel_name),
+ system_session_analyzer_(std::move(analyzer)) {}
int PostmortemReportCollector::CollectAndSubmitForUpload(
const base::FilePath& debug_info_dir,
@@ -310,21 +339,10 @@ PostmortemReportCollector::CollectionStatus PostmortemReportCollector::Collect(
}
// Collect global user data.
- google::protobuf::Map<std::string, TypedValue>& global_data =
- *(*report)->mutable_global_data();
- CollectUserData(global_data_snapshot, &global_data, report->get());
-
- // Add the reporting Chrome's details to the report.
- global_data[kStabilityReporterChannel].set_string_value(channel_name());
-#if defined(ARCH_CPU_X86)
- global_data[kStabilityReporterPlatform].set_string_value(
- std::string("Win32"));
-#elif defined(ARCH_CPU_X86_64)
- global_data[kStabilityReporterPlatform].set_string_value(
- std::string("Win64"));
-#endif
- global_data[kStabilityReporterProduct].set_string_value(product_name());
- global_data[kStabilityReporterVersion].set_string_value(version_number());
+ CollectUserData(global_data_snapshot, (*report)->mutable_global_data(),
+ report->get());
+ SetReporterDetails(report->get());
+ DetermineSystemSate(report->get());
// Collect thread activity data.
// Note: a single process is instrumented.
@@ -352,6 +370,65 @@ PostmortemReportCollector::CollectionStatus PostmortemReportCollector::Collect(
return SUCCESS;
}
+void PostmortemReportCollector::SetReporterDetails(
+ StabilityReport* report) const {
+ DCHECK(report);
+
+ google::protobuf::Map<std::string, TypedValue>& global_data =
+ *(report->mutable_global_data());
+
+ // Reporter version details.
+ global_data[kStabilityReporterChannel].set_string_value(channel_name());
+#if defined(ARCH_CPU_X86)
+ global_data[kStabilityReporterPlatform].set_string_value(
+ std::string("Win32"));
+#elif defined(ARCH_CPU_X86_64)
+ global_data[kStabilityReporterPlatform].set_string_value(
+ std::string("Win64"));
+#endif
+ global_data[kStabilityReporterProduct].set_string_value(product_name());
+ global_data[kStabilityReporterVersion].set_string_value(version_number());
+}
+
+void PostmortemReportCollector::DetermineSystemSate(
+ StabilityReport* report) const {
+ DCHECK(report);
+
+ // A session state for the stability report: was the system session clean?
+ SystemState::SessionState session_state = SystemState::UNKNOWN;
+ // An analasis status for metrics: did the analysis succeed?
+ SystemSessionAnalysisStatus status = SYSTEM_SESSION_ANALYSIS_SUCCESS;
+
+ base::Time time;
+ if (!GetStartTimestamp(report->global_data(), &time)) {
+ status = SYSTEM_SESSION_ANALYSIS_NO_TIMESTAMP;
+ } else if (!system_session_analyzer_) {
+ status = SYSTEM_SESSION_ANALYSIS_NO_ANALYZER;
+ } else {
+ SystemSessionAnalyzer::Status analyzer_status =
+ system_session_analyzer_->IsSessionUnclean(time);
+ switch (analyzer_status) {
+ case SystemSessionAnalyzer::FAILED:
+ status = SYSTEM_SESSION_ANALYSIS_FAILED;
+ break;
+ case SystemSessionAnalyzer::CLEAN:
+ session_state = SystemState::CLEAN;
+ break;
+ case SystemSessionAnalyzer::UNCLEAN:
+ session_state = SystemState::UNCLEAN;
+ break;
+ case SystemSessionAnalyzer::OUTSIDE_RANGE:
+ status = SYSTEM_SESSION_ANALYSIS_OUTSIDE_RANGE;
+ break;
+ }
+ }
+
+ report->mutable_system_state()->set_session_state(session_state);
+ UMA_HISTOGRAM_ENUMERATION(
+ "ActivityTracker.Collect.SystemSessionAnalysisStatus", status,
+ SYSTEM_SESSION_ANALYSIS_STATUS_MAX);
+}
+
void PostmortemReportCollector::CollectThread(
const base::debug::ThreadActivityAnalyzer::Snapshot& snapshot,
ThreadState* thread_state) {

Powered by Google App Engine
This is Rietveld 408576698