Merge "Don't log change reports to statsd from system server" into main (4dde0d73) · Commits · e / os / android_frameworks_base

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

+21 −8

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

			// Don't log if already reported
			return !isAlreadyReported;
	}		}

	/**		/**
	@@ -160,8 +162,6 @@ public final class ChangeReporter {
	boolean isAlreadyReported, int state, boolean isLoggableBySdk) {		boolean isAlreadyReported, int state, boolean isLoggableBySdk) {
	// If log all bit is on, always return true.		// If log all bit is on, always return true.
	if (mDebugLogAll) 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		// 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		// `adb setprop log.tag.CompatChangeReporter=DEBUG`, write to debug since the above checks
	@@ -224,6 +224,19 @@ public final class ChangeReporter {
	return mReportedChanges.getOrDefault(uid, EMPTY_SET).contains(report);		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) {		private void markAsReported(int uid, ChangeReport report) {
	mReportedChanges.computeIfAbsent(uid, NEW_CHANGE_REPORT_SET).add(report);		mReportedChanges.computeIfAbsent(uid, NEW_CHANGE_REPORT_SET).add(report);
	}		}

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

+26 −9

Original line number	Original line	Diff line number	Diff line
	@@ -31,21 +31,21 @@ public class ChangeReporterTest {
	@Rule public final SetFlagsRule mSetFlagsRule = new SetFlagsRule();		@Rule public final SetFlagsRule mSetFlagsRule = new SetFlagsRule();

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

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

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

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

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

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

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

	@Test		@Test
	@@ -196,4 +196,21 @@ public class ChangeReporterTest {
	// off.		// off.
	assertTrue(reporter.shouldWriteToDebug(myUid, myChangeId, myDisabledState, false));		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));
			}
	}		}