Stability instrumentation: metrics for collection on crash

components/browser_watcher/stability_report_user_stream_data_source.cc

 // Copyright 2017 The Chromium 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 "components/browser_watcher/stability_report_user_stream_data_source.h"
 
 #include <string>
 #include <utility>
 
 #include "base/files/file.h"
@@ skipping 55 matching lines @@
   DCHECK(!data_.empty());
   return data_.size();
 }
 
 bool BufferExtensionStreamDataSource::ReadStreamData(Delegate* delegate) {
   DCHECK(!data_.empty());
   return delegate->ExtensionStreamDataSourceRead(
       data_.size() ? data_.data() : nullptr, data_.size());
 }
 
+// TODO(manzagop): collection should factor in whether this is a true crash or
+// dump without crashing.
 std::unique_ptr<BufferExtensionStreamDataSource> CollectReport(
     const base::FilePath& path) {
   StabilityReport report;
   CollectionStatus status = Extract(path, &report);
   UMA_HISTOGRAM_ENUMERATION("ActivityTracker.CollectCrash.Status", status,
                             COLLECTION_STATUS_MAX);
   if (status != SUCCESS)
     return nullptr;
 
   // Open (with delete) and then immediately close the file by going out of
   // scope. This should cause the stability debugging file to be deleted prior
   // to the next execution.
   // TODO(manzagop): set the persistent allocator file's state to deleted in

[Sigurður Ásgeirsson, 2017/05/30 13:46:37]

This seems like a potentially important gotcha here, as failure to delete will
cause double-counting, no?
Is this hard to do?
Is there a metric for how often you encounter a nominally deleted file?

[manzagop (departed), 2017/05/30 16:16:54]

Correct. Note currently we undercount, as crashes are only counted if we fail to
delete on crash.
I thought I needed multi-process collection, but turns out no. Done.
Looks like it's 5% of the files (~60 out of 1100).

The activity analyzer logs when files are encountered with the deleted state:
ActivityTracker.Collect.AnalyzerCreationError
https://cs.chromium.org/chromium/src/base/debug/activity_analyzer.cc?l=120

   //     case the file can't be deleted.
   base::File file(path, base::File::FLAG_OPEN | base::File::FLAG_READ |
                             base::File::FLAG_DELETE_ON_CLOSE);
   UMA_HISTOGRAM_BOOLEAN("ActivityTracker.CollectCrash.OpenForDeleteSuccess",
                         file.IsValid());
+  UMA_STABILITY_HISTOGRAM_COUNTS_100("ActivityTracker.CollectCrash.CrashCount",

[Sigurður Ásgeirsson, 2017/05/30 13:46:37]

QQ: is this the best way to record this? I've seen enums used to record event
counts before. They're not super-convenient to decipher, but have the advantage
of being extensible to new events and presumably fairly dense in memory and on
wire.

[manzagop (departed), 2017/05/30 16:16:54]

I'd want the metric to be usable in a timeline formula for coverage estimation.
Is that possible?

+                                     1);
 
   std::unique_ptr<BufferExtensionStreamDataSource> source(
       new BufferExtensionStreamDataSource(kStabilityReportStreamType));
   return source->Init(report) ? std::move(source) : nullptr;
 }
 
 }  // namespace
 
 StabilityReportUserStreamDataSource::StabilityReportUserStreamDataSource(
     const base::FilePath& user_data_dir)
@@ skipping 12 matching lines @@
   if (!PathExists(stability_file)) {
     // Either this is not an instrumented process (currently only browser
     // processes can be instrumented), or the stability file cannot be found.
     return nullptr;
   }
 
   return CollectReport(stability_file);
 }
 
 }  // namespace browser_watcher