[CrOS PhoneHub] Add metrics for result of each Nearby operation

Adds a new metric for each API call made from SecureChannel to the
Nearby Connections library. Because values will now be used in metrics,
this CL also adds explicit numeric values to
nearby_connection_types.mojom.

Fixed: 1163979
Bug: 1106937
Change-Id: I15ddd5861cc22dce109970de0bea46e9adae043c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2617045Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Ryan Hansberry <hansberry@chromium.org>
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Auto-Submit: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#841770}

chrome/browser/chromeos/secure_channel/nearby_connection_broker_impl.cc

@@ -218,6 +218,8 @@ void NearbyConnectionBrokerImpl::OnDiscoveryFailure() {
 }
 
 void NearbyConnectionBrokerImpl::OnRequestConnectionResult(Status status) {
+  util::RecordRequestConnectionResult(status);
+
   // In the success case, OnConnectionInitiated() is expected to be called to
   // continue the flow, so nothing else needs to be done in this callback.
   if (status == Status::kSuccess)
@@ -228,6 +230,8 @@ void NearbyConnectionBrokerImpl::OnRequestConnectionResult(Status status) {
 }
 
 void NearbyConnectionBrokerImpl::OnAcceptConnectionResult(Status status) {
+  util::RecordAcceptConnectionResult(status);
+
   if (status == Status::kSuccess) {
     DCHECK_EQ(ConnectionStatus::kAcceptingConnection, connection_status_);
     TransitionToStatus(
@@ -242,6 +246,8 @@ void NearbyConnectionBrokerImpl::OnAcceptConnectionResult(Status status) {
 void NearbyConnectionBrokerImpl::OnSendPayloadResult(
     SendMessageCallback callback,
     Status status) {
+  util::RecordSendPayloadResult(status);
+
   bool success = status == Status::kSuccess;
   std::move(callback).Run(success);
 
@@ -256,6 +262,8 @@ void NearbyConnectionBrokerImpl::OnSendPayloadResult(
 }
 
 void NearbyConnectionBrokerImpl::OnDisconnectFromEndpointResult(Status status) {
+  util::RecordDisconnectFromEndpointResult(status);
+
   // If the disconnection was successful, wait for the OnDisconnected()
   // callback.
   if (status == Status::kSuccess)

chrome/browser/chromeos/secure_channel/nearby_endpoint_finder_impl.cc

@@ -6,6 +6,7 @@
 
 #include "base/memory/ptr_util.h"
 #include "base/rand_util.h"
+#include "chrome/browser/chromeos/secure_channel/util/histogram_util.h"
 #include "chromeos/components/multidevice/logging/logging.h"
 #include "chromeos/services/secure_channel/public/mojom/nearby_connector.mojom.h"
 
@@ -25,6 +26,8 @@ const size_t kEndpointIdLength = 4u;
 const size_t kEndpointInfoLength = 4u;
 
 void OnStopDiscoveryDestructorResult(Status status) {
+  util::RecordStopDiscoveryResult(status);
+
   if (status != Status::kSuccess)
     PA_LOG(WARNING) << "Failed to stop discovery as part of destructor";
 }
@@ -100,6 +103,8 @@ void NearbyEndpointFinderImpl::OnEndpointFound(const std::string& endpoint_id,
 }
 
 void NearbyEndpointFinderImpl::OnStartDiscoveryResult(Status status) {
+  util::RecordStartDiscoveryResult(status);
+
   if (status != Status::kSuccess) {
     PA_LOG(WARNING) << "Failed to start Nearby discovery: " << status;
     is_discovery_active_ = false;
@@ -117,6 +122,8 @@ void NearbyEndpointFinderImpl::OnStartDiscoveryResult(Status status) {
 }
 
 void NearbyEndpointFinderImpl::OnInjectBluetoothEndpointResult(Status status) {
+  util::RecordInjectEndpointResult(status);
+
   if (status != Status::kSuccess) {
     PA_LOG(WARNING) << "Failed to inject Bluetooth endpoint: " << status;
     NotifyEndpointDiscoveryFailure();
@@ -129,6 +136,8 @@ void NearbyEndpointFinderImpl::OnInjectBluetoothEndpointResult(Status status) {
 void NearbyEndpointFinderImpl::OnStopDiscoveryResult(
     location::nearby::connections::mojom::DiscoveredEndpointInfoPtr info,
     Status status) {
+  util::RecordStopDiscoveryResult(status);
+
   is_discovery_active_ = false;
 
   if (status != Status::kSuccess) {

chrome/browser/chromeos/secure_channel/util/histogram_util.cc

@@ -9,6 +9,49 @@
 namespace chromeos {
 namespace secure_channel {
 namespace util {
+namespace {
+using location::nearby::connections::mojom::Status;
+}  // namespace
+
+void RecordStartDiscoveryResult(Status status) {
+  base::UmaHistogramEnumeration(
+      "MultiDevice.SecureChannel.Nearby.OperationResult.StartDiscovery",
+      status);
+}
+
+void RecordInjectEndpointResult(Status status) {
+  base::UmaHistogramEnumeration(
+      "MultiDevice.SecureChannel.Nearby.OperationResult.InjectEndpoint",
+      status);
+}
+
+void RecordStopDiscoveryResult(Status status) {
+  base::UmaHistogramEnumeration(
+      "MultiDevice.SecureChannel.Nearby.OperationResult.StopDiscovery", status);
+}
+
+void RecordRequestConnectionResult(Status status) {
+  base::UmaHistogramEnumeration(
+      "MultiDevice.SecureChannel.Nearby.OperationResult.RequestConnection",
+      status);
+}
+
+void RecordAcceptConnectionResult(Status status) {
+  base::UmaHistogramEnumeration(
+      "MultiDevice.SecureChannel.Nearby.OperationResult.AcceptConnection",
+      status);
+}
+
+void RecordSendPayloadResult(Status status) {
+  base::UmaHistogramEnumeration(
+      "MultiDevice.SecureChannel.Nearby.OperationResult.SendPayload", status);
+}
+
+void RecordDisconnectFromEndpointResult(Status status) {
+  base::UmaHistogramEnumeration(
+      "MultiDevice.SecureChannel.Nearby.OperationResult.DisconnectFromEndpoint",
+      status);
+}
 
 void LogMessageAction(MessageAction message_action) {
   base::UmaHistogramEnumeration(

chrome/browser/chromeos/secure_channel/util/histogram_util.h

@@ -5,10 +5,28 @@
 #ifndef CHROME_BROWSER_CHROMEOS_SECURE_CHANNEL_UTIL_HISTOGRAM_UTIL_H_
 #define CHROME_BROWSER_CHROMEOS_SECURE_CHANNEL_UTIL_HISTOGRAM_UTIL_H_
 
+#include "chromeos/services/nearby/public/mojom/nearby_connections_types.mojom.h"
+
 namespace chromeos {
 namespace secure_channel {
 namespace util {
 
+// Logs the result of Nearby Connections API functions.
+void RecordStartDiscoveryResult(
+    location::nearby::connections::mojom::Status status);
+void RecordInjectEndpointResult(
+    location::nearby::connections::mojom::Status status);
+void RecordStopDiscoveryResult(
+    location::nearby::connections::mojom::Status status);
+void RecordRequestConnectionResult(
+    location::nearby::connections::mojom::Status status);
+void RecordAcceptConnectionResult(
+    location::nearby::connections::mojom::Status status);
+void RecordSendPayloadResult(
+    location::nearby::connections::mojom::Status status);
+void RecordDisconnectFromEndpointResult(
+    location::nearby::connections::mojom::Status status);
+
 // Enumeration of possible message transfer action via Nearby Connection
 // library. Keep in sync with corresponding enum in
 // tools/metrics/histograms/enums.xml. These values are persisted to logs.

chromeos/services/nearby/public/mojom/nearby_connections_types.mojom

@@ -7,51 +7,53 @@ module location.nearby.connections.mojom;
 import "device/bluetooth/public/mojom/uuid.mojom";
 import "mojo/public/mojom/base/file.mojom";
 
-// Generic result status of NearbyConnections API calls.
+// Generic result status of NearbyConnections API calls. These values are
+// persisted to logs. Entries should not be renumbered and numeric values should
+// never be reused.
 enum Status {
   // The operation was successful.
-  kSuccess,
+  kSuccess = 0,
   // The operation failed, without any more information.
-  kError,
+  kError = 1,
   // The app called an API method out of order (i.e. another method is expected
   // to be called first).
-  kOutOfOrderApiCall,
+  kOutOfOrderApiCall = 2,
   // The app already has active operations (advertising, discovering, or
   // connected to other devices) with another Strategy. Stop these operations on
   // the current Strategy before trying to advertise or discover with a new
   // Strategy.
-  kAlreadyHaveActiveStrategy,
+  kAlreadyHaveActiveStrategy = 3,
   // The app is already advertising; call StopAdvertising() before trying to
   // advertise again.
-  kAlreadyAdvertising,
+  kAlreadyAdvertising = 4,
   // The app is already discovering; call StopDiscovery() before trying to
   // discover again.
-  kAlreadyDiscovering,
+  kAlreadyDiscovering = 5,
   // An attempt to read from/write to a connected remote endpoint failed. If
   // this occurs repeatedly, consider invoking DisconnectFromEndpoint().
-  kEndpointIOError,
+  kEndpointIOError = 6,
   // An attempt to interact with a remote endpoint failed because it's unknown
   // to us -- it's either an endpoint that was never discovered, or an endpoint
   // that never connected to us (both of which are indicative of bad input from
   // the client app).
-  kEndpointUnknown,
+  kEndpointUnknown = 7,
   // The remote endpoint rejected the connection request.
-  kConnectionRejected,
+  kConnectionRejected = 8,
   // The app is already connected to the specified endpoint. Multiple
   // connections to a remote endpoint cannot be maintained simultaneously.
-  kAlreadyConnectedToEndpoint,
+  kAlreadyConnectedToEndpoint = 9,
   // The remote endpoint is not connected; messages cannot be sent to it.
-  kNotConnectedToEndpoint,
+  kNotConnectedToEndpoint = 10,
   // There was an error trying to use the device's Bluetooth capabilities.
-  kBluetoothError,
+  kBluetoothError = 11,
   // There was an error trying to use the device's Bluetooth Low Energy
   // capabilities.
-  kBleError,
+  kBleError = 12,
   // There was an error trying to use the device's WiFi capabilities.
-  kWifiLanError,
+  kWifiLanError = 13,
   // An attempt to interact with an in-flight Payload failed because it's
   // unknown to us.
-  kPayloadUnknown,
+  kPayloadUnknown = 14,
 };
 
 // Information about a connection that is being initiated.

tools/metrics/histograms/enums.xml

@@ -50940,6 +50940,24 @@ Called by update_use_counter_css.py.-->
   <int value="9" label="WebRTC"/>
 </enum>
 
+<enum name="NearbyConnectionsStatus">
+  <int value="0" label="Success"/>
+  <int value="1" label="Error"/>
+  <int value="2" label="Out-of-order API call"/>
+  <int value="3" label="Already have active strategy"/>
+  <int value="4" label="Already advertising"/>
+  <int value="5" label="Already discovering"/>
+  <int value="6" label="Endpoint IO error"/>
+  <int value="7" label="Endpoint unknown"/>
+  <int value="8" label="Connection rejected"/>
+  <int value="9" label="Already connected to endpoint"/>
+  <int value="10" label="Not connected to endpoint"/>
+  <int value="11" label="Bluetooth error"/>
+  <int value="12" label="BLE error"/>
+  <int value="13" label="Wi-Fi LAN error"/>
+  <int value="14" label="Payload unknown"/>
+</enum>
+
 <enum name="NearbyShareCertificateManagerGetDecryptedPublicCertificateResult">
   <int value="0" label="Success"/>
   <int value="1" label="No Match"/>

tools/metrics/histograms/histograms_xml/multi_device/histograms.xml

@@ -316,6 +316,27 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   </summary>
 </histogram>
 
+<histogram name="MultiDevice.SecureChannel.Nearby.OperationResult.{Function}"
+    enum="NearbyConnectionsStatus" expires_after="2022-01-01">
+  <owner>khorimoto@chromium.org</owner>
+  <owner>better-together-dev@google.com</owner>
+  <summary>
+    Records the result of invoking Nearby Connection's {Function} API function
+    within SecureChannel.
+
+    Emitted when the API call returns.
+  </summary>
+  <token key="Function">
+    <variant name="AcceptConnection"/>
+    <variant name="DisconnectFromEndpoint"/>
+    <variant name="InjectEndpoint"/>
+    <variant name="RequestConnection"/>
+    <variant name="SendPayload"/>
+    <variant name="StartDiscovery"/>
+    <variant name="StopDiscovery"/>
+  </token>
+</histogram>
+
 <histogram name="MultiDevice.SecureChannel.Nearby.SendMessageResult"
     enum="BooleanSuccess" expires_after="2021-11-30">
   <owner>khorimoto@chromium.org</owner>