Support needed to measure user and service traffic in Chrome.

components/data_use_measurement/content/data_use_measurement.cc

+// Copyright 2015 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 "components/data_use_measurement/content/data_use_measurement.h"
+
+#include "base/metrics/histogram.h"
+#include "base/metrics/sparse_histogram.h"
+#include "base/strings/stringprintf.h"
+#include "content/public/browser/resource_request_info.h"
+#include "net/base/network_change_notifier.h"
+#include "net/base/upload_data_stream.h"
+#include "net/http/http_response_headers.h"
+#include "net/url_request/url_request.h"
+
+namespace data_use_measurement {
+
+namespace {
+
+// Records the occurrence of |sample| in |name| histogram. Conventional UMA
+// histograms are not used because the |name| is not static.
+void RecordUMAHistogramCount(const std::string& name, int64_t sample) {
+  base::HistogramBase* histogram_pointer = base::Histogram::FactoryGet(
+      name,
+      1,        // Minimum sample size in bytes.
+      1000000,  // Maximum sample size in bytes. Should cover most of the
+                // requests by services.
+      50,       // Bucket count.
+      base::HistogramBase::kUmaTargetedHistogramFlag);
+  histogram_pointer->Add(sample);
+}
+
+// This function increases the value of |sample| bucket in |name| sparse
+// histogram by |value|. Conventional UMA histograms are not used because |name|
+// is not static.
+void IncreaseSparseHistogramByValue(const std::string& name,
+                                    int64_t sample,
+                                    int64_t value) {
+  base::HistogramBase* histogram = base::SparseHistogram::FactoryGet(
+      name, base::HistogramBase::kUmaTargetedHistogramFlag);
+  histogram->AddCount(sample, value);
+}
+
+}  // namespace
+
+DataUseMeasurement::DataUseMeasurement()
+#if defined(OS_ANDROID)
+    : app_state_(base::android::APPLICATION_STATE_HAS_RUNNING_ACTIVITIES),
+      app_listener_(new base::android::ApplicationStatusListener(
+          base::Bind(&DataUseMeasurement::OnApplicationStateChange,
+                     base::Unretained(this))))
+#endif
+{
+}
+
+DataUseMeasurement::~DataUseMeasurement(){};
+
+void DataUseMeasurement::ReportDataUseUMA(const net::URLRequest* request) {
+  const content::ResourceRequestInfo* info =
+      content::ResourceRequestInfo::ForRequest(request);
+  // Having |info| is the sign of a request for a web content from user. For now
+  // we could add a condition to check ProcessType in info is
+  // content::PROCESS_TYPE_RENDERER, but it won't be compatible with upcoming
+  // PlzNavigate architecture. So just existence of |info| is verified, and the
+  // current check should be compatible with upcoming changes in PlzNavigate.
+  bool is_user_traffic = info != nullptr;
+
+  // Counts rely on URLRequest::GetTotalReceivedBytes(), which does not include
+  // the send path, network layer overhead, TLS overhead, and DNS.
+  // TODO(amohammadkhan): Make these measured bytes more in line with number of
+  // bytes in lower levels.
+  int64_t request_body_bytes = 0;
+  int64_t request_header_bytes = 0;
+  int64_t total_upload_bytes = 0;
+  if (request->has_upload())
+    request_body_bytes = request->get_upload()->size();
+  net::HttpRequestHeaders request_headers;
+  if (request->GetFullRequestHeaders(&request_headers))
+    request_header_bytes = request_headers.ToString().length();
+  total_upload_bytes = request_body_bytes + request_header_bytes;

[mmenke, 2015/09/10 15:43:00]

Per earlier comments, all this upstream stuff is wrong.  We really should get
rid of it, until we can get the correct numbers.  Mixing good and bad numbers is
just a bad idea.

Suggest just waiting until https://codereview.chromium.org/1327763003/ lands
(Which I just signed off on), and then landing this with the correct new method
from the start.

[amohammadkhan, 2015/09/10 20:09:00]

Done. I made this CL dependent on that CL and used its function.

+  int64_t total_received_bytes = request->GetTotalReceivedBytes();
+
+  ReportDataUsageGeneral(
+      is_user_traffic ? REQUEST_INITIATOR_USER : REQUEST_INITIATOR_SERVICE,
+      UPSTREAM, total_upload_bytes);
+  ReportDataUsageGeneral(
+      is_user_traffic ? REQUEST_INITIATOR_USER : REQUEST_INITIATOR_SERVICE,
+      DOWNSTREAM, total_received_bytes);
+  DataUseUserData* attached_service_data = reinterpret_cast<DataUseUserData*>(
+      request->GetUserData(DataUseUserData::kUserDataKey));
+
+  if (!is_user_traffic) {

[mmenke, 2015/09/10 15:43:00]

optional:  I'd suggest just treating user traffic as its own service, rather
than recording some values in two histograms.

i.e.

DataUseUserData::ServiceName service_name = DataUseUserData::NOT_TAGGED;
if (content::ResourceRequestInfo::ForRequest(request)) {
  service_name = DataUseUserData::USER_DATA;
  DCHECK(!attached_service_data);
} else if (attached_service_data) {
  service_name = attached_service_data->service_type();
}

[amohammadkhan, 2015/09/10 20:09:00]

I see your point. But we are interested to have more granular info about user
traffic. (less data is recorded for each service in comparison to user data, for
example we have the distribution of request sizes for each dimension for user
traffic but we don't have it for services)

+    data_use_measurement::DataUseUserData::ServiceName service_type =
+        attached_service_data
+            ? attached_service_data->service_type()

[mmenke, 2015/09/10 15:43:00]

The naming here is inconsistent - the enumeration is ServiceName, but the local
variable and accessor are both called service_type - should be consistent.

[amohammadkhan, 2015/09/10 20:09:00]

Done.

+            : data_use_measurement::DataUseUserData::NOT_TAGGED;

[mmenke, 2015/09/10 15:43:00]

data_use_measurement:: namespace prefix not needed (Should search through this
file and remove it wherever it appears).

[amohammadkhan, 2015/09/10 20:09:00]

Done.

+    ReportDataUsageServices(service_type, UPSTREAM, total_upload_bytes);
+    ReportDataUsageServices(service_type, DOWNSTREAM, total_received_bytes);
+  }
+}
+
+#if defined(OS_ANDROID)
+void DataUseMeasurement::OnApplicationStateChange(
+    base::android::ApplicationState application_state) {
+  app_state_ = application_state;
+}
+#endif
+
+#if defined(OS_ANDROID)
+void DataUseMeasurement::OnApplicationStateChangeForTesting(

[mmenke, 2015/09/10 15:43:00]

Order in the cc file should match order in the header file.  Goes for all
methods.

[amohammadkhan, 2015/09/10 20:09:00]

Done.

+    base::android::ApplicationState application_state) {
+  app_state_ = application_state;
+}
+#endif
+
+DataUseMeasurement::AppState DataUseMeasurement::CurrentAppState() {
+#if defined(OS_ANDROID)
+  if (app_state_ != base::android::APPLICATION_STATE_HAS_RUNNING_ACTIVITIES)
+    return BACKGROUND;
+#endif
+  // If the OS is not Android, all the requests are considered Foreground.
+  return FOREGROUND;
+}
+
+const std::string DataUseMeasurement::GetSuffixForHistogramName(
+    TrafficDirection dir) {
+  AppState app_state = CurrentAppState();
+  bool is_conn_cellular = net::NetworkChangeNotifier::IsConnectionCellular(
+      net::NetworkChangeNotifier::GetConnectionType());
+  return base::StringPrintf(
+      ".%s.%s.%s", dir == UPSTREAM ? "Upstream" : "Downstream",
+      app_state == BACKGROUND ? "Background" : "Foreground",
+      is_conn_cellular ? "Cellular" : "NotCellular");
+}
+
+void DataUseMeasurement::ReportDataUsageServices(
+    data_use_measurement::DataUseUserData::ServiceName service,
+    TrafficDirection dir,
+    int64_t message_size) {
+  RecordUMAHistogramCount(
+      "DataUse.MessageSize." +
+          std::string(DataUseUserData::GetServiceNameAsString(service)),

[mmenke, 2015/09/10 15:43:00]

To reduce string operations by one, suggest passing in "DataUse.MessageSize" to
GetServiceNameAsString, and making it return the full string.

[amohammadkhan, 2015/09/10 20:09:00]

I am not sure if I understood it correctly. Please let me know if it is not what
you meant.

[mmenke, 2015/09/11 13:59:07]

below have:

     IncreaseSparseHistogramByValue(
          GetHistogramName("DataUse.MessageSize.AllServices",
GetSuffixForHistogramName(dir)));

[mmenke, 2015/09/11 14:00:03]

Oops...meant to delete the above comment - ignore it, just look at the two made
directly in your latest patch set.

+      message_size);
+  if (message_size > 0) {
+    IncreaseSparseHistogramByValue(
+        "DataUse.MessageSize.AllServices" + GetSuffixForHistogramName(dir),
+        service, message_size);

[mmenke, 2015/09/11 13:59:07]

Was thinking of having this here:

     IncreaseSparseHistogramByValue(
          GetHistogramName("DataUse.MessageSize.AllServices",
GetSuffixForHistogramName(dir)));

+  }
+}
+
+void DataUseMeasurement::ReportDataUsageGeneral(RequestInitiator service_type,

[mmenke, 2015/09/10 15:43:00]

Note that service_type and DataUseUserData::service_type() are of different
types, which is very confusing.  I've suggested renaming service_type(), but
this should also be renamed to request_initiator (And the same for the prototype
in the header)

[amohammadkhan, 2015/09/10 20:09:00]

Done.

+                                                TrafficDirection dir,
+                                                int64_t message_size) {
+  std::string histogram_name = base::StringPrintf(
+      "DataUse.TrafficSize.%s%s",
+      service_type == REQUEST_INITIATOR_USER ? "User" : "System",
+      GetSuffixForHistogramName(dir).c_str());
+  RecordUMAHistogramCount(histogram_name, message_size);
+}
+
+}  // namespace data_use_measurement