Fix ValueMetric should only pull on real state changes
In ValueMetricProducer, we now map the new and old state values from
onStateChanged to their correct group ids (no mapping happens if the
metric has no state map). We then check if the group ids are the same
and return if they are. Having the same group id means that the state
values are in the same group and no pull is needed.
onStateChanged was updated to take state values stored in FieldValue
objects instead of as ints. This makes it easier to utilize the
mapStateValue function in MetricProducer.
Bug: b/156428844
Test: m statsd_test && adb sync data && adb shell
data/nativetest/statsd_test/statsd_test
Change-Id: Id8f110db593470b8923e7c4259d70cc5f5bc9147
diff --git a/cmds/statsd/src/metrics/CountMetricProducer.cpp b/cmds/statsd/src/metrics/CountMetricProducer.cpp
index 21ffff3..d865c21 100644
--- a/cmds/statsd/src/metrics/CountMetricProducer.cpp
+++ b/cmds/statsd/src/metrics/CountMetricProducer.cpp
@@ -122,11 +122,11 @@
}
void CountMetricProducer::onStateChanged(const int64_t eventTimeNs, const int32_t atomId,
- const HashableDimensionKey& primaryKey, int oldState,
- int newState) {
+ const HashableDimensionKey& primaryKey,
+ const FieldValue& oldState, const FieldValue& newState) {
VLOG("CountMetric %lld onStateChanged time %lld, State%d, key %s, %d -> %d",
(long long)mMetricId, (long long)eventTimeNs, atomId, primaryKey.toString().c_str(),
- oldState, newState);
+ oldState.mValue.int_value, newState.mValue.int_value);
}
void CountMetricProducer::dumpStatesLocked(FILE* out, bool verbose) const {
diff --git a/cmds/statsd/src/metrics/CountMetricProducer.h b/cmds/statsd/src/metrics/CountMetricProducer.h
index f9a8842..26b3d3c 100644
--- a/cmds/statsd/src/metrics/CountMetricProducer.h
+++ b/cmds/statsd/src/metrics/CountMetricProducer.h
@@ -53,8 +53,8 @@
virtual ~CountMetricProducer();
void onStateChanged(const int64_t eventTimeNs, const int32_t atomId,
- const HashableDimensionKey& primaryKey, int oldState,
- int newState) override;
+ const HashableDimensionKey& primaryKey, const FieldValue& oldState,
+ const FieldValue& newState) override;
protected:
void onMatchedLogEventInternalLocked(
diff --git a/cmds/statsd/src/metrics/DurationMetricProducer.cpp b/cmds/statsd/src/metrics/DurationMetricProducer.cpp
index 0de92f3..6633659 100644
--- a/cmds/statsd/src/metrics/DurationMetricProducer.cpp
+++ b/cmds/statsd/src/metrics/DurationMetricProducer.cpp
@@ -161,13 +161,12 @@
void DurationMetricProducer::onStateChanged(const int64_t eventTimeNs, const int32_t atomId,
const HashableDimensionKey& primaryKey,
- const int32_t oldState, const int32_t newState) {
- // Create a FieldValue object to hold the new state.
- FieldValue value;
- value.mValue.setInt(newState);
+ const FieldValue& oldState,
+ const FieldValue& newState) {
// Check if this metric has a StateMap. If so, map the new state value to
// the correct state group id.
- mapStateValue(atomId, &value);
+ FieldValue newStateCopy = newState;
+ mapStateValue(atomId, &newStateCopy);
flushIfNeededLocked(eventTimeNs);
@@ -185,7 +184,7 @@
if (!containsLinkedStateValues(whatIt.first, primaryKey, mMetric2StateLinks, atomId)) {
continue;
}
- whatIt.second->onStateChanged(eventTimeNs, atomId, value);
+ whatIt.second->onStateChanged(eventTimeNs, atomId, newStateCopy);
}
}
diff --git a/cmds/statsd/src/metrics/DurationMetricProducer.h b/cmds/statsd/src/metrics/DurationMetricProducer.h
index 6f84076..53f0f28 100644
--- a/cmds/statsd/src/metrics/DurationMetricProducer.h
+++ b/cmds/statsd/src/metrics/DurationMetricProducer.h
@@ -55,8 +55,8 @@
const sp<AlarmMonitor>& anomalyAlarmMonitor) override;
void onStateChanged(const int64_t eventTimeNs, const int32_t atomId,
- const HashableDimensionKey& primaryKey, const int32_t oldState,
- const int32_t newState) override;
+ const HashableDimensionKey& primaryKey, const FieldValue& oldState,
+ const FieldValue& newState) override;
protected:
void onMatchedLogEventLocked(const size_t matcherIndex, const LogEvent& event) override;
diff --git a/cmds/statsd/src/metrics/MetricProducer.h b/cmds/statsd/src/metrics/MetricProducer.h
index 28563ad..5fabb5f 100644
--- a/cmds/statsd/src/metrics/MetricProducer.h
+++ b/cmds/statsd/src/metrics/MetricProducer.h
@@ -182,8 +182,8 @@
};
void onStateChanged(const int64_t eventTimeNs, const int32_t atomId,
- const HashableDimensionKey& primaryKey, const int32_t oldState,
- const int32_t newState){};
+ const HashableDimensionKey& primaryKey, const FieldValue& oldState,
+ const FieldValue& newState){};
// Output the metrics data to [protoOutput]. All metrics reports end with the same timestamp.
// This method clears all the past buckets.
diff --git a/cmds/statsd/src/metrics/ValueMetricProducer.cpp b/cmds/statsd/src/metrics/ValueMetricProducer.cpp
index bf636a4..f03ce45 100644
--- a/cmds/statsd/src/metrics/ValueMetricProducer.cpp
+++ b/cmds/statsd/src/metrics/ValueMetricProducer.cpp
@@ -182,15 +182,26 @@
}
void ValueMetricProducer::onStateChanged(int64_t eventTimeNs, int32_t atomId,
- const HashableDimensionKey& primaryKey, int oldState,
- int newState) {
+ const HashableDimensionKey& primaryKey,
+ const FieldValue& oldState, const FieldValue& newState) {
VLOG("ValueMetric %lld onStateChanged time %lld, State %d, key %s, %d -> %d",
(long long)mMetricId, (long long)eventTimeNs, atomId, primaryKey.toString().c_str(),
- oldState, newState);
+ oldState.mValue.int_value, newState.mValue.int_value);
// If condition is not true, we do not need to pull for this state change.
if (mCondition != ConditionState::kTrue) {
return;
}
+
+ // If old and new states are in the same StateGroup, then we do not need to
+ // pull for this state change.
+ FieldValue oldStateCopy = oldState;
+ FieldValue newStateCopy = newState;
+ mapStateValue(atomId, &oldStateCopy);
+ mapStateValue(atomId, &newStateCopy);
+ if (oldStateCopy == newStateCopy) {
+ return;
+ }
+
bool isEventLate = eventTimeNs < mCurrentBucketStartTimeNs;
if (isEventLate) {
VLOG("Skip event due to late arrival: %lld vs %lld", (long long)eventTimeNs,
diff --git a/cmds/statsd/src/metrics/ValueMetricProducer.h b/cmds/statsd/src/metrics/ValueMetricProducer.h
index c8dc8cc..751fef2 100644
--- a/cmds/statsd/src/metrics/ValueMetricProducer.h
+++ b/cmds/statsd/src/metrics/ValueMetricProducer.h
@@ -90,7 +90,7 @@
};
void onStateChanged(int64_t eventTimeNs, int32_t atomId, const HashableDimensionKey& primaryKey,
- int oldState, int newState) override;
+ const FieldValue& oldState, const FieldValue& newState) override;
protected:
void onMatchedLogEventInternalLocked(