Skip to content

Commit

Permalink
Merge pull request #1633 from bugsnag/PLAT-8185-delete-old-big-events
Browse files Browse the repository at this point in the history
[PLAT-8185] Limit retries of event payloads based on size and age, and session payloads based on age
  • Loading branch information
kstenerud authored Mar 30, 2022
2 parents f74d35a + 39cd765 commit 51b7eae
Show file tree
Hide file tree
Showing 15 changed files with 449 additions and 23 deletions.
3 changes: 3 additions & 0 deletions bugsnag-android-core/detekt-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
<ID>MagicNumber:DefaultDelivery.kt$DefaultDelivery$429</ID>
<ID>MagicNumber:DefaultDelivery.kt$DefaultDelivery$499</ID>
<ID>MagicNumber:LastRunInfoStore.kt$LastRunInfoStore$3</ID>
<ID>MagicNumber:SessionFilenameInfo.kt$SessionFilenameInfo.Companion$35</ID>
<ID>MaxLineLength:LastRunInfo.kt$LastRunInfo$return "LastRunInfo(consecutiveLaunchCrashes=$consecutiveLaunchCrashes, crashed=$crashed, crashedDuringLaunch=$crashedDuringLaunch)"</ID>
<ID>MaxLineLength:ThreadState.kt$ThreadState$"[${allThreads.size - maxThreadCount} threads omitted as the maxReportedThreads limit ($maxThreadCount) was exceeded]"</ID>
<ID>ProtectedMemberInFinalClass:ConfigInternal.kt$ConfigInternal$protected val plugins = HashSet&lt;Plugin>()</ID>
Expand All @@ -38,7 +39,9 @@
<ID>SwallowedException:DeviceDataCollector.kt$DeviceDataCollector$catch (exception: Exception) { logger.w("Could not get battery status") }</ID>
<ID>SwallowedException:DeviceDataCollector.kt$DeviceDataCollector$catch (exception: Exception) { logger.w("Could not get locationStatus") }</ID>
<ID>SwallowedException:DeviceIdStore.kt$DeviceIdStore$catch (exc: OverlappingFileLockException) { Thread.sleep(FILE_LOCK_WAIT_MS) }</ID>
<ID>SwallowedException:EventFilenameInfo.kt$EventFilenameInfo.Companion$catch (e: Exception) { return -1 }</ID>
<ID>SwallowedException:PluginClient.kt$PluginClient$catch (exc: ClassNotFoundException) { logger.d("Plugin '$clz' is not on the classpath - functionality will not be enabled.") null }</ID>
<ID>SwallowedException:SessionFilenameInfo.kt$SessionFilenameInfo.Companion$catch (e: Exception) { return 0 }</ID>
<ID>TooManyFunctions:ConfigInternal.kt$ConfigInternal : CallbackAwareMetadataAwareUserAwareFeatureFlagAware</ID>
<ID>TooManyFunctions:EventInternal.kt$EventInternal : FeatureFlagAwareStreamableMetadataAwareUserAware</ID>
<ID>UnnecessaryAbstractClass:DependencyModule.kt$DependencyModule</ID>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,8 @@ internal data class EventFilenameInfo(
val errorTypes: Set<ErrorType>
) {

/**
* Generates a filename for the Event in the format
* "[timestamp]_[apiKey]_[errorTypes]_[UUID]_[startupcrash|not-jvm].json"
*/
fun encode(): String {
return "${timestamp}_${apiKey}_${serializeErrorTypeHeader(errorTypes)}_${uuid}_$suffix.json"
return toFilename(apiKey, uuid, timestamp, suffix, errorTypes)
}

fun isLaunchCrashReport(): Boolean = suffix == STARTUP_CRASH
Expand All @@ -36,7 +32,21 @@ internal data class EventFilenameInfo(
private const val STARTUP_CRASH = "startupcrash"
private const val NON_JVM_CRASH = "not-jvm"

@JvmOverloads
/**
* Generates a filename for the Event in the format
* "[timestamp]_[apiKey]_[errorTypes]_[UUID]_[startupcrash|not-jvm].json"
*/
fun toFilename(
apiKey: String,
uuid: String,
timestamp: Long,
suffix: String,
errorTypes: Set<ErrorType>
): String {
return "${timestamp}_${apiKey}_${serializeErrorTypeHeader(errorTypes)}_${uuid}_$suffix.json"
}

@JvmOverloads @JvmStatic
fun fromEvent(
obj: Any,
uuid: String = UUID.randomUUID().toString(),
Expand All @@ -63,11 +73,12 @@ internal data class EventFilenameInfo(
/**
* Reads event information from a filename.
*/
@JvmStatic
fun fromFile(file: File, config: ImmutableConfig): EventFilenameInfo {
return EventFilenameInfo(
findApiKeyInFilename(file, config),
"", // ignore UUID field when reading from file as unused
-1, // ignore timestamp when reading from file as unused
findTimestampInFilename(file),
findSuffixInFilename(file),
findErrorTypesInFilename(file)
)
Expand All @@ -77,7 +88,7 @@ internal data class EventFilenameInfo(
* Retrieves the api key encoded in the filename, or an empty string if this information
* is not encoded for the given event
*/
private fun findApiKeyInFilename(file: File, config: ImmutableConfig): String {
internal fun findApiKeyInFilename(file: File, config: ImmutableConfig): String {
val name = file.name.removeSuffix("_$STARTUP_CRASH.json")
val start = name.indexOf("_") + 1
val end = name.indexOf("_", start)
Expand All @@ -93,7 +104,7 @@ internal data class EventFilenameInfo(
* Retrieves the error types encoded in the filename, or an empty string if this
* information is not encoded for the given event
*/
private fun findErrorTypesInFilename(eventFile: File): Set<ErrorType> {
internal fun findErrorTypesInFilename(eventFile: File): Set<ErrorType> {
val name = eventFile.name
val end = name.lastIndexOf("_", name.lastIndexOf("_") - 1)
val start = name.lastIndexOf("_", end - 1) + 1
Expand All @@ -111,7 +122,7 @@ internal data class EventFilenameInfo(
* Retrieves the error types encoded in the filename, or an empty string if this
* information is not encoded for the given event
*/
private fun findSuffixInFilename(eventFile: File): String {
internal fun findSuffixInFilename(eventFile: File): String {
val name = eventFile.nameWithoutExtension
val suffix = name.substring(name.lastIndexOf("_") + 1)
return when (suffix) {
Expand All @@ -120,10 +131,20 @@ internal data class EventFilenameInfo(
}
}

/**
* Retrieves the error types encoded in the filename, or an empty string if this
* information is not encoded for the given event
*/
@JvmStatic
fun findTimestampInFilename(eventFile: File): Long {
val name = eventFile.nameWithoutExtension
return name.substringBefore("_", missingDelimiterValue = "-1").toLongOrNull() ?: -1
}

/**
* Retrieves the error types for the given event
*/
private fun findErrorTypesForEvent(obj: Any): Set<ErrorType> {
internal fun findErrorTypesForEvent(obj: Any): Set<ErrorType> {
return when (obj) {
is Event -> obj.impl.getErrorTypesFromStackframes()
else -> setOf(ErrorType.C)
Expand All @@ -133,7 +154,7 @@ internal data class EventFilenameInfo(
/**
* Calculates the suffix for the given event
*/
private fun findSuffixForEvent(obj: Any, launching: Boolean?): String {
internal fun findSuffixForEvent(obj: Any, launching: Boolean?): String {
return when {
obj is Event && obj.app.isLaunching == true -> STARTUP_CRASH
launching == true -> STARTUP_CRASH
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@

import java.io.File;
import java.util.ArrayList;
import java.util.Calendar;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.Date;
import java.util.List;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;
Expand Down Expand Up @@ -119,7 +121,7 @@ File findLaunchCrashReport(Collection<File> storedFiles) {
List<File> launchCrashes = new ArrayList<>();

for (File file : storedFiles) {
EventFilenameInfo filenameInfo = EventFilenameInfo.Companion.fromFile(file, config);
EventFilenameInfo filenameInfo = EventFilenameInfo.fromFile(file, config);
if (filenameInfo.isLaunchCrashReport()) {
launchCrashes.add(file);
}
Expand Down Expand Up @@ -163,7 +165,7 @@ void flushReports(Collection<File> storedReports) {

private void flushEventFile(File eventFile) {
try {
EventFilenameInfo eventInfo = EventFilenameInfo.Companion.fromFile(eventFile, config);
EventFilenameInfo eventInfo = EventFilenameInfo.fromFile(eventFile, config);
String apiKey = eventInfo.getApiKey();
EventPayload payload = createEventPayload(eventFile, apiKey);

Expand All @@ -188,9 +190,21 @@ private void deliverEventPayload(File eventFile, EventPayload payload) {
logger.i("Deleting sent error file " + eventFile.getName());
break;
case UNDELIVERED:
cancelQueuedFiles(Collections.singleton(eventFile));
logger.w("Could not send previously saved error(s)"
+ " to Bugsnag, will try again later");
if (isTooBig(eventFile)) {
logger.w("Discarding over-sized event ("
+ eventFile.length()
+ ") after failed delivery");
deleteStoredFiles(Collections.singleton(eventFile));
} else if (isTooOld(eventFile)) {
logger.w("Discarding historical event (from "
+ getCreationDate(eventFile)
+ ") after failed delivery");
deleteStoredFiles(Collections.singleton(eventFile));
} else {
cancelQueuedFiles(Collections.singleton(eventFile));
logger.w("Could not send previously saved error(s)"
+ " to Bugsnag, will try again later");
}
break;
case FAILURE:
Exception exc = new RuntimeException("Failed to deliver event payload");
Expand Down Expand Up @@ -234,13 +248,29 @@ private void handleEventFlushFailure(Exception exc, File eventFile) {
@Override
String getFilename(Object object) {
EventFilenameInfo eventInfo
= EventFilenameInfo.Companion.fromEvent(object, null, config);
= EventFilenameInfo.fromEvent(object, null, config);
return eventInfo.encode();
}

String getNdkFilename(Object object, String apiKey) {
EventFilenameInfo eventInfo
= EventFilenameInfo.Companion.fromEvent(object, apiKey, config);
= EventFilenameInfo.fromEvent(object, apiKey, config);
return eventInfo.encode();
}

private static long oneMegabyte = 1024 * 1024;

public boolean isTooBig(File file) {
return file.length() > oneMegabyte;
}

public boolean isTooOld(File file) {
Calendar cal = Calendar.getInstance();
cal.add(Calendar.DATE, -60);
return EventFilenameInfo.findTimestampInFilename(file) < cal.getTimeInMillis();
}

public Date getCreationDate(File file) {
return new Date(EventFilenameInfo.findTimestampInFilename(file));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ interface Delegate {

private final Lock lock = new ReentrantLock();
private final Collection<File> queuedFiles = new ConcurrentSkipListSet<>();
private final Logger logger;
protected final Logger logger;
private final EventStore.Delegate delegate;

FileStore(@NonNull File storageDir,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package com.bugsnag.android

import java.io.File
import java.util.UUID

/**
* Represents important information about a session filename.
* Currently the following information is encoded:
*
* uuid - to disambiguate stored error reports
* timestamp - to sort error reports by time of capture
*/
internal data class SessionFilenameInfo(
val timestamp: Long,
val uuid: String,
) {

fun encode(): String {
return toFilename(timestamp, uuid)
}

internal companion object {

const val uuidLength = 36

/**
* Generates a filename for the session in the format
* "[UUID][timestamp]_v2.json"
*/
fun toFilename(timestamp: Long, uuid: String): String {
return "${uuid}${timestamp}_v2.json"
}

@JvmStatic
fun defaultFilename(): String {
return toFilename(System.currentTimeMillis(), UUID.randomUUID().toString())
}

fun fromFile(file: File): SessionFilenameInfo {
return SessionFilenameInfo(
findTimestampInFilename(file),
findUuidInFilename(file)
)
}

private fun findUuidInFilename(file: File): String {
return file.name.substring(0, uuidLength - 1)
}

@JvmStatic
fun findTimestampInFilename(file: File): Long {
return file.name.substring(uuidLength, file.name.indexOf("_")).toLongOrNull() ?: -1
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
import androidx.annotation.Nullable;

import java.io.File;
import java.util.Calendar;
import java.util.Comparator;
import java.util.Date;
import java.util.UUID;

/**
Expand Down Expand Up @@ -46,7 +48,16 @@ public int compare(File lhs, File rhs) {
@NonNull
@Override
String getFilename(Object object) {
return UUID.randomUUID().toString() + System.currentTimeMillis() + "_v2.json";
return SessionFilenameInfo.defaultFilename();
}

public boolean isTooOld(File file) {
Calendar cal = Calendar.getInstance();
cal.add(Calendar.DATE, -60);
return SessionFilenameInfo.findTimestampInFilename(file) < cal.getTimeInMillis();
}

public Date getCreationDate(File file) {
return new Date(SessionFilenameInfo.findTimestampInFilename(file));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -270,8 +270,15 @@ void flushStoredSession(File storedFile) {
logger.d("Sent 1 new session to Bugsnag");
break;
case UNDELIVERED:
sessionStore.cancelQueuedFiles(Collections.singletonList(storedFile));
logger.w("Leaving session payload for future delivery");
if (sessionStore.isTooOld(storedFile)) {
logger.w("Discarding historical session (from {"
+ sessionStore.getCreationDate(storedFile)
+ "}) after failed delivery");
sessionStore.deleteStoredFiles(Collections.singletonList(storedFile));
} else {
sessionStore.cancelQueuedFiles(Collections.singletonList(storedFile));
logger.w("Leaving session payload for future delivery");
}
break;
case FAILURE:
// drop bad data
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,14 @@
<ID>MagicNumber:AutoDetectAnrsFalseScenario.kt$AutoDetectAnrsFalseScenario$100000</ID>
<ID>MagicNumber:AutoDetectAnrsTrueScenario.kt$AutoDetectAnrsTrueScenario$100000</ID>
<ID>MagicNumber:BugsnagInitScenario.kt$BugsnagInitScenario$25</ID>
<ID>MagicNumber:DiscardBigEventsScenario.kt$DiscardBigEventsScenario$100</ID>
<ID>MagicNumber:DiscardBigEventsScenario.kt$DiscardBigEventsScenario$1024</ID>
<ID>MagicNumber:DiscardOldEventsScenario.kt$DiscardOldEventsScenario$100</ID>
<ID>MagicNumber:DiscardOldEventsScenario.kt$DiscardOldEventsScenario$60</ID>
<ID>MagicNumber:DiscardOldSessionScenario.kt$DiscardOldSessionScenario$100</ID>
<ID>MagicNumber:DiscardOldSessionScenario.kt$DiscardOldSessionScenario$35</ID>
<ID>MagicNumber:DiscardOldSessionScenario.kt$DiscardOldSessionScenario$60</ID>
<ID>MagicNumber:DiscardOldSessionScenarioPart2.kt$DiscardOldSessionScenarioPart2$1000</ID>
<ID>MagicNumber:HandledExceptionMaxThreadsScenario.kt$HandledExceptionMaxThreadsScenario$3</ID>
<ID>MagicNumber:HandledKotlinSmokeScenario.kt$HandledKotlinSmokeScenario$999</ID>
<ID>MagicNumber:JvmAnrSleepScenario.kt$JvmAnrSleepScenario$100000</ID>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package com.bugsnag.android.mazerunner.scenarios

import android.content.Context
import com.bugsnag.android.Bugsnag
import com.bugsnag.android.Configuration
import java.io.File

internal class DiscardBigEventsScenario(
config: Configuration,
context: Context,
eventMetadata: String
) : Scenario(config, context, eventMetadata) {

init {
config.launchDurationMillis = 0
config.addOnSend {
it.addMetadata("big", "stuff", generateBigText())
true
}
}

fun generateBigText(): String {
return "*".repeat(1024 * 1024)
}

fun waitForEventFile() {
val dir = File(context.cacheDir, "bugsnag-errors")
while (dir.listFiles()!!.isEmpty()) {
Thread.sleep(100)
}
}

override fun startScenario() {
super.startScenario()
Bugsnag.markLaunchCompleted()
Bugsnag.notify(MyThrowable("DiscardBigEventsScenario"))

waitForEventFile()

Bugsnag.notify(MyThrowable("To keep maze-runner from shutting me down prematurely"))
}
}
Loading

0 comments on commit 51b7eae

Please sign in to comment.