From 1a729f41637add03a9f79b8a1518a27faf175350 Mon Sep 17 00:00:00 2001 From: Kyle Horimoto Date: Thu, 7 Dec 2017 22:16:27 +0000 Subject: [PATCH] [CrOS Tether] Split up the "unknown error" metric for tether connections. This CL adds two additional error metrics: TETHERING_UNSUPPORTED and NO_CELL_DATA. These new metrics will enable us to better narrow down the cause of connection errors. Bug: 785514, 672263 Change-Id: I3f8b48173d522f3da463bbf5f5fc1101c7719cd5 Reviewed-on: https://chromium-review.googlesource.com/812604 Reviewed-by: Jeremy Klein Reviewed-by: Steven Holte Reviewed-by: Ryan Hansberry Commit-Queue: Kyle Horimoto Cr-Commit-Position: refs/heads/master@{#522572} --- .../tether/host_connection_metrics_logger.cc | 9 +++++++ .../tether/host_connection_metrics_logger.h | 10 +++++++- ...host_connection_metrics_logger_unittest.cc | 25 +++++++++++++++++++ chromeos/components/tether/proto/tether.proto | 2 ++ .../tether/tether_connector_impl.cc | 13 ++++++++++ .../tether/tether_connector_impl_unittest.cc | 19 ++++++++++++++ tools/metrics/histograms/enums.xml | 2 ++ tools/metrics/histograms/histograms.xml | 2 +- 8 files changed, 80 insertions(+), 2 deletions(-) diff --git a/chromeos/components/tether/host_connection_metrics_logger.cc b/chromeos/components/tether/host_connection_metrics_logger.cc index 4bbc6f0c37b231..2b4c4ebc76d6d4 100644 --- a/chromeos/components/tether/host_connection_metrics_logger.cc +++ b/chromeos/components/tether/host_connection_metrics_logger.cc @@ -59,6 +59,15 @@ void HostConnectionMetricsLogger::RecordConnectionToHostResult( ConnectionToHostResult_FailureTetheringTimeoutEventType:: FIRST_TIME_SETUP_WAS_NOT_REQUIRED); break; + case ConnectionToHostResult:: + CONNECTION_RESULT_FAILURE_TETHERING_UNSUPPORTED: + RecordConnectionResultFailure( + ConnectionToHostResult_FailureEventType::TETHERING_UNSUPPORTED); + break; + case ConnectionToHostResult::CONNECTION_RESULT_FAILURE_NO_CELL_DATA: + RecordConnectionResultFailure( + ConnectionToHostResult_FailureEventType::NO_CELL_DATA); + break; default: NOTREACHED(); }; diff --git a/chromeos/components/tether/host_connection_metrics_logger.h b/chromeos/components/tether/host_connection_metrics_logger.h index dfa0b7222f4ec8..06bb8e784b30d2 100644 --- a/chromeos/components/tether/host_connection_metrics_logger.h +++ b/chromeos/components/tether/host_connection_metrics_logger.h @@ -25,7 +25,9 @@ class HostConnectionMetricsLogger { CONNECTION_RESULT_FAILURE_CLIENT_CONNECTION_CANCELED_BY_USER, CONNECTION_RESULT_FAILURE_CLIENT_CONNECTION_INTERNAL_ERROR, CONNECTION_RESULT_FAILURE_TETHERING_TIMED_OUT_FIRST_TIME_SETUP_WAS_REQUIRED, - CONNECTION_RESULT_FAILURE_TETHERING_TIMED_OUT_FIRST_TIME_SETUP_WAS_NOT_REQUIRED + CONNECTION_RESULT_FAILURE_TETHERING_TIMED_OUT_FIRST_TIME_SETUP_WAS_NOT_REQUIRED, + CONNECTION_RESULT_FAILURE_TETHERING_UNSUPPORTED, + CONNECTION_RESULT_FAILURE_NO_CELL_DATA }; // Record the result of an attempted host connection. @@ -57,6 +59,10 @@ class HostConnectionMetricsLogger { FRIEND_TEST_ALL_PREFIXES( HostConnectionMetricsLoggerTest, RecordConnectionResultFailureTetheringTimeout_SetupNotRequired); + FRIEND_TEST_ALL_PREFIXES(HostConnectionMetricsLoggerTest, + RecordConnectionResultFailureTetheringUnsupported); + FRIEND_TEST_ALL_PREFIXES(HostConnectionMetricsLoggerTest, + RecordConnectionResultFailureNoCellData); // An Instant Tethering connection can fail for several different reasons. // Though traditionally success and each failure case would be logged to a @@ -88,6 +94,8 @@ class HostConnectionMetricsLogger { UNKNOWN_ERROR = 0, TETHERING_TIMED_OUT = 1, CLIENT_CONNECTION_ERROR = 2, + TETHERING_UNSUPPORTED = 3, + NO_CELL_DATA = 4, FAILURE_MAX }; diff --git a/chromeos/components/tether/host_connection_metrics_logger_unittest.cc b/chromeos/components/tether/host_connection_metrics_logger_unittest.cc index 8cd30397923d15..e4b4acb719737e 100644 --- a/chromeos/components/tether/host_connection_metrics_logger_unittest.cc +++ b/chromeos/components/tether/host_connection_metrics_logger_unittest.cc @@ -204,6 +204,31 @@ TEST_F(HostConnectionMetricsLoggerTest, ConnectionToHostResult_ProvisioningFailureEventType::OTHER); } +TEST_F(HostConnectionMetricsLoggerTest, + RecordConnectionResultFailureTetheringUnsupported) { + metrics_logger_->RecordConnectionToHostResult( + HostConnectionMetricsLogger::ConnectionToHostResult:: + CONNECTION_RESULT_FAILURE_TETHERING_UNSUPPORTED); + + VerifyFailure( + HostConnectionMetricsLogger::ConnectionToHostResult_FailureEventType:: + TETHERING_UNSUPPORTED); + VerifySuccess(HostConnectionMetricsLogger:: + ConnectionToHostResult_SuccessEventType::FAILURE); +} + +TEST_F(HostConnectionMetricsLoggerTest, + RecordConnectionResultFailureNoCellData) { + metrics_logger_->RecordConnectionToHostResult( + HostConnectionMetricsLogger::ConnectionToHostResult:: + CONNECTION_RESULT_FAILURE_NO_CELL_DATA); + + VerifyFailure(HostConnectionMetricsLogger:: + ConnectionToHostResult_FailureEventType::NO_CELL_DATA); + VerifySuccess(HostConnectionMetricsLogger:: + ConnectionToHostResult_SuccessEventType::FAILURE); +} + } // namespace tether } // namespace chromeos diff --git a/chromeos/components/tether/proto/tether.proto b/chromeos/components/tether/proto/tether.proto index 634429c86add89..17e1b813e63f17 100644 --- a/chromeos/components/tether/proto/tether.proto +++ b/chromeos/components/tether/proto/tether.proto @@ -88,6 +88,8 @@ message ConnectTetheringResponse { SUCCESS = 1; PROVISIONING_FAILED = 2; TETHERING_TIMEOUT = 3; + TETHERING_UNSUPPORTED = 4; + NO_CELL_DATA = 5; } optional ResponseCode response_code = 1; diff --git a/chromeos/components/tether/tether_connector_impl.cc b/chromeos/components/tether/tether_connector_impl.cc index 5e1d022ed8d958..9a53747427499a 100644 --- a/chromeos/components/tether/tether_connector_impl.cc +++ b/chromeos/components/tether/tether_connector_impl.cc @@ -384,6 +384,19 @@ TetherConnectorImpl::GetConnectionToHostResultFromErrorCode( CONNECTION_RESULT_FAILURE_TETHERING_TIMED_OUT_FIRST_TIME_SETUP_WAS_NOT_REQUIRED; } + if (error_code == + ConnectTetheringResponse_ResponseCode:: + ConnectTetheringResponse_ResponseCode_TETHERING_UNSUPPORTED) { + return HostConnectionMetricsLogger::ConnectionToHostResult:: + CONNECTION_RESULT_FAILURE_TETHERING_UNSUPPORTED; + } + + if (error_code == ConnectTetheringResponse_ResponseCode:: + ConnectTetheringResponse_ResponseCode_NO_CELL_DATA) { + return HostConnectionMetricsLogger::ConnectionToHostResult:: + CONNECTION_RESULT_FAILURE_NO_CELL_DATA; + } + return HostConnectionMetricsLogger::ConnectionToHostResult:: CONNECTION_RESULT_FAILURE_UNKNOWN_ERROR; } diff --git a/chromeos/components/tether/tether_connector_impl_unittest.cc b/chromeos/components/tether/tether_connector_impl_unittest.cc index dbf0f84335f99d..234b2f13ac50d4 100644 --- a/chromeos/components/tether/tether_connector_impl_unittest.cc +++ b/chromeos/components/tether/tether_connector_impl_unittest.cc @@ -437,6 +437,25 @@ TEST_F(TetherConnectorImplTest, CONNECTION_RESULT_FAILURE_TETHERING_TIMED_OUT_FIRST_TIME_SETUP_WAS_REQUIRED); } +TEST_F(TetherConnectorImplTest, + TestConnectTetheringOperationFails_TetheringUnsupported) { + VerifyConnectTetheringOperationFails( + ConnectTetheringResponse_ResponseCode:: + ConnectTetheringResponse_ResponseCode_TETHERING_UNSUPPORTED, + false /* setup_required */, + HostConnectionMetricsLogger::ConnectionToHostResult:: + CONNECTION_RESULT_FAILURE_TETHERING_UNSUPPORTED); +} + +TEST_F(TetherConnectorImplTest, TestConnectTetheringOperationFails_NoCellData) { + VerifyConnectTetheringOperationFails( + ConnectTetheringResponse_ResponseCode:: + ConnectTetheringResponse_ResponseCode_NO_CELL_DATA, + false /* setup_required */, + HostConnectionMetricsLogger::ConnectionToHostResult:: + CONNECTION_RESULT_FAILURE_NO_CELL_DATA); +} + TEST_F(TetherConnectorImplTest, ConnectionToHostFailedNotificationRemovedWhenConnectionStarts) { // Start with the "connection to host failed" notification showing. diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml index a9519453b52add..f319f9f83c7237 100644 --- a/tools/metrics/histograms/enums.xml +++ b/tools/metrics/histograms/enums.xml @@ -22479,6 +22479,8 @@ Called by update_gpu_driver_bug_workaround_entries.py.--> + + diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index 3677d1ac19f521..03f4f09b052415 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -30165,7 +30165,7 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries. An "unknown error" is caused by the host returning an "unknown error" response code. Tethering timing out and client - connection error and 3) are both broken down further in + connection error are both broken down further in InstantTethering.ConnectionToHostResult.Failure.TetheringTimeout and InstantTethering.ConnectionToHostResult.Failure.ClientConnection, respectively.