Set current state key earlier and do not reset hasCurrentState
- The current state key was updated to the new state key after the diff
calculations and base setting. There are a few cases where we exit the
current loop which results in the current state key not getting updated.
- When we reset the base, we should not reset the current state. All
instances where hasCurrentState was set to false have been removed.
- I added a unit test for a value metric that is sliced by state and has
a condition.
Bug: 157661456
Test: m statsd_test && adb sync data && adb shell
data/nativetest/statsd_test/statsd_test32
Change-Id: Ib75bd9d08e6785f9e03b4b5984dbdaf86a7e1749
diff --git a/cmds/statsd/src/metrics/MetricProducer.cpp b/cmds/statsd/src/metrics/MetricProducer.cpp
index cdd20cd..fe143e4 100644
--- a/cmds/statsd/src/metrics/MetricProducer.cpp
+++ b/cmds/statsd/src/metrics/MetricProducer.cpp
@@ -98,7 +98,7 @@
// Stores atom id to primary key pairs for each state atom that the metric is
// sliced by.
- std::map<int, HashableDimensionKey> statePrimaryKeys;
+ std::map<int32_t, HashableDimensionKey> statePrimaryKeys;
// For states with primary fields, use MetricStateLinks to get the primary
// field values from the log event. These values will form a primary key
diff --git a/cmds/statsd/src/metrics/ValueMetricProducer.cpp b/cmds/statsd/src/metrics/ValueMetricProducer.cpp
index c0d1174..8203f38 100644
--- a/cmds/statsd/src/metrics/ValueMetricProducer.cpp
+++ b/cmds/statsd/src/metrics/ValueMetricProducer.cpp
@@ -189,11 +189,6 @@
VLOG("ValueMetric %lld onStateChanged time %lld, State %d, key %s, %d -> %d",
(long long)mMetricId, (long long)eventTimeNs, atomId, primaryKey.toString().c_str(),
oldState.mValue.int_value, newState.mValue.int_value);
- // If condition is not true or metric is not active, we do not need to pull
- // for this state change.
- if (mCondition != ConditionState::kTrue || !mIsActive) {
- return;
- }
// If old and new states are in the same StateGroup, then we do not need to
// pull for this state change.
@@ -205,6 +200,12 @@
return;
}
+ // If condition is not true or metric is not active, we do not need to pull
+ // for this state change.
+ if (mCondition != ConditionState::kTrue || !mIsActive) {
+ return;
+ }
+
bool isEventLate = eventTimeNs < mCurrentBucketStartTimeNs;
if (isEventLate) {
VLOG("Skip event due to late arrival: %lld vs %lld", (long long)eventTimeNs,
@@ -412,7 +413,6 @@
for (auto& slice : mCurrentBaseInfo) {
for (auto& baseInfo : slice.second) {
baseInfo.hasBase = false;
- baseInfo.hasCurrentState = false;
}
}
mHasGlobalBase = false;
@@ -625,7 +625,6 @@
auto it = mCurrentBaseInfo.find(whatKey);
for (auto& baseInfo : it->second) {
baseInfo.hasBase = false;
- baseInfo.hasCurrentState = false;
}
}
}
@@ -820,6 +819,8 @@
Interval& interval = intervals[i];
interval.valueIndex = i;
Value value;
+ baseInfo.hasCurrentState = true;
+ baseInfo.currentState = stateKey;
if (!getDoubleOrLong(event, matcher, value)) {
VLOG("Failed to get value %d from event %s", i, event.ToString().c_str());
StatsdStats::getInstance().noteBadValueType(mMetricId);
@@ -907,7 +908,6 @@
interval.hasValue = true;
}
interval.sampleSize += 1;
- baseInfo.currentState = stateKey;
}
// Only trigger the tracker if all intervals are correct
diff --git a/cmds/statsd/src/metrics/ValueMetricProducer.h b/cmds/statsd/src/metrics/ValueMetricProducer.h
index 505b239..b359af7 100644
--- a/cmds/statsd/src/metrics/ValueMetricProducer.h
+++ b/cmds/statsd/src/metrics/ValueMetricProducer.h
@@ -313,6 +313,7 @@
FRIEND_TEST(ValueMetricProducerTest, TestSlicedState);
FRIEND_TEST(ValueMetricProducerTest, TestSlicedStateWithMap);
FRIEND_TEST(ValueMetricProducerTest, TestSlicedStateWithPrimaryField_WithDimensions);
+ FRIEND_TEST(ValueMetricProducerTest, TestSlicedStateWithCondition);
FRIEND_TEST(ValueMetricProducerTest, TestTrimUnusedDimensionKey);
FRIEND_TEST(ValueMetricProducerTest, TestUseZeroDefaultBase);
FRIEND_TEST(ValueMetricProducerTest, TestUseZeroDefaultBaseWithPullFailures);