Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 47234644 authored by Chenjie Yu's avatar Chenjie Yu
Browse files

Configurable data error action in value metric 

Right now in value metric, if a later pull produces a smaller number
than the previous one, we use absolute value of the current value.
This is not correct for some atoms as listed in the CL, which should
just take 0.
For some other atoms, this is unexpected error and should just dump
stale data.

Test: manual test
Bug: 79265262
Change-Id: I59fbfd96cbb57be22cd8d21cb57a7c60ca6856ee
parent d19cc3d7

cmds/statsd/src/metrics/ValueMetricProducer.cpp

+11 −5
Original line number Diff line number Diff line
@@ -87,7 +87,8 @@ ValueMetricProducer::ValueMetricProducer(const ConfigKey& key, const ValueMetric 
      mDimensionHardLimit(StatsdStats::kAtomDimensionKeySizeLimitMap.find(pullTagId) !=

                                          StatsdStats::kAtomDimensionKeySizeLimitMap.end()

                                  ? StatsdStats::kAtomDimensionKeySizeLimitMap.at(pullTagId).second

                                  : StatsdStats::kDimensionKeySizeHardLimit) {

                                  : StatsdStats::kDimensionKeySizeHardLimit),

      mUseAbsoluteValueOnReset(metric.use_absolute_value_on_reset()) {

    // TODO: valuemetric for pushed events may need unlimited bucket length

    int64_t bucketSizeMills = 0;

    if (metric.has_bucket()) {
@@ -393,15 +394,20 @@ void ValueMetricProducer::onMatchedLogEventInternalLocked( 
            }

        } else {

            // Generally we expect value to be monotonically increasing.

            // If not, there was a reset event. We take the absolute value as

            // diff in this case.

            // If not, take absolute value or drop it, based on config.

            if (interval.startUpdated) {

                if (value >= interval.start) {

                    interval.sum += (value - interval.start);

                    interval.hasValue = true;

                } else {

                    if (mUseAbsoluteValueOnReset) {

                        interval.sum += value;

                }

                        interval.hasValue = true;

                    } else {

                        VLOG("Dropping data for atom %d, prev: %lld, now: %lld", mPullTagId,

                             (long long)interval.start, (long long)value);

                    }

                }

                interval.startUpdated = false;

            } else {

                VLOG("No start for matching end %lld", (long long)value);

cmds/statsd/src/metrics/ValueMetricProducer.h

+2 −0
Original line number Diff line number Diff line
@@ -164,6 +164,8 @@ private: 


    const size_t mDimensionHardLimit;



    const bool mUseAbsoluteValueOnReset;



    FRIEND_TEST(ValueMetricProducerTest, TestNonDimensionalEvents);

    FRIEND_TEST(ValueMetricProducerTest, TestEventsWithNonSlicedCondition);

    FRIEND_TEST(ValueMetricProducerTest, TestPushedEventsWithUpgrade);

cmds/statsd/src/statsd_config.proto

+2 −0
Original line number Diff line number Diff line
@@ -265,6 +265,8 @@ message ValueMetric { 
  optional AggregationType aggregation_type = 8 [default = SUM];



  optional int64 min_bucket_size_nanos = 10;



  optional bool use_absolute_value_on_reset = 11 [default = false];

}



message Alert {