This CL logs the safe disconnection result in the analytics recorder.

PiperOrigin-RevId: 655757596
This commit is contained in:
hai007
2024-07-24 17:41:00 -07:00
committed by Copybara-Service
parent 65df1e8195
commit bc77c39d84
13 changed files with 98 additions and 21 deletions
@@ -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<LogicalConnection> &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());
@@ -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<location::nearby::analytics::proto::ConnectionsLog::Payload>
ResolvePendingPayloads(
absl::btree_map<std::int64_t, std::unique_ptr<PendingPayload>>
@@ -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();
@@ -20,6 +20,7 @@
#include <utility>
#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);
}
}
@@ -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;
@@ -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));
@@ -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::
@@ -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;
@@ -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;
@@ -17,15 +17,12 @@
#include <memory>
#include <string>
#include <utility>
#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"
@@ -16,13 +16,15 @@
#include <algorithm>
#include <cstdint>
#include <functional>
#include <memory>
#include <string>
#include <utility>
#include <vector>
#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"
@@ -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, (),
@@ -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::