NQE: Record correlation metric in UMA

net/nqe/network_quality_estimator.h

 // 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.
 
 #ifndef NET_NQE_NETWORK_QUALITY_ESTIMATOR_H_
 #define NET_NQE_NETWORK_QUALITY_ESTIMATOR_H_
 
 #include <stdint.h>
 
 #include <map>
@@ skipping 484 matching lines @@
     EXTERNAL_ESTIMATE_PROVIDER_STATUS_CALLBACK,
     EXTERNAL_ESTIMATE_PROVIDER_STATUS_RTT_AVAILABLE,
     EXTERNAL_ESTIMATE_PROVIDER_STATUS_DOWNLINK_BANDWIDTH_AVAILABLE,
     EXTERNAL_ESTIMATE_PROVIDER_STATUS_BOUNDARY
   };
 
   // Records the metrics related to external estimate provider.
   void RecordExternalEstimateProviderMetrics(
       NQEExternalEstimateProviderStatus status) const;
 
+  // Records correlation metric that can be used for computing the correlation

[bengr, 2016/07/08 16:49:52]

Records -> Records a
HTTP -> HTTP-layer
transport -> transport-layer

What does "time taken to fetch |request|" mean?

[tbansal1, 2016/07/08 17:45:51]

Done.
Re-worded. Not sure it is useful to explain more (code is pretty obvious).

+  // between HTTP RTT, transport RTT, throughput and the time taken to fetch
+  // |request|.
+  void RecordCorrelationMetric(const URLRequest& request) const;
+
   // Determines if the requests to local host can be used in estimating the
   // network quality. Set to true only for tests.
   bool use_localhost_requests_;
 
   // Determines if the responses smaller than |kMinTransferSizeInBytes|
   // or shorter than |kMinTransferSizeInBytes| can be used in estimating the
   // network quality. Set to true only for tests.
   bool use_small_responses_;
 
   // The factor by which the weight of an observation reduces every second.
@@ skipping 82 matching lines @@
   // |downstream_throughput_kbps_observations_|, which are later used for
   // estimating the throughput.
   std::unique_ptr<nqe::internal::ThroughputAnalyzer> throughput_analyzer_;
 
   // Current effective connection type. It is updated on connection change
   // events. It is also updated every time there is network traffic (provided
   // the last computation was more than
   // |effective_connection_type_recomputation_interval_| ago).
   EffectiveConnectionType effective_connection_type_;
 
+  // Correlation UMA is recorded with |correlation_logging_probability_|
+  // probability. e.g., if it is 0.0, then the UMA will never be recorded. On
+  // the other hand, if it is 1.0, then it will be recorded for all valid HTTP
+  // requests.

[bengr, 2016/07/08 16:49:52]

Why is it not recorded for all requests? If this is a performance optimization,
say so, and also provide some intuitions as to why performance would suffer with
a 1.0 probability.

[tbansal1, 2016/07/08 17:45:51]

Added more comments.

+  const double correlation_logging_probability_;

[bengr, 2016/07/08 16:49:52]

rename: correlation_uma_reporting_probability_;

[tbansal1, 2016/07/08 17:45:51]

Done.

+
   base::ThreadChecker thread_checker_;
 
   base::WeakPtrFactory<NetworkQualityEstimator> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(NetworkQualityEstimator);
 };
 
 }  // namespace net
 
 #endif  // NET_NQE_NETWORK_QUALITY_ESTIMATOR_H_