Skip to content

Commit

Permalink
[CrOS Tether] Split up the "unknown error" metric for tether connecti…
Browse files Browse the repository at this point in the history
…ons.

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 <jlklein@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522572}
  • Loading branch information
Kyle Horimoto authored and Commit Bot committed Dec 7, 2017
1 parent e55a1c0 commit 1a729f4
Show file tree
Hide file tree
Showing 8 changed files with 80 additions and 2 deletions.
9 changes: 9 additions & 0 deletions chromeos/components/tether/host_connection_metrics_logger.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
};
Expand Down
10 changes: 9 additions & 1 deletion chromeos/components/tether/host_connection_metrics_logger.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 2 additions & 0 deletions chromeos/components/tether/proto/tether.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
13 changes: 13 additions & 0 deletions chromeos/components/tether/tether_connector_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
19 changes: 19 additions & 0 deletions chromeos/components/tether/tether_connector_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 2 additions & 0 deletions tools/metrics/histograms/enums.xml
Original file line number Diff line number Diff line change
Expand Up @@ -22479,6 +22479,8 @@ Called by update_gpu_driver_bug_workaround_entries.py.-->
<int value="0" label="Unknown error"/>
<int value="1" label="Tethering timed out"/>
<int value="2" label="Client connection error"/>
<int value="3" label="Tethering unsupported"/>
<int value="4" label="No cellular data"/>
</enum>

<enum name="InstantTethering_ConnectionToHostResult_Failure_ClientConnection">
Expand Down
2 changes: 1 addition & 1 deletion tools/metrics/histograms/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -30165,7 +30165,7 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.

An &quot;unknown error&quot; is caused by the host returning an
&quot;unknown error&quot; 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.
Expand Down

0 comments on commit 1a729f4

Please sign in to comment.