shill: cellular: Add Network.Shill.Cellular.TimeToScan metric.
BUG=chromium-os:38298
TEST=Unit tests, check for new metric under chrome://histograms
Change-Id: I4209898b49314b462cdde695bc607a051a531245
Reviewed-on: https://gerrit.chromium.org/gerrit/42196
Reviewed-by: Arman Uguray <armansito@chromium.org>
Commit-Queue: Thieu Le <thieule@chromium.org>
Tested-by: Thieu Le <thieule@chromium.org>
diff --git a/cellular.cc b/cellular.cc
index cba1754..dee04da 100644
--- a/cellular.cc
+++ b/cellular.cc
@@ -379,6 +379,7 @@
SetState(kStateRegistered);
}
if (!service_.get()) {
+ metrics()->NotifyDeviceScanFinished(interface_index());
CreateService();
}
capability_->GetSignalQuality();
diff --git a/cellular_capability_classic.cc b/cellular_capability_classic.cc
index cdecbbd..0a23dde 100644
--- a/cellular_capability_classic.cc
+++ b/cellular_capability_classic.cc
@@ -121,7 +121,10 @@
callback.Run(Error());
GetRegistrationState();
GetSignalQuality();
+ // We expect the modem to start scanning after it has been enabled.
+ // Change this if this behavior is no longer the case in the future.
metrics()->NotifyDeviceEnableFinished(cellular()->interface_index());
+ metrics()->NotifyDeviceScanStarted(cellular()->interface_index());
}
void CellularCapabilityClassic::FinishDisable(const ResultCallback &callback) {
diff --git a/cellular_capability_universal.cc b/cellular_capability_universal.cc
index fe90e3a..c6553a5 100644
--- a/cellular_capability_universal.cc
+++ b/cellular_capability_universal.cc
@@ -272,8 +272,11 @@
// After modem is enabled, it should be possible to get properties
// TODO(jglasgow): handle errors from GetProperties
GetProperties();
- callback.Run(error);
+ // We expect the modem to start scanning after it has been enabled.
+ // Change this if this behavior is no longer the case in the future.
metrics()->NotifyDeviceEnableFinished(cellular()->interface_index());
+ metrics()->NotifyDeviceScanStarted(cellular()->interface_index());
+ callback.Run(error);
}
void CellularCapabilityUniversal::StopModem(Error *error,
diff --git a/metrics.cc b/metrics.cc
index c074499..2d9e290 100644
--- a/metrics.cc
+++ b/metrics.cc
@@ -95,6 +95,13 @@
"Network.Shill.%s.TimeToOnline";
const char Metrics::kMetricTimeToPortalMilliseconds[] =
"Network.Shill.%s.TimeToPortal";
+
+const char Metrics::kMetricTimeToScanMilliseconds[] =
+ "Network.Shill.%s.TimeToScan";
+const int Metrics::kMetricTimeToScanMillisecondsMax = 180 * 1000; // 3 minutes
+const int Metrics::kMetricTimeToScanMillisecondsMin = 1;
+const int Metrics::kMetricTimeToScanMillisecondsNumBuckets = 90;
+
const int Metrics::kTimerHistogramMillisecondsMax = 45 * 1000;
const int Metrics::kTimerHistogramMillisecondsMin = 1;
const int Metrics::kTimerHistogramNumBuckets = 50;
@@ -617,6 +624,14 @@
kMetricTimeToDisableMillisecondsMin,
kMetricTimeToDisableMillisecondsMax,
kMetricTimeToDisableMillisecondsNumBuckets));
+ histogram = GetFullMetricName(kMetricTimeToScanMilliseconds,
+ technology);
+ device_metrics->scan_timer.reset(
+ new chromeos_metrics::TimerReporter(
+ histogram,
+ kMetricTimeToScanMillisecondsMin,
+ kMetricTimeToScanMillisecondsMax,
+ kMetricTimeToScanMillisecondsNumBuckets));
histogram = GetFullMetricName(kMetricTimeToConnectMilliseconds,
technology);
device_metrics->connect_timer.reset(
@@ -681,6 +696,33 @@
device_metrics->disable_timer->ReportMilliseconds();
}
+void Metrics::NotifyDeviceScanStarted(int interface_index) {
+ DeviceMetrics *device_metrics = GetDeviceMetrics(interface_index);
+ if (device_metrics == NULL)
+ return;
+ // This metric is only supported for cellular devices.
+ if (device_metrics->technology != Technology::kCellular)
+ return;
+ device_metrics->scan_timer->Start();
+}
+
+void Metrics::NotifyDeviceScanFinished(int interface_index) {
+ DeviceMetrics *device_metrics = GetDeviceMetrics(interface_index);
+ if (device_metrics == NULL)
+ return;
+ // This metric is only supported for cellular devices.
+ if (device_metrics->technology != Technology::kCellular)
+ return;
+ device_metrics->scan_timer->Stop();
+ // Don't send TimeToScan metrics if the elapsed time exceeds the max
+ // metrics value. This usually means that the modem is in an area
+ // without service and we're not interested in this scenario.
+ base::TimeDelta elapsed_time;
+ device_metrics->scan_timer->GetElapsedTime(&elapsed_time);
+ if (elapsed_time.InMilliseconds() <= kMetricTimeToScanMillisecondsMax)
+ device_metrics->scan_timer->ReportMilliseconds();
+}
+
void Metrics::NotifyDeviceConnectStarted(int interface_index) {
DeviceMetrics *device_metrics = GetDeviceMetrics(interface_index);
if (device_metrics == NULL)
diff --git a/metrics.h b/metrics.h
index 73985c2..1136ca1 100644
--- a/metrics.h
+++ b/metrics.h
@@ -201,6 +201,10 @@
static const char kMetricTimeToJoinMilliseconds[];
static const char kMetricTimeToOnlineMilliseconds[];
static const char kMetricTimeToPortalMilliseconds[];
+ static const char kMetricTimeToScanMilliseconds[];
+ static const int kMetricTimeToScanMillisecondsMax;
+ static const int kMetricTimeToScanMillisecondsMin;
+ static const int kMetricTimeToScanMillisecondsNumBuckets;
static const int kTimerHistogramMillisecondsMax;
static const int kTimerHistogramMillisecondsMin;
static const int kTimerHistogramNumBuckets;
@@ -358,6 +362,12 @@
// Notifies this object that a device has completed the disable process.
void NotifyDeviceDisableFinished(int interface_index);
+ // Notifies this object that a device has started the scanning process.
+ void NotifyDeviceScanStarted(int interface_index);
+
+ // Notifies this object that a device has completed the scanning process.
+ void NotifyDeviceScanFinished(int interface_index);
+
// Notifies this object that a device has started the connect process.
void NotifyDeviceConnectStarted(int interface_index);
@@ -379,6 +389,7 @@
FRIEND_TEST(MetricsTest, TimeToConfig);
FRIEND_TEST(MetricsTest, TimeToOnline);
FRIEND_TEST(MetricsTest, TimeToPortal);
+ FRIEND_TEST(MetricsTest, TimeToScanIgnore);
FRIEND_TEST(MetricsTest, WiFiServiceChannel);
FRIEND_TEST(MetricsTest, WiFiServicePostReady);
FRIEND_TEST(WiFiMainTest, GetGeolocationObjects);
@@ -408,6 +419,7 @@
scoped_ptr<chromeos_metrics::TimerReporter> initialization_timer;
scoped_ptr<chromeos_metrics::TimerReporter> enable_timer;
scoped_ptr<chromeos_metrics::TimerReporter> disable_timer;
+ scoped_ptr<chromeos_metrics::TimerReporter> scan_timer;
scoped_ptr<chromeos_metrics::TimerReporter> connect_timer;
};
typedef std::map<const int, std::tr1::shared_ptr<DeviceMetrics> >
@@ -450,6 +462,11 @@
chromeos_metrics::Timer *timer) {
time_termination_actions_timer.reset(timer); // Passes ownership
}
+ void set_time_to_scan_timer(int interface_index,
+ chromeos_metrics::TimerReporter *timer) {
+ DeviceMetrics *device_metrics = GetDeviceMetrics(interface_index);
+ device_metrics->scan_timer.reset(timer); // Passes ownership
+ }
// |library_| points to |metrics_library_| when shill runs normally.
// However, in order to allow for unit testing, we point |library_| to a
diff --git a/metrics_unittest.cc b/metrics_unittest.cc
index abc020e..4e13811 100644
--- a/metrics_unittest.cc
+++ b/metrics_unittest.cc
@@ -381,6 +381,38 @@
metrics_.NotifyDeviceInitialized(kInterfaceIndex);
}
+TEST_F(MetricsTest, TimeToScan) {
+ EXPECT_CALL(library_,
+ SendToUMA("Network.Shill.Cellular.TimeToScan",
+ Ge(0),
+ Metrics::kMetricTimeToScanMillisecondsMin,
+ Metrics::kMetricTimeToScanMillisecondsMax,
+ Metrics::kMetricTimeToScanMillisecondsNumBuckets));
+ const int kInterfaceIndex = 1;
+ metrics_.RegisterDevice(kInterfaceIndex, Technology::kCellular);
+ metrics_.NotifyDeviceScanStarted(kInterfaceIndex);
+ metrics_.NotifyDeviceScanFinished(kInterfaceIndex);
+}
+
+TEST_F(MetricsTest, TimeToScanIgnore) {
+ // Make sure TimeToScan is not sent if the elapsed time exceeds the max
+ // value. This simulates the case where the device is in an area with no
+ // service.
+ const int kInterfaceIndex = 1;
+ metrics_.RegisterDevice(kInterfaceIndex, Technology::kCellular);
+ base::TimeDelta large_time_delta =
+ base::TimeDelta::FromMilliseconds(
+ Metrics::kMetricTimeToScanMillisecondsMax + 1);
+ chromeos_metrics::TimerReporterMock *mock_time_to_scan_timer =
+ new chromeos_metrics::TimerReporterMock;
+ metrics_.set_time_to_scan_timer(kInterfaceIndex, mock_time_to_scan_timer);
+ EXPECT_CALL(*mock_time_to_scan_timer, GetElapsedTime(_)).
+ WillOnce(DoAll(SetArgumentPointee<0>(large_time_delta), Return(true)));
+ EXPECT_CALL(library_, SendToUMA(_, _, _, _, _)).Times(0);
+ metrics_.NotifyDeviceScanStarted(kInterfaceIndex);
+ metrics_.NotifyDeviceScanFinished(kInterfaceIndex);
+}
+
#ifndef NDEBUG
typedef MetricsTest MetricsDeathTest;