From 8f749a77c2a80c12fe5f853ff3f4bc35193be74b Mon Sep 17 00:00:00 2001 From: Anna Bauza Date: Wed, 12 Oct 2022 15:05:39 +0000 Subject: [PATCH] Fix wrong userId being reported to CUJ metrics CUJ does not need to know user id before it is created as long as begin and end session is the same. User Journey Create User Begin will report user id -1 as it was not yet created. On finish it will semd corrent user id or -1 if user creation faild. Bug: 251357485 Bug: 251357372 Test: atest CtsStatsdAtomHostTestCases:android.cts.statsdatom.users.UserLifecycleEventStatsTests#testCreateFullUser Test: atest CtsStatsdAtomHostTestCases:android.cts.statsdatom.users.UserLifecycleEventStatsTests#testCreateGuestUser Change-Id: I4a81ba5c1a78a9123fb19697b03e6b8cb8530068 --- .../android/server/pm/UserManagerService.java | 73 ++++++++++++------- 1 file changed, 47 insertions(+), 26 deletions(-) diff --git a/services/core/java/com/android/server/pm/UserManagerService.java b/services/core/java/com/android/server/pm/UserManagerService.java index 0a4669c0aace6..60f247843bb7d 100644 --- a/services/core/java/com/android/server/pm/UserManagerService.java +++ b/services/core/java/com/android/server/pm/UserManagerService.java @@ -4466,17 +4466,19 @@ public class UserManagerService extends IUserManager.Stub { boolean preCreate, @Nullable String[] disallowedPackages, @Nullable Object token) throws UserManager.CheckedUserOperationException { - final int nextProbableUserId = getNextAvailableId(); + final int noneUserId = -1; final TimingsTraceAndSlog t = new TimingsTraceAndSlog(); t.traceBegin("createUser-" + flags); - final long sessionId = logUserCreateJourneyBegin(nextProbableUserId, userType, flags); + final long sessionId = logUserCreateJourneyBegin(noneUserId); UserInfo newUser = null; try { newUser = createUserInternalUncheckedNoTracing(name, userType, flags, parentId, preCreate, disallowedPackages, t, token); return newUser; } finally { - logUserCreateJourneyFinish(sessionId, nextProbableUserId, newUser != null); + logUserCreateJourneyFinish(sessionId, + newUser != null ? newUser.id : noneUserId, userType, flags, + newUser != null); t.traceEnd(); } } @@ -4969,42 +4971,60 @@ public class UserManagerService extends IUserManager.Stub { && !userTypeDetails.getName().equals(UserManager.USER_TYPE_FULL_RESTRICTED); } - private long logUserCreateJourneyBegin(@UserIdInt int userId, String userType, - @UserInfoFlag int flags) { + private long logUserCreateJourneyBegin(@UserIdInt int userId) { return logUserJourneyBegin( FrameworkStatsLog.USER_LIFECYCLE_JOURNEY_REPORTED__JOURNEY__USER_CREATE, - userId, userType, flags); + userId); } - private void logUserCreateJourneyFinish(long sessionId, @UserIdInt int userId, boolean finish) { - FrameworkStatsLog.write(FrameworkStatsLog.USER_LIFECYCLE_EVENT_OCCURRED, sessionId, userId, - FrameworkStatsLog.USER_LIFECYCLE_EVENT_OCCURRED__EVENT__CREATE_USER, - finish ? FrameworkStatsLog.USER_LIFECYCLE_EVENT_OCCURRED__STATE__FINISH - : FrameworkStatsLog.USER_LIFECYCLE_EVENT_OCCURRED__STATE__NONE); + private void logUserCreateJourneyFinish(long sessionId, @UserIdInt int userId, String userType, + @UserInfoFlag int flags, boolean finish) { + logUserJourneyFinish(sessionId, + FrameworkStatsLog.USER_LIFECYCLE_JOURNEY_REPORTED__JOURNEY__USER_CREATE, + userId, userType, flags, finish); } - private long logUserRemoveJourneyBegin(@UserIdInt int userId, String userType, - @UserInfoFlag int flags) { + private long logUserRemoveJourneyBegin(@UserIdInt int userId) { return logUserJourneyBegin( FrameworkStatsLog.USER_LIFECYCLE_JOURNEY_REPORTED__JOURNEY__USER_REMOVE, - userId, userType, flags); + userId); } - private void logUserRemoveJourneyFinish(long sessionId, @UserIdInt int userId, boolean finish) { - FrameworkStatsLog.write(FrameworkStatsLog.USER_LIFECYCLE_EVENT_OCCURRED, sessionId, userId, - FrameworkStatsLog.USER_LIFECYCLE_EVENT_OCCURRED__EVENT__REMOVE_USER, - finish ? FrameworkStatsLog.USER_LIFECYCLE_EVENT_OCCURRED__STATE__FINISH - : FrameworkStatsLog.USER_LIFECYCLE_EVENT_OCCURRED__STATE__NONE); + private void logUserRemoveJourneyFinish(long sessionId, @UserIdInt int userId, String userType, + @UserInfoFlag int flags, boolean finish) { + logUserJourneyFinish(sessionId, + FrameworkStatsLog.USER_LIFECYCLE_JOURNEY_REPORTED__JOURNEY__USER_REMOVE, + userId, userType, flags, finish); } - private long logUserJourneyBegin(int journey, @UserIdInt int userId, String userType, - @UserInfoFlag int flags) { - final long sessionId = ThreadLocalRandom.current().nextLong(1, Long.MAX_VALUE); + private void logUserJourneyFinish(long sessionId, int journey, @UserIdInt int userId, + String userType, @UserInfoFlag int flags, boolean finish) { + // log the journey atom with the user metadata FrameworkStatsLog.write(FrameworkStatsLog.USER_LIFECYCLE_JOURNEY_REPORTED, sessionId, journey, /* origin_user= */ -1, userId, UserManager.getUserTypeForStatsd(userType), flags); + int event; + switch (journey) { + case FrameworkStatsLog.USER_LIFECYCLE_JOURNEY_REPORTED__JOURNEY__USER_CREATE: + event = FrameworkStatsLog.USER_LIFECYCLE_EVENT_OCCURRED__EVENT__CREATE_USER; + break; + case FrameworkStatsLog.USER_LIFECYCLE_JOURNEY_REPORTED__JOURNEY__USER_REMOVE: + event = FrameworkStatsLog.USER_LIFECYCLE_EVENT_OCCURRED__EVENT__REMOVE_USER; + break; + default: + throw new IllegalArgumentException("Journey " + journey + " not expected."); + } + FrameworkStatsLog.write(FrameworkStatsLog.USER_LIFECYCLE_EVENT_OCCURRED, sessionId, userId, + event, + finish ? FrameworkStatsLog.USER_LIFECYCLE_EVENT_OCCURRED__STATE__FINISH + : FrameworkStatsLog.USER_LIFECYCLE_EVENT_OCCURRED__STATE__NONE); + } + + private long logUserJourneyBegin(int journey, @UserIdInt int userId) { + final long sessionId = ThreadLocalRandom.current().nextLong(1, Long.MAX_VALUE); + // log the event atom to indicate the event start int event; switch (journey) { @@ -5322,8 +5342,7 @@ public class UserManagerService extends IUserManager.Stub { writeUserLP(userData); } - final long sessionId = logUserRemoveJourneyBegin( - userId, userData.info.userType, userData.info.flags); + final long sessionId = logUserRemoveJourneyBegin(userId); try { mAppOpsService.removeUser(userId); @@ -5344,11 +5363,13 @@ public class UserManagerService extends IUserManager.Stub { @Override public void userStopped(int userIdParam) { finishRemoveUser(userIdParam); - logUserRemoveJourneyFinish(sessionId, userIdParam, true); + logUserRemoveJourneyFinish(sessionId, userIdParam, + userData.info.userType, userData.info.flags, true); } @Override public void userStopAborted(int userIdParam) { - logUserRemoveJourneyFinish(sessionId, userIdParam, false); + logUserRemoveJourneyFinish(sessionId, userIdParam, + userData.info.userType, userData.info.flags, false); } }); } catch (RemoteException e) {