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

Commit 3c0507b1 authored by Mark White's avatar Mark White
Browse files

Avoid logging change reports to statsd from system apps 

This change reduces statsd change report volumes by
skipping logging if we know an app is a system app

Test: atest CtsAppCompatHostTestCases PlatformCompatFrameworkTests PlatformCompatGating PlatformCompatTest
Fix: 343941691
Flag: EXEMPT bugfix
Change-Id: I54e822cd48c6614223921192f711277fadb549bc
parent ef5dd301

core/java/android/app/AppCompatCallbacks.java

+1 −1
Original line number Diff line number Diff line
@@ -82,7 +82,7 @@ public final class AppCompatCallbacks implements Compatibility.BehaviorChangeDel 


    private void reportChange(long changeId, int state, boolean isLoggable) {

        int uid = Process.myUid();

        mChangeReporter.reportChange(uid, changeId, state, isLoggable);

        mChangeReporter.reportChange(uid, changeId, state, false, isLoggable);

    }



}

core/java/com/android/internal/compat/ChangeReporter.java

+32 −15
Original line number Diff line number Diff line
@@ -42,7 +42,7 @@ import java.util.function.Function; 
 *

 * @hide

 */

public final class ChangeReporter {

public class ChangeReporter {

    private static final String TAG = "CompatChangeReporter";

    private static final Function<Integer, Set<ChangeReport>> NEW_CHANGE_REPORT_SET =

