From 270beb98f3c243245efcc72dc9c4b9ce70befa3a Mon Sep 17 00:00:00 2001 From: mlerman Date: Wed, 27 May 2015 08:50:48 -0700 Subject: [PATCH] Add histograms to entire MergeSession flow. This is part of refactoring ChromeOS to use the GaiaCookieManagerService BUG=483594 Review URL: https://codereview.chromium.org/1148283005 Cr-Commit-Position: refs/heads/master@{#331579} --- .../core/browser/gaia_cookie_manager_service.cc | 10 +++ .../gaia_cookie_manager_service_unittest.cc | 7 +++ google_apis/gaia/oauth2_token_service.cc | 5 ++ google_apis/gaia/ubertoken_fetcher.cc | 5 ++ tools/metrics/histograms/histograms.xml | 72 ++++++++++++++++++++-- 5 files changed, 94 insertions(+), 5 deletions(-) diff --git a/components/signin/core/browser/gaia_cookie_manager_service.cc b/components/signin/core/browser/gaia_cookie_manager_service.cc index a3a051784f6b..0926687da483 100644 --- a/components/signin/core/browser/gaia_cookie_manager_service.cc +++ b/components/signin/core/browser/gaia_cookie_manager_service.cc @@ -8,6 +8,7 @@ #include #include "base/json/json_reader.h" +#include "base/metrics/histogram_macros.h" #include "base/stl_util.h" #include "base/strings/string_util.h" #include "base/strings/stringprintf.h" @@ -525,6 +526,8 @@ void GaiaCookieManagerService::OnMergeSessionFailure( << " error=" << error.ToString(); if (++fetcher_retries_ < kMaxFetcherRetries && error.IsTransientError()) { fetcher_backoff_.InformOfRequest(false); + UMA_HISTOGRAM_ENUMERATION("OAuth2Login.MergeSessionRetry", + error.state(), GoogleServiceAuthError::NUM_STATES); fetcher_timer_.Start( FROM_HERE, fetcher_backoff_.GetTimeUntilRelease(), base::Bind(&SigninClient::DelayNetworkCall, @@ -537,6 +540,9 @@ void GaiaCookieManagerService::OnMergeSessionFailure( uber_token_ = std::string(); const std::string account_id = requests_.front().account_id(); + + UMA_HISTOGRAM_ENUMERATION("OAuth2Login.MergeSessionFailure", + error.state(), GoogleServiceAuthError::NUM_STATES); HandleNextRequest(); SignalComplete(account_id, error); } @@ -573,6 +579,8 @@ void GaiaCookieManagerService::OnListAccountsFailure( GaiaCookieRequestType::LIST_ACCOUNTS); if (++fetcher_retries_ < kMaxFetcherRetries && error.IsTransientError()) { fetcher_backoff_.InformOfRequest(false); + UMA_HISTOGRAM_ENUMERATION("Signin.ListAccountsRetry", + error.state(), GoogleServiceAuthError::NUM_STATES); fetcher_timer_.Start( FROM_HERE, fetcher_backoff_.GetTimeUntilRelease(), base::Bind(&SigninClient::DelayNetworkCall, @@ -583,6 +591,8 @@ void GaiaCookieManagerService::OnListAccountsFailure( return; } + UMA_HISTOGRAM_ENUMERATION("Signin.ListAccountsFailure", + error.state(), GoogleServiceAuthError::NUM_STATES); FOR_EACH_OBSERVER(Observer, observer_list_, OnGaiaAccountsInCookieUpdated(listed_accounts_, error)); HandleNextRequest(); diff --git a/components/signin/core/browser/gaia_cookie_manager_service_unittest.cc b/components/signin/core/browser/gaia_cookie_manager_service_unittest.cc index 2d0151c29d3f..f468c9f312e9 100644 --- a/components/signin/core/browser/gaia_cookie_manager_service_unittest.cc +++ b/components/signin/core/browser/gaia_cookie_manager_service_unittest.cc @@ -10,6 +10,7 @@ #include "base/memory/scoped_ptr.h" #include "base/message_loop/message_loop.h" #include "base/strings/stringprintf.h" +#include "base/test/histogram_tester.h" #include "components/signin/core/browser/gaia_cookie_manager_service.h" #include "components/signin/core/browser/test_signin_client.h" #include "google_apis/gaia/fake_oauth2_token_service.h" @@ -166,6 +167,7 @@ TEST_F(GaiaCookieManagerServiceTest, Success) { TEST_F(GaiaCookieManagerServiceTest, FailedMergeSession) { InstrumentedGaiaCookieManagerService helper(token_service(), signin_client()); MockObserver observer(&helper); + base::HistogramTester histograms; EXPECT_CALL(helper, StartFetchingUbertoken()); EXPECT_CALL(observer, OnAddAccountToCookieCompleted("acc1@gmail.com", @@ -175,6 +177,8 @@ TEST_F(GaiaCookieManagerServiceTest, FailedMergeSession) { SimulateMergeSessionFailure(&helper, error()); // Persistent error incurs no further retries. DCHECK(!helper.is_running()); + histograms.ExpectUniqueSample("OAuth2Login.MergeSessionFailure", + GoogleServiceAuthError::SERVICE_ERROR, 1); } TEST_F(GaiaCookieManagerServiceTest, AddAccountCookiesDisabled) { @@ -213,6 +217,7 @@ TEST_F(GaiaCookieManagerServiceTest, MergeSessionRetried) { TEST_F(GaiaCookieManagerServiceTest, MergeSessionRetriedTwice) { InstrumentedGaiaCookieManagerService helper(token_service(), signin_client()); MockObserver observer(&helper); + base::HistogramTester histograms; EXPECT_CALL(helper, StartFetchingUbertoken()); EXPECT_CALL(helper, StartFetchingMergeSession()).Times(2); @@ -238,6 +243,8 @@ TEST_F(GaiaCookieManagerServiceTest, MergeSessionRetriedTwice) { base::MessageLoop::current()->Run(); SimulateMergeSessionSuccess(&helper, "token"); DCHECK(!helper.is_running()); + histograms.ExpectUniqueSample("OAuth2Login.MergeSessionRetry", + GoogleServiceAuthError::REQUEST_CANCELED, 2); } TEST_F(GaiaCookieManagerServiceTest, FailedUbertoken) { diff --git a/google_apis/gaia/oauth2_token_service.cc b/google_apis/gaia/oauth2_token_service.cc index 34dfc1bda906..524564614d39 100644 --- a/google_apis/gaia/oauth2_token_service.cc +++ b/google_apis/gaia/oauth2_token_service.cc @@ -9,6 +9,7 @@ #include "base/bind.h" #include "base/memory/weak_ptr.h" #include "base/message_loop/message_loop.h" +#include "base/metrics/histogram_macros.h" #include "base/profiler/scoped_tracker.h" #include "base/rand_util.h" #include "base/stl_util.h" @@ -291,6 +292,8 @@ void OAuth2TokenService::Fetcher::OnGetTokenFailure( base::TimeDelta backoff = base::TimeDelta::FromMilliseconds( ComputeExponentialBackOffMilliseconds(retry_number_)); ++retry_number_; + UMA_HISTOGRAM_ENUMERATION("Signin.OAuth2TokenGetRetry", + error.state(), GoogleServiceAuthError::NUM_STATES); retry_timer_.Stop(); retry_timer_.Start(FROM_HERE, backoff, @@ -299,6 +302,8 @@ void OAuth2TokenService::Fetcher::OnGetTokenFailure( return; } + UMA_HISTOGRAM_ENUMERATION("Signin.OAuth2TokenGetFailure", + error.state(), GoogleServiceAuthError::NUM_STATES); error_ = error; InformWaitingRequestsAndDelete(); } diff --git a/google_apis/gaia/ubertoken_fetcher.cc b/google_apis/gaia/ubertoken_fetcher.cc index 6cfe15f1bba0..24cb53d8e428 100644 --- a/google_apis/gaia/ubertoken_fetcher.cc +++ b/google_apis/gaia/ubertoken_fetcher.cc @@ -7,6 +7,7 @@ #include #include "base/logging.h" +#include "base/metrics/histogram_macros.h" #include "base/rand_util.h" #include "base/time/time.h" #include "google_apis/gaia/gaia_auth_fetcher.h" @@ -68,6 +69,8 @@ void UbertokenFetcher::OnUberAuthTokenFailure( // Calculate an exponential backoff with randomness of less than 1 sec. double backoff = base::RandDouble() + (1 << retry_number_); ++retry_number_; + UMA_HISTOGRAM_ENUMERATION("Signin.UberTokenRetry", + error.state(), GoogleServiceAuthError::NUM_STATES); retry_timer_.Stop(); retry_timer_.Start(FROM_HERE, base::TimeDelta::FromSecondsD(backoff), @@ -89,6 +92,8 @@ void UbertokenFetcher::OnUberAuthTokenFailure( } } + UMA_HISTOGRAM_ENUMERATION("Signin.UberTokenFailure", + error.state(), GoogleServiceAuthError::NUM_STATES); consumer_->OnUbertokenFailure(error); } diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index 42d1dd7bdb4f..a80068122355 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -25667,30 +25667,40 @@ Therefore, the affected-histogram name has to have at least one dot in it. + + Deprecated 2015-05-22. Replaced by Signin.ListAccountsFailure. + zelidrag@chromium.org Failure reason of final ListAccounts call failure during Chrome OS login. + This data is now included in Signin.ListAccountsFailure. zelidrag@chromium.org + + Deprecated 2015-05-22. Replaced by Signin.ListAccountsRetry. + - Retry reason of failed ListAccounts call during Chrome OS login. + Retry reason of failed ListAccounts call during Chrome OS login. This data + is now included in Signin.ListAccountsRetry. zelidrag@chromium.org - Failure reason of final MergeSession call during Chrome OS login. + Failure reason of MergeSession call during Chrome OS login, Chrome Signin or + account addition. On all OSes as of M44 (previously CrOS only). zelidrag@chromium.org - Retry reason of failed MergeSession call during Chrome OS login. + Retry reason of failed MergeSession call during Chrome OS login, Chrome + Signin or account addition. On all OSes as of M44 (previously CrOS only). @@ -25714,19 +25724,25 @@ Therefore, the affected-histogram name has to have at least one dot in it. + + Deprecated 2015-05-22 + zelidrag@chromium.org Failure reason of final OAuthLogin (with uber token) call during Chrome OS - login. + login. This data is now (M44+) included in Signin.UberTokenFailure. + + Deprecated 2015-05-22 + zelidrag@chromium.org Retry reason of failed OAuthLogin (with uber token) call during Chrome OS - login. + login. This data is now (M44+) included in Signin.UberTokenRetry. @@ -37185,6 +37201,36 @@ Therefore, the affected-histogram name has to have at least one dot in it. + + mlerman@chromium.org + + Failure reason of ListAccounts call failure during account reconciliation, + ChromeOS login, or signin internals queries. + + + + + mlerman@chromium.org + + Retry reason of failed ListAccounts call during Chrome OS login during + account reconciliation, ChromeOS login, or signin internals queries. + + + + + mlerman@chromium.org + + Reason fetching an OAuth2 Token failed. Available on all OSes. + + + + + mlerman@chromium.org + + Reason fetching an OAuth2 Token is being retried. Available on all OSes. + + + noms@chromium.org @@ -37291,6 +37337,22 @@ Therefore, the affected-histogram name has to have at least one dot in it. Track how a profile gets signed out. + + mlerman@chromium.org + + Reason of failure to acquiring an ubertoken based on an already-minted + access token. Available on all OSes. + + + + + mlerman@chromium.org + + Retry reason of failure to acquire an ubertoken based on an already-minted + access token. Available on all OSes. + + + anthonyvd@chromium.org mlerman@chromium.org -- 2.11.4.GIT