[Remote suggestions] Report time distribution of triggers

components/ntp_snippets/remote/remote_suggestions_scheduler_impl.cc

 // Copyright 2016 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.
 
 #include "components/ntp_snippets/remote/remote_suggestions_scheduler_impl.h"
 
 #include <random>
 #include <string>
 #include <utility>
 
@@ skipping 127 matching lines @@
       break;
   }
 
   double value_hours = base::GetFieldTrialParamByFeatureAsDouble(
       ntp_snippets::kArticleSuggestionsFeature, param_name,
       default_value_hours);
 
   return base::TimeDelta::FromSecondsD(value_hours * 3600.0);
 }
 
+void ReportTimeUntilFirstTrigger(
+    UserClassifier::UserClass user_class,
+    const base::TimeDelta& time_until_first_failed_trigger) {

[Marc Treib, 2017/04/12 12:48:31]

s/time_until_first_failed_trigger/time_until_first_trigger/
(This records any, not just failed, right?)

And again, TimeDelta is fine to pass by value.

[jkrcal, 2017/04/12 13:26:44]

Done.

+  switch (user_class) {
+    case UserClassifier::UserClass::RARE_NTP_USER:
+      UMA_HISTOGRAM_CUSTOM_TIMES(
+          "NewTabPage.ContentSuggestions.TimeUntilFirstTrigger_RareNTPUser",
+          time_until_first_failed_trigger, base::TimeDelta::FromSeconds(1),
+          base::TimeDelta::FromDays(7),
+          /*bucket_count=*/100);
+      break;
+    case UserClassifier::UserClass::ACTIVE_NTP_USER:
+      UMA_HISTOGRAM_CUSTOM_TIMES(
+          "NewTabPage.ContentSuggestions.TimeUntilFirstTrigger_ActiveNTPUser",
+          time_until_first_failed_trigger, base::TimeDelta::FromSeconds(1),
+          base::TimeDelta::FromDays(7),
+          /*bucket_count=*/100);
+      break;
+    case UserClassifier::UserClass::ACTIVE_SUGGESTIONS_CONSUMER:
+      UMA_HISTOGRAM_CUSTOM_TIMES(
+          "NewTabPage.ContentSuggestions.TimeUntilFirstTrigger_"
+          "ActiveSuggestionsConsumer",
+          time_until_first_failed_trigger, base::TimeDelta::FromSeconds(1),
+          base::TimeDelta::FromDays(7),
+          /*bucket_count=*/100);
+      break;
+  }
+}
+
+void ReportTimeUntilFirstSuccessfulTrigger(
+    UserClassifier::UserClass user_class,
+    const base::TimeDelta& time_until_first_successful_trigger) {
+  switch (user_class) {
+    case UserClassifier::UserClass::RARE_NTP_USER:
+      UMA_HISTOGRAM_CUSTOM_TIMES(
+          "NewTabPage.ContentSuggestions.TimeUntilFirstSuccessfulTrigger_"
+          "RareNTPUser",
+          time_until_first_successful_trigger, base::TimeDelta::FromSeconds(1),
+          base::TimeDelta::FromDays(7),
+          /*bucket_count=*/100);
+      break;
+    case UserClassifier::UserClass::ACTIVE_NTP_USER:
+      UMA_HISTOGRAM_CUSTOM_TIMES(
+          "NewTabPage.ContentSuggestions.TimeUntilFirstSuccessfulTrigger_"
+          "ActiveNTPUser",
+          time_until_first_successful_trigger, base::TimeDelta::FromSeconds(1),
+          base::TimeDelta::FromDays(7),
+          /*bucket_count=*/100);
+      break;
+    case UserClassifier::UserClass::ACTIVE_SUGGESTIONS_CONSUMER:
+      UMA_HISTOGRAM_CUSTOM_TIMES(
+          "NewTabPage.ContentSuggestions.TimeUntilFirstSuccessfulTrigger_"
+          "ActiveSuggestionsConsumer",
+          time_until_first_successful_trigger, base::TimeDelta::FromSeconds(1),
+          base::TimeDelta::FromDays(7),
+          /*bucket_count=*/100);
+      break;
+  }
+}
+
 }  // namespace
 
 class EulaState : public web_resource::EulaAcceptedNotifier::Observer {
  public:
   EulaState(PrefService* local_state_prefs,
             RemoteSuggestionsScheduler* scheduler)
       : eula_notifier_(
             web_resource::EulaAcceptedNotifier::Create(local_state_prefs)),
         scheduler_(scheduler) {
     // EulaNotifier is not constructed on some platforms (such as desktop).
@@ skipping 81 matching lines @@
           RequestThrottler::RequestType::
               CONTENT_SUGGESTION_FETCHER_RARE_NTP_USER),
       request_throttler_active_ntp_user_(
           profile_prefs,
           RequestThrottler::RequestType::
               CONTENT_SUGGESTION_FETCHER_ACTIVE_NTP_USER),
       request_throttler_active_suggestions_consumer_(
           profile_prefs,
           RequestThrottler::RequestType::
               CONTENT_SUGGESTION_FETCHER_ACTIVE_SUGGESTIONS_CONSUMER),
+      time_until_first_trigger_reported_(false),
       eula_state_(base::MakeUnique<EulaState>(local_state_prefs, this)),
       profile_prefs_(profile_prefs),
       clock_(std::move(clock)),
       enabled_triggers_(GetEnabledTriggerTypes()) {
   DCHECK(user_classifier);
   DCHECK(profile_prefs);
 
   // Cleanup procedure in M59. Remove for M62.
   profile_prefs_->ClearPref(kSnippetSoftFetchingIntervalOnUsageEventDeprecated);
   profile_prefs_->ClearPref(kSnippetSoftFetchingIntervalOnNtpOpenedDeprecated);
@@ skipping 172 matching lines @@
 void RemoteSuggestionsSchedulerImpl::RefetchInTheBackgroundIfAppropriate(
     TriggerType trigger) {
   if (background_fetch_in_progress_) {
     return;
   }
 
   if (BackgroundFetchesDisabled(trigger)) {
     return;
   }
 
+  const base::Time last_fetch_attempt_time = base::Time::FromInternalValue(

[Marc Treib, 2017/04/12 12:48:31]

What exactly does fetch *attempt* mean? (Does it matter for the metrics?)

[jkrcal, 2017/04/12 13:26:44]

It is the last time we sent a request to the server. We do not care whether the
request was successful or not so that we continue with the regular schedule and
do not burn the server by frequent retries whenever if it is overloaded.

This time is used as reference for ShouldRefetchInTheBackgroundNow() so this is
also the right reference time for the metrics.

+      profile_prefs_->GetInt64(prefs::kSnippetLastFetchAttempt));
+  if (!time_until_first_trigger_reported_) {

[Marc Treib, 2017/04/12 12:48:31]

Should we exclude PERSISTENT_SCHEDULER_WAKE_UP here?

[jkrcal, 2017/04/12 13:26:44]

Good point, done!

+    time_until_first_trigger_reported_ = true;
+    ReportTimeUntilFirstTrigger(user_classifier_->GetUserClass(),
+                                clock_->Now() - last_fetch_attempt_time);
+  }
+
   if (trigger != TriggerType::PERSISTENT_SCHEDULER_WAKE_UP &&
-      !ShouldRefetchInTheBackgroundNow()) {
+      !ShouldRefetchInTheBackgroundNow(last_fetch_attempt_time)) {
     return;
   }
 
   if (!AcquireQuota(/*interactive_request=*/false)) {
     return;
   }
 
+  ReportTimeUntilFirstSuccessfulTrigger(

[Marc Treib, 2017/04/12 12:48:31]

and here?

[jkrcal, 2017/04/12 13:26:44]

Good point! I've introduced another histogram for persistent fetches because we
never really understood how they get triggered out there.

+      user_classifier_->GetUserClass(),
+      clock_->Now() - last_fetch_attempt_time);
+
   UMA_HISTOGRAM_ENUMERATION(
       "NewTabPage.ContentSuggestions.BackgroundFetchTrigger",
       static_cast<int>(trigger), static_cast<int>(TriggerType::COUNT));
 
   background_fetch_in_progress_ = true;
   provider_->RefetchInTheBackground(base::Bind(
       &RemoteSuggestionsSchedulerImpl::RefetchInTheBackgroundFinished,
       base::Unretained(this)));
 }
 
-bool RemoteSuggestionsSchedulerImpl::ShouldRefetchInTheBackgroundNow() {
-  const base::Time last_fetch_attempt_time = base::Time::FromInternalValue(
-      profile_prefs_->GetInt64(prefs::kSnippetLastFetchAttempt));
-
+bool RemoteSuggestionsSchedulerImpl::ShouldRefetchInTheBackgroundNow(
+    const base::Time& last_fetch_attempt_time) {
   // If we have no persistent scheduler to ask, err on the side of caution.
   bool wifi = false;
   if (persistent_scheduler_) {
     wifi = persistent_scheduler_->IsOnUnmeteredConnection();
   }
 
   base::Time first_allowed_fetch_time =
       last_fetch_attempt_time +
       (wifi ? schedule_.interval_soft_wifi : schedule_.interval_soft_fallback);
   base::Time now = clock_->Now();
@@ skipping 40 matching lines @@
 
 void RemoteSuggestionsSchedulerImpl::RefetchInTheBackgroundFinished(
     Status fetch_status) {
   background_fetch_in_progress_ = false;
   OnFetchCompleted(fetch_status);
 }
 
 void RemoteSuggestionsSchedulerImpl::OnFetchCompleted(Status fetch_status) {
   profile_prefs_->SetInt64(prefs::kSnippetLastFetchAttempt,
                            clock_->Now().ToInternalValue());
+  time_until_first_trigger_reported_ = false;
 
   // Reschedule after a fetch. The persistent schedule is applied only after a
   // successful fetch. After a failed fetch, we want to keep the previous
   // persistent schedule intact so that we eventually get a persistent
   // fallback fetch (if the wifi persistent fetches keep failing).
   if (fetch_status.code != StatusCode::SUCCESS) {
     return;
   }
   ApplyPersistentFetchingSchedule();
 }
@@ skipping 41 matching lines @@
   return enabled_types;
 }
 
 std::set<RemoteSuggestionsSchedulerImpl::TriggerType>
 RemoteSuggestionsSchedulerImpl::GetDefaultEnabledTriggerTypes() {
   return {TriggerType::PERSISTENT_SCHEDULER_WAKE_UP, TriggerType::NTP_OPENED,
           TriggerType::BROWSER_FOREGROUNDED};
 }
 
 }  // namespace ntp_snippets