[CrOS MultiDevice] Add metrics for

MultiDevice.DeviceSyncService.SetSoftwareFeatureState.Result.FailureReason

Add metrics which breaks down the percentages of reasons when
failure occurs on SetSoftwareFeatureState.

Bug: 870138
Change-Id: I277eeb3f0e0e3fe4418affb91977c7ddf74a9365
Reviewed-on: https://chromium-review.googlesource.com/c/1308979Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Regan Hsu <hsuregan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604660}

chromeos/services/device_sync/device_sync_impl.cc

@@ -43,6 +43,65 @@ void RegisterDeviceSyncPrefs(PrefRegistrySimple* registry) {
 constexpr base::TimeDelta kSetFeatureEnabledTimeout =
     base::TimeDelta::FromSeconds(5);
 
+// This enum is tied directly to a UMA enum defined in
+// //tools/metrics/histograms/enums.xml, and should always reflect it (do not
+// change one without changing the other). Entries should be never modified
+// or deleted. Only additions possible.
+enum class DeviceSyncRequestFailureReason {
+  kRequestSucceededButUnexpectedResult = 0,
+  kServiceNotYetInitialized = 1,
+  kOffline = 2,
+  kEndpointNotFound = 3,
+  kAuthenticationError = 4,
+  kBadRequest = 5,
+  kResponseMalformed = 6,
+  kInternalServerError = 7,
+  kUnknownNetworkError = 8,
+  kUnknown = 9,
+  kMaxValue = kUnknown
+};
+
+DeviceSyncRequestFailureReason GetDeviceSyncRequestFailureReason(
+    mojom::NetworkRequestResult failure_reason) {
+  switch (failure_reason) {
+    case mojom::NetworkRequestResult::kRequestSucceededButUnexpectedResult:
+      return DeviceSyncRequestFailureReason::
+          kRequestSucceededButUnexpectedResult;
+    case mojom::NetworkRequestResult::kServiceNotYetInitialized:
+      return DeviceSyncRequestFailureReason::kServiceNotYetInitialized;
+    case mojom::NetworkRequestResult::kOffline:
+      return DeviceSyncRequestFailureReason::kOffline;
+    case mojom::NetworkRequestResult::kEndpointNotFound:
+      return DeviceSyncRequestFailureReason::kEndpointNotFound;
+    case mojom::NetworkRequestResult::kAuthenticationError:
+      return DeviceSyncRequestFailureReason::kAuthenticationError;
+    case mojom::NetworkRequestResult::kBadRequest:
+      return DeviceSyncRequestFailureReason::kBadRequest;
+    case mojom::NetworkRequestResult::kResponseMalformed:
+      return DeviceSyncRequestFailureReason::kResponseMalformed;
+    case mojom::NetworkRequestResult::kInternalServerError:
+      return DeviceSyncRequestFailureReason::kInternalServerError;
+    case mojom::NetworkRequestResult::kUnknown:
+      return DeviceSyncRequestFailureReason::kUnknownNetworkError;
+    default:
+      return DeviceSyncRequestFailureReason::kUnknown;
+  }
+  NOTREACHED();
+}
+
+void RecordSetSoftwareFeatureStateResult(bool success) {
+  UMA_HISTOGRAM_BOOLEAN(
+      "MultiDevice.DeviceSyncService.SetSoftwareFeatureState.Result", success);
+}
+
+void RecordSetSoftwareFeatureStateResultFailureReason(
+    DeviceSyncRequestFailureReason failure_reason) {
+  UMA_HISTOGRAM_ENUMERATION(
+      "MultiDevice.DeviceSyncService.SetSoftwareFeatureState.Result."
+      "FailureReason",
+      failure_reason);
+}
+
 }  // namespace
 
 // static
@@ -240,6 +299,8 @@ void DeviceSyncImpl::SetSoftwareFeatureState(
         mojom::NetworkRequestResult::kServiceNotYetInitialized);
 
     RecordSetSoftwareFeatureStateResult(false /* success */);
+    RecordSetSoftwareFeatureStateResultFailureReason(
+        DeviceSyncRequestFailureReason::kServiceNotYetInitialized);
     return;
   }
 
@@ -506,6 +567,9 @@ void DeviceSyncImpl::OnSetSoftwareFeatureStateError(
   id_to_pending_set_software_feature_request_map_.erase(it);
 
   RecordSetSoftwareFeatureStateResult(false /* success */);
+  RecordSetSoftwareFeatureStateResultFailureReason(
+      GetDeviceSyncRequestFailureReason(
+          mojo::ConvertTo<mojom::NetworkRequestResult>(error)));
 }
 
 void DeviceSyncImpl::OnFindEligibleDevicesSuccess(
@@ -577,6 +641,8 @@ void DeviceSyncImpl::OnSetSoftwareFeatureTimerFired() {
     it = id_to_pending_set_software_feature_request_map_.erase(it);
 
     RecordSetSoftwareFeatureStateResult(false /* success */);
+    RecordSetSoftwareFeatureStateResultFailureReason(
+        DeviceSyncRequestFailureReason::kRequestSucceededButUnexpectedResult);
   }
 }
 
