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/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.
+      1000000,  // Maximum sample size. Should cover most of the requests by

[Alexei Svitkine (slow), 2015/08/31 17:26:31]

What's the units?

[amohammadkhan, 2015/09/01 04:52:48]

Done.

+                // 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 the
+// |name| is not static.
+void RecordSparseHistogramWithValue(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() {
+#if defined(OS_ANDROID)
+  // |app_listener_| is destroyed before DataUseMeasurement instance is
+  // destroyed, so a callback isn't sent between the DataUseMeasurement instance
+  // being destroyed and the |app_listener_| being destroyed.
+  app_listener_.reset();
+#endif
+}
+void DataUseMeasurement::ReportDataUseUMA(const net::URLRequest* request) {
+  bool is_user_traffic = false;
+
+  const content::ResourceRequestInfo* info =

[blundell, 2015/08/31 09:15:40]

IIUC, the only usage of //content in this code is to check whether a request is
coming from the user. I would move the DataUseMeasurement class to the core code
and have it take in a DataUseMeasurementDelegate, which would be an interface
defined in the core code with a //content-based implementation in
//components/data_use_measurement/content. Currently this interface would have
only one method: RequestIsFromUser(net::URLRequest* request). In the future the
interface could expand as needed.

This organization will set up the code better for sharing with iOS both now and
going forward IMO.

[amohammadkhan, 2015/09/01 04:52:48]

Thanks for heads up. I see your point. But currently IOS is out of scope of our
project and our main focus is on Android. So if you think it is not necessary, I
thought it may be better to land this CL without this change and start getting
data before I run out of time in my internship and improve the architecture
later For example while I am getting data (of course if you don't think this
change is crucial now)

+      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.
+  if (info) {
+    is_user_traffic = true;
+  }
+
+  // These number won't be the number of bytes handed to the kernel because
+  // session layer framing and compression is not being accounted for.
+  // 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();

[mmenke, 2015/08/31 17:41:29]

This is -1 for chunked uploads, which a lot of internal requests are.  I don't
think there's currently any reliable way to measure upstream traffic.

[amohammadkhan, 2015/09/01 04:52:48]

After reading your comment, I wanted to try to understand the cases which the
request is chunked but I couldn't find any cases. (I checked by using,
request->get_upload()->is_chunked() ) Is it a right function to check if the
upload is chunked? If yes, do you know why I couldn't find any chunked upload?
Are they more common in specific cases?
 
I know my method, for getting byte sizes for upload and even download is not
100% accurate, but do you think have they the minimum accuracy to get a high
level understanding to compare data use of different services? When I manually
check the numbers, they seem reasonable. 

[mmenke, 2015/09/01 19:23:37]

sclittle is working on adding methods to expose a reasonable approximation of
bytes sense, which works like GetTotalReceivedBytes.  Didn't know about the
effort when I made that comment.  The stuff he's working on is definitely the
method you want to use.

+  net::HttpRequestHeaders request_headers;
+  if (request->GetFullRequestHeaders(&request_headers))

[mmenke, 2015/08/31 17:41:29]

This method doesn't really work - it's not fully hooked up, or reliable.  You
shouldn't be using this.

[amohammadkhan, 2015/09/01 04:52:48]

Do you have any suggestion for a better substitute? Or you think we do not have
any good option to do it?

+    request_header_bytes = request_headers.ToString().length();

[mmenke, 2015/08/31 17:41:28]

Even if the method worked, this is only true for HTTP and SPDY.

[mmenke, 2015/08/31 17:42:06]

"For HTTP, and not SPDY (Or QUIC)", rather

[amohammadkhan, 2015/09/01 04:52:48]

I see your points. For now I am trying to get a rough estimation of data use of
different services, but I've heard that total send and total received bytes are
getting updated and accurate by someone else, so later I'll be able to update my
code and benefit those updates. But till then, I am trying to have some
estimation about them. 

So please let me know if you think if there is any good way without doing
complex plumbing to increase the accuracy. (Do you think can we have a rough
estimation by current values or the current values are not even accurate enough
for rough estimation?)

[amohammadkhan, 2015/09/01 04:52:48]

I agree.

+  total_upload_bytes = request_body_bytes + request_header_bytes;
+  int64_t total_received_bytes = request->GetTotalReceivedBytes();
+
+  ReportDataUsageURLRequest(is_user_traffic ? USER_TRAFFIC : SERVICE_TRAFFIC,
+                            UPLOAD, total_upload_bytes);
+  ReportDataUsageURLRequest(is_user_traffic ? USER_TRAFFIC : SERVICE_TRAFFIC,
+                            DOWNLOAD, total_received_bytes);

[mmenke, 2015/08/31 17:41:29]

Having "service traffic" an independent dimension from attached_service_data
seems confusing, and doubling up on UMA seems wasteful, with sparse histograms.

My suggestion:  Put SERVICE_TRAFFIC with no known service type into one bucket,
and bucket SERVICE_TRAFFIC with attached service data into the bucket
corresponding to that service.  Remove the redundant ReportDataUsageURLRequest /
ReportDataUsage calls.

[amohammadkhan, 2015/09/01 04:52:48]

Good idea. I made a bucket for all untagged services ("NotTagged"), but it may
still be good to keep the SERVICE_TRAFFIC, because it will have the traffic of
all services for different dimension that we do not have in detail for services.
(To reduce the number of histograms, just we have total exchanged bytes for each
service in each dimension and we do not have the distribution of exchanged
bytes)

+  DataUseUserData* attached_service_data = reinterpret_cast<DataUseUserData*>(
+      request->GetUserData(DataUseUserData::kUserDataKey));
+
+  if (attached_service_data) {
+    ReportDataUsage(attached_service_data->service_type(), UPLOAD,
+                    total_upload_bytes);
+    ReportDataUsage(attached_service_data->service_type(), DOWNLOAD,
+                    total_received_bytes);
+  }
+}
+
+#if defined(OS_ANDROID)
+void DataUseMeasurement::OnApplicationStateChange(
+    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;
+}
+
+DataUseMeasurement::ConnectionType DataUseMeasurement::CurrentConnectionType() {

[mmenke, 2015/08/31 17:41:29]

I don't think having this as a method, or mapping one enum to another, really
gets you anything here.  Suggest just inlining the logic in
GetDimensionForHistogramName, and using a bool rather than an enum.  Mapping the
enum returned by net::NetworkChangeNotifier::GetConnectionType to a bool to an
enum and then using that as a bool just seems a bit much.

[amohammadkhan, 2015/09/01 04:52:48]

Done.

+  if (net::NetworkChangeNotifier::IsConnectionCellular(
+          net::NetworkChangeNotifier::GetConnectionType())) {
+    return CELLULAR;
+  }
+  return NOT_CELLULAR;
+}
+
+std::string DataUseMeasurement::GetDimensionForHistogramName(

[mmenke, 2015/08/31 17:41:29]

Dimension -> Suffix?  This results a string,not dimensions.

[amohammadkhan, 2015/09/01 04:52:48]

Done.

+    TrafficDirection dir) {
+  AppState app_state = CurrentAppState();
+  ConnectionType conn_type = CurrentConnectionType();
+  return base::StringPrintf(
+      ".%s.%s.%s", dir == UPLOAD ? "Upload" : "Download",

[mmenke, 2015/08/31 17:41:29]

This isn't categorizing requests as uploads or downloads, right?  It's for bytes
in the two direction.  upstream and downstream would make that clearer.

[amohammadkhan, 2015/09/01 04:52:48]

Done.

+      app_state == BACKGROUND ? "Background" : "Foreground",
+      conn_type == NOT_CELLULAR ? "NotCellular" : "Cellular");
+}
+
+void DataUseMeasurement::ReportDataUsage(
+    data_use_measurement::DataUseUserData::ServiceType service,
+    TrafficDirection dir,
+    int64_t message_size) {
+  std::string service_histogram_name =
+      base::StringPrintf("DataUse.Service.%s", GetServiceName(service));
+  std::string combined_histogram_name =
+      "DataUse.Services" + GetDimensionForHistogramName(dir);

[mmenke, 2015/08/31 17:41:29]

Can't these two lines be combined, like in ReportDataUsageURLRequest?

[amohammadkhan, 2015/09/01 04:52:48]

Our ideal was to have all dimensions for all services, but finally we
compromised to these two histogram types:

DataUse.Service.ServiceName
   shows distribution of exchanged data for all dimensions combined.
DataUse.Services.Dimensions
   (Have ServiceNames as buckets)
   To have total dataUse of services in different dimensions.

+  RecordUMAHistogramCount(service_histogram_name, message_size);
+  if (message_size > 0) {
+    RecordSparseHistogramWithValue(combined_histogram_name, service,
+                                   message_size);
+  }
+}
+
+void DataUseMeasurement::ReportDataUsageURLRequest(
+    ChromeTrafficType service_type,
+    TrafficDirection dir,
+    int64_t message_size) {
+  std::string histogram_name = base::StringPrintf(
+      "DataUse.%s%s", service_type == USER_TRAFFIC ? "User" : "NotUser",
+      GetDimensionForHistogramName(dir).c_str());
+  RecordUMAHistogramCount(histogram_name, message_size);
+}
+
+}  // namespace data_use_measurement