[CrOS MultiDevice] Create metrics that break down failures

of enabling and disabling features.

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

chromeos/services/device_sync/device_sync_impl.cc

@@ -71,6 +71,20 @@ enum class ForceCryptAuthOperationResult {
   kMaxValue = kServiceNotReady
 };
 
+// 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 DeviceSyncSetSoftwareFeature {
+  kUnknown = 0,
+  kBetterTogetherSuite = 1,
+  kSmartLock = 2,
+  kInstantTethering = 3,
+  kMessages = 4,
+  kUnexpectedClientFeature = 5,
+  kMaxValue = kUnexpectedClientFeature
+};
+
 DeviceSyncRequestFailureReason GetDeviceSyncRequestFailureReason(
     mojom::NetworkRequestResult failure_reason) {
   switch (failure_reason) {
@@ -112,6 +126,40 @@ void RecordSetSoftwareFeatureStateResultFailureReason(
       failure_reason);
 }
 
+DeviceSyncSetSoftwareFeature GetDeviceSyncSoftwareFeature(
+    cryptauth::SoftwareFeature software_feature) {
+  switch (software_feature) {
+    case cryptauth::SoftwareFeature::UNKNOWN_FEATURE:
+      return DeviceSyncSetSoftwareFeature::kUnknown;
+    case cryptauth::SoftwareFeature::BETTER_TOGETHER_HOST:
+      return DeviceSyncSetSoftwareFeature::kBetterTogetherSuite;
+    case cryptauth::SoftwareFeature::EASY_UNLOCK_HOST:
+      return DeviceSyncSetSoftwareFeature::kSmartLock;
+    case cryptauth::SoftwareFeature::MAGIC_TETHER_HOST:
+      return DeviceSyncSetSoftwareFeature::kInstantTethering;
+    case cryptauth::SoftwareFeature::SMS_CONNECT_HOST:
+      return DeviceSyncSetSoftwareFeature::kMessages;
+    default:
+      NOTREACHED();
+      return DeviceSyncSetSoftwareFeature::kUnexpectedClientFeature;
+  }
+}
+
+void RecordSetSoftwareFailedFeature(bool enabled,
+                                    cryptauth::SoftwareFeature feature) {
+  if (enabled) {
+    UMA_HISTOGRAM_ENUMERATION(
+        "MultiDevice.DeviceSyncService.SetSoftwareFeatureState.Enable."
+        "FailedFeature",
+        GetDeviceSyncSoftwareFeature(feature));
+  } else {
+    UMA_HISTOGRAM_ENUMERATION(
+        "MultiDevice.DeviceSyncService.SetSoftwareFeatureState.Disable."
+        "FailedFeature",
+        GetDeviceSyncSoftwareFeature(feature));
+  }
+}
+
 void RecordFindEligibleDevicesResult(bool success) {
   UMA_HISTOGRAM_BOOLEAN(
       "MultiDevice.DeviceSyncService.FindEligibleDevices.Result", success);
@@ -342,6 +390,7 @@ void DeviceSyncImpl::SetSoftwareFeatureState(
     RecordSetSoftwareFeatureStateResult(false /* success */);
     RecordSetSoftwareFeatureStateResultFailureReason(
         DeviceSyncRequestFailureReason::kServiceNotYetInitialized);
+    RecordSetSoftwareFailedFeature(enabled, software_feature);
     return;
   }
 
@@ -605,14 +654,16 @@ void DeviceSyncImpl::OnSetSoftwareFeatureStateError(
     return;
   }
 
-  it->second->InvokeCallback(
-      mojo::ConvertTo<mojom::NetworkRequestResult>(error));
-  id_to_pending_set_software_feature_request_map_.erase(it);
-
   RecordSetSoftwareFeatureStateResult(false /* success */);
   RecordSetSoftwareFeatureStateResultFailureReason(
       GetDeviceSyncRequestFailureReason(
           mojo::ConvertTo<mojom::NetworkRequestResult>(error)));
+  RecordSetSoftwareFailedFeature(it->second->enabled(),
+                                 it->second->software_feature());
+
+  it->second->InvokeCallback(
+      mojo::ConvertTo<mojom::NetworkRequestResult>(error));
+  id_to_pending_set_software_feature_request_map_.erase(it);
 }
 
 void DeviceSyncImpl::OnFindEligibleDevicesSuccess(
@@ -686,13 +737,15 @@ void DeviceSyncImpl::OnSetSoftwareFeatureTimerFired() {
   // callbacks and remove them from the map.
   auto it = id_to_pending_set_software_feature_request_map_.begin();
   while (it != id_to_pending_set_software_feature_request_map_.end()) {
-    it->second->InvokeCallback(
-        mojom::NetworkRequestResult::kRequestSucceededButUnexpectedResult);
-    it = id_to_pending_set_software_feature_request_map_.erase(it);
-
     RecordSetSoftwareFeatureStateResult(false /* success */);
     RecordSetSoftwareFeatureStateResultFailureReason(
         DeviceSyncRequestFailureReason::kRequestSucceededButUnexpectedResult);
+    RecordSetSoftwareFailedFeature(it->second->enabled(),
+                                   it->second->software_feature());
+
+    it->second->InvokeCallback(
+        mojom::NetworkRequestResult::kRequestSucceededButUnexpectedResult);
+    it = id_to_pending_set_software_feature_request_map_.erase(it);
   }
 }
 

chromeos/services/device_sync/device_sync_impl.h

@@ -150,6 +150,12 @@ class DeviceSyncImpl : public DeviceSyncBase,
 
     void InvokeCallback(mojom::NetworkRequestResult result);
 
+    cryptauth::SoftwareFeature software_feature() const {
+      return software_feature_;
+    }
+
+    bool enabled() const { return enabled_; }
+
    private:
     std::string device_public_key_;
     cryptauth::SoftwareFeature software_feature_;

tools/metrics/histograms/enums.xml

@@ -33870,6 +33870,15 @@ Called by update_use_counter_css.py.-->
   <int value="9" label="Unknown error"/>
 </enum>
 
+<enum name="MultiDevice_DeviceSyncService_Features">
+  <int value="0" label="Unknown"/>
+  <int value="1" label="Better Together Suite"/>
+  <int value="2" label="Smart Lock"/>
+  <int value="3" label="Instant Tethering"/>
+  <int value="4" label="Messages"/>
+  <int value="5" label="Unexpected Client Feature"/>
+</enum>
+
 <enum name="MultiDevice_DeviceSyncService_ForceCryptAuthOperationResult">
   <int value="0" label="Success"/>
   <int value="1" label="Service not ready"/>

tools/metrics/histograms/histograms.xml

@@ -51423,6 +51423,22 @@ uploading your change for review.
   <summary>Result for when ForceSyncNow is called.</summary>
 </histogram>
 
+<histogram
+    name="MultiDevice.DeviceSyncService.SetSoftwareFeatureState.Disable.FailedFeature"
+    enum="MultiDevice_DeviceSyncService_Features">
+  <owner>hansberry@chromium.org</owner>
+  <summary>
+    Breaks down which features failed when attempted to disable.
+  </summary>
+</histogram>
+
+<histogram
+    name="MultiDevice.DeviceSyncService.SetSoftwareFeatureState.Enable.FailedFeature"
+    enum="MultiDevice_DeviceSyncService_Features">
+  <owner>hansberry@chromium.org</owner>
+  <summary>Breaks down which features failed when attempted to enable.</summary>
+</histogram>
+
 <histogram name="MultiDevice.DeviceSyncService.SetSoftwareFeatureState.Result"
     enum="BooleanSuccess">
   <owner>hansberry@chromium.org</owner>