ServiceWorker: Add UMA for ServiceWorkerDatabase

content/browser/service_worker/service_worker_database.cc

 // Copyright 2014 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 "content/browser/service_worker/service_worker_database.h"
 
 #include <string>
 
 #include "base/file_util.h"
 #include "base/location.h"
 #include "base/logging.h"
+#include "base/metrics/histogram.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "content/browser/service_worker/service_worker_database.pb.h"
 #include "third_party/leveldatabase/src/helpers/memenv/memenv.h"
 #include "third_party/leveldatabase/src/include/leveldb/db.h"
 #include "third_party/leveldatabase/src/include/leveldb/env.h"
 #include "third_party/leveldatabase/src/include/leveldb/write_batch.h"
@@ skipping 47 matching lines @@
 
 const char kRegKeyPrefix[] = "REG:";
 const char kResKeyPrefix[] = "RES:";
 const char kKeySeparator = '\x00';
 
 const char kUncommittedResIdKeyPrefix[] = "URES:";
 const char kPurgeableResIdKeyPrefix[] = "PRES:";
 
 const int64 kCurrentSchemaVersion = 1;
 
+// For histogram.
+const char kOpenHistogramLabel[] =
+    "ServiceWorker.Database.Open";
+const char kOperationErrorHistogramLabel[] =
+    "ServiceWorker.Database.OperationError";
+
 bool RemovePrefix(const std::string& str,
                   const std::string& prefix,
                   std::string* out) {
   if (!StartsWithASCII(str, prefix, true))
     return false;
   if (out)
     *out = str.substr(prefix.size());
   return true;
 }
 
@@ skipping 122 matching lines @@
   out->url = url;
   return true;
 }
 
 ServiceWorkerDatabase::Status LevelDBStatusToStatus(
     const leveldb::Status& status) {
   if (status.ok())
     return ServiceWorkerDatabase::STATUS_OK;
   else if (status.IsNotFound())
     return ServiceWorkerDatabase::STATUS_ERROR_NOT_FOUND;
+  else if (status.IsIOError())
+    return ServiceWorkerDatabase::STATUS_ERROR_IO_ERROR;
   else if (status.IsCorruption())
     return ServiceWorkerDatabase::STATUS_ERROR_CORRUPTED;
   else
     return ServiceWorkerDatabase::STATUS_ERROR_FAILED;
 }
 
 }  // namespace
 
 ServiceWorkerDatabase::RegistrationData::RegistrationData()
     : registration_id(-1),
@@ skipping 438 matching lines @@
   leveldb::Options options;
   options.create_if_missing = create_if_missing;
   if (use_in_memory_db) {
     env_.reset(leveldb::NewMemEnv(leveldb::Env::Default()));
     options.env = env_.get();
   }
 
   leveldb::DB* db = NULL;
   leveldb::Status db_status =
       leveldb::DB::Open(options, path_.AsUTF8Unsafe(), &db);
+  Status status = LevelDBStatusToStatus(db_status);
+  UMA_HISTOGRAM_ENUMERATION(kOpenHistogramLabel, status, STATUS_ERROR_MAX);
   if (!db_status.ok()) {
     DCHECK(!db);
     // TODO(nhiroki): Should we retry to open the database?
     HandleError(FROM_HERE, db_status);
-    return LevelDBStatusToStatus(db_status);
+    return status;
   }
   db_.reset(db);
 
   int64 db_version;
-  Status status = ReadDatabaseVersion(&db_version);
+  status = ReadDatabaseVersion(&db_version);
   if (status != STATUS_OK)
     return status;
   DCHECK_LE(0, db_version);
   if (db_version > 0)
     state_ = INITIALIZED;
   return STATUS_OK;
 }
 
 bool ServiceWorkerDatabase::IsNewOrNonexistentDatabase(
     ServiceWorkerDatabase::Status status) {
@@ skipping 288 matching lines @@
   }
 }
 
 bool ServiceWorkerDatabase::IsOpen() {
   return db_ != NULL;
 }
 
 void ServiceWorkerDatabase::HandleError(
     const tracked_objects::Location& from_here,
     const leveldb::Status& status) {
-  // TODO(nhiroki): Add an UMA histogram.
+  DCHECK(!status.ok());
   DLOG(ERROR) << "Failed at: " << from_here.ToString()
               << " with error: " << status.ToString();
+  UMA_HISTOGRAM_ENUMERATION(kOperationErrorHistogramLabel,

[michaeln, 2014/05/29 23:46:28]

I'm not sure how fine grained we want this to be?

Having the result of open be separate is definitely good and also reporting OK
events for that case is good so we can see rates.

Would it make sense to split ReadResults from WriteResult and to also log the OK
events so we can see rates?

[nhiroki, 2014/06/02 04:54:47]

SGTM. The latest patchset records read/write results separately.

+                            LevelDBStatusToStatus(status),
+                            STATUS_ERROR_MAX);
   state_ = DISABLED;
   db_.reset();
 }
 
 }  // namespace content