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

Commit 18e46929 authored by Muhammad Qureshi's avatar Muhammad Qureshi
Browse files

Handle condition changes when metric is not active.

Treat metric activation like conditions as much as possible. Keep track
of condition changes even when metric is not active. Right now, we
ignore all condition changes if metric is not active.

This is a band-aid fix. Ideally, we should re-think how metric activation is
implemented and consider implementing it as a condition with a timebomb.

Bug: 130838341
Test: statsd_test
Test: cts-tradefed run cts-dev -m CtsStatsdHostTestCases -t
android.cts.statsd.metric

Change-Id: I96bb7a7c6ee88359f310065e166f858be945eaff
parent c0201ca9
Loading
Loading
Loading
Loading
+1 −0
Original line number Diff line number Diff line
@@ -262,6 +262,7 @@ cc_test {
        "tests/e2e/Anomaly_duration_sum_e2e_test.cpp",
        "tests/e2e/ConfigTtl_e2e_test.cpp",
        "tests/e2e/PartialBucket_e2e_test.cpp",
        "tests/e2e/DurationMetric_e2e_test.cpp",
        "tests/shell/ShellSubscriber_test.cpp",
    ],

+7 −0
Original line number Diff line number Diff line
@@ -273,6 +273,13 @@ private:
    FRIEND_TEST(MetricActivationE2eTest, TestCountMetricWithOneDeactivation);
    FRIEND_TEST(MetricActivationE2eTest, TestCountMetricWithTwoDeactivations);
    FRIEND_TEST(MetricActivationE2eTest, TestCountMetricWithTwoMetricsTwoDeactivations);

    FRIEND_TEST(DurationMetricE2eTest, TestOneBucket);
    FRIEND_TEST(DurationMetricE2eTest, TestTwoBuckets);
    FRIEND_TEST(DurationMetricE2eTest, TestWithActivation);
    FRIEND_TEST(DurationMetricE2eTest, TestWithCondition);
    FRIEND_TEST(DurationMetricE2eTest, TestWithSlicedCondition);
    FRIEND_TEST(DurationMetricE2eTest, TestWithActivationAndSlicedCondition);
};

}  // namespace statsd
+67 −19
Original line number Diff line number Diff line
@@ -338,32 +338,25 @@ void DurationMetricProducer::onSlicedConditionMayChangeLocked_opt2(bool conditio
    }
}

void DurationMetricProducer::onSlicedConditionMayChangeLocked(bool overallCondition,
                                                              const int64_t eventTime) {
    VLOG("Metric %lld onSlicedConditionMayChange", (long long)mMetricId);
    flushIfNeededLocked(eventTime);

    if (!mConditionSliced) {
        return;
    }

void DurationMetricProducer::onSlicedConditionMayChangeInternalLocked(bool overallCondition,
        const int64_t eventTimeNs) {
    bool changeDimTrackable = mWizard->IsChangedDimensionTrackable(mConditionTrackerIndex);
    if (changeDimTrackable && mHasLinksToAllConditionDimensionsInTracker &&
        mDimensionsInCondition.empty()) {
        onSlicedConditionMayChangeLocked_opt1(overallCondition, eventTime);
        onSlicedConditionMayChangeLocked_opt1(overallCondition, eventTimeNs);
        return;
    }

    if (changeDimTrackable && mSameConditionDimensionsInTracker &&
        mMetric2ConditionLinks.size() <= 1) {
        onSlicedConditionMayChangeLocked_opt2(overallCondition, eventTime);
        onSlicedConditionMayChangeLocked_opt2(overallCondition, eventTimeNs);
        return;
    }

    // Now for each of the on-going event, check if the condition has changed for them.
    for (auto& whatIt : mCurrentSlicedDurationTrackerMap) {
        for (auto& pair : whatIt.second) {
            pair.second->onSlicedConditionMayChange(overallCondition, eventTime);
            pair.second->onSlicedConditionMayChange(overallCondition, eventTimeNs);
        }
    }

@@ -389,10 +382,10 @@ void DurationMetricProducer::onSlicedConditionMayChangeLocked(bool overallCondit
                        continue;
                    }
                    unique_ptr<DurationTracker> newTracker =
                        whatIt.second.begin()->second->clone(eventTime);
                        whatIt.second.begin()->second->clone(eventTimeNs);
                    if (newTracker != nullptr) {
                        newTracker->setEventKey(MetricDimensionKey(newEventKey));
                        newTracker->onSlicedConditionMayChange(overallCondition, eventTime);
                        newTracker->onSlicedConditionMayChange(overallCondition, eventTimeNs);
                        whatIt.second[conditionDimension] = std::move(newTracker);
                    }
                }
@@ -418,10 +411,10 @@ void DurationMetricProducer::onSlicedConditionMayChangeLocked(bool overallCondit
                    if (hitGuardRailLocked(newEventKey)) {
                        continue;
                    }
                    auto newTracker = whatIt.second.begin()->second->clone(eventTime);
                    auto newTracker = whatIt.second.begin()->second->clone(eventTimeNs);
                    if (newTracker != nullptr) {
                        newTracker->setEventKey(newEventKey);
                        newTracker->onSlicedConditionMayChange(overallCondition, eventTime);
                        newTracker->onSlicedConditionMayChange(overallCondition, eventTimeNs);
                        whatIt.second[conditionDimension] = std::move(newTracker);
                    }
                }
@@ -430,10 +423,61 @@ void DurationMetricProducer::onSlicedConditionMayChangeLocked(bool overallCondit
    }
}

