From b19fc200dcf3dbe7745f1f8dd6204da203b3b488 Mon Sep 17 00:00:00 2001 From: Yandry Perez Clemente Date: Thu, 13 May 2021 13:38:45 +0100 Subject: [PATCH] Add a unique id to Perfetto trace and ANR dropbox header. The unique id is added so that the trace and ANR can be easily linked server side. Also moves the logging of the ANR atom before the ANR dump takes place so that the Perfetto trace captures data closer to the point in time when the ANR happens. Bug: b/188122403 Test: Manual. 1. Enabled the flag: adb shell device_config put perfetto_error_logger append_perfetto_uuid true 2. Triggered an ANR using a test app. Logs from PerfettoTraceErrorLogger: adb logcat -s PerfettoTraceErrorLogger --------- beginning of main 04-30 14:07:02.527 1782 12583 I PerfettoTraceErrorLogger: Pushing trace counter: 6c555dc6-3f46-4997-8844-706d91156600#PerfettoMonitoredCrash 3. The UUID is present in the ANR dropbox headers: adb shell dumpsys dropbox data_app_anr --print | grep 6c555dc6-3f46-4997-8844-706d91156600 PerfettoUUID: 6c555dc6-3f46-4997-8844-706d91156600 Change-Id: If7110692cb0856e693f595e24b067ce1bc7d0a38 --- core/java/android/provider/DeviceConfig.java | 7 +++ .../java/com/android/server/Watchdog.java | 2 +- .../server/am/ActivityManagerService.java | 17 ++++-- .../com/android/server/am/AppProfiler.java | 2 +- .../server/am/ProcessErrorStateRecord.java | 15 ++++- .../android/server/am/TraceErrorLogger.java | 58 +++++++++++++++++++ 6 files changed, 93 insertions(+), 8 deletions(-) create mode 100644 services/core/java/com/android/server/am/TraceErrorLogger.java diff --git a/core/java/android/provider/DeviceConfig.java b/core/java/android/provider/DeviceConfig.java index 1092adf65f1fb..1259b843d7bf0 100644 --- a/core/java/android/provider/DeviceConfig.java +++ b/core/java/android/provider/DeviceConfig.java @@ -587,6 +587,13 @@ public final class DeviceConfig { */ public static final String NAMESPACE_CONSTRAIN_DISPLAY_APIS = "constrain_display_apis"; + /** + * Trace error logger properties definitions. + * + * @hide + */ + public static final String NAMESPACE_TRACE_ERROR_LOGGER = "trace_error_logger"; + private static final Object sLock = new Object(); @GuardedBy("sLock") private static ArrayMap> sListeners = diff --git a/services/core/java/com/android/server/Watchdog.java b/services/core/java/com/android/server/Watchdog.java index 7a0a3a7640d46..670f557b81e97 100644 --- a/services/core/java/com/android/server/Watchdog.java +++ b/services/core/java/com/android/server/Watchdog.java @@ -699,7 +699,7 @@ public class Watchdog { if (mActivity != null) { mActivity.addErrorToDropBox( "watchdog", null, "system_server", null, null, null, - localSubject, report.toString(), stack, null, null, null); + localSubject, report.toString(), stack, null, null, null, null); } FrameworkStatsLog.write(FrameworkStatsLog.SYSTEM_SERVER_WATCHDOG_OCCURRED, localSubject); diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 6661f88f3fdfe..07ded2689dfee 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -420,6 +420,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Set; +import java.util.UUID; import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicInteger; @@ -621,6 +622,8 @@ public class ActivityManagerService extends IActivityManager.Stub @GuardedBy("this") BroadcastStats mCurBroadcastStats; + TraceErrorLogger mTraceErrorLogger; + BroadcastQueue broadcastQueueForIntent(Intent intent) { if (isOnOffloadQueue(intent.getFlags())) { if (DEBUG_BROADCAST_BACKGROUND) { @@ -2336,6 +2339,7 @@ public class ActivityManagerService extends IActivityManager.Stub mInternal = new LocalService(); mPendingStartActivityUids = new PendingStartActivityUids(mContext); + mTraceErrorLogger = new TraceErrorLogger(); } public void setSystemServiceManager(SystemServiceManager mgr) { @@ -7810,7 +7814,7 @@ public class ActivityManagerService extends IActivityManager.Stub addErrorToDropBox( eventType, r, processName, null, null, null, null, null, null, crashInfo, - new Float(loadingProgress), incrementalMetrics); + new Float(loadingProgress), incrementalMetrics, null); mAppErrors.crashApplication(r, crashInfo); } @@ -7993,7 +7997,7 @@ public class ActivityManagerService extends IActivityManager.Stub callingPid, (r != null) ? r.getProcessClassEnum() : 0); addErrorToDropBox("wtf", r, processName, null, null, null, tag, null, null, crashInfo, - null, null); + null, null, null); return r; } @@ -8018,7 +8022,7 @@ public class ActivityManagerService extends IActivityManager.Stub for (Pair p = list.poll(); p != null; p = list.poll()) { addErrorToDropBox("wtf", proc, "system_server", null, null, null, p.first, null, null, - p.second, null, null); + p.second, null, null, null); } } @@ -8109,13 +8113,15 @@ public class ActivityManagerService extends IActivityManager.Stub * @param crashInfo giving an application stack trace, null if absent * @param loadingProgress the loading progress of an installed package, range in [0, 1]. * @param incrementalMetrics metrics for apps installed on Incremental. + * @param errorId a unique id to append to the dropbox headers. */ public void addErrorToDropBox(String eventType, ProcessRecord process, String processName, String activityShortComponentName, String parentShortComponentName, ProcessRecord parentProcess, String subject, final String report, final File dataFile, final ApplicationErrorReport.CrashInfo crashInfo, - @Nullable Float loadingProgress, @Nullable IncrementalMetrics incrementalMetrics) { + @Nullable Float loadingProgress, @Nullable IncrementalMetrics incrementalMetrics, + @Nullable UUID errorId) { // NOTE -- this must never acquire the ActivityManagerService lock, // otherwise the watchdog may be prevented from resetting the system. @@ -8169,6 +8175,9 @@ public class ActivityManagerService extends IActivityManager.Stub if (subject != null) { sb.append("Subject: ").append(subject).append("\n"); } + if (errorId != null) { + sb.append("ErrorId: ").append(errorId.toString()).append("\n"); + } sb.append("Build: ").append(Build.FINGERPRINT).append("\n"); if (Debug.isDebuggerConnected()) { sb.append("Debugger: Connected\n"); diff --git a/services/core/java/com/android/server/am/AppProfiler.java b/services/core/java/com/android/server/am/AppProfiler.java index 074e8fe42016f..74094e500de7a 100644 --- a/services/core/java/com/android/server/am/AppProfiler.java +++ b/services/core/java/com/android/server/am/AppProfiler.java @@ -1627,7 +1627,7 @@ public class AppProfiler { dropBuilder.append(catSw.toString()); FrameworkStatsLog.write(FrameworkStatsLog.LOW_MEM_REPORTED); mService.addErrorToDropBox("lowmem", null, "system_server", null, - null, null, tag.toString(), dropBuilder.toString(), null, null, null, null); + null, null, tag.toString(), dropBuilder.toString(), null, null, null, null, null); synchronized (mService) { long now = SystemClock.uptimeMillis(); if (mLastMemUsageReportTime < now) { diff --git a/services/core/java/com/android/server/am/ProcessErrorStateRecord.java b/services/core/java/com/android/server/am/ProcessErrorStateRecord.java index 88bd010806b68..66d4779698bdc 100644 --- a/services/core/java/com/android/server/am/ProcessErrorStateRecord.java +++ b/services/core/java/com/android/server/am/ProcessErrorStateRecord.java @@ -55,7 +55,7 @@ import java.io.File; import java.io.PrintWriter; import java.io.StringWriter; import java.util.ArrayList; - +import java.util.UUID; /** * The error state of the process, such as if it's crashing/ANR etc. */ @@ -235,6 +235,7 @@ class ProcessErrorStateRecord { final boolean isSilentAnr; final int pid = mApp.getPid(); + final UUID errorId; synchronized (mService) { // PowerManager.reboot() can block for a long time, so ignore ANRs while shutting down. if (mService.mAtmInternal.isShuttingDown()) { @@ -264,6 +265,13 @@ class ProcessErrorStateRecord { EventLog.writeEvent(EventLogTags.AM_ANR, mApp.userId, pid, mApp.processName, mApp.info.flags, annotation); + if (mService.mTraceErrorLogger.isAddErrorIdEnabled()) { + errorId = mService.mTraceErrorLogger.generateErrorId(); + mService.mTraceErrorLogger.addErrorIdToTrace(errorId); + } else { + errorId = null; + } + // Dump thread traces as quickly as we can, starting with "interesting" processes. firstPids.add(pid); @@ -315,6 +323,9 @@ class ProcessErrorStateRecord { && parentShortComponentName.equals(activityShortComponentName)) { info.append("Parent: ").append(parentShortComponentName).append("\n"); } + if (errorId != null) { + info.append("ErrorId: ").append(errorId.toString()).append("\n"); + } // Retrieve controller with max ANR delay from AnrControllers // Note that we retrieve the controller before dumping stacks because dumping stacks can @@ -457,7 +468,7 @@ class ProcessErrorStateRecord { ? (ProcessRecord) parentProcess.mOwner : null; mService.addErrorToDropBox("anr", mApp, mApp.processName, activityShortComponentName, parentShortComponentName, parentPr, annotation, report.toString(), tracesFile, - null, new Float(loadingProgress), incrementalMetrics); + null, new Float(loadingProgress), incrementalMetrics, errorId); if (mApp.getWindowProcessController().appNotResponding(info.toString(), () -> { diff --git a/services/core/java/com/android/server/am/TraceErrorLogger.java b/services/core/java/com/android/server/am/TraceErrorLogger.java new file mode 100644 index 0000000000000..f055be2357169 --- /dev/null +++ b/services/core/java/com/android/server/am/TraceErrorLogger.java @@ -0,0 +1,58 @@ +/* + * Copyright 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.am; + +import android.os.Trace; +import android.provider.DeviceConfig; + +import java.util.UUID; + +/** + * Adds a unique id to a trace. + * + * @hide + */ +class TraceErrorLogger { + private static final String COUNTER_PREFIX = "ErrorId:"; + private static final String ADD_ERROR_ID = "add_error_id"; + private static final int PLACEHOLDER_VALUE = 1; + + public boolean isAddErrorIdEnabled() { + return DeviceConfig + .getBoolean(DeviceConfig.NAMESPACE_TRACE_ERROR_LOGGER, ADD_ERROR_ID, + false); + } + + /** + * Generates a unique id with which to tag a trace. + */ + public UUID generateErrorId() { + return UUID.randomUUID(); + } + + /** + * Pushes a counter containing a unique id and a label {@link #COUNTER_PREFIX} so that traces + * can be uniquely identified. We also add the same id to the dropbox entry of the error, so + * that we can join the trace and the error server-side. + * + * @param errorId The unique id with which to tag the trace. + */ + public void addErrorIdToTrace(UUID errorId) { + Trace.traceCounter(Trace.TRACE_TAG_ACTIVITY_MANAGER, COUNTER_PREFIX + errorId.toString(), + PLACEHOLDER_VALUE); + } +}