BluetoothMetrics: Bypass counter metrics caching for profile connection failures to get timestamp
Test: atest BluetoothInstrumentationTests
Bug: 193268714
Change-Id: I0bdbf89ddda03f27acda1f7f6d3112eb40393996
diff --git a/android/app/src/com/android/bluetooth/btservice/MetricsLogger.java b/android/app/src/com/android/bluetooth/btservice/MetricsLogger.java
index 12a4f81..c3f5b65 100644
--- a/android/app/src/com/android/bluetooth/btservice/MetricsLogger.java
+++ b/android/app/src/com/android/bluetooth/btservice/MetricsLogger.java
@@ -97,7 +97,7 @@
return true;
}
- public boolean count(int key, long count) {
+ public boolean cacheCount(int key, long count) {
if (!mInitialized) {
Log.w(TAG, "MetricsLogger isn't initialized");
return false;
@@ -167,9 +167,18 @@
getDrainIntent());
}
- protected void writeCounter(int key, long count) {
+ public boolean count(int key, long count) {
+ if (!mInitialized) {
+ Log.w(TAG, "MetricsLogger isn't initialized");
+ return false;
+ }
+ if (count <= 0) {
+ Log.w(TAG, "count is not larger than 0. count: " + count + " key: " + key);
+ return false;
+ }
BluetoothStatsLog.write(
BluetoothStatsLog.BLUETOOTH_CODE_PATH_COUNTER, key, count);
+ return true;
}
protected void drainBufferedCounters() {
@@ -177,7 +186,7 @@
synchronized (mLock) {
// send mCounters to statsd
for (int key : mCounters.keySet()) {
- writeCounter(key, mCounters.get(key));
+ count(key, mCounters.get(key));
}
mCounters.clear();
}
diff --git a/android/app/tests/unit/src/com/android/bluetooth/btservice/MetricsLoggerTest.java b/android/app/tests/unit/src/com/android/bluetooth/btservice/MetricsLoggerTest.java
index 9566ef9..851e998 100644
--- a/android/app/tests/unit/src/com/android/bluetooth/btservice/MetricsLoggerTest.java
+++ b/android/app/tests/unit/src/com/android/bluetooth/btservice/MetricsLoggerTest.java
@@ -51,8 +51,9 @@
public HashMap<Integer, Long> mTestableCounters = new HashMap<>();
@Override
- protected void writeCounter(int key, long count) {
+ public boolean count(int key, long count) {
mTestableCounters.put(key, count);
+ return true;
}
@Override
@@ -141,18 +142,18 @@
@Test
public void testAddAndSendCountersNormalCases() {
mTestableMetricsLogger.init(mMockAdapterService);
- mTestableMetricsLogger.count(1, 10);
- mTestableMetricsLogger.count(1, 10);
- mTestableMetricsLogger.count(2, 5);
+ mTestableMetricsLogger.cacheCount(1, 10);
+ mTestableMetricsLogger.cacheCount(1, 10);
+ mTestableMetricsLogger.cacheCount(2, 5);
mTestableMetricsLogger.drainBufferedCounters();
Assert.assertEquals(20L, mTestableMetricsLogger.mTestableCounters.get(1).longValue());
Assert.assertEquals(5L, mTestableMetricsLogger.mTestableCounters.get(2).longValue());
- mTestableMetricsLogger.count(1, 3);
- mTestableMetricsLogger.count(2, 5);
- mTestableMetricsLogger.count(2, 5);
- mTestableMetricsLogger.count(3, 1);
+ mTestableMetricsLogger.cacheCount(1, 3);
+ mTestableMetricsLogger.cacheCount(2, 5);
+ mTestableMetricsLogger.cacheCount(2, 5);
+ mTestableMetricsLogger.cacheCount(3, 1);
mTestableMetricsLogger.drainBufferedCounters();
Assert.assertEquals(
3L, mTestableMetricsLogger.mTestableCounters.get(1).longValue());
@@ -166,10 +167,10 @@
public void testAddAndSendCountersCornerCases() {
mTestableMetricsLogger.init(mMockAdapterService);
Assert.assertTrue(mTestableMetricsLogger.isInitialized());
- mTestableMetricsLogger.count(1, -1);
- mTestableMetricsLogger.count(3, 0);
- mTestableMetricsLogger.count(2, 10);
- mTestableMetricsLogger.count(2, Long.MAX_VALUE - 8L);
+ mTestableMetricsLogger.cacheCount(1, -1);
+ mTestableMetricsLogger.cacheCount(3, 0);
+ mTestableMetricsLogger.cacheCount(2, 10);
+ mTestableMetricsLogger.cacheCount(2, Long.MAX_VALUE - 8L);
mTestableMetricsLogger.drainBufferedCounters();
Assert.assertFalse(mTestableMetricsLogger.mTestableCounters.containsKey(1));
@@ -181,9 +182,9 @@
@Test
public void testMetricsLoggerClose() {
mTestableMetricsLogger.init(mMockAdapterService);
- mTestableMetricsLogger.count(1, 1);
- mTestableMetricsLogger.count(2, 10);
- mTestableMetricsLogger.count(2, Long.MAX_VALUE);
+ mTestableMetricsLogger.cacheCount(1, 1);
+ mTestableMetricsLogger.cacheCount(2, 10);
+ mTestableMetricsLogger.cacheCount(2, Long.MAX_VALUE);
mTestableMetricsLogger.close();
Assert.assertEquals(
@@ -194,7 +195,7 @@
@Test
public void testMetricsLoggerNotInit() {
- Assert.assertFalse(mTestableMetricsLogger.count(1, 1));
+ Assert.assertFalse(mTestableMetricsLogger.cacheCount(1, 1));
mTestableMetricsLogger.drainBufferedCounters();
Assert.assertFalse(mTestableMetricsLogger.mTestableCounters.containsKey(1));
Assert.assertFalse(mTestableMetricsLogger.close());
diff --git a/system/gd/metrics/counter_metrics.cc b/system/gd/metrics/counter_metrics.cc
index 4474b39..820990e 100644
--- a/system/gd/metrics/counter_metrics.cc
+++ b/system/gd/metrics/counter_metrics.cc
@@ -49,7 +49,7 @@
LOG_INFO("Counter metrics canceled");
}
-bool CounterMetrics::Count(int32_t key, int64_t count) {
+bool CounterMetrics::CacheCount(int32_t key, int64_t count) {
if (!IsInitialized()) {
LOG_WARN("Counter metrics isn't initialized");
return false;
@@ -73,8 +73,17 @@
return true;
}
-void CounterMetrics::WriteCounter(int32_t key, int64_t count) {
+bool CounterMetrics::Count(int32_t key, int64_t count) {
+ if (!IsInitialized()) {
+ LOG_WARN("Counter metrics isn't initialized");
+ return false;
+ }
+ if (count <= 0) {
+ LOG_WARN("count is not larger than 0. count: %s, key: %d", std::to_string(count).c_str(), key);
+ return false;
+ }
os::LogMetricBluetoothCodePathCounterMetrics(key, count);
+ return true;
}
void CounterMetrics::DrainBufferedCounters() {
@@ -85,7 +94,7 @@
std::lock_guard<std::mutex> lock(mutex_);
LOG_INFO("Draining buffered counters");
for (auto const& pair : counters_) {
- WriteCounter(pair.first, pair.second);
+ Count(pair.first, pair.second);
}
counters_.clear();
}
diff --git a/system/gd/metrics/counter_metrics.h b/system/gd/metrics/counter_metrics.h
index c21e9be..8148e63e 100644
--- a/system/gd/metrics/counter_metrics.h
+++ b/system/gd/metrics/counter_metrics.h
@@ -25,7 +25,8 @@
class CounterMetrics : public bluetooth::Module {
public:
- bool Count(int32_t key, int64_t value);
+ bool CacheCount(int32_t key, int64_t value);
+ virtual bool Count(int32_t key, int64_t count);
void Stop() override;
static const ModuleFactory Factory;
@@ -36,7 +37,6 @@
return std::string("BluetoothCounterMetrics");
}
void DrainBufferedCounters();
- virtual void WriteCounter(int32_t key, int64_t count);
virtual bool IsInitialized() {
return initialized_;
}
diff --git a/system/gd/metrics/counter_metrics_unittest.cc b/system/gd/metrics/counter_metrics_unittest.cc
index d9d89a2..f09a6d9 100644
--- a/system/gd/metrics/counter_metrics_unittest.cc
+++ b/system/gd/metrics/counter_metrics_unittest.cc
@@ -33,8 +33,9 @@
}
std::unordered_map<int32_t, int64_t> test_counters_;
private:
- void WriteCounter(int32_t key, int64_t count) override {
+ bool Count(int32_t key, int64_t count) override {
test_counters_[key] = count;
+ return true;
}
bool IsInitialized() override {
return true;
@@ -44,26 +45,26 @@
};
TEST_F(CounterMetricsTest, normal_case) {
- ASSERT_TRUE(testable_counter_metrics_.Count(1, 2));
- ASSERT_TRUE(testable_counter_metrics_.Count(1, 3));
- ASSERT_TRUE(testable_counter_metrics_.Count(2, 4));
+ ASSERT_TRUE(testable_counter_metrics_.CacheCount(1, 2));
+ ASSERT_TRUE(testable_counter_metrics_.CacheCount(1, 3));
+ ASSERT_TRUE(testable_counter_metrics_.CacheCount(2, 4));
testable_counter_metrics_.DrainBuffer();
ASSERT_EQ(testable_counter_metrics_.test_counters_[1], 5);
ASSERT_EQ(testable_counter_metrics_.test_counters_[2], 4);
}
TEST_F(CounterMetricsTest, multiple_drain) {
- ASSERT_TRUE(testable_counter_metrics_.Count(1, 2));
- ASSERT_TRUE(testable_counter_metrics_.Count(1, 3));
- ASSERT_TRUE(testable_counter_metrics_.Count(2, 4));
+ ASSERT_TRUE(testable_counter_metrics_.CacheCount(1, 2));
+ ASSERT_TRUE(testable_counter_metrics_.CacheCount(1, 3));
+ ASSERT_TRUE(testable_counter_metrics_.CacheCount(2, 4));
testable_counter_metrics_.DrainBuffer();
ASSERT_EQ(testable_counter_metrics_.test_counters_[1], 5);
ASSERT_EQ(testable_counter_metrics_.test_counters_[2], 4);
testable_counter_metrics_.test_counters_.clear();
- ASSERT_TRUE(testable_counter_metrics_.Count(1, 20));
- ASSERT_TRUE(testable_counter_metrics_.Count(1, 30));
- ASSERT_TRUE(testable_counter_metrics_.Count(2, 40));
- ASSERT_TRUE(testable_counter_metrics_.Count(3, 100));
+ ASSERT_TRUE(testable_counter_metrics_.CacheCount(1, 20));
+ ASSERT_TRUE(testable_counter_metrics_.CacheCount(1, 30));
+ ASSERT_TRUE(testable_counter_metrics_.CacheCount(2, 40));
+ ASSERT_TRUE(testable_counter_metrics_.CacheCount(3, 100));
testable_counter_metrics_.DrainBuffer();
ASSERT_EQ(testable_counter_metrics_.test_counters_[1], 50);
ASSERT_EQ(testable_counter_metrics_.test_counters_[2], 40);
@@ -71,17 +72,17 @@
}
TEST_F(CounterMetricsTest, overflow) {
- ASSERT_TRUE(testable_counter_metrics_.Count(1, LLONG_MAX));
- ASSERT_FALSE(testable_counter_metrics_.Count(1, 1));
- ASSERT_FALSE(testable_counter_metrics_.Count(1, 2));
+ ASSERT_TRUE(testable_counter_metrics_.CacheCount(1, LLONG_MAX));
+ ASSERT_FALSE(testable_counter_metrics_.CacheCount(1, 1));
+ ASSERT_FALSE(testable_counter_metrics_.CacheCount(1, 2));
testable_counter_metrics_.DrainBuffer();
ASSERT_EQ(testable_counter_metrics_.test_counters_[1], LLONG_MAX);
}
TEST_F(CounterMetricsTest, non_positive) {
- ASSERT_TRUE(testable_counter_metrics_.Count(1, 5));
- ASSERT_FALSE(testable_counter_metrics_.Count(1, 0));
- ASSERT_FALSE(testable_counter_metrics_.Count(1, -1));
+ ASSERT_TRUE(testable_counter_metrics_.CacheCount(1, 5));
+ ASSERT_FALSE(testable_counter_metrics_.CacheCount(1, 0));
+ ASSERT_FALSE(testable_counter_metrics_.CacheCount(1, -1));
testable_counter_metrics_.DrainBuffer();
ASSERT_EQ(testable_counter_metrics_.test_counters_[1], 5);
}
