leveldb: Moved to LevelDBEnv.IOError.BFE histogram entries.

third_party/leveldatabase/env_chromium.cc

Index: third_party/leveldatabase/env_chromium.cc
diff --git a/third_party/leveldatabase/env_chromium.cc b/third_party/leveldatabase/env_chromium.cc
index 42bdb42269373db25644e415dc94d8b5e907741b..343f5e367604b0601c54465f12c75af9a8846435 100644
--- a/third_party/leveldatabase/env_chromium.cc
+++ b/third_party/leveldatabase/env_chromium.cc
@@ -15,6 +15,7 @@
 #include "base/metrics/histogram.h"
 #include "base/process/process_metrics.h"
 #include "base/stl_util.h"
+#include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/trace_event/trace_event.h"
 #include "third_party/leveldatabase/chromium_logger.h"
@@ -326,6 +327,7 @@ class IDBEnv : public ChromiumEnv {
  public:
   IDBEnv() : ChromiumEnv() {
     name_ = "LevelDBEnv.IDB";
+    uma_ioerror_base_name_ = name_ + ".IOError.BFE";
     make_backup_ = true;
   }
 };
@@ -396,7 +398,7 @@ Status MakeIOError(Slice filename,
                    base::File::Error error) {
   DCHECK_LT(error, 0);
   char buf[512];
-  snprintf(buf, sizeof(buf), "%s (ChromeMethodPFE: %d::%s::%d)",
+  snprintf(buf, sizeof(buf), "%s (ChromeMethodBFE: %d::%s::%d)",
            message.c_str(), method, MethodIDToString(method), -error);
   return Status::IOError(filename, buf);
 }
@@ -422,13 +424,13 @@ ErrorParsingResult ParseMethodAndError(const leveldb::Status& status,
   }
   int parsed_error;
   if (RE2::PartialMatch(status_string.c_str(),
-                        "ChromeMethodPFE: (\\d+)::.*::(\\d+)", &method,
+                        "ChromeMethodBFE: (\\d+)::.*::(\\d+)", &method,
                         &parsed_error)) {
     *method_param = static_cast<MethodID>(method);
     *error = static_cast<base::File::Error>(-parsed_error);
     DCHECK_LT(*error, base::File::FILE_OK);
     DCHECK_GT(*error, base::File::FILE_ERROR_MAX);
-    return METHOD_AND_PFE;
+    return METHOD_AND_BFE;
   }
   return NONE;
 }
@@ -507,7 +509,7 @@ bool IndicatesDiskFull(const leveldb::Status& status) {
   base::File::Error error = base::File::FILE_OK;
   leveldb_env::ErrorParsingResult result =
       leveldb_env::ParseMethodAndError(status, &method, &error);
-  return (result == leveldb_env::METHOD_AND_PFE &&
+  return (result == leveldb_env::METHOD_AND_BFE &&
           static_cast<base::File::Error>(error) ==
               base::File::FILE_ERROR_NO_SPACE);
 }
@@ -525,6 +527,7 @@ ChromiumEnv::ChromiumEnv()
       bgsignal_(&mu_),
       started_bgthread_(false),
       kMaxRetryTimeMillis(1000) {
+  uma_ioerror_base_name_ = name_ + ".IOError.BFE";
 }
 
 ChromiumEnv::~ChromiumEnv() {
@@ -951,18 +954,17 @@ void ChromiumEnv::RecordBackupResult(bool result) const {
 
 base::HistogramBase* ChromiumEnv::GetOSErrorHistogram(MethodID method,
                                                       int limit) const {
-  std::string uma_name(name_);
-  // TODO(dgrogan): This is probably not the best way to concatenate strings.
-  uma_name.append(".IOError.").append(MethodIDToString(method));
+  std::string uma_name;
+  base::StringAppendF(&uma_name, "%s.%s", uma_ioerror_base_name_.c_str(),
+                      MethodIDToString(method));
   return base::LinearHistogram::FactoryGet(uma_name, 1, limit, limit + 1,
       base::Histogram::kUmaTargetedHistogramFlag);
 }
 
 base::HistogramBase* ChromiumEnv::GetMethodIOErrorHistogram() const {

[dgrogan, 2015/03/14 05:57:37]

It looks like this one didn't need to change. It just records the Env methods
where IOErrors were encountered, it doesn't record the IOErrors themselves.

Edit (after looking at xml):

Changing this like it is here brings us into a world of histograms.xml pain. I
think you should revert the change in this method and leave its histograms named
"LevelDBEnv.IOError" and "LevelDBEnv.IDB.IOError".

As it stands now the data this CL logs to the new
"LevelDBEnv.IDB.IOError.BFE.<methodName>" histograms would show up in the UMA UI
with LevelDBIOErrorMethods labels, not PlatformFileError labels.

[cmumford, 2015/03/17 21:25:52]

Yes, I see your point - and agree.

-  std::string uma_name(name_);
-  uma_name.append(".IOError");
-  return base::LinearHistogram::FactoryGet(uma_name, 1, kNumEntries,
-      kNumEntries + 1, base::Histogram::kUmaTargetedHistogramFlag);
+  return base::LinearHistogram::FactoryGet(
+      uma_ioerror_base_name_, 1, kNumEntries, kNumEntries + 1,
+      base::Histogram::kUmaTargetedHistogramFlag);
 }
 
 base::HistogramBase* ChromiumEnv::GetMaxFDHistogram(