[ios] Add histogram to investigate loss off UKM.LogSize.OnSuccess records

There is a significant discrepancy between number of UKM records
uploaded to google servers and numbder of UKM.LogSize.Success records.
This either means that UMA data was not written to disk or that app was
terminated after server received the data, but before UMA was recorded.
Data loss caused by failure to write on disk is highly plausible if the
app is running in the background.

This CL adds another metric, which counts the number of times
UKM.LogSize.Success is emitted. The counter is stored in NSUserDefaults
(hence have higher chance to persist). The counter is recorded as UMA
when the app starts or foregrounded, hence have higher changes to get
written to disk.

Bug: 1154678
Change-Id: Ifa71355e02f568713f9d87f45692ae70fd496b72
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2576077
Auto-Submit: Eugene But <eugenebut@chromium.org>
Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Commit-Queue: Olivier Robin <olivierrobin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835191}

components/ukm/BUILD.gn

@@ -70,6 +70,9 @@ static_library("ukm") {
     "//components/variations",
     "//url",
   ]
+  if (is_ios) {
+    deps += [ "//components/ukm/ios:ukm_reporting_ios_util" ]
+  }
 }
 
 # Helper library for observing signals that we need to clear any local data.

components/ukm/ios/BUILD.gn

@@ -33,6 +33,16 @@ source_set("ukm_url_recorder") {
   ]
 }
 
+source_set("ukm_reporting_ios_util") {
+  configs += [ "//build/config/compiler:enable_arc" ]
+  deps = [ "//base" ]
+
+  sources = [
+    "ukm_reporting_ios_util.h",
+    "ukm_reporting_ios_util.mm",
+  ]
+}
+
 source_set("unit_tests") {
   configs += [ "//build/config/compiler:enable_arc" ]
   testonly = true

components/ukm/ios/ukm_reporting_ios_util.h

+// Copyright 2020 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.
+
+#ifndef COMPONENTS_UKM_IOS_UKM_REPORTING_IOS_UTIL_H_
+#define COMPONENTS_UKM_IOS_UKM_REPORTING_IOS_UTIL_H_
+
+// Utilities in this file should help to figure out the root cause of
+// crbug.com/1154678 (Data loss on UMA in iOS). If total sum of
+// "UKM.IOSLog.OnSuccess" is greater than total count of "UKM.LogSize.OnSuccess"
+// records, then data loss is caused by failure to write the histogram to the
+// disk on background thread. Otherwise (if total sum of
+// "UKM.IOSLog.OnSuccess" is equal to total count of "UKM.LogSize.OnSuccess"
+// records) then there is actually no data loss and app simply gets terminated
+// in a short window between UKM reached the server and the API call which
+// records the data.
+
+// Records "UKM.IOSLog.OnSuccess" histogram and resets the counter.
+void RecordAndResetUkmLogSizeOnSuccessCounter();
+
+// Increments the counter which will be recorded as UMA histogram when
+// RecordAndResetUkmLogSizeOnSuccessCounter is called. Counter represents
+// number of times "UKM.LogSize.OnSuccess" was recorded.
+void IncrementUkmLogSizeOnSuccessCounter();
+
+#endif  // COMPONENTS_UKM_IOS_UKM_REPORTING_IOS_UTIL_H_

components/ukm/ios/ukm_reporting_ios_util.mm

+// Copyright 2020 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/ukm/ios/ukm_reporting_ios_util.h"
+
+#import <UIKit/UIKit.h>
+
+#include "base/metrics/histogram_functions.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+namespace {
+// Key in NSUserDefaults to store the count of "UKM.LogSize.OnSuccess" records.
+NSString* LogSizeOnSuccessCounterKey = @"IOSUKMLogSizeOnSuccessCounter";
+}
+
+void RecordAndResetUkmLogSizeOnSuccessCounter() {
+  NSUserDefaults* defaults = NSUserDefaults.standardUserDefaults;
+  NSInteger counter = [defaults integerForKey:LogSizeOnSuccessCounterKey];
+  base::UmaHistogramCounts10000("UKM.IOSLog.OnSuccess", counter);
+  [defaults removeObjectForKey:LogSizeOnSuccessCounterKey];
+}
+
+void IncrementUkmLogSizeOnSuccessCounter() {
+  NSUserDefaults* defaults = NSUserDefaults.standardUserDefaults;
+  NSInteger counter = [defaults integerForKey:LogSizeOnSuccessCounterKey] + 1;
+  [defaults setInteger:counter forKey:LogSizeOnSuccessCounterKey];
+}

