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

Commit 12fe42f3 authored by Liana Kazanova's avatar Liana Kazanova Committed by Android (Google) Code Review
Browse files

Revert "Don't log change reports to statsd from system server"

This reverts commit 84ee5e0e.

Reason for revert: Potential culprit for b/344525050  - verifying through ABTD before revert submission. This is part of the standard investigation process, and does not mean your CL will be reverted

Change-Id: I62d3cff9a92ab5a0220f4d6cf983038c5493f282
parent 84ee5e0e
Loading
Loading
Loading
Loading
+8 −21
Original line number Diff line number Diff line
@@ -98,7 +98,7 @@ public final class ChangeReporter {
    public void reportChange(int uid, long changeId, int state, boolean isLoggableBySdk) {
        boolean isAlreadyReported =
                checkAndSetIsAlreadyReported(uid, new ChangeReport(changeId, state));
        if (shouldWriteToStatsLog(isAlreadyReported)) {
        if (!isAlreadyReported) {
            FrameworkStatsLog.write(FrameworkStatsLog.APP_COMPATIBILITY_CHANGE_REPORTED, uid,
                    changeId, state, mSource);
        }
@@ -136,16 +136,14 @@ public final class ChangeReporter {
    /**
     * Returns whether the next report should be logged to FrameworkStatsLog.
     *
     * @param isAlreadyReported is the change already reported
     * @param uid      affected by the change
     * @param changeId the reported change id
     * @param state    of the reported change - enabled/disabled/only logged
     * @return true if the report should be logged
     */
    @VisibleForTesting
    boolean shouldWriteToStatsLog(boolean isAlreadyReported) {
        // We don't log for system server
        if (mSource == SOURCE_SYSTEM_SERVER) return false;

        // Don't log if already reported
        return !isAlreadyReported;
    boolean shouldWriteToStatsLog(int uid, long changeId, int state) {
        return !isAlreadyReported(uid, new ChangeReport(changeId, state));
    }

    /**
@@ -162,6 +160,8 @@ public final class ChangeReporter {
            boolean isAlreadyReported, int state, boolean isLoggableBySdk) {
        // If log all bit is on, always return true.
        if (mDebugLogAll) return true;
        // If the change has already been reported, do not write.
        if (isAlreadyReported) return false;

        // If the flag is turned off or the TAG's logging is forced to debug level with
        // `adb setprop log.tag.CompatChangeReporter=DEBUG`, write to debug since the above checks
@@ -224,19 +224,6 @@ public final class ChangeReporter {
        return mReportedChanges.getOrDefault(uid, EMPTY_SET).contains(report);
    }

    /**
     * Returns whether the next report should be logged.
     *
     * @param uid      affected by the change
     * @param changeId the reported change id
     * @param state    of the reported change - enabled/disabled/only logged
     * @return true if the report should be logged
     */
    @VisibleForTesting
    boolean isAlreadyReported(int uid, long changeId, int state) {
        return isAlreadyReported(uid, new ChangeReport(changeId, state));
    }

    private void markAsReported(int uid, ChangeReport report) {
        mReportedChanges.computeIfAbsent(uid, NEW_CHANGE_REPORT_SET).add(report);
    }
+9 −26
Original line number Diff line number Diff line
@@ -31,21 +31,21 @@ public class ChangeReporterTest {
    @Rule public final SetFlagsRule mSetFlagsRule = new SetFlagsRule();

    @Test
    public void testIsAlreadyReportedOnce() {
    public void testStatsLogOnce() {
        ChangeReporter reporter = new ChangeReporter(ChangeReporter.SOURCE_UNKNOWN_SOURCE);
        int myUid = 1022, otherUid = 1023;
        long myChangeId = 500L, otherChangeId = 600L;
        int myState = ChangeReporter.STATE_ENABLED, otherState = ChangeReporter.STATE_DISABLED;

        assertFalse(reporter.isAlreadyReported(myUid, myChangeId, myState));
        assertTrue(reporter.shouldWriteToStatsLog(myUid, myChangeId, myState));
        reporter.reportChange(myUid, myChangeId, myState);

        // Same report will not be logged again.
        assertTrue(reporter.isAlreadyReported(myUid, myChangeId, myState));
        assertFalse(reporter.shouldWriteToStatsLog(myUid, myChangeId, myState));
        // Other reports will be logged.
        assertFalse(reporter.isAlreadyReported(otherUid, myChangeId, myState));
        assertFalse(reporter.isAlreadyReported(myUid, otherChangeId, myState));
        assertFalse(reporter.isAlreadyReported(myUid, myChangeId, otherState));
        assertTrue(reporter.shouldWriteToStatsLog(otherUid, myChangeId, myState));
        assertTrue(reporter.shouldWriteToStatsLog(myUid, otherChangeId, myState));
        assertTrue(reporter.shouldWriteToStatsLog(myUid, myChangeId, otherState));
    }

    @Test
@@ -55,15 +55,15 @@ public class ChangeReporterTest {
        long myChangeId = 500L;
        int myState = ChangeReporter.STATE_ENABLED;

        assertFalse(reporter.isAlreadyReported(myUid, myChangeId, myState));
        assertTrue(reporter.shouldWriteToStatsLog(myUid, myChangeId, myState));
        reporter.reportChange(myUid, myChangeId, myState);

        // Same report will not be logged again.
        assertTrue(reporter.isAlreadyReported(myUid, myChangeId, myState));
        assertFalse(reporter.shouldWriteToStatsLog(myUid, myChangeId, myState));
        reporter.resetReportedChanges(myUid);

        // Same report will be logged again after reset.
        assertFalse(reporter.isAlreadyReported(myUid, myChangeId, myState));
        assertTrue(reporter.shouldWriteToStatsLog(myUid, myChangeId, myState));
    }

    @Test
@@ -196,21 +196,4 @@ public class ChangeReporterTest {
        // off.
        assertTrue(reporter.shouldWriteToDebug(myUid, myChangeId, myDisabledState, false));
    }

    @Test
    public void testDontLogSystemServer() {
        ChangeReporter systemServerReporter =
                new ChangeReporter(ChangeReporter.SOURCE_SYSTEM_SERVER);

        // We never log from system server, even if already reported.
        assertFalse(systemServerReporter.shouldWriteToStatsLog(false));
        assertFalse(systemServerReporter.shouldWriteToStatsLog(true));

        ChangeReporter unknownSourceReporter =
                new ChangeReporter(ChangeReporter.SOURCE_UNKNOWN_SOURCE);

        // Otherwise we log if it's not already been reported.
        assertTrue(unknownSourceReporter.shouldWriteToStatsLog(false));
        assertFalse(unknownSourceReporter.shouldWriteToStatsLog(true));
    }
}