diff --git a/connections/implementation/analytics/analytics_recorder.cc b/connections/implementation/analytics/analytics_recorder.cc index cb50febf..c06d5669 100644 --- a/connections/implementation/analytics/analytics_recorder.cc +++ b/connections/implementation/analytics/analytics_recorder.cc @@ -27,6 +27,7 @@ #include "absl/container/btree_map.h" #include "absl/strings/string_view.h" #include "absl/time/time.h" +#include "connections/payload_type.h" #include "internal/analytics/event_logger.h" #include "internal/platform/count_down_latch.h" #include "internal/platform/error_code_params.h" @@ -95,6 +96,8 @@ using ::location::nearby::proto::connections::UPGRADE_SUCCESS; using ::location::nearby::proto::connections::UPGRADE_UNFINISHED; using ::location::nearby::proto::connections::UPGRADED; using ::nearby::analytics::EventLogger; +using SafeDisconnectionResult = ::location::nearby::analytics::proto:: + ConnectionsLog::EstablishedConnection::SafeDisconnectionResult; AnalyticsRecorder::AnalyticsRecorder(EventLogger *event_logger) : event_logger_(event_logger) { @@ -475,12 +478,14 @@ void AnalyticsRecorder::OnConnectionEstablished( void AnalyticsRecorder::OnConnectionClosed(const std::string &endpoint_id, Medium medium, - DisconnectionReason reason) { + DisconnectionReason reason, + SafeDisconnectionResult result) { MutexLock lock(&mutex_); NEARBY_LOGS(INFO) << __func__ << ": OnConnectionClosed is called with endpoint_id:" << endpoint_id << ", medium:" << Medium_Name(medium) - << ", reason:" << DisconnectionReason_Name(reason); + << ", reason:" << DisconnectionReason_Name(reason) + << ", result:" << result; if (!CanRecordAnalyticsLocked("OnConnectionClosed")) { return; @@ -498,7 +503,7 @@ void AnalyticsRecorder::OnConnectionClosed(const std::string &endpoint_id, return; } const std::unique_ptr &logical_connection = it->second; - logical_connection->PhysicalConnectionClosed(medium, reason); + logical_connection->PhysicalConnectionClosed(medium, reason, result); if (reason != UPGRADED) { // Unless this is an upgraded connection, remove this from our active // connections. Any future communication with an endpoint will need to be @@ -1127,7 +1132,7 @@ void AnalyticsRecorder::LogicalConnection::PhysicalConnectionEstablished( } void AnalyticsRecorder::LogicalConnection::PhysicalConnectionClosed( - Medium medium, DisconnectionReason reason) { + Medium medium, DisconnectionReason reason, SafeDisconnectionResult result) { if (current_medium_ == UNKNOWN_MEDIUM) { NEARBY_LOGS(WARNING) << "Unexpected call to PhysicalConnectionClosed() for medium " @@ -1159,7 +1164,7 @@ void AnalyticsRecorder::LogicalConnection::PhysicalConnectionClosed( established_connection->disconnection_reason()); return; } - FinishPhysicalConnection(established_connection, reason); + FinishPhysicalConnection(established_connection, reason, result); if (medium == current_medium_) { // If the EstablishedConnection we just closed was the one that we have @@ -1173,7 +1178,9 @@ void AnalyticsRecorder::LogicalConnection::CloseAllPhysicalConnections() { ConnectionsLog::EstablishedConnection *established_connection = physical_connection.second.get(); if (!established_connection->has_disconnection_reason()) { - FinishPhysicalConnection(established_connection, UNFINISHED); + FinishPhysicalConnection( + established_connection, UNFINISHED, + ConnectionsLog::EstablishedConnection::SAFE_DISCONNECTION); } } current_medium_ = UNKNOWN_MEDIUM; @@ -1269,8 +1276,9 @@ void AnalyticsRecorder::LogicalConnection::OutgoingPayloadDone( void AnalyticsRecorder::LogicalConnection::FinishPhysicalConnection( ConnectionsLog::EstablishedConnection *established_connection, - DisconnectionReason reason) { + DisconnectionReason reason, SafeDisconnectionResult result) { established_connection->set_disconnection_reason(reason); + established_connection->set_safe_disconnection_result(result); established_connection->set_duration_millis( absl::ToUnixMillis(SystemClock::ElapsedRealtime()) - established_connection->duration_millis()); diff --git a/connections/implementation/analytics/analytics_recorder.h b/connections/implementation/analytics/analytics_recorder.h index 141022af..7145f1ac 100644 --- a/connections/implementation/analytics/analytics_recorder.h +++ b/connections/implementation/analytics/analytics_recorder.h @@ -119,7 +119,7 @@ class AnalyticsRecorder { bool wifi_hotspot_enabled = false, int max_wifi_tx_speed = 0, int max_wifi_rx_speed = 0, int channel_width = -1); - // Connection established + // Connection establishedSafeDisconnectionResult void OnConnectionEstablished( const std::string &endpoint_id, location::nearby::proto::connections::Medium medium, @@ -127,7 +127,9 @@ class AnalyticsRecorder { void OnConnectionClosed( const std::string &endpoint_id, location::nearby::proto::connections::Medium medium, - location::nearby::proto::connections ::DisconnectionReason reason) + location::nearby::proto::connections::DisconnectionReason reason, + location::nearby::analytics::proto::ConnectionsLog:: + EstablishedConnection::SafeDisconnectionResult result) ABSL_LOCKS_EXCLUDED(mutex_); // Payload @@ -246,7 +248,9 @@ class AnalyticsRecorder { const std::string &connection_token); void PhysicalConnectionClosed( location::nearby::proto::connections::Medium medium, - location::nearby::proto::connections::DisconnectionReason reason); + location::nearby::proto::connections::DisconnectionReason reason, + location::nearby::analytics::proto::ConnectionsLog:: + EstablishedConnection::SafeDisconnectionResult result); void CloseAllPhysicalConnections(); void IncomingPayloadStarted( @@ -274,7 +278,9 @@ class AnalyticsRecorder { void FinishPhysicalConnection( location::nearby::analytics::proto::ConnectionsLog:: EstablishedConnection *established_connection, - location::nearby::proto::connections::DisconnectionReason reason); + location::nearby::proto::connections::DisconnectionReason reason, + location::nearby::analytics::proto::ConnectionsLog:: + EstablishedConnection::SafeDisconnectionResult result); std::vector ResolvePendingPayloads( absl::btree_map> diff --git a/connections/implementation/analytics/analytics_recorder_test.cc b/connections/implementation/analytics/analytics_recorder_test.cc index c65ed311..a2bc091e 100644 --- a/connections/implementation/analytics/analytics_recorder_test.cc +++ b/connections/implementation/analytics/analytics_recorder_test.cc @@ -734,7 +734,9 @@ TEST(AnalyticsRecorderTest, UnfinishedEstablishedConnectionsAddedAsUnfinished) { /*mediums=*/{BLE, BLUETOOTH}); analytics_recorder.OnConnectionEstablished(endpoint_id, BLUETOOTH, connection_token); - analytics_recorder.OnConnectionClosed(endpoint_id, BLUETOOTH, UPGRADED); + analytics_recorder.OnConnectionClosed( + endpoint_id, BLUETOOTH, UPGRADED, ConnectionsLog::EstablishedConnection:: + UNKNOWN_SAFE_DISCONNECTION_RESULT); analytics_recorder.OnConnectionEstablished(endpoint_id, WIFI_LAN, connection_token); @@ -788,7 +790,9 @@ TEST(AnalyticsRecorderTest, OutgoingPayloadUpgraded) { {endpoint_id}, payload_id, connections::PayloadType::kFile, 50); analytics_recorder.OnPayloadChunkSent(endpoint_id, payload_id, 10); analytics_recorder.OnPayloadChunkSent(endpoint_id, payload_id, 10); - analytics_recorder.OnConnectionClosed(endpoint_id, BLUETOOTH, UPGRADED); + analytics_recorder.OnConnectionClosed( + endpoint_id, BLUETOOTH, UPGRADED, + ConnectionsLog::EstablishedConnection::SAFE_DISCONNECTION); analytics_recorder.OnConnectionEstablished(endpoint_id, WIFI_LAN, connection_token); analytics_recorder.OnPayloadChunkSent(endpoint_id, payload_id, 10); @@ -796,7 +800,9 @@ TEST(AnalyticsRecorderTest, OutgoingPayloadUpgraded) { analytics_recorder.OnPayloadChunkSent(endpoint_id, payload_id, 10); analytics_recorder.OnOutgoingPayloadDone(endpoint_id, payload_id, SUCCESS); analytics_recorder.OnConnectionClosed(endpoint_id, WIFI_LAN, - LOCAL_DISCONNECTION); + LOCAL_DISCONNECTION, + ConnectionsLog::EstablishedConnection:: + SAFE_DISCONNECTION); analytics_recorder.LogSession(); ASSERT_TRUE(client_session_done_latch.Await(kDefaultTimeout).result()); @@ -1899,7 +1905,9 @@ TEST(AnalyticsRecorderOnConnectionClosedTest, // current_strategy_session_. analytics_recorder.OnConnectionEstablished(endpoint_id, BLUETOOTH, /*connection_token=*/""); - analytics_recorder.OnConnectionClosed(endpoint_id, BLUETOOTH, UPGRADED); + analytics_recorder.OnConnectionClosed( + endpoint_id, BLUETOOTH, UPGRADED, + ConnectionsLog::EstablishedConnection::SAFE_DISCONNECTION); analytics_recorder.LogSession(); diff --git a/connections/implementation/base_endpoint_channel.cc b/connections/implementation/base_endpoint_channel.cc index d404d7aa..560e624e 100644 --- a/connections/implementation/base_endpoint_channel.cc +++ b/connections/implementation/base_endpoint_channel.cc @@ -20,6 +20,7 @@ #include #include "absl/strings/str_cat.h" +#include "connections/implementation/endpoint_channel_manager.h" #include "connections/implementation/offline_frames.h" #include "internal/platform/byte_array.h" #include "internal/platform/exception.h" @@ -31,6 +32,9 @@ namespace nearby { namespace connections { namespace { +using ::location::nearby::analytics::proto::ConnectionsLog; +using DisconnectionReason = + ::location::nearby::proto::connections::DisconnectionReason; std::int32_t BytesToInt(const ByteArray& bytes) { const char* int_bytes = bytes.data(); @@ -309,12 +313,19 @@ void BaseEndpointChannel::SetAnalyticsRecorder( void BaseEndpointChannel::Close( location::nearby::proto::connections::DisconnectionReason reason) { + Close(reason, ConnectionsLog::EstablishedConnection::SAFE_DISCONNECTION); +} + +void BaseEndpointChannel::Close( + location::nearby::proto::connections::DisconnectionReason reason, + SafeDisconnectionResult result) { NEARBY_LOGS(INFO) << __func__ << ": Closing endpoint channel, reason: " << reason; Close(); if (analytics_recorder_ != nullptr && !endpoint_id_.empty()) { - analytics_recorder_->OnConnectionClosed(endpoint_id_, GetMedium(), reason); + analytics_recorder_->OnConnectionClosed(endpoint_id_, GetMedium(), reason, + result); } } diff --git a/connections/implementation/base_endpoint_channel.h b/connections/implementation/base_endpoint_channel.h index 3908de52..570e96ac 100644 --- a/connections/implementation/base_endpoint_channel.h +++ b/connections/implementation/base_endpoint_channel.h @@ -59,6 +59,10 @@ class BaseEndpointChannel : public EndpointChannel { void Close() ABSL_LOCKS_EXCLUDED(is_paused_mutex_) override; void Close(location::nearby::proto::connections::DisconnectionReason reason) override; + void Close( + location::nearby::proto::connections::DisconnectionReason reason, + location::nearby::analytics::proto::ConnectionsLog:: + EstablishedConnection::SafeDisconnectionResult result) override; std::string GetType() const override; std::string GetServiceId() const override; std::string GetName() const override; diff --git a/connections/implementation/connections_authentication_transport_test.cc b/connections/implementation/connections_authentication_transport_test.cc index ef72c23e..84774aa5 100644 --- a/connections/implementation/connections_authentication_transport_test.cc +++ b/connections/implementation/connections_authentication_transport_test.cc @@ -21,6 +21,8 @@ #include "gmock/gmock.h" #include "protobuf-matchers/protocol-buffer-matchers.h" #include "gtest/gtest.h" +#include "absl/time/time.h" +#include "connections/implementation/analytics/analytics_recorder.h" #include "connections/implementation/endpoint_channel.h" #include "internal/platform/byte_array.h" #include "internal/platform/exception.h" @@ -44,6 +46,11 @@ class MockEndpointChannel : public EndpointChannel { void, Close, (location::nearby::proto::connections::DisconnectionReason reason), (override)); + MOCK_METHOD(void, Close, + (location::nearby::proto::connections::DisconnectionReason reason, + location::nearby::analytics::proto::ConnectionsLog:: + EstablishedConnection::SafeDisconnectionResult result), + (override)); MOCK_METHOD(std::string, GetType, (), (const override)); MOCK_METHOD(std::string, GetServiceId, (), (const override)); MOCK_METHOD(std::string, GetName, (), (const override)); diff --git a/connections/implementation/encryption_runner_test.cc b/connections/implementation/encryption_runner_test.cc index 4dbd624e..5702c284 100644 --- a/connections/implementation/encryption_runner_test.cc +++ b/connections/implementation/encryption_runner_test.cc @@ -68,6 +68,12 @@ class FakeEndpointChannel : public EndpointChannel { override { Close(); } + void Close( + location::nearby::proto::connections::DisconnectionReason reason, + location::nearby::analytics::proto::ConnectionsLog:: + EstablishedConnection::SafeDisconnectionResult result) override { + Close(); + } location::nearby::proto::connections::ConnectionTechnology GetTechnology() const override { return location::nearby::proto::connections::ConnectionTechnology:: diff --git a/connections/implementation/endpoint_channel.h b/connections/implementation/endpoint_channel.h index 2626866e..19d8b0f3 100644 --- a/connections/implementation/endpoint_channel.h +++ b/connections/implementation/endpoint_channel.h @@ -52,6 +52,13 @@ class EndpointChannel { virtual void Close( location::nearby::proto::connections::DisconnectionReason reason) = 0; + // Closes this EndpointChannel and records the closure with the given reason + // and safe disconnection result. + virtual void Close( + location::nearby::proto::connections::DisconnectionReason reason, + location::nearby::analytics::proto::ConnectionsLog:: + EstablishedConnection::SafeDisconnectionResult result) = 0; + // Returns a one-word type descriptor for the concrete EndpointChannel // implementation that can be used in log messages; eg: BLUETOOTH, BLE, WIFI. virtual std::string GetType() const = 0; diff --git a/connections/implementation/endpoint_channel_manager.cc b/connections/implementation/endpoint_channel_manager.cc index 17970452..005fa92e 100644 --- a/connections/implementation/endpoint_channel_manager.cc +++ b/connections/implementation/endpoint_channel_manager.cc @@ -22,6 +22,7 @@ #include "connections/implementation/offline_frames.h" #include "internal/platform/condition_variable.h" #include "internal/platform/feature_flags.h" +#include "internal/platform/implementation/system_clock.h" #include "internal/platform/logging.h" #include "internal/platform/mutex.h" #include "internal/platform/mutex_lock.h" @@ -239,6 +240,7 @@ bool EndpointChannelManager::ChannelState::RemoveEndpoint( << endpoint_id; SystemClock::Sleep(kDataTransferDelay); } + NEARBY_LOGS(INFO) << "Remove Endpoint: " << endpoint_id; endpoints_.erase(item); return true; diff --git a/connections/implementation/endpoint_channel_manager.h b/connections/implementation/endpoint_channel_manager.h index df38a538..828bd3ac 100644 --- a/connections/implementation/endpoint_channel_manager.h +++ b/connections/implementation/endpoint_channel_manager.h @@ -17,15 +17,12 @@ #include #include -#include -#include "securegcm/d2d_connection_context_v1.h" #include "absl/base/thread_annotations.h" #include "absl/container/flat_hash_map.h" #include "absl/time/time.h" #include "connections/implementation/client_proxy.h" #include "connections/implementation/endpoint_channel.h" -#include "internal/platform/feature_flags.h" #include "internal/platform/mutex.h" #include "internal/proto/analytics/connections_log.pb.h" #include "proto/connections_enums.pb.h" diff --git a/connections/implementation/endpoint_manager.cc b/connections/implementation/endpoint_manager.cc index 2038f1d4..af223fab 100644 --- a/connections/implementation/endpoint_manager.cc +++ b/connections/implementation/endpoint_manager.cc @@ -16,13 +16,15 @@ #include #include -#include #include #include #include #include +#include "absl/functional/any_invocable.h" #include "absl/time/time.h" +#include "connections/connection_options.h" +#include "connections/implementation/analytics/packet_meta_data.h" #include "connections/implementation/analytics/throughput_recorder.h" #include "connections/implementation/client_proxy.h" #include "connections/implementation/endpoint_channel.h" @@ -31,13 +33,19 @@ #include "connections/implementation/payload_manager.h" #include "connections/implementation/proto/offline_wire_formats.pb.h" #include "connections/implementation/service_id_constants.h" +#include "connections/listeners.h" +#include "connections/medium_selector.h" +#include "connections/payload_type.h" #include "internal/platform/byte_array.h" #include "internal/platform/count_down_latch.h" #include "internal/platform/exception.h" #include "internal/platform/feature_flags.h" +#include "internal/platform/implementation/system_clock.h" #include "internal/platform/logging.h" #include "internal/platform/mutex.h" #include "internal/platform/mutex_lock.h" +#include "internal/platform/runnable.h" +#include "internal/platform/single_thread_executor.h" #include "internal/proto/analytics/connections_log.pb.h" #include "proto/connections_enums.pb.h" diff --git a/connections/implementation/endpoint_manager_test.cc b/connections/implementation/endpoint_manager_test.cc index 38ffc3b8..2666380d 100644 --- a/connections/implementation/endpoint_manager_test.cc +++ b/connections/implementation/endpoint_manager_test.cc @@ -28,7 +28,9 @@ #include "absl/time/clock.h" #include "absl/time/time.h" #include "connections/connection_options.h" +#include "connections/implementation/analytics/analytics_recorder.h" #include "connections/implementation/client_proxy.h" +#include "connections/implementation/endpoint_channel.h" #include "connections/implementation/endpoint_channel_manager.h" #include "connections/implementation/flags/nearby_connections_feature_flags.h" #include "connections/implementation/offline_frames.h" @@ -38,7 +40,6 @@ #include "internal/platform/byte_array.h" #include "internal/platform/count_down_latch.h" #include "internal/platform/exception.h" -// #include "internal/platform/feature_flags.h" #include "internal/platform/logging.h" #include "internal/platform/single_thread_executor.h" #include "internal/test/fake_single_thread_executor.h" @@ -70,6 +71,11 @@ class MockEndpointChannel : public EndpointChannel { (override)); MOCK_METHOD(void, Close, (), (override)); MOCK_METHOD(void, Close, (DisconnectionReason reason), (override)); + MOCK_METHOD(void, Close, + (DisconnectionReason reason, + location::nearby::analytics::proto::ConnectionsLog:: + EstablishedConnection::SafeDisconnectionResult result), + (override)); MOCK_METHOD(location::nearby::proto::connections::ConnectionTechnology, GetTechnology, (), (const override)); MOCK_METHOD(location::nearby::proto::connections::ConnectionBand, GetBand, (), diff --git a/connections/implementation/fake_endpoint_channel.h b/connections/implementation/fake_endpoint_channel.h index 24744c76..72725980 100644 --- a/connections/implementation/fake_endpoint_channel.h +++ b/connections/implementation/fake_endpoint_channel.h @@ -19,6 +19,7 @@ #include "connections/implementation/endpoint_channel.h" #include "internal/platform/byte_array.h" +#include "internal/platform/exception.h" namespace nearby { namespace connections { @@ -58,6 +59,12 @@ class FakeEndpointChannel : public EndpointChannel { is_closed_ = true; disconnection_reason_ = reason; } + void Close( + location::nearby::proto::connections::DisconnectionReason reason, + location::nearby::analytics::proto::ConnectionsLog:: + EstablishedConnection::SafeDisconnectionResult result) override { + Close(reason); + } location::nearby::proto::connections::ConnectionTechnology GetTechnology() const override { return location::nearby::proto::connections::ConnectionTechnology::