Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filter out app starts with more than 60s #2127

Merged
merged 11 commits into from
Jun 24, 2022
6 changes: 6 additions & 0 deletions CHANGELOG.md
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

### Fixes

- Filter out app starts with more than 60s ([#2127](https://github.com/getsentry/sentry-java/pull/2127))

## 6.1.2

### Fixes
Expand Down
Expand Up @@ -13,6 +13,9 @@ public final class AppStartState {

private static @NotNull AppStartState instance = new AppStartState();

/** // We filter out App starts more than 60s */
private static final int MAX_APP_START = 60000;
marandaneto marked this conversation as resolved.
Show resolved Hide resolved

private @Nullable Long appStartMillis;

private @Nullable Long appStartEndMillis;
Expand Down Expand Up @@ -48,7 +51,16 @@ public synchronized Long getAppStartInterval() {
if (appStartMillis == null || appStartEndMillis == null || coldStart == null) {
return null;
}
return appStartEndMillis - appStartMillis;
final long appStart = appStartEndMillis - appStartMillis;

// we filter out app start more than 60s.
// this could be due to many different reasons.
marandaneto marked this conversation as resolved.
Show resolved Hide resolved
// we've seen app starts with hours, days and even months.
if (appStart >= MAX_APP_START) {
marandaneto marked this conversation as resolved.
Show resolved Hide resolved
return null;
}

return appStart;
}

public @Nullable Boolean isColdStart() {
Expand Down
Expand Up @@ -20,7 +20,8 @@
/**
* SentryPerformanceProvider is responsible for collecting data (eg appStart) as early as possible
* as ContentProvider is the only reliable hook for libraries that works across all the supported
* SDK versions. When minSDK is >= 24, we could use Process.getStartUptimeMillis()
* SDK versions. When minSDK is >= 24, we could use Process.getStartUptimeMillis() We could also use
* AppComponentFactory but it depends on androidx.core.app.AppComponentFactory
*/
@ApiStatus.Internal
public final class SentryPerformanceProvider extends ContentProvider
Expand Down
Expand Up @@ -77,4 +77,16 @@ class AppStartStateTest {

assertEquals(400, sut.appStartInterval)
}

@Test
fun `getAppStartInterval returns null if more than 60s`() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m: We could add another test with the max app start duration of 59999 ms to validate it is working correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is unnecessary, we already test above and lower the boundary.
The comparison is a simple >= with a long value rather than some more complicated calculation.
I can do it but sounds like not needed, your call.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If someone changes the MAX_APP_START_MILLIS to 10000000, no test will fail. Up to you.

val sut = AppStartState.getInstance()

val date = Date()
sut.setAppStartTime(100, date)
sut.setAppStartEnd(70000)
marandaneto marked this conversation as resolved.
Show resolved Hide resolved
sut.setColdStart(true)

assertNull(sut.appStartInterval)
}
}