Merge "Add rate limiting to addTombstoneToDropbox." into tm-dev

This commit is contained in:
Siim Sammul
2022-05-24 09:47:53 +00:00
committed by Android (Google) Code Review
4 changed files with 53 additions and 23 deletions

View File

@@ -46,6 +46,7 @@ import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.logging.MetricsLogger; import com.android.internal.logging.MetricsLogger;
import com.android.internal.util.FrameworkStatsLog; import com.android.internal.util.FrameworkStatsLog;
import com.android.internal.util.XmlUtils; import com.android.internal.util.XmlUtils;
import com.android.server.am.DropboxRateLimiter;
import org.xmlpull.v1.XmlPullParser; import org.xmlpull.v1.XmlPullParser;
import org.xmlpull.v1.XmlPullParserException; import org.xmlpull.v1.XmlPullParserException;
@@ -308,19 +309,31 @@ public class BootReceiver extends BroadcastReceiver {
writeTimestamps(timestamps); writeTimestamps(timestamps);
} }
private static final DropboxRateLimiter sDropboxRateLimiter = new DropboxRateLimiter();
/** /**
* Add a tombstone to the DropBox. * Add a tombstone to the DropBox.
* *
* @param ctx Context * @param ctx Context
* @param tombstone path to the tombstone * @param tombstone path to the tombstone
* @param proto whether the tombstone is stored as proto
* @param processName the name of the process corresponding to the tombstone
*/ */
public static void addTombstoneToDropBox(Context ctx, File tombstone, boolean proto) { public static void addTombstoneToDropBox(
Context ctx, File tombstone, boolean proto, String processName) {
final DropBoxManager db = ctx.getSystemService(DropBoxManager.class); final DropBoxManager db = ctx.getSystemService(DropBoxManager.class);
if (db == null) { if (db == null) {
Slog.e(TAG, "Can't log tombstone: DropBoxManager not available"); Slog.e(TAG, "Can't log tombstone: DropBoxManager not available");
return; return;
} }
// Check if we should rate limit and abort early if needed. Do this for both proto and
// non-proto tombstones, even though proto tombstones do not support including the counter
// of events dropped since rate limiting activated yet.
DropboxRateLimiter.RateLimitResult rateLimitResult =
sDropboxRateLimiter.shouldRateLimit(TAG_TOMBSTONE, processName);
if (rateLimitResult.shouldRateLimit()) return;
HashMap<String, Long> timestamps = readTimestamps(); HashMap<String, Long> timestamps = readTimestamps();
try { try {
if (proto) { if (proto) {
@@ -328,7 +341,9 @@ public class BootReceiver extends BroadcastReceiver {
db.addFile(TAG_TOMBSTONE_PROTO, tombstone, 0); db.addFile(TAG_TOMBSTONE_PROTO, tombstone, 0);
} }
} else { } else {
final String headers = getBootHeadersToLogAndUpdate(); // Add the header indicating how many events have been dropped due to rate limiting.
final String headers = getBootHeadersToLogAndUpdate()
+ rateLimitResult.createHeader();
addFileToDropBox(db, timestamps, headers, tombstone.getPath(), LOG_SIZE, addFileToDropBox(db, timestamps, headers, tombstone.getPath(), LOG_SIZE,
TAG_TOMBSTONE); TAG_TOMBSTONE);
} }

View File

@@ -8819,8 +8819,7 @@ public class ActivityManagerService extends IActivityManager.Stub
millisSinceOldestPendingRead).append("\n"); millisSinceOldestPendingRead).append("\n");
} }
} }
sb.append("Dropped-Count: ").append( sb.append(rateLimitResult.createHeader());
rateLimitResult.droppedCountSinceRateLimitActivated()).append("\n");
sb.append("\n"); sb.append("\n");
// Do the rest in a worker thread to avoid blocking the caller on I/O // Do the rest in a worker thread to avoid blocking the caller on I/O

View File

