analytics: Implement to pass values for OperatoinResultWithMedium.

edwinwugoog · copybara-github · commit 9a434f3ec3a4 · 2024-12-09T09:02:05.000-08:00
PiperOrigin-RevId: 704304596
diff --git a/connections/implementation/analytics/analytics_recorder.cc b/connections/implementation/analytics/analytics_recorder.cc
@@ -91,9 +91,9 @@ using ::location::nearby::proto::connections::START_CLIENT_SESSION;
 using ::location::nearby::proto::connections::START_STRATEGY_SESSION;
 using ::location::nearby::proto::connections::STOP_CLIENT_SESSION;
 using ::location::nearby::proto::connections::STOP_STRATEGY_SESSION;
-using ::location::nearby::proto::connections::STREAM;
 using ::location::nearby::proto::connections::StopAdvertisingReason;
 using ::location::nearby::proto::connections::StopDiscoveringReason;
+using ::location::nearby::proto::connections::STREAM;
 using ::location::nearby::proto::connections::UNFINISHED;
 using ::location::nearby::proto::connections::UNFINISHED_ERROR;
 using ::location::nearby::proto::connections::UNKNOWN_MEDIUM;
@@ -108,7 +108,7 @@ using SafeDisconnectionResult = ::location::nearby::analytics::proto::
     ConnectionsLog::EstablishedConnection::SafeDisconnectionResult;
 
 // TODO(edwinwu): Add ifttt in Android counterpart.