            uid -> Collections.synchronizedSet(new HashSet<>());
@@ -91,14 +91,17 @@ public final class ChangeReporter { 
     * @param uid              affected by the change

     * @param changeId         the reported change id

     * @param state            of the reported change - enabled/disabled/only logged

     * @param isKnownSystemApp do we know that the affected app is a system app? (if true is

     *                         definitely a system app, if false it may or may not be a system app)

     * @param isLoggableBySdk  whether debug logging is allowed for this change based on target

     *                        SDK version. This is combined with other logic to determine whether to

     *                        actually log. If the sdk version does not matter, should be true.

     *                         SDK version. This is combined with other logic to determine whether

     *                         to actually log. If the sdk version does not matter, should be true.

     */

    public void reportChange(int uid, long changeId, int state, boolean isLoggableBySdk) {

    public void reportChange(int uid, long changeId, int state, boolean isKnownSystemApp,

            boolean isLoggableBySdk) {

        boolean isAlreadyReported =

                checkAndSetIsAlreadyReported(uid, new ChangeReport(changeId, state));

        if (!isAlreadyReported) {

        if (shouldWriteToStatsLog(isKnownSystemApp, isAlreadyReported)) {

            FrameworkStatsLog.write(FrameworkStatsLog.APP_COMPATIBILITY_CHANGE_REPORTED, uid,

                    changeId, state, mSource);

        }
@@ -116,7 +119,7 @@ public final class ChangeReporter { 
     * @param state    of the reported change - enabled/disabled/only logged

     */

    public void reportChange(int uid, long changeId, int state) {

        reportChange(uid, changeId, state, true);

        reportChange(uid, changeId, state, false, true);

    }



    /**
@@ -136,14 +139,15 @@ public final class ChangeReporter { 
    /**

     * Returns whether the next report should be logged to FrameworkStatsLog.

     *

     * @param uid      affected by the change

     * @param changeId the reported change id

     * @param state    of the reported change - enabled/disabled/only logged

     * @param isKnownSystemApp do we know that the affected app is a system app? (if true is

     *                         definitely a system app, if false it may or may not be a system app)

     * @param isAlreadyReported is the change already reported

     * @return true if the report should be logged

     */

    @VisibleForTesting

    boolean shouldWriteToStatsLog(int uid, long changeId, int state) {

        return !isAlreadyReported(uid, new ChangeReport(changeId, state));

    boolean shouldWriteToStatsLog(boolean isKnownSystemApp, boolean isAlreadyReported) {

        // We don't log where we know the source is a system app or is already reported

        return !isKnownSystemApp && !isAlreadyReported;

    }



    /**
@@ -224,6 +228,19 @@ 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);

    }

core/tests/PlatformCompatFramework/src/com/android/internal/compat/ChangeReporterTest.java

+33 −8
Original line number Diff line number Diff line
@@ -37,15 +37,20 @@ public class ChangeReporterTest { 
        long myChangeId = 500L, otherChangeId = 600L;

        int myState = ChangeReporter.STATE_ENABLED, otherState = ChangeReporter.STATE_DISABLED;



        assertTrue(reporter.shouldWriteToStatsLog(myUid, myChangeId, myState));

        assertTrue(reporter.shouldWriteToStatsLog(false,

                reporter.isAlreadyReported(myUid, myChangeId, myState)));

        reporter.reportChange(myUid, myChangeId, myState);



        // Same report will not be logged again.

        assertFalse(reporter.shouldWriteToStatsLog(myUid, myChangeId, myState));

        assertFalse(reporter.shouldWriteToStatsLog(false,

                reporter.isAlreadyReported(myUid, myChangeId, myState)));

        // Other reports will be logged.

        assertTrue(reporter.shouldWriteToStatsLog(otherUid, myChangeId, myState));

        assertTrue(reporter.shouldWriteToStatsLog(myUid, otherChangeId, myState));

        assertTrue(reporter.shouldWriteToStatsLog(myUid, myChangeId, otherState));

        assertTrue(reporter.shouldWriteToStatsLog(false,

                reporter.isAlreadyReported(otherUid, myChangeId, myState)));

        assertTrue(reporter.shouldWriteToStatsLog(false,

                reporter.isAlreadyReported(myUid, otherChangeId, myState)));

        assertTrue(reporter.shouldWriteToStatsLog(false,

                reporter.isAlreadyReported(myUid, myChangeId, otherState)));

    }



    @Test
@@ -55,15 +60,18 @@ public class ChangeReporterTest { 
        long myChangeId = 500L;

        int myState = ChangeReporter.STATE_ENABLED;



        assertTrue(reporter.shouldWriteToStatsLog(myUid, myChangeId, myState));

        assertTrue(reporter.shouldWriteToStatsLog(false,

                reporter.isAlreadyReported(myUid, myChangeId, myState)));

        reporter.reportChange(myUid, myChangeId, myState);



        // Same report will not be logged again.

        assertFalse(reporter.shouldWriteToStatsLog(myUid, myChangeId, myState));

        assertFalse(reporter.shouldWriteToStatsLog(false,

                reporter.isAlreadyReported(myUid, myChangeId, myState)));

        reporter.resetReportedChanges(myUid);



        // Same report will be logged again after reset.

        assertTrue(reporter.shouldWriteToStatsLog(myUid, myChangeId, myState));

        assertTrue(reporter.shouldWriteToStatsLog(false,

                reporter.isAlreadyReported(myUid, myChangeId, myState)));

    }



    @Test
@@ -196,4 +204,21 @@ public class ChangeReporterTest { 
        // off.

        assertTrue(reporter.shouldWriteToDebug(myUid, myChangeId, myDisabledState, false));

    }



    @Test

    public void testDontLogSystemApps() {

        // Verify we don't log an app if we know it's a system app when source is system server.

        ChangeReporter systemServerReporter =

                new ChangeReporter(ChangeReporter.SOURCE_SYSTEM_SERVER);



        assertFalse(systemServerReporter.shouldWriteToStatsLog(true, false));

        assertFalse(systemServerReporter.shouldWriteToStatsLog(true, true));



        // Verify we don't log an app if we know it's a system app when source is unknown.

        ChangeReporter unknownReporter =

                new ChangeReporter(ChangeReporter.SOURCE_UNKNOWN_SOURCE);



        assertFalse(unknownReporter.shouldWriteToStatsLog(true, false));

        assertFalse(unknownReporter.shouldWriteToStatsLog(true, true));

    }

}

services/core/java/com/android/server/compat/PlatformCompat.java

+21 −9
Original line number Diff line number Diff line
@@ -83,9 +83,10 @@ public class PlatformCompat extends IPlatformCompat.Stub { 


    @VisibleForTesting

    PlatformCompat(Context context, CompatConfig compatConfig,

            AndroidBuildClassifier buildClassifier) {

            AndroidBuildClassifier buildClassifier,

            ChangeReporter changeReporter) {

        mContext = context;

        mChangeReporter = new ChangeReporter(ChangeReporter.SOURCE_SYSTEM_SERVER);

        mChangeReporter = changeReporter;

        mCompatConfig = compatConfig;

        mBuildClassifier = buildClassifier;
@@ -96,8 +97,11 @@ public class PlatformCompat extends IPlatformCompat.Stub { 
    @EnforcePermission(LOG_COMPAT_CHANGE)

    public void reportChange(long changeId, ApplicationInfo appInfo) {

        super.reportChange_enforcePermission();



        reportChangeInternal(changeId, appInfo.uid, ChangeReporter.STATE_LOGGED);

        reportChangeInternal(

                changeId,

                appInfo.uid,

                appInfo.isSystemApp(),

                ChangeReporter.STATE_LOGGED);

    }



    @Override
@@ -108,7 +112,11 @@ public class PlatformCompat extends IPlatformCompat.Stub { 


        ApplicationInfo appInfo = getApplicationInfo(packageName, userId);

        if (appInfo != null) {

            reportChangeInternal(changeId, appInfo.uid, ChangeReporter.STATE_LOGGED);

            reportChangeInternal(

                    changeId,

                    appInfo.uid,

                    appInfo.isSystemApp(),

                    ChangeReporter.STATE_LOGGED);

        }

    }
@@ -117,7 +125,7 @@ public class PlatformCompat extends IPlatformCompat.Stub { 
    public void reportChangeByUid(long changeId, int uid) {

        super.reportChangeByUid_enforcePermission();



        reportChangeInternal(changeId, uid, ChangeReporter.STATE_LOGGED);

        reportChangeInternal(changeId, uid, false, ChangeReporter.STATE_LOGGED);

    }



    /**
@@ -128,8 +136,8 @@ public class PlatformCompat extends IPlatformCompat.Stub { 
     * @param uid             of the user

     * @param state           of the change - enabled/disabled/logged

     */

    private void reportChangeInternal(long changeId, int uid, int state) {

        mChangeReporter.reportChange(uid, changeId, state, true);

    private void reportChangeInternal(long changeId, int uid, boolean isKnownSystemApp, int state) {

        mChangeReporter.reportChange(uid, changeId, state, isKnownSystemApp, true);

    }



    @Override
@@ -190,7 +198,11 @@ public class PlatformCompat extends IPlatformCompat.Stub { 
        if (appInfo != null) {

            boolean isTargetingLatestSdk =

                    mCompatConfig.isChangeTargetingLatestSdk(c, appInfo.targetSdkVersion);

            mChangeReporter.reportChange(appInfo.uid, changeId, state, isTargetingLatestSdk);

            mChangeReporter.reportChange(appInfo.uid,

                    changeId,

                    state,

                    appInfo.isSystemApp(),

                    isTargetingLatestSdk);

        }

        return enabled;

    }

services/tests/servicestests/src/com/android/server/compat/ApplicationInfoBuilder.java

+16 −0
Original line number Diff line number Diff line
@@ -21,8 +21,10 @@ import android.content.pm.ApplicationInfo; 
class ApplicationInfoBuilder {

    private boolean mIsDebuggable;

    private int mTargetSdk;

    private int mUid;

    private String mPackageName;

    private long mVersionCode;

    private boolean mIsSystemApp;



    private ApplicationInfoBuilder() {

        mTargetSdk = -1;
@@ -42,6 +44,16 @@ class ApplicationInfoBuilder { 
        return this;

    }



    ApplicationInfoBuilder systemApp() {

        mIsSystemApp = true;

        return this;

    }



    ApplicationInfoBuilder withUid(int uid) {

        mUid = uid;

        return this;

    }



    ApplicationInfoBuilder withPackageName(String packageName) {

        mPackageName = packageName;

        return this;
@@ -60,6 +72,10 @@ class ApplicationInfoBuilder { 
        applicationInfo.packageName = mPackageName;

        applicationInfo.targetSdkVersion = mTargetSdk;

        applicationInfo.longVersionCode = mVersionCode;

        applicationInfo.uid = mUid;

        if (mIsSystemApp) {

            applicationInfo.flags |= ApplicationInfo.FLAG_SYSTEM;

        }

        return applicationInfo;

    }

}