@@ -111,8 +111,8 @@ public class DropboxRateLimiter {
/** Holds information on whether we should rate limit and how many events have been dropped. */ /** Holds information on whether we should rate limit and how many events have been dropped. */
public class RateLimitResult { public class RateLimitResult {
boolean mShouldRateLimit; final boolean mShouldRateLimit;
int mDroppedCountSinceRateLimitActivated; final int mDroppedCountSinceRateLimitActivated;
public RateLimitResult(boolean shouldRateLimit, int droppedCountSinceRateLimitActivated) { public RateLimitResult(boolean shouldRateLimit, int droppedCountSinceRateLimitActivated) {
mShouldRateLimit = shouldRateLimit; mShouldRateLimit = shouldRateLimit;
@@ -128,6 +128,11 @@ public class DropboxRateLimiter {
public int droppedCountSinceRateLimitActivated() { public int droppedCountSinceRateLimitActivated() {
return mDroppedCountSinceRateLimitActivated; return mDroppedCountSinceRateLimitActivated;
} }
/** Returns a header indicating the number of dropped events. */
public String createHeader() {
return "Dropped-Count: " + mDroppedCountSinceRateLimitActivated + "\n";
}
} }
private class ErrorRecord { private class ErrorRecord {

View File

@@ -113,19 +113,22 @@ public final class NativeTombstoneManager {
return; return;
} }
if (filename.endsWith(".pb")) { String processName = "UNKNOWN";
handleProtoTombstone(path); final boolean isProtoFile = filename.endsWith(".pb");
BootReceiver.addTombstoneToDropBox(mContext, path, true); File protoPath = isProtoFile ? path : new File(path.getAbsolutePath() + ".pb");
} else {
BootReceiver.addTombstoneToDropBox(mContext, path, false); Optional<TombstoneFile> parsedTombstone = handleProtoTombstone(protoPath, isProtoFile);
if (parsedTombstone.isPresent()) {
processName = parsedTombstone.get().getProcessName();
} }
BootReceiver.addTombstoneToDropBox(mContext, path, isProtoFile, processName);
} }
private void handleProtoTombstone(File path) { private Optional<TombstoneFile> handleProtoTombstone(File path, boolean addToList) {
final String filename = path.getName(); final String filename = path.getName();
if (!filename.endsWith(".pb")) { if (!filename.endsWith(".pb")) {
Slog.w(TAG, "unexpected tombstone name: " + path); Slog.w(TAG, "unexpected tombstone name: " + path);
return; return Optional.empty();
} }
final String suffix = filename.substring("tombstone_".length()); final String suffix = filename.substring("tombstone_".length());
@@ -136,11 +139,11 @@ public final class NativeTombstoneManager {
number = Integer.parseInt(numberStr); number = Integer.parseInt(numberStr);
if (number < 0 || number > 99) { if (number < 0 || number > 99) {
Slog.w(TAG, "unexpected tombstone name: " + path); Slog.w(TAG, "unexpected tombstone name: " + path);
return; return Optional.empty();
} }
} catch (NumberFormatException ex) { } catch (NumberFormatException ex) {
Slog.w(TAG, "unexpected tombstone name: " + path); Slog.w(TAG, "unexpected tombstone name: " + path);
return; return Optional.empty();
} }
ParcelFileDescriptor pfd; ParcelFileDescriptor pfd;
@@ -148,15 +151,16 @@ public final class NativeTombstoneManager {
pfd = ParcelFileDescriptor.open(path, MODE_READ_WRITE); pfd = ParcelFileDescriptor.open(path, MODE_READ_WRITE);
} catch (FileNotFoundException ex) { } catch (FileNotFoundException ex) {
Slog.w(TAG, "failed to open " + path, ex); Slog.w(TAG, "failed to open " + path, ex);
return; return Optional.empty();
} }
final Optional<TombstoneFile> parsedTombstone = TombstoneFile.parse(pfd); final Optional<TombstoneFile> parsedTombstone = TombstoneFile.parse(pfd);
if (!parsedTombstone.isPresent()) { if (!parsedTombstone.isPresent()) {
IoUtils.closeQuietly(pfd); IoUtils.closeQuietly(pfd);
return; return Optional.empty();
} }
if (addToList) {
synchronized (mLock) { synchronized (mLock) {
TombstoneFile previous = mTombstones.get(number); TombstoneFile previous = mTombstones.get(number);
if (previous != null) { if (previous != null) {
@@ -167,6 +171,9 @@ public final class NativeTombstoneManager {
} }
} }
return parsedTombstone;
}
/** /**
* Remove native tombstones matching a user and/or app. * Remove native tombstones matching a user and/or app.
* *
@@ -363,6 +370,10 @@ public final class NativeTombstoneManager {
return true; return true;
} }
public String getProcessName() {
return mProcessName;
}
public void dispose() { public void dispose() {
IoUtils.closeQuietly(mPfd); IoUtils.closeQuietly(mPfd);
} }