[Storage] Added histograms for blob storage

storage/browser/blob/blob_storage_context.cc

Index: storage/browser/blob/blob_storage_context.cc
diff --git a/storage/browser/blob/blob_storage_context.cc b/storage/browser/blob/blob_storage_context.cc
index 554c84e278e7725404cafe4386b69029bf114e01..2c733c603a98d82461866effcd6444bfa5746f05 100644
--- a/storage/browser/blob/blob_storage_context.cc
+++ b/storage/browser/blob/blob_storage_context.cc
@@ -8,6 +8,7 @@
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop_proxy.h"
+#include "base/metrics/histogram.h"
 #include "storage/browser/blob/blob_data_handle.h"
 #include "storage/common/blob/blob_data.h"
 #include "url/gurl.h"
@@ -143,29 +144,36 @@ void BlobStorageContext::AppendBlobDataItem(
   // 4) The Blob items are expanded.
   // TODO(michaeln): Would be nice to avoid copying Data items when expanding.
 
-  DCHECK(item.length() > 0);
+  uint64 length = item.length();
+  DCHECK(length > 0);

[Alexei Svitkine (slow), 2015/01/06 16:47:42]

Nit: DCHECK_GT

[dmurph, 2015/01/06 20:15:44]

Done.

+  UMA_HISTOGRAM_COUNTS("Storage.Blob.StorageSizeBeforeAppend",
+                       memory_usage_ / 1024);
   switch (item.type()) {
     case BlobData::Item::TYPE_BYTES:
+      UMA_HISTOGRAM_COUNTS("Storage.BlobItemSize.Bytes", length);
       DCHECK(!item.offset());
       exceeded_memory = !AppendBytesItem(target_blob_data,
                                          item.bytes(),
-                                         static_cast<int64>(item.length()));
+                                         static_cast<int64>(length));
       break;
     case BlobData::Item::TYPE_FILE:
+      UMA_HISTOGRAM_COUNTS("Storage.BlobItemSize.File", length);
       AppendFileItem(target_blob_data,
                      item.path(),
                      item.offset(),
-                     item.length(),
+                     length,
                      item.expected_modification_time());
       break;
     case BlobData::Item::TYPE_FILE_FILESYSTEM:
+      UMA_HISTOGRAM_COUNTS("Storage.BlobItemSize.FileSystem", length);
       AppendFileSystemFileItem(target_blob_data,
                                item.filesystem_url(),
                                item.offset(),
-                               item.length(),
+                               length,
                                item.expected_modification_time());
       break;
     case BlobData::Item::TYPE_BLOB: {
+      UMA_HISTOGRAM_COUNTS("Storage.BlobItemSize.Blob", length);
       scoped_ptr<BlobDataHandle> src = GetBlobDataFromUUID(item.blob_uuid());
       if (src)
         exceeded_memory = !ExpandStorageItems(target_blob_data,
@@ -178,10 +186,13 @@ void BlobStorageContext::AppendBlobDataItem(
       NOTREACHED();
       break;
   }
+  UMA_HISTOGRAM_COUNTS("Storage.Blob.StorageSizeAfterAppend",
+                       memory_usage_ / 1024);

[Alexei Svitkine (slow), 2015/01/06 16:47:41]

Just wondering why you actually need both the Before and After versions of this
histogram.

I suspect their shapes would look rather similar - what's the purpose of needing
both?

[dmurph, 2015/01/06 20:15:44]

I want to find the distribution of the final storage size (or maximum storage
size) in all machines.  I can do this by logging all of the 'befores' and
'afters', and then subtracting all of the befores from the afters, which gives
me the final 'after', which is the maximum (or final) storage size on that
machine.

If you know a better way to do this I would definitely be interested.

[Alexei Svitkine (slow), 2015/01/06 20:22:48]

Ah, I see. You'll have to deal with some skew (i.e. when the before was logged
on a previous day than the after and when the next after will be on the next
day, etc), but I don't have a better suggestion for this, unfortunately - so
this is OK.

 
   // If we're using too much memory, drop this blob's data.
   // TODO(michaeln): Blob memory storage does not yet spill over to disk,
   // as a stop gap, we'll prevent memory usage over a max amount.
+  UMA_HISTOGRAM_BOOLEAN("Storage.BlobItem.ExceededMemory", exceeded_memory);

[Alexei Svitkine (slow), 2015/01/06 16:47:42]

Since exceeded memory can only happen on TYPE_BLOB, would it make more sense to
log this in that block? That way, you get the ratio of TYPE_BLOB's that exceeded
memory rather than of everything...

I guess you can derive one from the other with the help of the other metrics,
but if you're actually interested in ratio of blobs that exceeded, would be
better to be in the case.

[dmurph, 2015/01/06 20:15:44]

It's set in both TYPE_BLOB and TYPE_BYTES
However, I believe this isn't the best stat for us, as we want memory exceeded
ratio for blobs, not blob items.  Changed.

   if (exceeded_memory) {
     memory_usage_ -= target_blob_data->GetMemoryUsage();
     found->second.flags |= EXCEEDED_MEMORY;