components/ukm/ukm_reporting_service.cc

@@ -11,6 +11,7 @@
 #include "base/metrics/field_trial_params.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/metrics/histogram_macros.h"
+#include "build/build_config.h"
 #include "components/metrics/metrics_service_client.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/ukm/ukm_pref_names.h"
@@ -18,6 +19,10 @@
 #include "components/ukm/unsent_log_store_metrics_impl.h"
 #include "third_party/zlib/google/compression_utils.h"
 
+#if defined(OS_IOS)
+#include "components/ukm/ios/ukm_reporting_ios_util.h"
+#endif
+
 namespace ukm {
 
 namespace {
@@ -108,6 +113,9 @@ void UkmReportingService::LogResponseOrErrorCode(int response_code,
 }
 
 void UkmReportingService::LogSuccessLogSize(size_t log_size) {
+#if defined(OS_IOS)
+  IncrementUkmLogSizeOnSuccessCounter();
+#endif
   UMA_HISTOGRAM_COUNTS_10000("UKM.LogSize.OnSuccess", log_size / 1024);
 }
 

ios/chrome/app/application_delegate/BUILD.gn

@@ -178,6 +178,7 @@ source_set("application_delegate_internal") {
     "//components/previous_session_info",
     "//components/search_engines",
     "//components/ukm/ios:features",
+    "//components/ukm/ios:ukm_reporting_ios_util",
     "//ios/chrome/app",
     "//ios/chrome/app:mode",
     "//ios/chrome/app/intents",

ios/chrome/app/application_delegate/metrics_mediator.mm

@@ -19,6 +19,7 @@
 #include "components/prefs/pref_service.h"
 #import "components/previous_session_info/previous_session_info.h"
 #include "components/ukm/ios/features.h"
+#include "components/ukm/ios/ukm_reporting_ios_util.h"
 #import "ios/chrome/app/application_delegate/metric_kit_subscriber.h"
 #import "ios/chrome/app/application_delegate/startup_information.h"
 #include "ios/chrome/browser/application_context.h"
@@ -155,6 +156,8 @@ using metrics_mediator::kAppEnteredBackgroundDateKey;
 + (void)logLaunchMetricsWithStartupInformation:
             (id<StartupInformation>)startupInformation
                                connectedScenes:(NSArray<SceneState*>*)scenes {
+  RecordAndResetUkmLogSizeOnSuccessCounter();
+
   int numTabs = 0;
   int numNTPTabs = 0;
   for (SceneState* scene in scenes) {

tools/metrics/histograms/histograms_xml/ukm/histograms.xml

@@ -134,6 +134,24 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   </summary>
 </histogram>
 
+<histogram name="UKM.IOSLog.OnSuccess" units="records"
+    expires_after="2021-04-11">
+  <owner>eugenebut@chromium.org</owner>
+  <owner>rkaplow@chromium.org</owner>
+  <summary>
+    Number of times when UKM.LogSize.OnSuccess was recorded on iOS. Recorded
+    when the app transitions to the foreground. This histogram should help to
+    figure out the root cause of crbug.com/1154678 (Data loss on UMA in iOS). If
+    total count of UKM.IOSLog.OnSuccess is greater than number of
+    UKM.LogSize.OnSuccess records, then data loss is caused by failure to write
+    the histogram to the disk on background thread. Otherwise (if total sum of
+    UKM.IOSLog.OnSuccess is equal to total count of UKM.LogSize.OnSuccess
+    records) then there is actually no data loss and app simply gets terminated
+    in a short window between UKM reached the server and the API call which
+    records the data.
+  </summary>
+</histogram>
+
 <histogram name="UKM.LogSize.OnSuccess" units="KB" expires_after="2021-04-11">
   <owner>rkaplow@chromium.org</owner>
   <owner>ukm-team@google.com</owner>