-OperationResultCategory GetOperationResultCateory(
+OperationResultCategory ConvertToOperationResultCateory(
     OperationResultCode result_code) {
   if (result_code == OperationResultCode::DETAIL_SUCCESS) {
     return OperationResultCategory::CATEGORY_SUCCESS;
@@ -301,7 +301,7 @@ void AnalyticsRecorder::OnStopDiscovery() {
   if (!CanRecordAnalyticsLocked("OnStopDiscovery")) {
     return;
   }
-  RecordDiscoveryPhaseDurationAndReasonLocked(/*on_stop=*/ true);
+  RecordDiscoveryPhaseDurationAndReasonLocked(/*on_stop=*/true);
 }
 
 void AnalyticsRecorder::OnStartedIncomingConnectionListening(
@@ -501,7 +501,7 @@ void AnalyticsRecorder::OnIncomingConnectionAttemptLocked(
       std::make_unique<ConnectionsLog::OperationResult>();
   operation_result_proto->set_result_code(
       connection_attempt_metadata_params->operation_result_code);
-  operation_result_proto->set_result_category(GetOperationResultCateory(
+  operation_result_proto->set_result_category(ConvertToOperationResultCateory(
       connection_attempt_metadata_params->operation_result_code));
   connection_attempt->set_allocated_operation_result(
       operation_result_proto.release());
@@ -589,7 +589,7 @@ void AnalyticsRecorder::OnOutgoingConnectionAttemptLocked(
       std::make_unique<ConnectionsLog::OperationResult>();
   operation_result_proto->set_result_code(
       connection_attempt_metadata_params->operation_result_code);
-  operation_result_proto->set_result_category(GetOperationResultCateory(
+  operation_result_proto->set_result_category(ConvertToOperationResultCateory(
       connection_attempt_metadata_params->operation_result_code));
   connection_attempt->set_allocated_operation_result(
       operation_result_proto.release());
@@ -1144,7 +1144,7 @@ void AnalyticsRecorder::FinishDiscoveryPhaseLocked() {
       MarkConnectionRequestIgnoredLocked(connection_request.get());
       UpdateDiscovererConnectionRequestLocked(connection_request.get());
     }
-    RecordDiscoveryPhaseDurationAndReasonLocked(/* on_stop=*/ false);
+    RecordDiscoveryPhaseDurationAndReasonLocked(/* on_stop=*/false);
     *current_strategy_session_->add_discovery_phase() =
         *std::move(current_discovery_phase_);
   }
@@ -1323,7 +1323,7 @@ void AnalyticsRecorder::FinishUpgradeAttemptLocked(
         std::make_unique<ConnectionsLog::OperationResult>();
     operation_result_proto->set_result_code(operation_result_code);
     operation_result_proto->set_result_category(
-        GetOperationResultCateory(operation_result_code));
+        ConvertToOperationResultCateory(operation_result_code));
     attempt->set_allocated_operation_result(operation_result_proto.release());
     *current_strategy_session_->add_upgrade_attempt() = *attempt;
     if (erase_item) {
@@ -1426,7 +1426,7 @@ ConnectionsLog::Payload AnalyticsRecorder::PendingPayload::GetProtoPayload(
       std::make_unique<ConnectionsLog::OperationResult>();
   operation_result_proto->set_result_code(operation_result_code_);
   operation_result_proto->set_result_category(
-      GetOperationResultCateory(operation_result_code_));
+      ConvertToOperationResultCateory(operation_result_code_));
   payload.set_allocated_operation_result(operation_result_proto.release());
 
   return payload;
@@ -1696,6 +1696,11 @@ AnalyticsRecorder::LogicalConnection::GetPendingPayloadResultCodeFromReason(
   }
 }
 
+OperationResultCategory AnalyticsRecorder::GetOperationResultCateory(
+    location::nearby::proto::connections::OperationResultCode result_code) {
+  return ConvertToOperationResultCateory(result_code);
+}
+
 void AnalyticsRecorder::Sync() {
   CountDownLatch latch(1);
   serial_executor_.Execute([&]() { latch.CountDown(); });
diff --git a/connections/implementation/analytics/analytics_recorder.h b/connections/implementation/analytics/analytics_recorder.h
@@ -214,6 +214,10 @@ class AnalyticsRecorder {
 
   bool IsSessionLogged();
 
+  location::nearby::proto::connections::OperationResultCategory
+  GetOperationResultCateory(
+      location::nearby::proto::connections::OperationResultCode result_code);
+
   // Waits until all logs are sent to the backend.
   // For testing only.
   void Sync();
diff --git a/connections/implementation/base_pcp_handler.cc b/connections/implementation/base_pcp_handler.cc
@@ -99,6 +99,7 @@ std::string AuthenticationStatusToString(nearby::AuthenticationStatus status) {
 }
 }  // namespace
 
+using ::location::nearby::analytics::proto::ConnectionsLog;
 using ::location::nearby::connections::ConnectionRequestFrame;
 using ::location::nearby::connections::ConnectionResponseFrame;
 using ::location::nearby::connections::ConnectionsDevice;
@@ -267,9 +268,11 @@ Status BasePcpHandler::StartAdvertising(
         // Save the advertising options for local reference in later process
         // like upgrading bandwidth.
         advertising_listener_ = info.listener;
-        client->StartedAdvertising(service_id, GetStrategy(), info.listener,
-                                   absl::MakeSpan(result.mediums),
-                                   compatible_advertising_options);
+        client->StartedAdvertising(
+            service_id, GetStrategy(), info.listener,
+            absl::MakeSpan(result.mediums),
+            std::move(result.operation_result_with_mediums),
+            compatible_advertising_options);
         client->UpdateLocalEndpointInfo(info.endpoint_info.string_data());
         response.Set({Status::kSuccess});
       });
@@ -451,7 +454,9 @@ Status BasePcpHandler::StartDiscovery(ClientProxy* client,
             }
             client->StartedDiscovery(
                 service_id, GetStrategy(), std::move(listener),
-                absl::MakeSpan(result.mediums), stripped_discovery_options);
+                absl::MakeSpan(result.mediums),
+                std::move(result.operation_result_with_mediums),
+                stripped_discovery_options);
             response.Set({Status::kSuccess});
           });
   return WaitForResult(absl::StrCat("StartDiscovery(", service_id, ")"),
@@ -1142,6 +1147,25 @@ void BasePcpHandler::StripOutUnavailableMediums(
   }
 }
 
+ConnectionsLog::OperationResultWithMedium
+BasePcpHandler::GetOperationResultWithMediumByResultCode(
+    ClientProxy* client, location::nearby::proto::connections::Medium medium,
+    int update_index,
+    location::nearby::proto::connections::OperationResultCode
+        operation_result_code,
+    location::nearby::proto::connections::ConnectionMode connection_mode) {
+  ConnectionsLog::OperationResultWithMedium operation_result_with_medium;
+  operation_result_with_medium.set_medium(medium);
+  operation_result_with_medium.set_result_code(operation_result_code);
+  operation_result_with_medium.set_result_category(
+      client->GetAnalyticsRecorder().GetOperationResultCateory(
+          operation_result_code));
+  operation_result_with_medium.set_connection_mode(connection_mode);
+  operation_result_with_medium.set_update_index(update_index);
+
+  return operation_result_with_medium;
+}
+
 void BasePcpHandler::StripOutUnavailableMediums(
     DiscoveryOptions& discovery_options) {
   BooleanMediumSelector& allowed = discovery_options.allowed;
diff --git a/connections/implementation/base_pcp_handler.h b/connections/implementation/base_pcp_handler.h
@@ -195,6 +195,9 @@ class BasePcpHandler : public PcpHandler,
     // If success, the mediums on which we are now advertising/discovering, for
     // analytics.
     std::vector<location::nearby::proto::connections::Medium> mediums;
+    std::vector<location::nearby::analytics::proto::ConnectionsLog::
+                    OperationResultWithMedium>
+        operation_result_with_mediums;
   };
 
   // Represents an endpoint that we've discovered. Typically, the implementation
@@ -415,6 +418,15 @@ class BasePcpHandler : public PcpHandler,
 
   void StripOutWifiHotspotMedium(ConnectionInfo& connection_info);
 
+  location::nearby::analytics::proto::ConnectionsLog::OperationResultWithMedium
+  GetOperationResultWithMediumByResultCode(
+      ClientProxy* client, location::nearby::proto::connections::Medium medium,
+      int update_index,
+      location::nearby::proto::connections::OperationResultCode
+          operation_result_code,
+      location::nearby::proto::connections::ConnectionMode connection_mode =
+          location::nearby::proto::connections::ConnectionMode::LEGACY);
+
   // Test only.
   int GetEndpointLostByMediumAlarmsCount() RUN_ON_PCP_HANDLER_THREAD() {
     return endpoint_lost_by_medium_alarms_.size();
diff --git a/connections/implementation/client_proxy.cc b/connections/implementation/client_proxy.cc
@@ -70,6 +70,7 @@ namespace nearby {
 namespace connections {
 
 namespace {
+using ::location::nearby::analytics::proto::ConnectionsLog;
 using ::location::nearby::connections::OsInfo;
 
 constexpr char kEndpointIdChars[] = {
@@ -214,6 +215,8 @@ void ClientProxy::StartedAdvertising(
     const std::string& service_id, Strategy strategy,
     const ConnectionListener& listener,
     absl::Span<location::nearby::proto::connections::Medium> mediums,
+    const std::vector<ConnectionsLog::OperationResultWithMedium>&
+        operation_result_with_mediums,
     const AdvertisingOptions& advertising_options) {
   MutexLock lock(&mutex_);
   NEARBY_LOGS(INFO) << "ClientProxy [StartedAdvertising]: client="
@@ -245,7 +248,8 @@ void ClientProxy::StartedAdvertising(
   std::unique_ptr<AdvertisingMetadataParams> advertising_metadata_params;
   advertising_metadata_params =
       GetAnalyticsRecorder().BuildAdvertisingMetadataParams();
-  // TODO(edwinwu): Implement to pass real values for OperatoinResultWithMedium.
+  advertising_metadata_params->operation_result_with_mediums =
+      std::move(operation_result_with_mediums);
   analytics_recorder_->OnStartAdvertising(strategy, medium_vector,
                                           advertising_metadata_params.get());
 }
@@ -366,6 +370,8 @@ void ClientProxy::StartedDiscovery(
     const std::string& service_id, Strategy strategy,
     DiscoveryListener listener,
     absl::Span<location::nearby::proto::connections::Medium> mediums,
+    const std::vector<ConnectionsLog::OperationResultWithMedium>&
+        operation_result_with_mediums,
     const DiscoveryOptions& discovery_options) {
   MutexLock lock(&mutex_);
   discovery_info_ = DiscoveryInfo{service_id, std::move(listener)};
@@ -376,10 +382,10 @@ void ClientProxy::StartedDiscovery(
   std::unique_ptr<DiscoveryMetadataParams> discovery_metadata_params;
   discovery_metadata_params =
       GetAnalyticsRecorder().BuildDiscoveryMetadataParams();
-  // TODO(edwinwu): Implement to pass real values for OperatoinResultWithMedium.
-  analytics_recorder_->OnStartDiscovery(
-      strategy, medium_vector,
-      discovery_metadata_params.get());
+  discovery_metadata_params->operation_result_with_mediums =
+      std::move(operation_result_with_mediums);
+  analytics_recorder_->OnStartDiscovery(strategy, medium_vector,
+                                        discovery_metadata_params.get());
 }
 
 void ClientProxy::StoppedDiscovery() {
diff --git a/connections/implementation/client_proxy.h b/connections/implementation/client_proxy.h
@@ -53,6 +53,7 @@
 #include "absl/types/span.h"
 #include "internal/platform/os_name.h"
 #include "internal/platform/scheduled_executor.h"
+#include "internal/proto/analytics/connections_log.pb.h"
 
 namespace nearby {
 namespace connections {
@@ -101,6 +102,9 @@ class ClientProxy final {
       const std::string& service_id, Strategy strategy,
       const ConnectionListener& connection_lifecycle_listener,
       absl::Span<location::nearby::proto::connections::Medium> mediums,
+      const std::vector<location::nearby::analytics::proto::ConnectionsLog::
+                            OperationResultWithMedium>&
+          operation_result_with_medium,
       const AdvertisingOptions& advertising_options = AdvertisingOptions{});
   // Marks this client as not advertising.
   void StoppedAdvertising();
@@ -123,6 +127,9 @@ class ClientProxy final {
       const std::string& service_id, Strategy strategy,
       DiscoveryListener discovery_listener,
       absl::Span<location::nearby::proto::connections::Medium> mediums,
+      const std::vector<location::nearby::analytics::proto::ConnectionsLog::
+                            OperationResultWithMedium>&
+          operation_result_with_medium,
       const DiscoveryOptions& discovery_options = DiscoveryOptions{});
   // Marks this client as not discovering at all.
   void StoppedDiscovery();
diff --git a/connections/implementation/client_proxy_test.cc b/connections/implementation/client_proxy_test.cc
@@ -210,8 +210,9 @@ class ClientProxyTest : public ::testing::TestWithParam<FeatureFlags::Flags> {
         .info = ByteArray{"advertising endpoint name"},
         .id = client->GetLocalEndpointId(),
     };
-    client->StartedAdvertising(service_id_, strategy_, listener,
-                               absl::MakeSpan(mediums_), advertising_options);
+    client->StartedAdvertising(
+        service_id_, strategy_, listener, absl::MakeSpan(mediums_),
+        /*operation_result_with_medium=*/{}, advertising_options);
     return endpoint;
   }
 
@@ -256,7 +257,8 @@ class ClientProxyTest : public ::testing::TestWithParam<FeatureFlags::Flags> {
         .id = client->GetLocalEndpointId(),
     };
     client->StartedDiscovery(service_id_, strategy_, std::move(listener),
-                             absl::MakeSpan(mediums_));
+                             absl::MakeSpan(mediums_),
+                             /*operation_result_with_medium=*/{});
     return endpoint;
   }
 
@@ -607,7 +609,7 @@ TEST_F(ClientProxyTest, ResetClearsState) {
 }
 
 TEST_F(ClientProxyTest, StartedAdvertisingChangesStateFromIdle) {
-  client1()->StartedAdvertising(service_id_, strategy_, {}, {});
+  client1()->StartedAdvertising(service_id_, strategy_, {}, {}, {});
 
   EXPECT_TRUE(client1()->IsAdvertising());
   EXPECT_FALSE(client1()->IsDiscovering());
@@ -616,7 +618,7 @@ TEST_F(ClientProxyTest, StartedAdvertisingChangesStateFromIdle) {
 }
 
 TEST_F(ClientProxyTest, StartedDiscoveryChangesStateFromIdle) {
-  client1()->StartedDiscovery(service_id_, strategy_, {}, {});
+  client1()->StartedDiscovery(service_id_, strategy_, {}, {}, {});
 
   EXPECT_FALSE(client1()->IsAdvertising());
   EXPECT_TRUE(client1()->IsDiscovering());
diff --git a/connections/implementation/p2p_cluster_pcp_handler.cc b/connections/implementation/p2p_cluster_pcp_handler.cc
diff --git a/connections/implementation/service_controller_router_test.cc b/connections/implementation/service_controller_router_test.cc
