Add histogram for hints request fetch latency

Bug: 1011483
Change-Id: I45db77c672ee662a368491f8c3ff1895ece10701
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1845871Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Michael Crouse <mcrouse@chromium.org>
Commit-Queue: Sophie Chang <sophiechang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703471}

components/optimization_guide/hints_fetcher.cc

@@ -103,6 +103,8 @@ bool HintsFetcher::FetchOptimizationGuideServiceHints(
   if (url_loader_)
     return false;
 
+  hints_fetch_start_time_ = base::TimeTicks::Now();
+
   request_context_ = request_context;
 
   get_hints_request_ = std::make_unique<proto::GetHintsRequest>();
@@ -184,6 +186,8 @@ bool HintsFetcher::FetchOptimizationGuideServiceHints(
 void HintsFetcher::HandleResponse(const std::string& get_hints_response_data,
                                   int net_status,
                                   int response_code) {
+  SEQUENCE_CHECKER(sequence_checker_);
+
   std::unique_ptr<proto::GetHintsResponse> get_hints_response =
       std::make_unique<proto::GetHintsResponse>();
 
@@ -201,6 +205,9 @@ void HintsFetcher::HandleResponse(const std::string& get_hints_response_data,
     UMA_HISTOGRAM_COUNTS_100(
         "OptimizationGuide.HintsFetcher.GetHintsRequest.HintCount",
         get_hints_response->hints_size());
+    UMA_HISTOGRAM_MEDIUM_TIMES(
+        "OptimizationGuide.HintsFetcher.GetHintsRequest.FetchLatency",
+        base::TimeTicks::Now() - hints_fetch_start_time_);
     UpdateHostsSuccessfullyFetched();
     std::move(hints_fetched_callback_)
         .Run(request_context_, std::move(get_hints_response));
@@ -261,6 +268,8 @@ void HintsFetcher::UpdateHostsSuccessfullyFetched() {
 
 void HintsFetcher::OnURLLoadComplete(
     std::unique_ptr<std::string> response_body) {
+  SEQUENCE_CHECKER(sequence_checker_);
+
   int response_code = -1;
   if (url_loader_->ResponseInfo() && url_loader_->ResponseInfo()->headers) {
     response_code = url_loader_->ResponseInfo()->headers->response_code();

components/optimization_guide/hints_fetcher.h

@@ -15,6 +15,7 @@
 #include "base/optional.h"
 #include "base/sequence_checker.h"
 #include "base/time/clock.h"
+#include "base/time/time.h"
 #include "components/optimization_guide/proto/hints.pb.h"
 #include "url/gurl.h"
 
@@ -122,6 +123,10 @@ class HintsFetcher {
   // Used for creating a |url_loader_| when needed for request hints.
   scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
 
+  // The start time of the current hints fetch, used to determine the latency in
+  // retrieving hints from the remote Optimization Guide Service.
+  base::TimeTicks hints_fetch_start_time_;
+
   SEQUENCE_CHECKER(sequence_checker_);
 
   DISALLOW_COPY_AND_ASSIGN(HintsFetcher);

components/optimization_guide/hints_fetcher_unittest.cc

@@ -150,11 +150,16 @@ class HintsFetcherTest : public testing::Test {
 };
 
 TEST_F(HintsFetcherTest, FetchOptimizationGuideServiceHints) {
+  base::HistogramTester histogram_tester;
+
   std::string response_content;
   EXPECT_TRUE(FetchHints(std::vector<std::string>()));
   VerifyHasPendingFetchRequests();
   EXPECT_TRUE(SimulateResponse(response_content, net::HTTP_OK));
   EXPECT_TRUE(hints_fetched());
+
+  histogram_tester.ExpectTotalCount(
+      "OptimizationGuide.HintsFetcher.GetHintsRequest.FetchLatency", 1);
 }
 
 // Tests to ensure that multiple hint fetches by the same object cannot be in
@@ -173,6 +178,8 @@ TEST_F(HintsFetcherTest, FetchInProcess) {
 
 // Tests 404 response from request.
 TEST_F(HintsFetcherTest, FetchReturned404) {
+  base::HistogramTester histogram_tester;
+
   std::string response_content;
 
   EXPECT_TRUE(FetchHints(std::vector<std::string>()));
@@ -180,27 +187,46 @@ TEST_F(HintsFetcherTest, FetchReturned404) {
   // Send a 404 to HintsFetcher.
   SimulateResponse(response_content, net::HTTP_NOT_FOUND);
   EXPECT_FALSE(hints_fetched());
+
+  // Make sure histogram not recorded on bad response.
+  histogram_tester.ExpectTotalCount(
+      "OptimizationGuide.HintsFetcher.GetHintsRequest.FetchLatency", 0);
 }
 
 TEST_F(HintsFetcherTest, FetchReturnBadResponse) {
+  base::HistogramTester histogram_tester;
+
   std::string response_content = "not proto";
   EXPECT_TRUE(FetchHints(std::vector<std::string>()));
   VerifyHasPendingFetchRequests();
   EXPECT_TRUE(SimulateResponse(response_content, net::HTTP_OK));
   EXPECT_FALSE(hints_fetched());
+
+  // Make sure histogram not recorded on bad response.
+  histogram_tester.ExpectTotalCount(
+      "OptimizationGuide.HintsFetcher.GetHintsRequest.FetchLatency", 0);
 }
 
 TEST_F(HintsFetcherTest, FetchAttemptWhenNetworkOffline) {
+  base::HistogramTester histogram_tester;
+
   SetConnectionOffline();
   std::string response_content;
   EXPECT_FALSE(FetchHints(std::vector<std::string>()));
   EXPECT_FALSE(hints_fetched());
 
+  // Make sure histogram not recorded on bad response.
+  histogram_tester.ExpectTotalCount(
+      "OptimizationGuide.HintsFetcher.GetHintsRequest.FetchLatency", 0);
+
   SetConnectionOnline();
   EXPECT_TRUE(FetchHints(std::vector<std::string>()));
   VerifyHasPendingFetchRequests();
   EXPECT_TRUE(SimulateResponse(response_content, net::HTTP_OK));
   EXPECT_TRUE(hints_fetched());
+
+  histogram_tester.ExpectTotalCount(
+      "OptimizationGuide.HintsFetcher.GetHintsRequest.FetchLatency", 1);
 }
 
 TEST_F(HintsFetcherTest, HintsFetchSuccessfulHostsRecorded) {

tools/metrics/histograms/histograms.xml

@@ -95335,6 +95335,17 @@ uploading your change for review.
   </summary>
 </histogram>
 
+<histogram name="OptimizationGuide.HintsFetcher.GetHintsRequest.FetchLatency"
+    units="ms" expires_after="M85">
+  <owner>sophiechang@chromium.org</owner>
+  <owner>mcrouse@chromium.org</owner>
+  <summary>
+    The duration of a request to fetch hints from the remote Optimization Guide
+    Service starts until it completes. Recorded every time hints are fetched and
+    parsed successfully.
+  </summary>
+</histogram>
+
 <histogram name="OptimizationGuide.HintsFetcher.GetHintsRequest.HintCount"
     units="units" expires_after="M85">
   <owner>mcrouse@chromium.org</owner>