void DurationMetricProducer::onSlicedConditionMayChangeLocked(bool overallCondition,
                                                              const int64_t eventTime) {
    VLOG("Metric %lld onSlicedConditionMayChange", (long long)mMetricId);

    if (!mIsActive) {
        return;
    }

    flushIfNeededLocked(eventTime);

    if (!mConditionSliced) {
        return;
    }

    onSlicedConditionMayChangeInternalLocked(overallCondition, eventTime);
}

void DurationMetricProducer::onActiveStateChangedLocked(const int64_t& eventTimeNs) {
    MetricProducer::onActiveStateChangedLocked(eventTimeNs);

    if (!mConditionSliced) {
        if (ConditionState::kTrue != mCondition) {
            return;
        }

        if (mIsActive) {
            flushIfNeededLocked(eventTimeNs);
        }

        for (auto& whatIt : mCurrentSlicedDurationTrackerMap) {
            for (auto& pair : whatIt.second) {
                pair.second->onConditionChanged(mIsActive, eventTimeNs);
            }
        }
    } else if (mIsActive) {
        flushIfNeededLocked(eventTimeNs);
        onSlicedConditionMayChangeInternalLocked(mIsActive, eventTimeNs);
    } else { // mConditionSliced == true && !mIsActive
        for (auto& whatIt : mCurrentSlicedDurationTrackerMap) {
            for (auto& pair : whatIt.second) {
                pair.second->onConditionChanged(mIsActive, eventTimeNs);
            }
        }
    }
}

void DurationMetricProducer::onConditionChangedLocked(const bool conditionMet,
                                                      const int64_t eventTime) {
    VLOG("Metric %lld onConditionChanged", (long long)mMetricId);
    mCondition = conditionMet ? ConditionState::kTrue : ConditionState::kFalse;

    if (!mIsActive) {
        return;
    }

    flushIfNeededLocked(eventTime);
    for (auto& whatIt : mCurrentSlicedDurationTrackerMap) {
        for (auto& pair : whatIt.second) {
@@ -696,7 +740,9 @@ void DurationMetricProducer::onMatchedLogEventLocked(const size_t matcherIndex,
        return;
    }

    if (mIsActive) {
        flushIfNeededLocked(event.GetElapsedTimestampNs());
    }

    // Handles Stopall events.
    if (matcherIndex == mStopAllIndex) {
@@ -767,6 +813,8 @@ void DurationMetricProducer::onMatchedLogEventLocked(const size_t matcherIndex,
        }
    }

    condition = condition && mIsActive;

    if (dimensionKeysInCondition.empty()) {
        handleStartEvent(MetricDimensionKey(dimensionInWhat, DEFAULT_DIMENSION_KEY),
                         conditionKey, condition, event);
+6 −0
Original line number Diff line number Diff line
@@ -73,9 +73,15 @@ private:
    // Internal interface to handle condition change.
    void onConditionChangedLocked(const bool conditionMet, const int64_t eventTime) override;

    // Internal interface to handle active state change.
    void onActiveStateChangedLocked(const int64_t& eventTimeNs) override;

    // Internal interface to handle sliced condition change.
    void onSlicedConditionMayChangeLocked(bool overallCondition, const int64_t eventTime) override;

    void onSlicedConditionMayChangeInternalLocked(bool overallCondition,
                                                  const int64_t eventTimeNs);

    void onSlicedConditionMayChangeLocked_opt1(bool overallCondition, const int64_t eventTime);
    void onSlicedConditionMayChangeLocked_opt2(bool overallCondition, const int64_t eventTime);

+22 −2
Original line number Diff line number Diff line
@@ -364,11 +364,27 @@ void GaugeMetricProducer::pullAndMatchEventsLocked(const int64_t timestampNs) {
    }
}

void GaugeMetricProducer::onActiveStateChangedLocked(const int64_t& eventTimeNs) {
    MetricProducer::onActiveStateChangedLocked(eventTimeNs);
    if (ConditionState::kTrue != mCondition || !mIsPulled) {
        return;
    }
    if (mTriggerAtomId == -1 || (mIsActive && mSamplingType == GaugeMetric::RANDOM_ONE_SAMPLE)) {
        pullAndMatchEventsLocked(eventTimeNs);
    }

}

void GaugeMetricProducer::onConditionChangedLocked(const bool conditionMet,
                                                   const int64_t eventTimeNs) {
    VLOG("GaugeMetric %lld onConditionChanged", (long long)mMetricId);
    flushIfNeededLocked(eventTimeNs);

    mCondition = conditionMet ? ConditionState::kTrue : ConditionState::kFalse;
    if (!mIsActive) {
        return;
    }

    flushIfNeededLocked(eventTimeNs);
    if (mIsPulled && mTriggerAtomId == -1) {
        pullAndMatchEventsLocked(eventTimeNs);
    }  // else: Push mode. No need to proactively pull the gauge data.
@@ -378,10 +394,14 @@ void GaugeMetricProducer::onSlicedConditionMayChangeLocked(bool overallCondition
                                                           const int64_t eventTimeNs) {
    VLOG("GaugeMetric %lld onSlicedConditionMayChange overall condition %d", (long long)mMetricId,
         overallCondition);
    mCondition = overallCondition ? ConditionState::kTrue : ConditionState::kFalse;
    if (!mIsActive) {
        return;
    }

    flushIfNeededLocked(eventTimeNs);
    // If the condition is sliced, mCondition is true if any of the dimensions is true. And we will
    // pull for every dimension.
    mCondition = overallCondition ? ConditionState::kTrue : ConditionState::kFalse;
    if (mIsPulled && mTriggerAtomId == -1) {
        pullAndMatchEventsLocked(eventTimeNs);
    }  // else: Push mode. No need to proactively pull the gauge data.
Loading