Obsolete the DataUse.Sync.* histograms

These histograms that record datause of sync service by sync message
types. This level of detail is not needed. Overall datause of sync
service is tracked by DataUse.MessageSize.AllServices.* histograms.

Bug: 691866
Change-Id: Iddd456c1d3d3d550111b6f9c5fe78aa21306749a
Reviewed-on: https://chromium-review.googlesource.com/742702Reviewed-by: Stanislav Chiknavaryan <stanisc@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Commit-Queue: rajendrant <rajendrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512971}

components/sync/base/data_type_histogram.cc

@@ -5,16 +5,9 @@
 #include "components/sync/base/data_type_histogram.h"
 
 #include "base/metrics/histogram_functions.h"
-#include "base/metrics/sparse_histogram.h"
 
 const char kModelTypeMemoryHistogramPrefix[] = "Sync.ModelTypeMemoryKB.";
 
-void SyncRecordDatatypeBin(const std::string& name, int sample, int value) {
-  base::HistogramBase* histogram = base::SparseHistogram::FactoryGet(
-      name, base::HistogramBase::kUmaTargetedHistogramFlag);
-  histogram->AddCount(sample, value);
-}
-
 void SyncRecordMemoryKbHistogram(const std::string& histogram_name_prefix,
                                  syncer::ModelType model_type,
                                  size_t value) {

components/sync/base/data_type_histogram.h

@@ -14,12 +14,6 @@
 // Prefix for histogram recording datatype's memory usage.
 extern const char kModelTypeMemoryHistogramPrefix[];
 
-// This function adds |value| to |sample| bucket of histogram |name|. |value|
-// should be greater or equal to 1 and |name| can be variable. DataTypes are
-// mapped to proper |sample| bucket by using ModelTypeToHistogramInt() function.
-// So different DataTypes play the role of different buckets in this histogram.
-void SyncRecordDatatypeBin(const std::string& name, int sample, int value);
-
 // Converts memory size |value| into kilobytes and records it into |model_type|
 // related histogram with prefix |histogram_name_prefix|.
 void SyncRecordMemoryKbHistogram(const std::string& histogram_name_prefix,

components/sync/engine_impl/commit.cc

@@ -94,19 +94,9 @@ std::unique_ptr<Commit> Commit::Init(ModelTypeSet requested_types,
   commit_util::AddClientConfigParamsToMessage(
       enabled_types, cookie_jar_mismatch, commit_message);
 
-  int previous_message_size = message.ByteSize();
   // Finally, serialize all our contributions.
   for (const auto& contribution : contributions) {
     contribution.second->AddToCommitMessage(&message);
-    int current_entry_size = message.ByteSize() - previous_message_size;
-    previous_message_size = message.ByteSize();
-    int local_integer_model_type = ModelTypeToHistogramInt(contribution.first);
-    if (current_entry_size > 0) {
-      SyncRecordDatatypeBin("DataUse.Sync.Upload.Bytes",
-                            local_integer_model_type, current_entry_size);
-    }
-    UMA_HISTOGRAM_SPARSE_SLOWLY("DataUse.Sync.Upload.Count",
-                                local_integer_model_type);
   }
 
   // If we made it this far, then we've successfully prepared a commit message.

components/sync/engine_impl/directory_update_handler.cc

@@ -59,11 +59,6 @@ SyncerError DirectoryUpdateHandler::ProcessGetUpdatesResponse(
     const SyncEntityList& applicable_updates,
     StatusController* status) {
   syncable::ModelNeutralWriteTransaction trans(FROM_HERE, SYNCER, dir_);
-  if (progress_marker.ByteSize() > 0) {
-    SyncRecordDatatypeBin("DataUse.Sync.ProgressMarker.Bytes",
-                          ModelTypeToHistogramInt(type_),
-                          progress_marker.ByteSize());
-  }
   if (mutated_context.has_context()) {
     sync_pb::DataTypeContext local_context;
     dir_->GetDataTypeContext(&trans, type_, &local_context);

components/sync/engine_impl/process_updates_util.cc

@@ -295,12 +295,6 @@ void ProcessDownloadedUpdates(syncable::Directory* dir,
     if (verify_result != VERIFY_SUCCESS && verify_result != VERIFY_UNDELETE)
       continue;
     ProcessUpdate(*update, dir->GetCryptographer(trans), trans);
-    if (update->ByteSize() > 0) {
-      SyncRecordDatatypeBin("DataUse.Sync.Download.Bytes",
-                            ModelTypeToHistogramInt(type), update->ByteSize());
-    }
-    UMA_HISTOGRAM_SPARSE_SLOWLY("DataUse.Sync.Download.Count",
-                                ModelTypeToHistogramInt(type));
   }
 }
 

components/sync/engine_impl/syncer_unittest.cc

@@ -629,115 +629,6 @@ TEST_F(SyncerTest, GetCommitIdsFiltersThrottledEntries) {
   }
 }
 
-// This test has three steps. In the first step, a BOOKMARK update is received.
-// In the next step, syncing BOOKMARKS is disabled, so no BOOKMARK is sent or
-// received. In the last step, a BOOKMARK update is committed.
-TEST_F(SyncerTest, DataUseHistogramsTest) {
-  base::HistogramTester histogram_tester;
-  sync_pb::EntitySpecifics bookmark_data;
-  AddDefaultFieldValue(BOOKMARKS, &bookmark_data);
-
-  mock_server_->AddUpdateDirectory(1, 0, "A", 10, 10, foreign_cache_guid(),
-                                   "-1");
-  int download_bytes_bookmark = 0;
-  vector<unsigned int> progress_bookmark(3, 0);
-  vector<unsigned int> progress_all(3, 0);
-  vector<base::Bucket> samples;
-  EXPECT_TRUE(SyncShareNudge());
-  {
-    histogram_tester.ExpectTotalCount("DataUse.Sync.Upload.Count", 0);
-    histogram_tester.ExpectTotalCount("DataUse.Sync.Upload.Bytes", 0);
-    histogram_tester.ExpectTotalCount("DataUse.Sync.Download.Count", 1);
-    histogram_tester.ExpectUniqueSample("DataUse.Sync.Download.Count",
-                                        BOOKMARKS, 1);
-    samples = histogram_tester.GetAllSamples("DataUse.Sync.Download.Bytes");
-    EXPECT_EQ(1u, samples.size());
-    EXPECT_EQ(BOOKMARKS, samples.at(0).min);
-    EXPECT_GE(samples.at(0).count, 0);
-    download_bytes_bookmark = samples.at(0).count;
-
-    samples =
-        histogram_tester.GetAllSamples("DataUse.Sync.ProgressMarker.Bytes");
-
-    for (const base::Bucket& bucket : samples) {
-      if (bucket.min == BOOKMARKS)
-        progress_bookmark.at(0) += bucket.count;
-      progress_all.at(0) += bucket.count;
-    }
-    EXPECT_GT(progress_bookmark.at(0), 0u);
-    EXPECT_GT(progress_all.at(0), 0u);
-
-    syncable::WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
-    MutableEntry A(&wtrans, GET_BY_ID, ids_.FromNumber(1));
-    A.PutIsUnsynced(true);
-    A.PutSpecifics(bookmark_data);
-    A.PutNonUniqueName("bookmark");
-  }
-
-  // Now sync without enabling bookmarks.
-  mock_server_->ExpectGetUpdatesRequestTypes(
-      Difference(context_->GetEnabledTypes(), ModelTypeSet(BOOKMARKS)));
-  ResetCycle();
-  syncer_->NormalSyncShare(
-      Difference(context_->GetEnabledTypes(), ModelTypeSet(BOOKMARKS)),
-      &nudge_tracker_, cycle_.get());
-
-  {
-    // Nothing should have been committed as bookmarks is throttled.
-    histogram_tester.ExpectTotalCount("DataUse.Sync.Upload.Count", 0);
-    histogram_tester.ExpectTotalCount("DataUse.Sync.Upload.Bytes", 0);
-    histogram_tester.ExpectTotalCount("DataUse.Sync.Download.Count", 1);
-    histogram_tester.ExpectUniqueSample("DataUse.Sync.Download.Count",
-                                        BOOKMARKS, 1);
-
-    samples = histogram_tester.GetAllSamples("DataUse.Sync.Download.Bytes");
-    EXPECT_EQ(1u, samples.size());
-    EXPECT_EQ(BOOKMARKS, samples.at(0).min);
-    EXPECT_EQ(download_bytes_bookmark, samples.at(0).count);
-
-    samples =
-        histogram_tester.GetAllSamples("DataUse.Sync.ProgressMarker.Bytes");
-    for (const base::Bucket& bucket : samples) {
-      if (bucket.min == BOOKMARKS)
-        progress_bookmark.at(1) += bucket.count;
-      progress_all.at(1) += bucket.count;
-    }
-    EXPECT_EQ(progress_bookmark.at(1), progress_bookmark.at(0));
-    EXPECT_GT(progress_all.at(1), progress_all.at(0));
-  }
-
-  // Sync again with bookmarks enabled.
-  mock_server_->ExpectGetUpdatesRequestTypes(context_->GetEnabledTypes());
-  EXPECT_TRUE(SyncShareNudge());
-  {
-    // It should have been committed.
-    histogram_tester.ExpectTotalCount("DataUse.Sync.Upload.Count", 1);
-    histogram_tester.ExpectUniqueSample("DataUse.Sync.Upload.Count", BOOKMARKS,
-                                        1);
-    samples = histogram_tester.GetAllSamples("DataUse.Sync.Upload.Bytes");
-    EXPECT_EQ(1u, samples.size());
-    EXPECT_EQ(BOOKMARKS, samples.at(0).min);
-    EXPECT_GE(samples.at(0).count, 0);
-
-    samples = histogram_tester.GetAllSamples("DataUse.Sync.Download.Bytes");
-    EXPECT_EQ(1u, samples.size());
-    EXPECT_EQ(BOOKMARKS, samples.at(0).min);
-    EXPECT_EQ(download_bytes_bookmark, samples.at(0).count);
-
-    histogram_tester.ExpectTotalCount("DataUse.Sync.Download.Count", 1);
-
-    samples =
-        histogram_tester.GetAllSamples("DataUse.Sync.ProgressMarker.Bytes");
-    for (const base::Bucket& bucket : samples) {
-      if (bucket.min == BOOKMARKS)
-        progress_bookmark.at(2) += bucket.count;
-      progress_all.at(2) += bucket.count;
-    }
-    EXPECT_GT(progress_bookmark.at(2), progress_bookmark.at(1));
-    EXPECT_GT(progress_all.at(2), progress_all.at(1));
-  }
-}
-
 // We use a macro so we can preserve the error location.
 #define VERIFY_ENTRY(id, is_unapplied, is_unsynced, prev_initialized,         \
                      parent_id, version, server_version, id_fac, rtrans)      \

tools/metrics/histograms/histograms.xml

@@ -13496,6 +13496,9 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
 </histogram>
 
 <histogram name="DataUse.Sync.Download.Bytes" enum="SyncModelTypes">
+  <obsolete>
+    Deprecated 10/2017.
+  </obsolete>
   <owner>amohammadkhan@chromium.org</owner>
   <owner>bengr@chromium.org</owner>
   <summary>
@@ -13506,6 +13509,9 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
 </histogram>
 
 <histogram name="DataUse.Sync.Download.Count" enum="SyncModelTypes">
+  <obsolete>
+    Deprecated 10/2017.
+  </obsolete>
   <owner>amohammadkhan@chromium.org</owner>
   <owner>bengr@chromium.org</owner>
   <summary>
@@ -13516,6 +13522,9 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
 </histogram>
 
 <histogram name="DataUse.Sync.ProgressMarker.Bytes" enum="SyncModelTypes">
+  <obsolete>
+    Deprecated 10/2017.
+  </obsolete>
   <owner>amohammadkhan@chromium.org</owner>
   <owner>bengr@chromium.org</owner>
   <summary>
@@ -13526,6 +13535,9 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
 </histogram>
 
 <histogram name="DataUse.Sync.Upload.Bytes" enum="SyncModelTypes">
+  <obsolete>
+    Deprecated 10/2017.
+  </obsolete>
   <owner>amohammadkhan@chromium.org</owner>
   <owner>bengr@chromium.org</owner>
   <summary>
@@ -13535,6 +13547,9 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
 </histogram>
 
 <histogram name="DataUse.Sync.Upload.Count" enum="SyncModelTypes">
+  <obsolete>
+    Deprecated 10/2017.
+  </obsolete>
   <owner>amohammadkhan@chromium.org</owner>
   <owner>bengr@chromium.org</owner>
   <summary>