@@ -585,11 +651,6 @@ void DeviceSyncImpl::SetPrefConnectionDelegateForTesting(
   pref_connection_delegate_ = std::move(pref_connection_delegate);
 }
 
-void DeviceSyncImpl::RecordSetSoftwareFeatureStateResult(bool success) {
-  UMA_HISTOGRAM_BOOLEAN(
-      "MultiDevice.DeviceSyncService.SetSoftwareFeatureState.Result", success);
-}
-
 }  // namespace device_sync
 
 }  // namespace chromeos

chromeos/services/device_sync/device_sync_impl.h

@@ -203,8 +203,6 @@ class DeviceSyncImpl : public DeviceSyncBase,
   void SetPrefConnectionDelegateForTesting(
       std::unique_ptr<PrefConnectionDelegate> pref_connection_delegate);
 
-  static void RecordSetSoftwareFeatureStateResult(bool success);
-
   identity::IdentityManager* identity_manager_;
   gcm::GCMDriver* gcm_driver_;
   service_manager::Connector* connector_;

chromeos/services/device_sync/device_sync_service_unittest.cc

@@ -1171,6 +1171,10 @@ TEST_F(DeviceSyncServiceTest, SetSoftwareFeatureState_Success) {
       "MultiDevice.DeviceSyncService.SetSoftwareFeatureState.Result", false, 0);
   histogram_tester().ExpectBucketCount<bool>(
       "MultiDevice.DeviceSyncService.SetSoftwareFeatureState.Result", true, 1);
+  histogram_tester().ExpectTotalCount(
+      "MultiDevice.DeviceSyncService.SetSoftwareFeatureState.Result."
+      "FailureReason",
+      0);
 }
 
 TEST_F(DeviceSyncServiceTest,
@@ -1215,6 +1219,10 @@ TEST_F(DeviceSyncServiceTest,
 
   histogram_tester().ExpectBucketCount<bool>(
       "MultiDevice.DeviceSyncService.SetSoftwareFeatureState.Result", false, 1);
+  histogram_tester().ExpectTotalCount(
+      "MultiDevice.DeviceSyncService.SetSoftwareFeatureState.Result."
+      "FailureReason",
+      1);
   histogram_tester().ExpectBucketCount<bool>(
       "MultiDevice.DeviceSyncService.SetSoftwareFeatureState.Result", true, 0);
 }
@@ -1257,6 +1265,10 @@ TEST_F(DeviceSyncServiceTest, SetSoftwareFeatureState_Error) {
 
   histogram_tester().ExpectBucketCount<bool>(
       "MultiDevice.DeviceSyncService.SetSoftwareFeatureState.Result", false, 1);
+  histogram_tester().ExpectTotalCount(
+      "MultiDevice.DeviceSyncService.SetSoftwareFeatureState.Result."
+      "FailureReason",
+      1);
   histogram_tester().ExpectBucketCount<bool>(
       "MultiDevice.DeviceSyncService.SetSoftwareFeatureState.Result", true, 0);
 }

tools/metrics/histograms/enums.xml

@@ -10714,6 +10714,19 @@ Called by update_net_error_codes.py.-->
   <int value="12" label="LOG_IN_STARTED_FROM_REGISTER_OVERLAY_PROMO"/>
 </enum>
 
+<enum name="DeviceSyncRequestFailureReason">
+  <int value="0" label="Requested succeeded but unexpected result"/>
+  <int value="1" label="Service not yet initialized"/>
+  <int value="2" label="Device offline"/>
+  <int value="3" label="Server endpoint not found"/>
+  <int value="4" label="Authentication error"/>
+  <int value="5" label="Bad request"/>
+  <int value="6" label="Response malformed"/>
+  <int value="7" label="Internal server error"/>
+  <int value="8" label="Unknown network error"/>
+  <int value="9" label="Unknown error"/>
+</enum>
+
 <enum name="DeviceTechnologyType">
   <int value="0" label="Unknown"/>
   <int value="1" label="Ethernet"/>

tools/metrics/histograms/histograms.xml

@@ -50874,6 +50874,16 @@ uploading your change for review.
   <summary>Result of enabling and disabling features for devices.</summary>
 </histogram>
 
+<histogram
+    name="MultiDevice.DeviceSyncService.SetSoftwareFeatureState.Result.FailureReason"
+    enum="DeviceSyncRequestFailureReason">
+  <owner>hansberry@chromium.org</owner>
+  <summary>
+    Breaks down the percentages of reasons when failure occurs on
+    SetSoftwareFeatureState.
+  </summary>
+</histogram>
+
 <histogram name="MultiDevice.PostOOBESetupFlow.PageShown"
     enum="MultiDevice_PostOOBESetupFlow_Page">
   <owner>hansberry@chromium.org</owner>