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

Commit 3e8cd35b authored by Bookatz's avatar Bookatz
Browse files

Statsd AnomalyDetection stopAlarm also checks old 

Every time stopAlarm() is called, it should also,
right then, check to see if the alarm should actually
have already fired (but didn't due to AlarmManager lag).
Right now, the client needs to do this check separately,
but they always go together. Indeed, MaxDurationTracker
forgot to do the check, which is a bug. It would make
much more sense if the stopAlarm takes care of it for
them, to prevent such mistakes.

Bug: 75273733
Test: make statsd_test && adb sync data && adb shell data/nativetest64/statsd_test/statsd_test
Test: cts-tradefed run cts-dev -m CtsStatsdHostTestCases -t android.cts.statsd.alert
Change-Id: I689df13690df822090ac34b1171e948be1ad0d9f
parent 6bf9825b

cmds/statsd/src/anomaly/DurationAnomalyTracker.cpp

+21 −29
Original line number Diff line number Diff line
@@ -30,7 +30,7 @@ DurationAnomalyTracker::DurationAnomalyTracker(const Alert& alert, const ConfigK 
}



DurationAnomalyTracker::~DurationAnomalyTracker() {

    stopAllAlarms();

    cancelAllAlarms();

}



void DurationAnomalyTracker::resetStorage() {
@@ -38,20 +38,6 @@ void DurationAnomalyTracker::resetStorage() { 
    if (!mAlarms.empty()) ALOGW("AnomalyTracker.resetStorage() called but mAlarms is NOT empty!");

}



void DurationAnomalyTracker::declareAnomalyIfAlarmExpired(const MetricDimensionKey& dimensionKey,

                                                          const uint64_t& timestampNs) {

    auto itr = mAlarms.find(dimensionKey);

    if (itr == mAlarms.end()) {

        return;

    }



    if (itr->second != nullptr &&

        static_cast<uint32_t>(timestampNs / NS_PER_SEC) >= itr->second->timestampSec) {

        declareAnomaly(timestampNs, dimensionKey);

        stopAlarm(dimensionKey);

    }

}



void DurationAnomalyTracker::startAlarm(const MetricDimensionKey& dimensionKey,

                                        const uint64_t& timestampNs) {

    // Alarms are stored in secs. Must round up, since if it fires early, it is ignored completely.
@@ -74,24 +60,30 @@ void DurationAnomalyTracker::startAlarm(const MetricDimensionKey& dimensionKey, 
    }

}



void DurationAnomalyTracker::stopAlarm(const MetricDimensionKey& dimensionKey) {

    auto itr = mAlarms.find(dimensionKey);

    if (itr != mAlarms.end()) {

void DurationAnomalyTracker::stopAlarm(const MetricDimensionKey& dimensionKey,

                                       const uint64_t& timestampNs) {

    const auto itr = mAlarms.find(dimensionKey);

    if (itr == mAlarms.end()) {

        return;

    }



    // If the alarm is set in the past but hasn't fired yet (due to lag), catch it now.

    if (itr->second != nullptr && timestampNs >= NS_PER_SEC * itr->second->timestampSec) {

        declareAnomaly(timestampNs, dimensionKey);

    }

    mAlarms.erase(dimensionKey);

    if (mAlarmMonitor != nullptr) {

        mAlarmMonitor->remove(itr->second);

    }

}

}



void DurationAnomalyTracker::stopAllAlarms() {

    std::set<MetricDimensionKey> keys;

    for (auto itr = mAlarms.begin(); itr != mAlarms.end(); ++itr) {

        keys.insert(itr->first);

void DurationAnomalyTracker::cancelAllAlarms() {

    if (mAlarmMonitor != nullptr) {

        for (const auto& itr : mAlarms) {

            mAlarmMonitor->remove(itr.second);

        }

    for (auto key : keys) {

        stopAlarm(key);

    }

    mAlarms.clear();

}



void DurationAnomalyTracker::informAlarmsFired(const uint64_t& timestampNs,

cmds/statsd/src/anomaly/DurationAnomalyTracker.h

+5 −7
Original line number Diff line number Diff line
@@ -37,14 +37,12 @@ public: 
    void startAlarm(const MetricDimensionKey& dimensionKey, const uint64_t& eventTime);



    // Stops the alarm.

    void stopAlarm(const MetricDimensionKey& dimensionKey);

    // If it should have already fired, but hasn't yet (e.g. because the AlarmManager is delayed),

    // declare the anomaly now.

    void stopAlarm(const MetricDimensionKey& dimensionKey, const uint64_t& timestampNs);



    // Stop all the alarms owned by this tracker.

    void stopAllAlarms();



    // Declares the anomaly when the alarm expired given the current timestamp.

    void declareAnomalyIfAlarmExpired(const MetricDimensionKey& dimensionKey,

                                      const uint64_t& timestampNs);

    // Stop all the alarms owned by this tracker. Does not declare any anomalies.

    void cancelAllAlarms();



    // Declares an anomaly for each alarm in firedAlarms that belongs to this DurationAnomalyTracker

    // and removes it from firedAlarms. The AlarmMonitor is not informed.

cmds/statsd/src/metrics/duration_helper/DurationTracker.h

+3 −11
Original line number Diff line number Diff line
@@ -128,11 +128,11 @@ protected: 
        }

    }



    // Stops the anomaly alarm.

    void stopAnomalyAlarm() {

    // Stops the anomaly alarm. If it should have already fired, declare the anomaly now.

    void stopAnomalyAlarm(const uint64_t timestamp) {

        for (auto& anomalyTracker : mAnomalyTrackers) {

            if (anomalyTracker != nullptr) {

                anomalyTracker->stopAlarm(mEventKey);

                anomalyTracker->stopAlarm(mEventKey, timestamp);

            }

        }

    }
@@ -155,14 +155,6 @@ protected: 
        }

    }



    void declareAnomalyIfAlarmExpired(const uint64_t& timestamp) {

        for (auto& anomalyTracker : mAnomalyTrackers) {

            if (anomalyTracker != nullptr) {

                anomalyTracker->declareAnomalyIfAlarmExpired(mEventKey, timestamp);

            }

        }

    }



    // Convenience to compute the current bucket's end time, which is always aligned with the

    // start time of the metric.

    uint64_t getCurrentBucketEndTimeNs() {

cmds/statsd/src/metrics/duration_helper/MaxDurationTracker.cpp

+2 −2
Original line number Diff line number Diff line
@@ -130,7 +130,7 @@ void MaxDurationTracker::noteStop(const HashableDimensionKey& key, const uint64_ 
        case DurationState::kStarted: {

            duration.startCount--;

            if (forceStop || !mNested || duration.startCount <= 0) {

                stopAnomalyAlarm();

                stopAnomalyAlarm(eventTime);

                duration.state = DurationState::kStopped;

                int64_t durationTime = eventTime - duration.lastStartTime;

                VLOG("Max, key %s, Stop %lld %lld %lld", key.toString().c_str(),
@@ -284,7 +284,7 @@ void MaxDurationTracker::noteConditionChanged(const HashableDimensionKey& key, b 
            // If condition becomes false, kStarted -> kPaused. Record the current duration and

            // stop anomaly alarm.

            if (!conditionMet) {

                stopAnomalyAlarm();

                stopAnomalyAlarm(timestamp);

                it->second.state = DurationState::kPaused;

                it->second.lastDuration += (timestamp - it->second.lastStartTime);

                if (anyStarted()) {

cmds/statsd/src/metrics/duration_helper/OringDurationTracker.cpp

+4 −8
Original line number Diff line number Diff line
@@ -92,7 +92,6 @@ void OringDurationTracker::noteStart(const HashableDimensionKey& key, bool condi 


void OringDurationTracker::noteStop(const HashableDimensionKey& key, const uint64_t timestamp,

                                    const bool stopAll) {

    declareAnomalyIfAlarmExpired(timestamp);

    VLOG("Oring: %s stop", key.toString().c_str());

    auto it = mStarted.find(key);

    if (it != mStarted.end()) {
@@ -118,12 +117,11 @@ void OringDurationTracker::noteStop(const HashableDimensionKey& key, const uint6 
        }

    }

    if (mStarted.empty()) {

        stopAnomalyAlarm();

        stopAnomalyAlarm(timestamp);

    }

}



void OringDurationTracker::noteStopAll(const uint64_t timestamp) {

    declareAnomalyIfAlarmExpired(timestamp);

    if (!mStarted.empty()) {

        mDuration += (timestamp - mLastStartTime);

        VLOG("Oring Stop all: record duration %lld %lld ", (long long)timestamp - mLastStartTime,
@@ -131,7 +129,7 @@ void OringDurationTracker::noteStopAll(const uint64_t timestamp) { 
        detectAndDeclareAnomaly(timestamp, mCurrentBucketNum, mDuration + mDurationFullBucket);

    }



    stopAnomalyAlarm();

    stopAnomalyAlarm(timestamp);

    mStarted.clear();

    mPaused.clear();

    mConditionKeyMap.clear();
@@ -213,7 +211,6 @@ bool OringDurationTracker::flushIfNeeded( 
}



void OringDurationTracker::onSlicedConditionMayChange(const uint64_t timestamp) {

    declareAnomalyIfAlarmExpired(timestamp);

    vector<pair<HashableDimensionKey, int>> startedToPaused;

    vector<pair<HashableDimensionKey, int>> pausedToStarted;

    if (!mStarted.empty()) {
@@ -291,12 +288,11 @@ void OringDurationTracker::onSlicedConditionMayChange(const uint64_t timestamp) 
    mPaused.insert(startedToPaused.begin(), startedToPaused.end());



    if (mStarted.empty()) {

        stopAnomalyAlarm();

        stopAnomalyAlarm(timestamp);

    }

}



void OringDurationTracker::onConditionChanged(bool condition, const uint64_t timestamp) {

    declareAnomalyIfAlarmExpired(timestamp);

    if (condition) {

        if (!mPaused.empty()) {

            VLOG("Condition true, all started");
@@ -319,7 +315,7 @@ void OringDurationTracker::onConditionChanged(bool condition, const uint64_t tim 
        }

    }

    if (mStarted.empty()) {

        stopAnomalyAlarm();

        stopAnomalyAlarm(timestamp);

    }

}

cmds/statsd/src/anomaly/AnomalyTracker.h

+2 −2

File changed.

Contains only whitespace changes.