From f9907e347b6f3ef2c747ec3e46278f41d72faa2e Mon Sep 17 00:00:00 2001 From: "rtenneti@chromium.org" Date: Thu, 10 May 2012 04:26:32 +0000 Subject: NetConnectivity - Collect stats for TCP/UDP network connectivity when there is no proxy server. This will allow us to see if there is a difference in connectivity characteristics if proxy server is not involved. "NetConnectivity..NoProxy.Success...RTT" collects the RTT for RTT when there is no proxy. "NetConnectivity..NoProxy.Status.." collects the status of connectivity when there is no proxy. Exisiting histograms are not impacted. R=jar,eroman TEST=browser unit tests. Review URL: https://chromiumcodereview.appspot.com/10206035 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@136241 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/net/network_stats.cc | 153 +++++++++++++++++++++------ chrome/browser/net/network_stats.h | 67 +++++++++++- chrome/browser/net/network_stats_unittest.cc | 52 ++++++--- 3 files changed, 224 insertions(+), 48 deletions(-) (limited to 'chrome') diff --git a/chrome/browser/net/network_stats.cc b/chrome/browser/net/network_stats.cc index ae1457c..64023a4 100644 --- a/chrome/browser/net/network_stats.cc +++ b/chrome/browser/net/network_stats.cc @@ -16,10 +16,12 @@ #include "base/tuple.h" #include "chrome/common/chrome_version_info.h" #include "content/public/browser/browser_thread.h" +#include "googleurl/src/gurl.h" #include "net/base/net_errors.h" #include "net/base/net_util.h" #include "net/base/network_change_notifier.h" #include "net/base/test_completion_callback.h" +#include "net/proxy/proxy_service.h" #include "net/socket/tcp_client_socket.h" #include "net/udp/udp_client_socket.h" #include "net/udp/udp_server_socket.h" @@ -93,6 +95,7 @@ NetworkStats::NetworkStats() : load_size_(0), bytes_to_read_(0), bytes_to_send_(0), + has_proxy_server_(false), packets_to_send_(0), packets_sent_(0), base_packet_number_(0), @@ -107,13 +110,19 @@ NetworkStats::~NetworkStats() { bool NetworkStats::Start(net::HostResolver* host_resolver, const net::HostPortPair& server_host_port_pair, HistogramPortSelector histogram_port, + bool has_proxy_server, uint32 bytes_to_send, uint32 packets_to_send, const net::CompletionCallback& finished_callback) { + DCHECK(host_resolver); DCHECK(bytes_to_send); // We should have data to send. DCHECK_LE(packets_to_send, kMaximumPackets); - Initialize(bytes_to_send, histogram_port, packets_to_send, finished_callback); + Initialize(bytes_to_send, + histogram_port, + has_proxy_server, + packets_to_send, + finished_callback); net::HostResolver::RequestInfo request(server_host_port_pair); int rv = host_resolver->Resolve( @@ -129,6 +138,7 @@ bool NetworkStats::Start(net::HostResolver* host_resolver, void NetworkStats::Initialize( uint32 bytes_to_send, HistogramPortSelector histogram_port, + bool has_proxy_server, uint32 packets_to_send, const net::CompletionCallback& finished_callback) { DCHECK(bytes_to_send); // We should have data to send. @@ -139,6 +149,7 @@ void NetworkStats::Initialize( packets_to_send_ = packets_to_send; histogram_port_ = histogram_port; + has_proxy_server_ = has_proxy_server; finished_callback_ = finished_callback; } @@ -537,7 +548,7 @@ void NetworkStats::GetHistogramNames(const ProtocolValue& protocol, // Build "NetConnectivity..Status.." histogram // name. Total number of histograms are 2*5*2. - *status_histogram_name = base::StringPrintf( + *status_histogram_name = base::StringPrintf( "NetConnectivity.%s.Status.%d.%s", protocol_string, kPorts[port], @@ -546,7 +557,7 @@ void NetworkStats::GetHistogramNames(const ProtocolValue& protocol, // Build "NetConnectivity..PacketLoss.." histogram // name. Total number of histograms are 5 (because we do this test for UDP // only). - *packet_loss_histogram_name = base::StringPrintf( + *packet_loss_histogram_name = base::StringPrintf( "NetConnectivity.%s.PacketLoss6.%d.%s", protocol_string, kPorts[port], @@ -569,31 +580,40 @@ void NetworkStats::RecordHistograms(const ProtocolValue& protocol, &status_histogram_name, &packet_loss_histogram_name); - // For packet loss test, just record packet loss data. - if (packets_to_send_ > 1) { - base::Histogram* packet_loss_histogram = base::LinearHistogram::FactoryGet( - packet_loss_histogram_name, - 1, - 2 << kMaximumPackets, - (2 << kMaximumPackets) + 1, - base::Histogram::kUmaTargetedHistogramFlag); - packet_loss_histogram->Add(packets_received_mask_); - return; - } + // If we are running without a proxy, we'll generate 2 distinct histograms in + // each case, one will have the ".NoProxy" suffix. + size_t histogram_count = has_proxy_server_ ? 1 : 2; + for (size_t i = 0; i < histogram_count; i++) { + // For packet loss test, just record packet loss data. + if (packets_to_send_ > 1) { + base::Histogram* histogram = base::LinearHistogram::FactoryGet( + packet_loss_histogram_name, + 1, + 2 << kMaximumPackets, + (2 << kMaximumPackets) + 1, + base::Histogram::kUmaTargetedHistogramFlag); + histogram->Add(packets_received_mask_); + packet_loss_histogram_name.append(".NoProxy"); + // Packet loss histograms don't measure times or status. + continue; + } - if (result == net::OK) { - base::Histogram* rtt_histogram = base::Histogram::FactoryTimeGet( - rtt_histogram_name, - base::TimeDelta::FromMilliseconds(10), - base::TimeDelta::FromSeconds(60), 50, + if (result == net::OK) { + base::Histogram* rtt_histogram = base::Histogram::FactoryTimeGet( + rtt_histogram_name, + base::TimeDelta::FromMilliseconds(10), + base::TimeDelta::FromSeconds(60), 50, + base::Histogram::kUmaTargetedHistogramFlag); + rtt_histogram->AddTime(duration); + rtt_histogram_name.append(".NoProxy"); + } + + base::Histogram* status_histogram = base::LinearHistogram::FactoryGet( + status_histogram_name, 1, STATUS_MAX, STATUS_MAX+1, base::Histogram::kUmaTargetedHistogramFlag); - rtt_histogram->AddTime(duration); + status_histogram->Add(status); + status_histogram_name.append(".NoProxy"); } - - base::Histogram* status_histogram = base::LinearHistogram::FactoryGet( - status_histogram_name, 1, STATUS_MAX, STATUS_MAX+1, - base::Histogram::kUmaTargetedHistogramFlag); - status_histogram->Add(status); } // UDPStatsClient methods and members. @@ -714,6 +734,55 @@ void TCPStatsClient::Finish(Status status, int result) { delete this; } +// ProxyDetector methods and members. +ProxyDetector::ProxyDetector(net::ProxyService* proxy_service, + const net::HostPortPair& server_address, + OnResolvedCallback callback) + : proxy_service_(proxy_service), + server_address_(server_address), + callback_(callback), + has_pending_proxy_resolution_(false) { +} + +ProxyDetector::~ProxyDetector() { + CHECK(!has_pending_proxy_resolution_); +} + +void ProxyDetector::StartResolveProxy() { + std::string url = + base::StringPrintf("https://%s", server_address_.ToString().c_str()); + GURL gurl(url); + + has_pending_proxy_resolution_ = true; + DCHECK(proxy_service_); + int rv = proxy_service_->ResolveProxy( + gurl, + &proxy_info_, + base::Bind(&ProxyDetector::OnResolveProxyComplete, + base::Unretained(this)), + NULL, + net::BoundNetLog()); + if (rv != net::ERR_IO_PENDING) + OnResolveProxyComplete(rv); +} + +void ProxyDetector::OnResolveProxyComplete(int result) { + has_pending_proxy_resolution_ = false; + bool has_proxy_server = (result == net::OK && + proxy_info_.proxy_server().is_valid() && + !proxy_info_.proxy_server().is_direct()); + + OnResolvedCallback callback = callback_; + BrowserThread::PostTask( + BrowserThread::IO, + FROM_HERE, + base::Bind(callback, has_proxy_server)); + + // TODO(rtenneti): Will we leak if ProxyResolve is cancelled (or proxy + // resolution never completes). + delete this; +} + // static void CollectNetworkStats(const std::string& network_stats_server, IOThread* io_thread) { @@ -739,8 +808,6 @@ void CollectNetworkStats(const std::string& network_stats_server, CR_DEFINE_STATIC_LOCAL(scoped_refptr, trial, ()); static bool collect_stats = false; - - static uint32 port; static NetworkStats::HistogramPortSelector histogram_port; if (!trial.get()) { @@ -779,7 +846,6 @@ void CollectNetworkStats(const std::string& network_stats_server, base::RandInt(NetworkStats::PORT_53, NetworkStats::PORT_8080)); DCHECK_GE(histogram_port, NetworkStats::PORT_53); DCHECK_LE(histogram_port, NetworkStats::PORT_8080); - port = kPorts[histogram_port]; } } @@ -797,15 +863,34 @@ void CollectNetworkStats(const std::string& network_stats_server, net::HostResolver* host_resolver = io_thread->globals()->host_resolver.get(); DCHECK(host_resolver); - net::HostPortPair server_address(network_stats_server, port); + net::HostPortPair server_address(network_stats_server, + kPorts[histogram_port]); + + net::ProxyService* proxy_service = + io_thread->globals()->system_proxy_service.get(); + DCHECK(proxy_service); + + ProxyDetector::OnResolvedCallback callback = + base::Bind(&StartNetworkStatsTest, + host_resolver, server_address, histogram_port); + ProxyDetector* proxy_client = new ProxyDetector( + proxy_service, server_address, callback); + proxy_client->StartResolveProxy(); +} + +// static +void StartNetworkStatsTest(net::HostResolver* host_resolver, + const net::HostPortPair& server_address, + NetworkStats::HistogramPortSelector histogram_port, + bool has_proxy_server) { int experiment_to_run = base::RandInt(1, 5); switch (experiment_to_run) { case 1: { UDPStatsClient* small_udp_stats = new UDPStatsClient(); small_udp_stats->Start( - host_resolver, server_address, histogram_port, + host_resolver, server_address, histogram_port, has_proxy_server, kSmallTestBytesToSend, 1, net::CompletionCallback()); } break; @@ -814,7 +899,7 @@ void CollectNetworkStats(const std::string& network_stats_server, { UDPStatsClient* large_udp_stats = new UDPStatsClient(); large_udp_stats->Start( - host_resolver, server_address, histogram_port, + host_resolver, server_address, histogram_port, has_proxy_server, kLargeTestBytesToSend, 1, net::CompletionCallback()); } break; @@ -823,7 +908,7 @@ void CollectNetworkStats(const std::string& network_stats_server, { TCPStatsClient* small_tcp_client = new TCPStatsClient(); small_tcp_client->Start( - host_resolver, server_address, histogram_port, + host_resolver, server_address, histogram_port, has_proxy_server, kSmallTestBytesToSend, 1, net::CompletionCallback()); } break; @@ -832,7 +917,7 @@ void CollectNetworkStats(const std::string& network_stats_server, { TCPStatsClient* large_tcp_client = new TCPStatsClient(); large_tcp_client->Start( - host_resolver, server_address, histogram_port, + host_resolver, server_address, histogram_port, has_proxy_server, kLargeTestBytesToSend, 1, net::CompletionCallback()); } break; @@ -841,7 +926,7 @@ void CollectNetworkStats(const std::string& network_stats_server, { UDPStatsClient* packet_loss_udp_stats = new UDPStatsClient(); packet_loss_udp_stats->Start( - host_resolver, server_address, histogram_port, + host_resolver, server_address, histogram_port, has_proxy_server, kLargeTestBytesToSend, kMaximumPackets, net::CompletionCallback()); } break; diff --git a/chrome/browser/net/network_stats.h b/chrome/browser/net/network_stats.h index b36cbec..0a58d33 100644 --- a/chrome/browser/net/network_stats.h +++ b/chrome/browser/net/network_stats.h @@ -21,6 +21,7 @@ #include "net/base/io_buffer.h" #include "net/base/ip_endpoint.h" #include "net/base/test_data_stream.h" +#include "net/proxy/proxy_info.h" #include "net/socket/socket.h" namespace chrome_browser_net { @@ -101,6 +102,7 @@ class NetworkStats { bool Start(net::HostResolver* host_resolver, const net::HostPortPair& server, HistogramPortSelector histogram_port, + bool has_proxy_server, uint32 bytes_to_send, uint32 packets_to_send, const net::CompletionCallback& callback); @@ -109,6 +111,7 @@ class NetworkStats { // Constructs an NetworkStats object that collects metrics for network // connectivity (either TCP or UDP). NetworkStats(); + // NetworkStats is deleted when Finish() is called. virtual ~NetworkStats(); // Initializes |finished_callback_| and the number of bytes to send to the @@ -116,6 +119,7 @@ class NetworkStats { // |finished_callback| is mainly useful for unittests. void Initialize(uint32 bytes_to_send, HistogramPortSelector histogram_port, + bool has_proxy_server, uint32 packets_to_send, const net::CompletionCallback& finished_callback); @@ -271,6 +275,9 @@ class NetworkStats { // connectivity. HistogramPortSelector histogram_port_; + // |has_proxy_server_| specifies if there is a proxy server or not. + bool has_proxy_server_; + // HostResolver fills out the |addresses_| after host resolution is completed. net::AddressList addresses_; @@ -298,6 +305,7 @@ class UDPStatsClient : public NetworkStats { // Constructs an UDPStatsClient object that collects metrics for UDP // connectivity. UDPStatsClient(); + // UDPStatsClient is deleted when Finish() is called. virtual ~UDPStatsClient(); protected: @@ -315,7 +323,8 @@ class UDPStatsClient : public NetworkStats { virtual bool ReadComplete(int result) OVERRIDE; // Collects stats for UDP connectivity. This is called when all the data from - // server is read or when there is a failure during connect/read/write. + // server is read or when there is a failure during connect/read/write. This + // object is deleted at the end of this method. virtual void Finish(Status status, int result) OVERRIDE; }; @@ -324,6 +333,7 @@ class TCPStatsClient : public NetworkStats { // Constructs a TCPStatsClient object that collects metrics for TCP // connectivity. TCPStatsClient(); + // TCPStatsClient is deleted when Finish() is called. virtual ~TCPStatsClient(); protected: @@ -339,7 +349,8 @@ class TCPStatsClient : public NetworkStats { virtual bool ReadComplete(int result) OVERRIDE; // Collects stats for TCP connectivity. This is called when all the data from - // server is read or when there is a failure during connect/read/write. + // server is read or when there is a failure during connect/read/write. This + // object is deleted at the end of this method. virtual void Finish(Status status, int result) OVERRIDE; private: @@ -348,6 +359,49 @@ class TCPStatsClient : public NetworkStats { void OnConnectComplete(int result); }; +class ProxyDetector { + public: + // Used for the callback that is called from |OnResolveProxyComplete|. + typedef base::Callback OnResolvedCallback; + + // Constructs a ProxyDetector object that finds out if access to + // |server_address| goes through a proxy server or not. Calls the |callback| + // after proxy resolution is completed by currying the proxy resolution + // status. + ProxyDetector(net::ProxyService* proxy_service, + const net::HostPortPair& server_address, + OnResolvedCallback callback); + + // This method uses |proxy_service_| to resolve the proxy for + // |server_address_|. + void StartResolveProxy(); + + private: + // This object is deleted from |OnResolveProxyComplete|. + ~ProxyDetector(); + + // Calls the |callback_| by currying the proxy resolution status. + void OnResolveProxyComplete(int result); + + // |proxy_service_| specifies the proxy service that is to be used to find + // if access to |server_address_| goes through proxy server or not. + net::ProxyService* proxy_service_; + + // |server_address_| specifies the server host and port pair for which we are + // trying to see if access to it, goes through proxy or not. + net::HostPortPair server_address_; + + // |callback_| will be called after proxy resolution is completed. + OnResolvedCallback callback_; + + // |proxy_info_| holds proxy information returned by ResolveProxy. + net::ProxyInfo proxy_info_; + + // Indicates if there is a pending a proxy resolution. We use this to assert + // that there is no in-progress proxy resolution request. + bool has_pending_proxy_resolution_; +}; + // This collects the network connectivity stats for UDP and TCP for small // percentage of users who are participating in the experiment. All users must // have enabled "UMA upload". This method gets called only if UMA upload to the @@ -355,6 +409,15 @@ class TCPStatsClient : public NetworkStats { void CollectNetworkStats(const std::string& network_stats_server_url, IOThread* io_thread); +// This starts a test randomly selected among "TCP test with small packet size", +// "TCP test with large packet size", "UDP test with small packet size", "UDP +// test with large packet size" and "UDP multi packet loss" tests to collect the +// network connectivity stats. +void StartNetworkStatsTest(net::HostResolver* host_resolver, + const net::HostPortPair& server_address, + NetworkStats::HistogramPortSelector histogram_port, + bool has_proxy_server); + } // namespace chrome_browser_net #endif // CHROME_BROWSER_NET_NETWORK_STATS_H_ diff --git a/chrome/browser/net/network_stats_unittest.cc b/chrome/browser/net/network_stats_unittest.cc index ec287c9..2bd531c 100644 --- a/chrome/browser/net/network_stats_unittest.cc +++ b/chrome/browser/net/network_stats_unittest.cc @@ -36,7 +36,7 @@ class NetworkStatsTestUDP : public NetworkStatsTest { } protected: - void RunUDPEchoTest(int bytes, int packets) { + void RunUDPEchoTest(int bytes, int packets, bool has_proxy) { net::TestCompletionCallback cb; scoped_ptr host_resolver( @@ -48,6 +48,7 @@ class NetworkStatsTestUDP : public NetworkStatsTest { EXPECT_TRUE(udp_stats_client->Start(host_resolver.get(), test_server_.host_port_pair(), NetworkStats::HISTOGRAM_PORT_MAX, + has_proxy, bytes, packets, cb.callback())); @@ -68,7 +69,7 @@ class NetworkStatsTestTCP : public NetworkStatsTest { } protected: - void RunTCPEchoTest(int bytes, int packets) { + void RunTCPEchoTest(int bytes, int packets, bool has_proxy) { net::TestCompletionCallback cb; scoped_ptr host_resolver( @@ -80,6 +81,7 @@ class NetworkStatsTestTCP : public NetworkStatsTest { EXPECT_TRUE(tcp_stats_client->Start(host_resolver.get(), test_server_.host_port_pair(), NetworkStats::HISTOGRAM_PORT_MAX, + has_proxy, bytes, packets, cb.callback())); @@ -91,29 +93,54 @@ class NetworkStatsTestTCP : public NetworkStatsTest { net::TestServer test_server_; }; -TEST_F(NetworkStatsTestUDP, UDPEcho_100B_Of_Data) { +TEST_F(NetworkStatsTestUDP, UDPEcho100BytesHasProxy) { ASSERT_TRUE(test_server_.Start()); - RunUDPEchoTest(100, 1); + RunUDPEchoTest(100, 1, true); } -TEST_F(NetworkStatsTestUDP, UDPEcho_1K_Of_Data) { +TEST_F(NetworkStatsTestUDP, UDPEcho100BytesNoProxy) { ASSERT_TRUE(test_server_.Start()); - RunUDPEchoTest(1024, 1); + RunUDPEchoTest(100, 1, false); } -TEST_F(NetworkStatsTestUDP, UDPEchoMultiplePackets) { +TEST_F(NetworkStatsTestUDP, UDPEcho1KBytesHasProxy) { ASSERT_TRUE(test_server_.Start()); - RunUDPEchoTest(1024, 4); + RunUDPEchoTest(1024, 1, true); } -TEST_F(NetworkStatsTestTCP, TCPEcho_100B_Of_Data) { +TEST_F(NetworkStatsTestUDP, UDPEcho1KBytesNoProxy) { ASSERT_TRUE(test_server_.Start()); - RunTCPEchoTest(100, 1); + RunUDPEchoTest(1024, 1, false); } -TEST_F(NetworkStatsTestTCP, TCPEcho_1K_Of_Data) { +TEST_F(NetworkStatsTestUDP, UDPEchoMultiplePacketsHasProxy) { ASSERT_TRUE(test_server_.Start()); - RunTCPEchoTest(1024, 1); + RunUDPEchoTest(1024, 4, true); +} + +TEST_F(NetworkStatsTestUDP, UDPEchoMultiplePacketsNoProxy) { + ASSERT_TRUE(test_server_.Start()); + RunUDPEchoTest(1024, 4, false); +} + +TEST_F(NetworkStatsTestTCP, TCPEcho100BytesHasProxy) { + ASSERT_TRUE(test_server_.Start()); + RunTCPEchoTest(100, 1, true); +} + +TEST_F(NetworkStatsTestTCP, TCPEcho100BytesNoProxy) { + ASSERT_TRUE(test_server_.Start()); + RunTCPEchoTest(100, 1, false); +} + +TEST_F(NetworkStatsTestTCP, TCPEcho1KBytesHasProxy) { + ASSERT_TRUE(test_server_.Start()); + RunTCPEchoTest(1024, 1, true); +} + +TEST_F(NetworkStatsTestTCP, TCPEcho1KBytesNoProxy) { + ASSERT_TRUE(test_server_.Start()); + RunTCPEchoTest(1024, 1, false); } TEST_F(NetworkStatsTestTCP, VerifyBytes) { @@ -124,6 +151,7 @@ TEST_F(NetworkStatsTestTCP, VerifyBytes) { net::TestCompletionCallback cb; network_stats.Initialize(KBytesToSend, NetworkStats::HISTOGRAM_PORT_MAX, + true, packets_to_send, cb.callback()); -- cgit v1.1