1/ Use FieldMatcher to specify the value fields in value metric.
2/ rename number_of_buckets as num_buckets
3/ use double for the Alert's threshold
Test: statsd unit tests passed.
Change-Id: Id1f55f14d3712eddee561681e3cd77343f086c7a
diff --git a/cmds/statsd/src/anomaly/AnomalyTracker.cpp b/cmds/statsd/src/anomaly/AnomalyTracker.cpp
index 9c797dc..05c68e1 100644
--- a/cmds/statsd/src/anomaly/AnomalyTracker.cpp
+++ b/cmds/statsd/src/anomaly/AnomalyTracker.cpp
@@ -34,11 +34,11 @@
AnomalyTracker::AnomalyTracker(const Alert& alert, const ConfigKey& configKey)
: mAlert(alert),
mConfigKey(configKey),
- mNumOfPastBuckets(mAlert.number_of_buckets() - 1) {
+ mNumOfPastBuckets(mAlert.num_buckets() - 1) {
VLOG("AnomalyTracker() called");
- if (mAlert.number_of_buckets() <= 0) {
+ if (mAlert.num_buckets() <= 0) {
ALOGE("Cannot create AnomalyTracker with %lld buckets",
- (long long)mAlert.number_of_buckets());
+ (long long)mAlert.num_buckets());
return;
}
if (!mAlert.has_trigger_if_sum_gt()) {
diff --git a/cmds/statsd/src/config/ConfigManager.cpp b/cmds/statsd/src/config/ConfigManager.cpp
index 5cf8b9b..de75c71 100644
--- a/cmds/statsd/src/config/ConfigManager.cpp
+++ b/cmds/statsd/src/config/ConfigManager.cpp
@@ -377,7 +377,8 @@
ValueMetric* valueMetric = config.add_value_metric();
valueMetric->set_id(11);
valueMetric->set_what(109);
- valueMetric->set_value_field(KERNEL_WAKELOCK_COUNT_KEY);
+ valueMetric->mutable_value_field()->set_field(KERNEL_WAKELOCK_TAG_ID);
+ valueMetric->mutable_value_field()->add_child()->set_field(KERNEL_WAKELOCK_COUNT_KEY);
valueMetric->set_condition(201);
dimensions = valueMetric->mutable_dimensions();
dimensions->set_field(KERNEL_WAKELOCK_TAG_ID);
diff --git a/cmds/statsd/src/dimension.cpp b/cmds/statsd/src/dimension.cpp
index 886a33bf..8c0ae97 100644
--- a/cmds/statsd/src/dimension.cpp
+++ b/cmds/statsd/src/dimension.cpp
@@ -351,6 +351,23 @@
}
}
+long getLongFromDimenValue(const DimensionsValue& dimensionValue) {
+ switch (dimensionValue.value_case()) {
+ case DimensionsValue::ValueCase::kValueInt:
+ return dimensionValue.value_int();
+ case DimensionsValue::ValueCase::kValueLong:
+ return dimensionValue.value_long();
+ case DimensionsValue::ValueCase::kValueBool:
+ return dimensionValue.value_bool() ? 1 : 0;
+ case DimensionsValue::ValueCase::kValueFloat:
+ return (int64_t)dimensionValue.value_float();
+ case DimensionsValue::ValueCase::kValueTuple:
+ case DimensionsValue::ValueCase::kValueStr:
+ case DimensionsValue::ValueCase::VALUE_NOT_SET:
+ return 0;
+ }
+}
+
} // namespace statsd
} // namespace os
} // namespace android
diff --git a/cmds/statsd/src/dimension.h b/cmds/statsd/src/dimension.h
index c866958..5bb64a9 100644
--- a/cmds/statsd/src/dimension.h
+++ b/cmds/statsd/src/dimension.h
@@ -56,6 +56,8 @@
bool IsSubDimension(const DimensionsValue& dimension, const DimensionsValue& sub);
+// Helper function to get long value from the DimensionsValue proto.
+long getLongFromDimenValue(const DimensionsValue& dimensionValue);
} // namespace statsd
} // namespace os
} // namespace android
diff --git a/cmds/statsd/src/metrics/DurationMetricProducer.cpp b/cmds/statsd/src/metrics/DurationMetricProducer.cpp
index 99545ae..0117b6d 100644
--- a/cmds/statsd/src/metrics/DurationMetricProducer.cpp
+++ b/cmds/statsd/src/metrics/DurationMetricProducer.cpp
@@ -98,9 +98,9 @@
sp<AnomalyTracker> DurationMetricProducer::addAnomalyTracker(const Alert &alert) {
std::lock_guard<std::mutex> lock(mMutex);
- if (alert.trigger_if_sum_gt() > alert.number_of_buckets() * mBucketSizeNs) {
- ALOGW("invalid alert: threshold (%lld) > possible recordable value (%d x %lld)",
- alert.trigger_if_sum_gt(), alert.number_of_buckets(),
+ if (alert.trigger_if_sum_gt() > alert.num_buckets() * mBucketSizeNs) {
+ ALOGW("invalid alert: threshold (%f) > possible recordable value (%d x %lld)",
+ alert.trigger_if_sum_gt(), alert.num_buckets(),
(long long)mBucketSizeNs);
return nullptr;
}
diff --git a/cmds/statsd/src/metrics/ValueMetricProducer.cpp b/cmds/statsd/src/metrics/ValueMetricProducer.cpp
index ea90170..5f7d761 100644
--- a/cmds/statsd/src/metrics/ValueMetricProducer.cpp
+++ b/cmds/statsd/src/metrics/ValueMetricProducer.cpp
@@ -17,6 +17,7 @@
#define DEBUG false // STOPSHIP if true
#include "Log.h"
+#include "dimension.h"
#include "ValueMetricProducer.h"
#include "guardrail/StatsdStats.h"
#include "stats_log_util.h"
@@ -218,7 +219,8 @@
// For scheduled pulled data, the effective event time is snap to the nearest
// bucket boundary to make bucket finalize.
uint64_t realEventTime = allData.at(0)->GetTimestampNs();
- uint64_t eventTime = mStartTimeNs + ((realEventTime - mStartTimeNs)/mBucketSizeNs) * mBucketSizeNs;
+ uint64_t eventTime = mStartTimeNs +
+ ((realEventTime - mStartTimeNs)/mBucketSizeNs) * mBucketSizeNs;
mCondition = false;
for (const auto& data : allData) {
@@ -272,7 +274,11 @@
}
Interval& interval = mCurrentSlicedBucket[eventKey];
- long value = get_value(event);
+ std::shared_ptr<FieldValueMap> valueFieldMap = getValueFields(event);
+ if (valueFieldMap->empty() || valueFieldMap->size() > 1) {
+ return;
+ }
+ const long value = getLongFromDimenValue(valueFieldMap->begin()->second);
if (mPullTagId != -1) { // for pulled events
if (mCondition == true) {
@@ -296,15 +302,11 @@
}
}
-long ValueMetricProducer::get_value(const LogEvent& event) {
- status_t err = NO_ERROR;
- long val = event.GetLong(mValueField, &err);
- if (err == NO_ERROR) {
- return val;
- } else {
- VLOG("Can't find value in message. %s", event.ToString().c_str());
- return 0;
- }
+std::shared_ptr<FieldValueMap> ValueMetricProducer::getValueFields(const LogEvent& event) {
+ std::shared_ptr<FieldValueMap> valueFields =
+ std::make_shared<FieldValueMap>(event.getFieldValueMap());
+ filterFields(mValueField, valueFields.get());
+ return valueFields;
}
void ValueMetricProducer::flushIfNeededLocked(const uint64_t& eventTimeNs) {
diff --git a/cmds/statsd/src/metrics/ValueMetricProducer.h b/cmds/statsd/src/metrics/ValueMetricProducer.h
index 6f3b1d1..3e7032d 100644
--- a/cmds/statsd/src/metrics/ValueMetricProducer.h
+++ b/cmds/statsd/src/metrics/ValueMetricProducer.h
@@ -70,7 +70,7 @@
// Util function to flush the old packet.
void flushIfNeededLocked(const uint64_t& eventTime);
- const int32_t mValueField;
+ const FieldMatcher mValueField;
std::shared_ptr<StatsPullerManager> mStatsPullerManager;
@@ -103,7 +103,7 @@
// TODO: Add a lock to mPastBuckets.
std::unordered_map<HashableDimensionKey, std::vector<ValueBucket>> mPastBuckets;
- long get_value(const LogEvent& event);
+ std::shared_ptr<FieldValueMap> getValueFields(const LogEvent& event);
// Util function to check whether the specified dimension hits the guardrail.
bool hitGuardRailLocked(const HashableDimensionKey& newKey);
diff --git a/cmds/statsd/src/metrics/metrics_manager_util.cpp b/cmds/statsd/src/metrics/metrics_manager_util.cpp
index a4ae4d6..89d54c7b 100644
--- a/cmds/statsd/src/metrics/metrics_manager_util.cpp
+++ b/cmds/statsd/src/metrics/metrics_manager_util.cpp
@@ -486,9 +486,9 @@
(long long)alert.metric_id());
return false;
}
- if (alert.trigger_if_sum_gt() < 0 || alert.number_of_buckets() <= 0) {
- ALOGW("invalid alert: threshold=%lld num_buckets= %d",
- alert.trigger_if_sum_gt(), alert.number_of_buckets());
+ if (alert.trigger_if_sum_gt() < 0 || alert.num_buckets() <= 0) {
+ ALOGW("invalid alert: threshold=%f num_buckets= %d",
+ alert.trigger_if_sum_gt(), alert.num_buckets());
return false;
}
const int metricIndex = itr->second;
diff --git a/cmds/statsd/src/statsd_config.proto b/cmds/statsd/src/statsd_config.proto
index 6ac0e28..a5057da 100644
--- a/cmds/statsd/src/statsd_config.proto
+++ b/cmds/statsd/src/statsd_config.proto
@@ -213,7 +213,7 @@
optional int64 what = 2;
- optional int32 value_field = 3;
+ optional FieldMatcher value_field = 3;
optional int64 condition = 4;
@@ -232,11 +232,11 @@
optional int64 metric_id = 2;
- optional int32 number_of_buckets = 3;
+ optional int32 num_buckets = 3;
optional int32 refractory_period_secs = 4;
- optional int64 trigger_if_sum_gt = 5;
+ optional double trigger_if_sum_gt = 5;
}
message AllowedLogSource {
diff --git a/cmds/statsd/tests/MetricsManager_test.cpp b/cmds/statsd/tests/MetricsManager_test.cpp
index 96ee9c5..cb212a7 100644
--- a/cmds/statsd/tests/MetricsManager_test.cpp
+++ b/cmds/statsd/tests/MetricsManager_test.cpp
@@ -89,7 +89,7 @@
auto alert = config.add_alert();
alert->set_id(3);
alert->set_metric_id(3);
- alert->set_number_of_buckets(10);
+ alert->set_num_buckets(10);
alert->set_refractory_period_secs(100);
alert->set_trigger_if_sum_gt(100);
return config;
@@ -138,7 +138,7 @@
auto alert = config.add_alert();
alert->set_id(3);
alert->set_metric_id(2);
- alert->set_number_of_buckets(10);
+ alert->set_num_buckets(10);
alert->set_refractory_period_secs(100);
alert->set_trigger_if_sum_gt(100);
return config;
@@ -222,7 +222,7 @@
auto alert = config.add_alert();
alert->set_id(103);
alert->set_metric_id(3);
- alert->set_number_of_buckets(10);
+ alert->set_num_buckets(10);
alert->set_refractory_period_secs(100);
alert->set_trigger_if_sum_gt(100);
return config;
diff --git a/cmds/statsd/tests/anomaly/AnomalyTracker_test.cpp b/cmds/statsd/tests/anomaly/AnomalyTracker_test.cpp
index 751180d..5842bc8 100644
--- a/cmds/statsd/tests/anomaly/AnomalyTracker_test.cpp
+++ b/cmds/statsd/tests/anomaly/AnomalyTracker_test.cpp
@@ -57,7 +57,7 @@
TEST(AnomalyTrackerTest, TestConsecutiveBuckets) {
const int64_t bucketSizeNs = 30 * NS_PER_SEC;
Alert alert;
- alert.set_number_of_buckets(3);
+ alert.set_num_buckets(3);
alert.set_refractory_period_secs(2 * bucketSizeNs / NS_PER_SEC);
alert.set_trigger_if_sum_gt(2);
@@ -177,7 +177,7 @@
TEST(AnomalyTrackerTest, TestSparseBuckets) {
const int64_t bucketSizeNs = 30 * NS_PER_SEC;
Alert alert;
- alert.set_number_of_buckets(3);
+ alert.set_num_buckets(3);
alert.set_refractory_period_secs(2 * bucketSizeNs / NS_PER_SEC);
alert.set_trigger_if_sum_gt(2);
diff --git a/cmds/statsd/tests/metrics/CountMetricProducer_test.cpp b/cmds/statsd/tests/metrics/CountMetricProducer_test.cpp
index 4c2d472..4cb242a 100644
--- a/cmds/statsd/tests/metrics/CountMetricProducer_test.cpp
+++ b/cmds/statsd/tests/metrics/CountMetricProducer_test.cpp
@@ -195,7 +195,7 @@
alert.set_id(11);
alert.set_metric_id(1);
alert.set_trigger_if_sum_gt(2);
- alert.set_number_of_buckets(2);
+ alert.set_num_buckets(2);
alert.set_refractory_period_secs(1);
int64_t bucketStartTimeNs = 10000000000;
diff --git a/cmds/statsd/tests/metrics/GaugeMetricProducer_test.cpp b/cmds/statsd/tests/metrics/GaugeMetricProducer_test.cpp
index cde50c1..749cf26 100644
--- a/cmds/statsd/tests/metrics/GaugeMetricProducer_test.cpp
+++ b/cmds/statsd/tests/metrics/GaugeMetricProducer_test.cpp
@@ -199,7 +199,7 @@
alert.set_id(101);
alert.set_metric_id(metricId);
alert.set_trigger_if_sum_gt(25);
- alert.set_number_of_buckets(2);
+ alert.set_num_buckets(2);
sp<AnomalyTracker> anomalyTracker = gaugeProducer.addAnomalyTracker(alert);
int tagId = 1;
diff --git a/cmds/statsd/tests/metrics/MaxDurationTracker_test.cpp b/cmds/statsd/tests/metrics/MaxDurationTracker_test.cpp
index 6d32329..704a466 100644
--- a/cmds/statsd/tests/metrics/MaxDurationTracker_test.cpp
+++ b/cmds/statsd/tests/metrics/MaxDurationTracker_test.cpp
@@ -217,7 +217,7 @@
alert.set_id(101);
alert.set_metric_id(metricId);
alert.set_trigger_if_sum_gt(32 * NS_PER_SEC);
- alert.set_number_of_buckets(2);
+ alert.set_num_buckets(2);
alert.set_refractory_period_secs(1);
unordered_map<HashableDimensionKey, vector<DurationBucket>> buckets;
diff --git a/cmds/statsd/tests/metrics/OringDurationTracker_test.cpp b/cmds/statsd/tests/metrics/OringDurationTracker_test.cpp
index d34c85b..36cdaae 100644
--- a/cmds/statsd/tests/metrics/OringDurationTracker_test.cpp
+++ b/cmds/statsd/tests/metrics/OringDurationTracker_test.cpp
@@ -265,7 +265,7 @@
alert.set_id(101);
alert.set_metric_id(1);
alert.set_trigger_if_sum_gt(40 * NS_PER_SEC);
- alert.set_number_of_buckets(2);
+ alert.set_num_buckets(2);
alert.set_refractory_period_secs(1);
unordered_map<HashableDimensionKey, vector<DurationBucket>> buckets;
@@ -327,7 +327,7 @@
alert.set_id(101);
alert.set_metric_id(1);
alert.set_trigger_if_sum_gt(40 * NS_PER_SEC);
- alert.set_number_of_buckets(2);
+ alert.set_num_buckets(2);
alert.set_refractory_period_secs(1);
unordered_map<HashableDimensionKey, vector<DurationBucket>> buckets;
diff --git a/cmds/statsd/tests/metrics/ValueMetricProducer_test.cpp b/cmds/statsd/tests/metrics/ValueMetricProducer_test.cpp
index acbfbba..15acca4 100644
--- a/cmds/statsd/tests/metrics/ValueMetricProducer_test.cpp
+++ b/cmds/statsd/tests/metrics/ValueMetricProducer_test.cpp
@@ -51,7 +51,8 @@
ValueMetric metric;
metric.set_id(metricId);
metric.mutable_bucket()->set_bucket_size_millis(bucketSizeNs / 1000000);
- metric.set_value_field(2);
+ metric.mutable_value_field()->set_field(tagId);
+ metric.mutable_value_field()->add_child()->set_field(2);
sp<MockConditionWizard> wizard = new NaggyMock<MockConditionWizard>();
// TODO: pending refactor of StatsPullerManager
@@ -127,7 +128,8 @@
ValueMetric metric;
metric.set_id(metricId);
metric.mutable_bucket()->set_bucket_size_millis(bucketSizeNs / 1000000);
- metric.set_value_field(2);
+ metric.mutable_value_field()->set_field(tagId);
+ metric.mutable_value_field()->add_child()->set_field(2);
metric.set_condition(StringToId("SCREEN_ON"));
sp<MockConditionWizard> wizard = new NaggyMock<MockConditionWizard>();
@@ -203,7 +205,8 @@
ValueMetric metric;
metric.set_id(metricId);
metric.mutable_bucket()->set_bucket_size_millis(bucketSizeNs / 1000000);
- metric.set_value_field(2);
+ metric.mutable_value_field()->set_field(tagId);
+ metric.mutable_value_field()->add_child()->set_field(2);
sp<MockConditionWizard> wizard = new NaggyMock<MockConditionWizard>();
shared_ptr<MockStatsPullerManager> pullerManager =
@@ -244,13 +247,14 @@
alert.set_id(101);
alert.set_metric_id(metricId);
alert.set_trigger_if_sum_gt(130);
- alert.set_number_of_buckets(2);
+ alert.set_num_buckets(2);
alert.set_refractory_period_secs(3);
ValueMetric metric;
metric.set_id(metricId);
metric.mutable_bucket()->set_bucket_size_millis(bucketSizeNs / 1000000);
- metric.set_value_field(2);
+ metric.mutable_value_field()->set_field(tagId);
+ metric.mutable_value_field()->add_child()->set_field(2);
sp<MockConditionWizard> wizard = new NaggyMock<MockConditionWizard>();
ValueMetricProducer valueProducer(kConfigKey, metric, -1 /*-1 meaning no condition*/, wizard,