diff options
author | megjablon@chromium.org <megjablon@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-07-01 19:01:27 +0000 |
---|---|---|
committer | megjablon@chromium.org <megjablon@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-07-01 19:01:27 +0000 |
commit | adc6eb89e31942c80eda20db6abb1391c5b0a159 (patch) | |
tree | d1952d7146f9887cf2af1d4dd15e8cc2bd5fb8fe | |
parent | 741a177f3c1c8930e84dc3ced31bb50150c0b547 (diff) | |
download | chromium_src-adc6eb89e31942c80eda20db6abb1391c5b0a159.zip chromium_src-adc6eb89e31942c80eda20db6abb1391c5b0a159.tar.gz chromium_src-adc6eb89e31942c80eda20db6abb1391c5b0a159.tar.bz2 |
Update data reduction bypass UMA to reflect modern usage.
Created new UMAs DataReductionProxy.BypassType{Primary|Fallback} and DataReductionProxy.BlockType{Primary|Fallback} that add bypass types to more specifically track the bypass reason. Deprecated DataReductionProxy.BypassInfo{Primary|Fallback}.
BUG=373978
Review URL: https://codereview.chromium.org/348553006
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@280857 0039d316-1c4b-4281-b951-d872f2087c98
7 files changed, 187 insertions, 60 deletions
diff --git a/components/data_reduction_proxy/browser/data_reduction_proxy_protocol.cc b/components/data_reduction_proxy/browser/data_reduction_proxy_protocol.cc index 2ec31e7..a013f72 100644 --- a/components/data_reduction_proxy/browser/data_reduction_proxy_protocol.cc +++ b/components/data_reduction_proxy/browser/data_reduction_proxy_protocol.cc @@ -56,8 +56,8 @@ bool MaybeBypassProxyAndPrepareToRetry( return false; DataReductionProxyInfo data_reduction_proxy_info; - net::ProxyService::DataReductionProxyBypassEventType bypass_type = - GetDataReductionProxyBypassEventType( + net::ProxyService::DataReductionProxyBypassType bypass_type = + GetDataReductionProxyBypassType( original_response_headers, &data_reduction_proxy_info); if (bypass_type == net::ProxyService::BYPASS_EVENT_TYPE_MAX) { return false; @@ -68,7 +68,10 @@ bool MaybeBypassProxyAndPrepareToRetry( net::ProxyServer proxy_server; SetProxyServerFromGURL(data_reduction_proxies.first, &proxy_server); request->context()->proxy_service()->RecordDataReductionProxyBypassInfo( - !data_reduction_proxies.second.is_empty(), proxy_server, bypass_type); + !data_reduction_proxies.second.is_empty(), + data_reduction_proxy_info.bypass_all, + proxy_server, + bypass_type); MarkProxiesAsBadUntil(request, data_reduction_proxy_info.bypass_duration, diff --git a/components/data_reduction_proxy/common/data_reduction_proxy_headers.cc b/components/data_reduction_proxy/common/data_reduction_proxy_headers.cc index 773d14c..b5c6119 100644 --- a/components/data_reduction_proxy/common/data_reduction_proxy_headers.cc +++ b/components/data_reduction_proxy/common/data_reduction_proxy_headers.cc @@ -103,23 +103,35 @@ bool HasDataReductionProxyViaHeader(const net::HttpResponseHeaders* headers) { return false; } -net::ProxyService::DataReductionProxyBypassEventType -GetDataReductionProxyBypassEventType( +const int kShortBypassMaxSeconds = 59; +const int kMediumBypassMaxSeconds = 300; +net::ProxyService::DataReductionProxyBypassType +GetDataReductionProxyBypassType( const net::HttpResponseHeaders* headers, DataReductionProxyInfo* data_reduction_proxy_info) { DCHECK(data_reduction_proxy_info); if (GetDataReductionProxyInfo(headers, data_reduction_proxy_info)) { // A chrome-proxy response header is only present in a 502. For proper // reporting, this check must come before the 5xx checks below. - if (data_reduction_proxy_info->bypass_duration < TimeDelta::FromMinutes(30)) + const TimeDelta& duration = data_reduction_proxy_info->bypass_duration; + if (duration <= TimeDelta::FromSeconds(kShortBypassMaxSeconds)) return ProxyService::SHORT_BYPASS; + if (duration <= TimeDelta::FromSeconds(kMediumBypassMaxSeconds)) + return ProxyService::MEDIUM_BYPASS; return ProxyService::LONG_BYPASS; } - if (headers->response_code() == net::HTTP_INTERNAL_SERVER_ERROR || - headers->response_code() == net::HTTP_BAD_GATEWAY || - headers->response_code() == net::HTTP_SERVICE_UNAVAILABLE) { - // Fall back if a 500, 502 or 503 is returned. - return ProxyService::INTERNAL_SERVER_ERROR_BYPASS; + // Fall back if a 500, 502 or 503 is returned. + if (headers->response_code() == net::HTTP_INTERNAL_SERVER_ERROR) + return ProxyService::STATUS_500_HTTP_INTERNAL_SERVER_ERROR; + if (headers->response_code() == net::HTTP_BAD_GATEWAY) + return ProxyService::STATUS_502_HTTP_BAD_GATEWAY; + if (headers->response_code() == net::HTTP_SERVICE_UNAVAILABLE) + return ProxyService::STATUS_503_HTTP_SERVICE_UNAVAILABLE; + // TODO(kundaji): Bypass if Proxy-Authenticate header value cannot be + // interpreted by data reduction proxy. + if (headers->response_code() == net::HTTP_PROXY_AUTHENTICATION_REQUIRED && + !headers->HasHeader("Proxy-Authenticate")) { + return ProxyService::MALFORMED_407; } if (!HasDataReductionProxyViaHeader(headers) && (headers->response_code() != net::HTTP_NOT_MODIFIED)) { @@ -132,9 +144,9 @@ GetDataReductionProxyBypassEventType( // Separate this case from other responses that are missing the header. if (headers->response_code() >= net::HTTP_BAD_REQUEST && headers->response_code() < net::HTTP_INTERNAL_SERVER_ERROR) { - return ProxyService::PROXY_4XX_BYPASS; + return ProxyService::MISSING_VIA_HEADER_4XX; } - return ProxyService::MISSING_VIA_HEADER; + return ProxyService::MISSING_VIA_HEADER_OTHER; } // There is no bypass event. return ProxyService::BYPASS_EVENT_TYPE_MAX; diff --git a/components/data_reduction_proxy/common/data_reduction_proxy_headers.h b/components/data_reduction_proxy/common/data_reduction_proxy_headers.h index 88b0cd0..50aea24 100644 --- a/components/data_reduction_proxy/common/data_reduction_proxy_headers.h +++ b/components/data_reduction_proxy/common/data_reduction_proxy_headers.h @@ -12,6 +12,8 @@ #include "net/http/http_response_headers.h" #include "net/proxy/proxy_service.h" +using net::ProxyService; + namespace data_reduction_proxy { // Contains instructions contained in the Chrome-Proxy header. @@ -43,8 +45,7 @@ bool HasDataReductionProxyViaHeader(const net::HttpResponseHeaders* headers); // Returns the reason why the Chrome proxy should be bypassed or not, and // populates |proxy_info| with information on how long to bypass if // applicable. -net::ProxyService::DataReductionProxyBypassEventType -GetDataReductionProxyBypassEventType( +ProxyService::DataReductionProxyBypassType GetDataReductionProxyBypassType( const net::HttpResponseHeaders* headers, DataReductionProxyInfo* proxy_info); diff --git a/components/data_reduction_proxy/common/data_reduction_proxy_headers_unittest.cc b/components/data_reduction_proxy/common/data_reduction_proxy_headers_unittest.cc index 871d665..250cf32 100644 --- a/components/data_reduction_proxy/common/data_reduction_proxy_headers_unittest.cc +++ b/components/data_reduction_proxy/common/data_reduction_proxy_headers_unittest.cc @@ -269,7 +269,7 @@ TEST_F(DataReductionProxyHeadersTest, HasDataReductionProxyViaHeader) { TEST_F(DataReductionProxyHeadersTest, GetDataReductionProxyBypassEventType) { const struct { const char* headers; - net::ProxyService::DataReductionProxyBypassEventType expected_result; + net::ProxyService::DataReductionProxyBypassType expected_result; } tests[] = { { "HTTP/1.1 200 OK\n" "Chrome-Proxy: bypass=0\n" @@ -277,18 +277,28 @@ TEST_F(DataReductionProxyHeadersTest, GetDataReductionProxyBypassEventType) { net::ProxyService::SHORT_BYPASS, }, { "HTTP/1.1 200 OK\n" - "Chrome-Proxy: bypass=1799\n" + "Chrome-Proxy: bypass=59\n" "Via: 1.1 Chrome-Compression-Proxy\n", net::ProxyService::SHORT_BYPASS, }, { "HTTP/1.1 200 OK\n" - "Chrome-Proxy: bypass=1800\n" + "Chrome-Proxy: bypass=60\n" + "Via: 1.1 Chrome-Compression-Proxy\n", + net::ProxyService::MEDIUM_BYPASS, + }, + { "HTTP/1.1 200 OK\n" + "Chrome-Proxy: bypass=300\n" + "Via: 1.1 Chrome-Compression-Proxy\n", + net::ProxyService::MEDIUM_BYPASS, + }, + { "HTTP/1.1 200 OK\n" + "Chrome-Proxy: bypass=301\n" "Via: 1.1 Chrome-Compression-Proxy\n", net::ProxyService::LONG_BYPASS, }, { "HTTP/1.1 500 Internal Server Error\n" "Via: 1.1 Chrome-Compression-Proxy\n", - net::ProxyService::INTERNAL_SERVER_ERROR_BYPASS, + net::ProxyService::STATUS_500_HTTP_INTERNAL_SERVER_ERROR, }, { "HTTP/1.1 501 Not Implemented\n" "Via: 1.1 Chrome-Compression-Proxy\n", @@ -296,11 +306,11 @@ TEST_F(DataReductionProxyHeadersTest, GetDataReductionProxyBypassEventType) { }, { "HTTP/1.1 502 Bad Gateway\n" "Via: 1.1 Chrome-Compression-Proxy\n", - net::ProxyService::INTERNAL_SERVER_ERROR_BYPASS, + net::ProxyService::STATUS_502_HTTP_BAD_GATEWAY, }, { "HTTP/1.1 503 Service Unavailable\n" "Via: 1.1 Chrome-Compression-Proxy\n", - net::ProxyService::INTERNAL_SERVER_ERROR_BYPASS, + net::ProxyService::STATUS_503_HTTP_SERVICE_UNAVAILABLE, }, { "HTTP/1.1 504 Gateway Timeout\n" "Via: 1.1 Chrome-Compression-Proxy\n", @@ -311,32 +321,40 @@ TEST_F(DataReductionProxyHeadersTest, GetDataReductionProxyBypassEventType) { net::ProxyService::BYPASS_EVENT_TYPE_MAX, }, { "HTTP/1.1 304 Not Modified\n", - net::ProxyService::BYPASS_EVENT_TYPE_MAX, + net::ProxyService::BYPASS_EVENT_TYPE_MAX, }, { "HTTP/1.1 200 OK\n", - net::ProxyService::MISSING_VIA_HEADER, + net::ProxyService::MISSING_VIA_HEADER_OTHER, }, { "HTTP/1.1 200 OK\n" - "Chrome-Proxy: bypass=1799\n", + "Chrome-Proxy: bypass=59\n", net::ProxyService::SHORT_BYPASS, }, { "HTTP/1.1 502 Bad Gateway\n", - net::ProxyService::INTERNAL_SERVER_ERROR_BYPASS, + net::ProxyService::STATUS_502_HTTP_BAD_GATEWAY, }, { "HTTP/1.1 502 Bad Gateway\n" - "Chrome-Proxy: bypass=1799\n", + "Chrome-Proxy: bypass=59\n", net::ProxyService::SHORT_BYPASS, }, { "HTTP/1.1 502 Bad Gateway\n" - "Chrome-Proxy: bypass=1799\n", + "Chrome-Proxy: bypass=59\n", net::ProxyService::SHORT_BYPASS, }, { "HTTP/1.1 414 Request-URI Too Long\n", - net::ProxyService::PROXY_4XX_BYPASS, + net::ProxyService::MISSING_VIA_HEADER_4XX, }, { "HTTP/1.1 414 Request-URI Too Long\n" "Via: 1.1 Chrome-Compression-Proxy\n", net::ProxyService::BYPASS_EVENT_TYPE_MAX, + }, + { "HTTP/1.1 407 Proxy Authentication Required\n", + net::ProxyService::MALFORMED_407, + }, + { "HTTP/1.1 407 Proxy Authentication Required\n" + "Proxy-Authenticate: Basic\n" + "Via: 1.1 Chrome-Compression-Proxy\n", + net::ProxyService::BYPASS_EVENT_TYPE_MAX, } }; for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) { @@ -346,7 +364,7 @@ TEST_F(DataReductionProxyHeadersTest, GetDataReductionProxyBypassEventType) { new net::HttpResponseHeaders(headers)); DataReductionProxyInfo chrome_proxy_info; EXPECT_EQ(tests[i].expected_result, - GetDataReductionProxyBypassEventType(parsed, &chrome_proxy_info)); + GetDataReductionProxyBypassType(parsed, &chrome_proxy_info)); } } } // namespace data_reduction_proxy diff --git a/net/proxy/proxy_service.cc b/net/proxy/proxy_service.cc index c5cd6ac..b80b31c 100644 --- a/net/proxy/proxy_service.cc +++ b/net/proxy/proxy_service.cc @@ -1189,12 +1189,12 @@ int ProxyService::ReconsiderProxyAfterError(const GURL& url, #if defined(SPDY_PROXY_AUTH_ORIGIN) if (result->proxy_server().isDataReductionProxy()) { RecordDataReductionProxyBypassInfo( - true, result->proxy_server(), ERROR_BYPASS); + true, false, result->proxy_server(), NETWORK_ERROR); RecordDataReductionProxyBypassOnNetworkError( true, result->proxy_server(), net_error); } else if (result->proxy_server().isDataReductionProxyFallback()) { RecordDataReductionProxyBypassInfo( - false, result->proxy_server(), ERROR_BYPASS); + false, false, result->proxy_server(), NETWORK_ERROR); RecordDataReductionProxyBypassOnNetworkError( false, result->proxy_server(), net_error); } @@ -1427,18 +1427,29 @@ scoped_ptr<ProxyService::PacPollPolicy> void ProxyService::RecordDataReductionProxyBypassInfo( bool is_primary, + bool bypass_all, const ProxyServer& proxy_server, - DataReductionProxyBypassEventType bypass_type) const { + DataReductionProxyBypassType bypass_type) const { // Only record UMA if the proxy isn't already on the retry list. if (proxy_retry_info_.find(proxy_server.ToURI()) != proxy_retry_info_.end()) return; - if (is_primary) { - UMA_HISTOGRAM_ENUMERATION("DataReductionProxy.BypassInfoPrimary", - bypass_type, BYPASS_EVENT_TYPE_MAX); + if (bypass_all) { + if (is_primary) { + UMA_HISTOGRAM_ENUMERATION("DataReductionProxy.BlockTypePrimary", + bypass_type, BYPASS_EVENT_TYPE_MAX); + } else { + UMA_HISTOGRAM_ENUMERATION("DataReductionProxy.BlockTypeFallback", + bypass_type, BYPASS_EVENT_TYPE_MAX); + } } else { - UMA_HISTOGRAM_ENUMERATION("DataReductionProxy.BypassInfoFallback", - bypass_type, BYPASS_EVENT_TYPE_MAX); + if (is_primary) { + UMA_HISTOGRAM_ENUMERATION("DataReductionProxy.BypassTypePrimary", + bypass_type, BYPASS_EVENT_TYPE_MAX); + } else { + UMA_HISTOGRAM_ENUMERATION("DataReductionProxy.BypassTypeFallback", + bypass_type, BYPASS_EVENT_TYPE_MAX); + } } } diff --git a/net/proxy/proxy_service.h b/net/proxy/proxy_service.h index b6ef447..146acde 100644 --- a/net/proxy/proxy_service.h +++ b/net/proxy/proxy_service.h @@ -269,41 +269,57 @@ class NET_EXPORT ProxyService : public NetworkChangeNotifier::IPAddressObserver, bool quick_check_enabled() const { return quick_check_enabled_; } - // Values of the UMA DataReductionProxy.BypassInfo{Primary|Fallback} - // histograms. This enum must remain synchronized with the enum of the same + // Values of the UMA DataReductionProxy.BypassType{Primary|Fallback} + // and DataReductionProxy.BlockType{Primary|Fallback} histograms. + // This enum must remain synchronized with the enum of the same // name in metrics/histograms/histograms.xml. - enum DataReductionProxyBypassEventType { - // Bypass the proxy for less than 30 minutes. - SHORT_BYPASS = 0, + enum DataReductionProxyBypassType { + // Bypass due to explicit instruction for the current request. + // Not yet supported. + CURRENT_BYPASS = 0, - // Bypass the proxy for 30 minutes or more. - LONG_BYPASS, + // Bypass the proxy for less than one minute. + SHORT_BYPASS = 1, - // Bypass the proxy because of an internal server error. - INTERNAL_SERVER_ERROR_BYPASS, + // Bypass the proxy for one to five minutes. + MEDIUM_BYPASS = 2, - // Bypass the proxy because of any other error. - ERROR_BYPASS, + // Bypass the proxy for more than five minutes. + LONG_BYPASS = 3, - // Bypass the proxy because responses appear not to be coming via it. - MISSING_VIA_HEADER, + // Bypass due to a 4xx missing via header. + MISSING_VIA_HEADER_4XX = 4, - // Bypass the proxy because the proxy, not the origin, sent a 4xx response. - PROXY_4XX_BYPASS, + // Bypass due to other missing via header, excluding 4xx errors. + MISSING_VIA_HEADER_OTHER = 5, - // Bypass the proxy because we got a 407 from the proxy without a challenge. - MALFORMED_407_BYPASS, + // Bypass due to 407 response from proxy without a challenge. + MALFORMED_407 = 6, + + // Bypass due to a 500 internal server error + STATUS_500_HTTP_INTERNAL_SERVER_ERROR = 7, + + // Bypass because the request URI was too long. + STATUS_502_HTTP_BAD_GATEWAY = 8, + + // Bypass due to a 503 response. + STATUS_503_HTTP_SERVICE_UNAVAILABLE = 9, + + // Bypass due to any network error. + NETWORK_ERROR = 10, // This must always be last. - BYPASS_EVENT_TYPE_MAX + BYPASS_EVENT_TYPE_MAX = 11 }; - // Records a |DataReductionProxyBypassEventType| for either the data reduction + // Records a |DataReductionProxyBypassEventType| or + // |DataReductionProxyBlockEventType| for either the data reduction // proxy (|is_primary| is true) or the data reduction proxy fallback. void RecordDataReductionProxyBypassInfo( bool is_primary, + bool bypass_all, const ProxyServer& proxy_server, - DataReductionProxyBypassEventType bypass_type) const; + DataReductionProxyBypassType block_type) const; // Records a net error code that resulted in bypassing the data reduction // proxy (|is_primary| is true) or the data reduction proxy fallback. diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index 6b101a8..fcb388a 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -2937,8 +2937,31 @@ Therefore, the affected-histogram name has to have at least one dot in it. </summary> </histogram> +<histogram name="DataReductionProxy.BlockTypeFallback" + enum="DataReductionProxyBypassType"> + <owner>bengr@chromium.org</owner> + <owner>marq@chromium.org</owner> + <summary> + Counts various events that trigger Chrome to block the fallback + configuration of the data reduction proxy. + </summary> +</histogram> + +<histogram name="DataReductionProxy.BlockTypePrimary" + enum="DataReductionProxyBypassType"> + <owner>bengr@chromium.org</owner> + <owner>marq@chromium.org</owner> + <summary> + Counts various events that trigger Chrome to block the primary configuration + of the data reduction proxy. + </summary> +</histogram> + <histogram name="DataReductionProxy.BypassInfoFallback" - enum="DataReductionProxyBypassEventType"> + enum="DataReductionProxyBypassEventType_Deprecated"> + <obsolete> + Deprecated as of 6/2014, replaced by DataReductionProxy.BypassTypeFallback. + </obsolete> <owner>bengr@chromium.org</owner> <owner>marq@chromium.org</owner> <summary> @@ -2948,7 +2971,10 @@ Therefore, the affected-histogram name has to have at least one dot in it. </histogram> <histogram name="DataReductionProxy.BypassInfoPrimary" - enum="DataReductionProxyBypassEventType"> + enum="DataReductionProxyBypassEventType_Deprecated"> + <obsolete> + Deprecated as of 6/2014, replaced by DataReductionProxy.BypassTypePrimary. + </obsolete> <owner>bengr@chromium.org</owner> <owner>marq@chromium.org</owner> <summary> @@ -2977,6 +3003,26 @@ Therefore, the affected-histogram name has to have at least one dot in it. </summary> </histogram> +<histogram name="DataReductionProxy.BypassTypeFallback" + enum="DataReductionProxyBypassType"> + <owner>bengr@chromium.org</owner> + <owner>marq@chromium.org</owner> + <summary> + Counts various events that trigger Chrome to bypass the fallback + configuration of the data reduction proxy. + </summary> +</histogram> + +<histogram name="DataReductionProxy.BypassTypePrimary" + enum="DataReductionProxyBypassType"> + <owner>bengr@chromium.org</owner> + <owner>marq@chromium.org</owner> + <summary> + Counts various events that trigger Chrome to bypass the primary + configuration of the data reduction proxy. + </summary> +</histogram> + <histogram name="DataReductionProxy.ProbeURL" enum="DataReductionProxyProbeURLFetchResult"> <owner>bengr@chromium.org</owner> @@ -35944,7 +35990,7 @@ Therefore, the affected-histogram name has to have at least one dot in it. <int value="4" label="Channel is negotiated."/> </enum> -<enum name="DataReductionProxyBypassEventType" type="int"> +<enum name="DataReductionProxyBypassEventType_Deprecated" type="int"> <int value="0" label="Short bypass"/> <int value="1" label="Long bypass"/> <int value="2" label="Bypass due to internal server error"/> @@ -35955,6 +36001,26 @@ Therefore, the affected-histogram name has to have at least one dot in it. label="Bypass due to 407 response from proxy without a challenge"/> </enum> +<enum name="DataReductionProxyBypassType" type="int"> + <int value="0" + label="Bypass due to explicit instruction for the current request"/> + <int value="1" + label="Short bypass: Bypass the proxy for less than one minute"/> + <int value="2" + label="Medium bypass: Bypass the proxy for one to five minutes"/> + <int value="3" + label="Long bypass: Bypass the proxy for more than five minutes"/> + <int value="4" label="Bypass due to a 4xx missing via header"/> + <int value="5" + label="Bypass due to other missing via header, excluding 4xx errors"/> + <int value="6" + label="Bypass due to 407 response from proxy without a challenge"/> + <int value="7" label="Bypass due to a 500 internal server error"/> + <int value="8" label="Bypass because the request URI was too long"/> + <int value="9" label="Bypass due to a 503 response"/> + <int value="10" label="Bypass due to any network error"/> +</enum> + <enum name="DataReductionProxyProbeURLFetchResult" type="int"> <int value="0" label="Internet disconnected"/> <int value="1" label="Probe failed, proxy disabled"/> |