shill: LinkMonitor: Add Metrics
BUG=chromium-os:32600
TEST=Unit tests
Change-Id: I4a7625fb006e939a5ea63efeede8ca9982115332
Reviewed-on: https://gerrit.chromium.org/gerrit/29421
Commit-Ready: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
Reviewed-by: Paul Stewart <pstew@chromium.org>
diff --git a/connection.h b/connection.h
index ba0c7cb..9bdbabd 100644
--- a/connection.h
+++ b/connection.h
@@ -98,6 +98,7 @@
virtual const IPAddress &local() const { return local_; }
virtual const IPAddress &gateway() const { return gateway_; }
+ Technology::Identifier technology() const { return technology_; }
protected:
friend class base::RefCounted<Connection>;
diff --git a/link_monitor.cc b/link_monitor.cc
index 2485ce5..7db78be 100644
--- a/link_monitor.cc
+++ b/link_monitor.cc
@@ -18,6 +18,7 @@
#include "shill/device_info.h"
#include "shill/event_dispatcher.h"
#include "shill/ip_address.h"
+#include "shill/metrics.h"
#include "shill/scope_logger.h"
#include "shill/shill_time.h"
@@ -33,10 +34,12 @@
LinkMonitor::LinkMonitor(const ConnectionRefPtr &connection,
EventDispatcher *dispatcher,
+ Metrics *metrics,
DeviceInfo *device_info,
const FailureCallback &failure_callback)
: connection_(connection),
dispatcher_(dispatcher),
+ metrics_(metrics),
device_info_(device_info),
failure_callback_(failure_callback),
broadcast_failure_count_(0),
@@ -56,6 +59,8 @@
if (!device_info_->GetMACAddress(
connection_->interface_index(), &local_mac_address_)) {
LOG(ERROR) << "Could not get local MAC address.";
+ metrics_->NotifyLinkMonitorFailure(
+ connection_->technology(), Metrics::kLinkMonitorMacAddressNotFound);
Stop();
return false;
}
@@ -89,6 +94,8 @@
unsigned int response_time_milliseconds) {
SLOG(Link, 2) << "In " << __func__ << " with sample "
<< response_time_milliseconds << ".";
+ metrics_->NotifyLinkMonitorResponseTimeSampleAdded(
+ connection_->technology(), response_time_milliseconds);
response_sample_bucket_ += response_time_milliseconds;
if (response_sample_count_ < kMaxResponseSampleFilterDepth) {
++response_sample_count_;
@@ -113,7 +120,7 @@
arp_client_.reset(new ArpClient(connection_->interface_index()));
if (!arp_client_->Start()) {
- return false;
+ return false;
}
receive_response_handler_.reset(
dispatcher_->CreateReadyHandler(
@@ -141,6 +148,9 @@
<< " unicast failures.";
failure_callback_.Run();
Stop();
+ metrics_->NotifyLinkMonitorFailure(
+ connection_->technology(),
+ Metrics::kLinkMonitorFailureThresholdReached);
return true;
}
is_unicast_ = !is_unicast_;
@@ -206,6 +216,8 @@
if (!CreateClient()) {
LOG(ERROR) << "Failed to start ARP client.";
Stop();
+ metrics_->NotifyLinkMonitorFailure(
+ connection_->technology(), Metrics::kLinkMonitorClientStartFailure);
return false;
}
} else if (AddMissedResponse()) {
@@ -239,6 +251,8 @@
if (!arp_client_->TransmitRequest(request)) {
LOG(ERROR) << "Failed to send ARP request. Stopping.";
Stop();
+ metrics_->NotifyLinkMonitorFailure(
+ connection_->technology(), Metrics::kLinkMonitorTransmitFailure);
return false;
}
diff --git a/link_monitor.h b/link_monitor.h
index 9a3276b..a3e0a79 100644
--- a/link_monitor.h
+++ b/link_monitor.h
@@ -20,6 +20,7 @@
class DeviceInfo;
class EventDispatcher;
class IOHandler;
+class Metrics;
class Time;
// LinkMonitor tracks the status of a connection by sending ARP
@@ -31,8 +32,12 @@
public:
typedef base::Closure FailureCallback;
+ // The number of milliseconds between ARP requests. Needed by Metrics.
+ static const unsigned int kTestPeriodMilliseconds;
+
LinkMonitor(const ConnectionRefPtr &connection,
EventDispatcher *dispatcher,
+ Metrics *metrics,
DeviceInfo *device_info,
const FailureCallback &failure_callback);
virtual ~LinkMonitor();
@@ -52,9 +57,6 @@
friend class LinkMonitorForTest;
friend class LinkMonitorTest;
- // The number of milliseconds between ARP requests.
- static const unsigned int kTestPeriodMilliseconds;
-
// When the sum of consecutive unicast and broadcast failures
// equals this value, the failure callback is called, the counters
// are reset, and the link monitoring quiesces.
@@ -87,6 +89,7 @@
ConnectionRefPtr connection_;
EventDispatcher *dispatcher_;
+ Metrics *metrics_;
DeviceInfo *device_info_;
FailureCallback failure_callback_;
ByteString local_mac_address_;
diff --git a/link_monitor_unittest.cc b/link_monitor_unittest.cc
index 9851c34..cea0a9f 100644
--- a/link_monitor_unittest.cc
+++ b/link_monitor_unittest.cc
@@ -15,6 +15,7 @@
#include "shill/mock_device_info.h"
#include "shill/mock_event_dispatcher.h"
#include "shill/mock_log.h"
+#include "shill/mock_metrics.h"
#include "shill/mock_sockets.h"
#include "shill/mock_time.h"
#include "shill/scope_logger.h"
@@ -48,8 +49,9 @@
public:
LinkMonitorForTest(const ConnectionRefPtr &connection,
EventDispatcher *dispatcher,
+ Metrics *metrics,
DeviceInfo *device_info)
- : LinkMonitor(connection, dispatcher, device_info,
+ : LinkMonitor(connection, dispatcher, metrics, device_info,
Bind(&LinkMonitorForTest::FailureCallback,
Unretained(this))) {}
@@ -76,7 +78,7 @@
reinterpret_cast<Metrics*>(NULL),
reinterpret_cast<Manager*>(NULL)),
connection_(new StrictMock<MockConnection>(&device_info_)),
- monitor_(connection_, &dispatcher_, &device_info_),
+ monitor_(connection_, &dispatcher_, &metrics_, &device_info_),
client_(NULL),
next_client_(new StrictMock<MockArpClient>()),
gateway_ip_(IPAddress::kFamilyIPv4),
@@ -221,6 +223,7 @@
}
MockEventDispatcher dispatcher_;
+ StrictMock<MockMetrics> metrics_;
MockControl control_;
NiceMock<MockDeviceInfo> device_info_;
scoped_refptr<MockConnection> connection_;
@@ -253,6 +256,9 @@
Log(logging::LOG_ERROR, _,
HasSubstr("Could not get local MAC address"))).Times(1);
EXPECT_CALL(device_info_, GetMACAddress(0, _)).WillOnce(Return(false));
+ EXPECT_CALL(metrics_, SendEnumToUMA(
+ HasSubstr("LinkMonitorFailure"), Metrics::kLinkMonitorMacAddressNotFound,
+ _));
EXPECT_CALL(monitor_, CreateClient()).Times(0);
EXPECT_FALSE(monitor_.Start());
ExpectReset();
@@ -264,6 +270,9 @@
EXPECT_CALL(log,
Log(logging::LOG_ERROR, _,
HasSubstr("Failed to start ARP client"))).Times(1);
+ EXPECT_CALL(metrics_, SendEnumToUMA(
+ HasSubstr("LinkMonitorFailure"), Metrics::kLinkMonitorClientStartFailure,
+ _));
EXPECT_CALL(device_info_, GetMACAddress(0, _)).WillOnce(Return(true));
EXPECT_CALL(monitor_, CreateClient()).WillOnce(Return(false));
EXPECT_FALSE(monitor_.Start());
@@ -276,6 +285,9 @@
EXPECT_CALL(log,
Log(logging::LOG_ERROR, _,
HasSubstr("Failed to send ARP"))).Times(1);
+ EXPECT_CALL(metrics_, SendEnumToUMA(
+ HasSubstr("LinkMonitorFailure"), Metrics::kLinkMonitorTransmitFailure,
+ _));
EXPECT_CALL(device_info_, GetMACAddress(0, _)).WillOnce(Return(true));
EXPECT_CALL(monitor_, CreateClient())
.WillOnce(Invoke(this, &LinkMonitorTest::CreateMockClient));
@@ -319,6 +331,9 @@
EXPECT_FALSE(monitor_.GetResponseTimeMilliseconds());
EXPECT_CALL(log, Log(_, _, _)).Times(AnyNumber());
EXPECT_CALL(log, Log(_, _, HasSubstr("Found gateway"))).Times(1);
+ EXPECT_CALL(metrics_, SendToUMA(
+ HasSubstr("LinkMonitorResponseTimeSample"), kResponseTime,
+ _, _, _)).Times(1);
ReceiveCorrectResponse();
EXPECT_FALSE(GetArpClient());
EXPECT_EQ(kResponseTime, monitor_.GetResponseTimeMilliseconds());
@@ -326,6 +341,9 @@
}
TEST_F(LinkMonitorTest, TimeoutBroadcast) {
+ EXPECT_CALL(metrics_, SendToUMA(
+ HasSubstr("LinkMonitorResponseTimeSample"), GetTestPeriodMilliseconds(),
+ _, _, _)).Times(GetFailureThreshold());
StartMonitor();
for (unsigned int i = 1; i < GetFailureThreshold(); ++i) {
ExpectTransmit(false);
@@ -341,6 +359,9 @@
EXPECT_CALL(log,
Log(logging::LOG_ERROR, _,
HasSubstr("monitor has reached the failure threshold"))).Times(1);
+ EXPECT_CALL(metrics_, SendEnumToUMA(
+ HasSubstr("LinkMonitorFailure"),
+ Metrics::kLinkMonitorFailureThresholdReached, _));
EXPECT_FALSE(GetSendRequestCallback().IsCancelled());
ExpectNoTransmit();
EXPECT_CALL(monitor_, FailureCallback());
@@ -350,6 +371,14 @@
TEST_F(LinkMonitorTest, TimeoutUnicast) {
StartMonitor();
+ // Successful broadcast receptions.
+ EXPECT_CALL(metrics_, SendToUMA(
+ HasSubstr("LinkMonitorResponseTimeSample"), 0, _, _, _))
+ .Times(GetFailureThreshold());
+ // Unsuccessful unicast receptions.
+ EXPECT_CALL(metrics_, SendToUMA(
+ HasSubstr("LinkMonitorResponseTimeSample"), GetTestPeriodMilliseconds(),
+ _, _, _)).Times(GetFailureThreshold());
ReceiveCorrectResponse();
for (unsigned int i = 1; i < GetFailureThreshold(); ++i) {
// Failed unicast ARP.
@@ -373,6 +402,9 @@
EXPECT_CALL(log,
Log(logging::LOG_ERROR, _,
HasSubstr("monitor has reached the failure threshold"))).Times(1);
+ EXPECT_CALL(metrics_, SendEnumToUMA(
+ HasSubstr("LinkMonitorFailure"),
+ Metrics::kLinkMonitorFailureThresholdReached, _));
EXPECT_FALSE(GetSendRequestCallback().IsCancelled());
ExpectNoTransmit();
EXPECT_CALL(monitor_, FailureCallback());
@@ -384,6 +416,9 @@
const unsigned int kSamples[] = { 200, 950, 1200, 4096, 5000,
86, 120, 3060, 842, 750 };
const unsigned int filter_depth = GetMaxResponseSampleFilterDepth();
+ EXPECT_CALL(metrics_, SendToUMA(
+ HasSubstr("LinkMonitorResponseTimeSample"), _, _, _, _))
+ .Times(arraysize(kSamples));
ASSERT_GT(arraysize(kSamples), filter_depth);
StartMonitor();
unsigned int i = 0;
@@ -408,6 +443,9 @@
const unsigned int kNormalValue = 50;
const unsigned int kExceptionalValue = 5000;
const unsigned int filter_depth = GetMaxResponseSampleFilterDepth();
+ EXPECT_CALL(metrics_, SendToUMA(
+ HasSubstr("LinkMonitorResponseTimeSample"), _, _, _, _))
+ .Times(AnyNumber());
StartMonitor();
for (unsigned int i = 0; i < filter_depth * 2; ++i) {
AdvanceTime(kNormalValue);
diff --git a/metrics.cc b/metrics.cc
index 85e37cd..4162f64 100644
--- a/metrics.cc
+++ b/metrics.cc
@@ -10,6 +10,7 @@
#include <chromeos/dbus/service_constants.h>
#include <metrics/bootstat.h>
+#include "shill/link_monitor.h"
#include "shill/scope_logger.h"
#include "shill/wifi_service.h"
@@ -100,6 +101,17 @@
// static
const char Metrics::kMetricPowerManagerKey[] = "metrics";
+// static
+const char Metrics::kMetricLinkMonitorFailure[] =
+ "Network.Shill.%s.LinkMonitorFailure";
+const char Metrics::kMetricLinkMonitorResponseTimeSample[] =
+ "Network.Shill.%s.LinkMonitorResponseTimeSample";
+const int Metrics::kMetricLinkMonitorResponseTimeSampleMin = 0;
+const int Metrics::kMetricLinkMonitorResponseTimeSampleMax =
+ LinkMonitor::kTestPeriodMilliseconds;
+const int Metrics::kMetricLinkMonitorResponseTimeSampleNumBuckets = 50;
+
+
Metrics::Metrics()
: library_(&metrics_library_),
last_default_technology_(Technology::kUnknown),
@@ -377,6 +389,25 @@
}
}
+void Metrics::NotifyLinkMonitorFailure(
+ Technology::Identifier technology, LinkMonitorFailure failure) {
+ string histogram = GetFullMetricName(kMetricLinkMonitorFailure,
+ technology);
+ SendEnumToUMA(histogram, failure, kLinkMonitorFailureMax);
+}
+
+void Metrics::NotifyLinkMonitorResponseTimeSampleAdded(
+ Technology::Identifier technology,
+ unsigned int response_time_milliseconds) {
+ string histogram = GetFullMetricName(kMetricLinkMonitorResponseTimeSample,
+ technology);
+ SendToUMA(histogram,
+ response_time_milliseconds,
+ kMetricLinkMonitorResponseTimeSampleMin,
+ kMetricLinkMonitorResponseTimeSampleMax,
+ kMetricLinkMonitorResponseTimeSampleNumBuckets);
+}
+
bool Metrics::SendEnumToUMA(const string &name, int sample, int max) {
return library_->SendEnumToUMA(name, sample, max);
}
diff --git a/metrics.h b/metrics.h
index bcd1276..d241249 100644
--- a/metrics.h
+++ b/metrics.h
@@ -116,6 +116,15 @@
kPortalResultMax
};
+ enum LinkMonitorFailure {
+ kLinkMonitorMacAddressNotFound = 0,
+ kLinkMonitorClientStartFailure = 1,
+ kLinkMonitorTransmitFailure = 2,
+ kLinkMonitorFailureThresholdReached = 3,
+
+ kLinkMonitorFailureMax
+ };
+
static const char kMetricDisconnect[];
static const int kMetricDisconnectMax;
static const int kMetricDisconnectMin;
@@ -165,6 +174,13 @@
static const char kMetricPowerManagerKey[];
+ // LinkMonitor statistics.
+ static const char kMetricLinkMonitorFailure[];
+ static const char kMetricLinkMonitorResponseTimeSample[];
+ static const int kMetricLinkMonitorResponseTimeSampleMin;
+ static const int kMetricLinkMonitorResponseTimeSampleMax;
+ static const int kMetricLinkMonitorResponseTimeSampleNumBuckets;
+
Metrics();
virtual ~Metrics();
@@ -211,6 +227,16 @@
// Notifies this object of a power management state change.
void NotifyPowerStateChange(PowerManager::SuspendState new_state);
+ // Notifies this object of a failure in LinkMonitor.
+ void NotifyLinkMonitorFailure(
+ Technology::Identifier technology, LinkMonitorFailure failure);
+
+ // Notifies this object that LinkMonitor has added a response time sample
+ // for |connection| with a value of |response_time_milliseconds|.
+ void NotifyLinkMonitorResponseTimeSampleAdded(
+ Technology::Identifier technology,
+ unsigned int response_time_milliseconds);
+
// Sends linear histogram data to UMA.
virtual bool SendEnumToUMA(const std::string &name, int sample, int max);
diff --git a/mock_link_monitor.cc b/mock_link_monitor.cc
index 1567748..bd27d16 100644
--- a/mock_link_monitor.cc
+++ b/mock_link_monitor.cc
@@ -9,7 +9,7 @@
namespace shill {
MockLinkMonitor::MockLinkMonitor()
- : LinkMonitor(NULL, NULL, NULL, FailureCallback()) {}
+ : LinkMonitor(NULL, NULL, NULL, NULL, FailureCallback()) {}
MockLinkMonitor::~MockLinkMonitor() {}