Histogram for UMA log upload connection type.

chrome/browser/metrics/chrome_metrics_service_client.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 "chrome/browser/metrics/chrome_metrics_service_client.h"
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/logging.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
+#include "base/metrics/sparse_histogram.h"

[Ilya Sherman, 2015/03/24 21:55:44]

nit: No longer needed?

[gayane -on leave until 09-2017, 2015/03/25 21:02:33]

Done.

 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/platform_thread.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/google/google_brand.h"
 #include "chrome/browser/metrics/chrome_stability_metrics_provider.h"
@@ skipping 90 matching lines @@
   return group_name == "Enabled";
 }
 #endif
 
 }  // namespace
 
 ChromeMetricsServiceClient::ChromeMetricsServiceClient(
     metrics::MetricsStateManager* state_manager)
     : metrics_state_manager_(state_manager),
       chromeos_metrics_provider_(NULL),
+      network_metrics_provider_(NULL),
       waiting_for_collect_final_metrics_step_(false),
       num_async_histogram_fetches_in_progress_(0),
       weak_ptr_factory_(this) {
   DCHECK(thread_checker_.CalledOnValidThread());
   RecordCommandLineMetrics();
   RegisterForNotifications();
 
 #if defined(OS_WIN)
   CountBrowserCrashDumpAttempts();
 #endif  // defined(OS_WIN)
@@ skipping 62 matching lines @@
   chrome::VersionInfo version_info;
   std::string version = version_info.Version();
 #if defined(ARCH_CPU_64_BITS)
   version += "-64";
 #endif  // defined(ARCH_CPU_64_BITS)
   if (!version_info.IsOfficialBuild())
     version.append("-devel");
   return version;
 }
 
-void ChromeMetricsServiceClient::OnLogUploadComplete() {
+void ChromeMetricsServiceClient::OnLogUploadComplete(bool upload_succeeded) {
+  // Record the network connection type.
+  if (upload_succeeded) {
+    UMA_HISTOGRAM_ENUMERATION(
+        "UMA.LogUpload.ConnetionType",
+        network_metrics_provider_->GetConnectionType(),
+        metrics::SystemProfileProto::Network::CONNECTION_BLUETOOTH + 1);

[Ilya Sherman, 2015/03/24 21:55:44]

Rather than using the enum type from the protocol buffer, which is really not
designed to be used in this way, how about skipping the
network_metrics_provider_ and just calling
net::NetworkChangeNotifier::GetConnectionType() directly?  In fact, you could
probably do this from the metrics component's code, rather than needing to
change the clients.

Actually, it might also be worth defining a separate enum specifically for
backing this histogram, since (a) the protobuf isn't guaranteed to preserve
CONNECTION_BLUETOOTH as the final element, and (b) the net layer code isn't
guaranteed to treat its enum as append-only.

[gayane -on leave until 09-2017, 2015/03/25 21:02:33]

For the enum eventually I have used net::NetworkChangeNotifier::ConnectionType.

+  }
+
   // Collect network stats after each UMA upload.
   network_stats_uploader_.CollectAndReportNetworkStats();
 }
 
 void ChromeMetricsServiceClient::StartGatheringMetrics(
     const base::Closure& done_callback) {
   finished_gathering_initial_metrics_callback_ = done_callback;
   base::Closure got_hardware_class_callback =
       base::Bind(&ChromeMetricsServiceClient::OnInitTaskGotHardwareClass,
                  weak_ptr_factory_.GetWeakPtr());
@@ skipping 41 matching lines @@
   return scoped_ptr<metrics::MetricsLogUploader>(
       new metrics::NetMetricsLogUploader(
           g_browser_process->system_request_context(),
           metrics::kDefaultMetricsServerUrl,
           metrics::kDefaultMetricsMimeType,
           on_upload_complete));
 }
 
 base::TimeDelta ChromeMetricsServiceClient::GetStandardUploadInterval() {
 #if defined(OS_ANDROID) || defined(OS_IOS)
-  bool is_cellular = false;
-  cellular_callback_.Run(&is_cellular);
-
-  if (is_cellular && IsCellularEnabledByExperiment())
+  if (network_metrics_provider_->IsCellularConnection() &&
+      IsCellularEnabledByExperiment())
     return base::TimeDelta::FromSeconds(kStandardUploadIntervalCellularSeconds);
 #endif
   return base::TimeDelta::FromSeconds(kStandardUploadIntervalSeconds);
 }
 
 base::string16 ChromeMetricsServiceClient::GetRegistryBackupKey() {
 #if defined(OS_WIN)
   return L"Software\\" PRODUCT_STRING_PATH L"\\StabilityMetrics";
 #else
   return base::string16();
@@ skipping 12 matching lines @@
 void ChromeMetricsServiceClient::Initialize() {
   metrics_service_.reset(new metrics::MetricsService(
       metrics_state_manager_, this, g_browser_process->local_state()));
 
   // Register metrics providers.
 #if defined(ENABLE_EXTENSIONS)
   metrics_service_->RegisterMetricsProvider(
       scoped_ptr<metrics::MetricsProvider>(
           new ExtensionsMetricsProvider(metrics_state_manager_)));
 #endif
-  scoped_ptr<metrics::NetworkMetricsProvider> network_metrics_provider(
-      new metrics::NetworkMetricsProvider(
-          content::BrowserThread::GetBlockingPool()));
-  cellular_callback_ = network_metrics_provider->GetConnectionCallback();
-  metrics_service_->RegisterMetricsProvider(network_metrics_provider.Pass());
+
+  network_metrics_provider_ = new metrics::NetworkMetricsProvider(
+      content::BrowserThread::GetBlockingPool());
+  metrics_service_->RegisterMetricsProvider(
+      scoped_ptr<metrics::NetworkMetricsProvider>(network_metrics_provider_));
 
   metrics_service_->RegisterMetricsProvider(
       scoped_ptr<metrics::MetricsProvider>(new OmniboxMetricsProvider));
   metrics_service_->RegisterMetricsProvider(
       scoped_ptr<metrics::MetricsProvider>(new ChromeStabilityMetricsProvider));
   metrics_service_->RegisterMetricsProvider(
       scoped_ptr<metrics::MetricsProvider>(new metrics::GPUMetricsProvider()));
 
-  profiler_metrics_provider_ =
-      new metrics::ProfilerMetricsProvider(cellular_callback_);
+  profiler_metrics_provider_ = new metrics::ProfilerMetricsProvider(
+      network_metrics_provider_->GetConnectionCallback());
   metrics_service_->RegisterMetricsProvider(
       scoped_ptr<metrics::MetricsProvider>(profiler_metrics_provider_));
 
 #if defined(OS_ANDROID)
   metrics_service_->RegisterMetricsProvider(
       scoped_ptr<metrics::MetricsProvider>(
           new AndroidMetricsProvider(g_browser_process->local_state())));
 #endif  // defined(OS_ANDROID)
 
 #if defined(OS_WIN)
@@ skipping 250 matching lines @@
   // Capture the histogram samples.
   if (dumps_with_crash != 0)
     UMA_HISTOGRAM_COUNTS("Chrome.BrowserDumpsWithCrash", dumps_with_crash);
   if (dumps_with_no_crash != 0)
     UMA_HISTOGRAM_COUNTS("Chrome.BrowserDumpsWithNoCrash", dumps_with_no_crash);
   int total_dumps = dumps_with_crash + dumps_with_no_crash;
   if (total_dumps != 0)
     UMA_HISTOGRAM_COUNTS("Chrome.BrowserCrashDumpAttempts", total_dumps);
 }
 #endif  // defined(OS_WIN)