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

Add main flag to threads and in_foreground flag for app contexts #2516

Merged
merged 9 commits into from
Feb 8, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Features

- Add `main` flag to threads and `in_foreground` flag for app contexts ([#2516](https://github.com/getsentry/sentry-java/pull/2516))

### Fixes

- Ignore Shutdown in progress when closing ShutdownHookIntegration ([#2521](https://github.com/getsentry/sentry-java/pull/2521))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public DefaultAndroidEventProcessor(
this.options = Objects.requireNonNull(options, "The options object is required.");

ExecutorService executorService = Executors.newSingleThreadExecutor();
// dont ref. to method reference, theres a bug on it
// don't ref. to method reference, theres a bug on it
//noinspection Convert2MethodRef
contextData = executorService.submit(() -> loadContextData());

Expand Down Expand Up @@ -128,8 +128,8 @@ public DefaultAndroidEventProcessor(
// we only set memory data if it's not a hard crash, when it's a hard crash the event is
// enriched on restart, so non static data might be wrong, eg lowMemory or availMem will
// be different if the App. crashes because of OOM.
processNonCachedEvent(event);
setThreads(event);
processNonCachedEvent(event, hint);
setThreads(event, hint);
}

setCommons(event, true, applyScopeData);
Expand Down Expand Up @@ -201,23 +201,34 @@ private void mergeOS(final @NotNull SentryBaseEvent event) {
}

// Data to be applied to events that was created in the running process
private void processNonCachedEvent(final @NotNull SentryBaseEvent event) {
private void processNonCachedEvent(
final @NotNull SentryBaseEvent event, final @NotNull Hint hint) {
App app = event.getContexts().getApp();
if (app == null) {
app = new App();
}
setAppExtras(app);
setAppExtras(app, hint);

setPackageInfo(event, app);

event.getContexts().setApp(app);
}

private void setThreads(final @NotNull SentryEvent event) {
private void setThreads(final @NotNull SentryEvent event, final @NotNull Hint hint) {
if (event.getThreads() != null) {
for (SentryThread thread : event.getThreads()) {
final boolean isHybridSDK = HintUtils.isFromHybridSdk(hint);

for (final SentryThread thread : event.getThreads()) {
final boolean isMainThread = AndroidMainThreadChecker.getInstance().isMainThread(thread);

// TODO: Fix https://github.com/getsentry/team-mobile/issues/47
if (thread.isCurrent() == null) {
thread.setCurrent(AndroidMainThreadChecker.getInstance().isMainThread(thread));
thread.setCurrent(isMainThread);
}

// This should not be set by Hybrid SDKs since they have their own threading model
if (!isHybridSDK && thread.isMain() == null) {
Comment on lines +229 to +230
Copy link
Contributor Author

Choose a reason for hiding this comment

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

thread.setMain(isMainThread);
}
}
}
Expand All @@ -241,9 +252,19 @@ private void setDist(final @NotNull SentryBaseEvent event, final @NotNull String
}
}

private void setAppExtras(final @NotNull App app) {
private void setAppExtras(final @NotNull App app, final @NotNull Hint hint) {
app.setAppName(getApplicationName());
app.setAppStartTime(DateUtils.toUtilDate(AppStartState.getInstance().getAppStartTime()));

// This should not be set by Hybrid SDKs since they have their own app's lifecycle
if (!HintUtils.isFromHybridSdk(hint) && app.getInForeground() == null) {
Comment on lines +259 to +260
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Running into #2525

// This feature depends on the AppLifecycleIntegration being installed, so only if
// enableAutoSessionTracking or enableAppLifecycleBreadcrumbs are enabled.
final @Nullable Boolean isBackground = AppState.getInstance().isInBackground();
if (isBackground != null) {
app.setInForeground(!isBackground);
}
}
}

@SuppressWarnings("deprecation")
Expand All @@ -256,21 +277,21 @@ private void setAppExtras(final @NotNull App app) {
return Build.CPU_ABI2;
}

@SuppressWarnings({"ObsoleteSdkInt", "deprecation"})
@SuppressWarnings({"ObsoleteSdkInt", "deprecation", "NewApi"})
private void setArchitectures(final @NotNull Device device) {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
String[] supportedAbis = Build.SUPPORTED_ABIS;
device.setArchs(supportedAbis);
final String[] supportedAbis;
if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.LOLLIPOP) {
supportedAbis = Build.SUPPORTED_ABIS;
} else {
String[] supportedAbis = {getAbi(), getAbi2()};
device.setArchs(supportedAbis);
supportedAbis = new String[] {getAbi(), getAbi2()};
// we were not checking CPU_ABI2, but I've added to the list now
}
device.setArchs(supportedAbis);
}

@SuppressWarnings("ObsoleteSdkInt")
@SuppressWarnings({"ObsoleteSdkInt", "NewApi"})
private @NotNull Long getMemorySize(final @NotNull ActivityManager.MemoryInfo memInfo) {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN) {
if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.JELLY_BEAN) {
return memInfo.totalMem;
}
// using Runtime as a fallback
Expand Down Expand Up @@ -393,17 +414,18 @@ private void setDeviceIO(final @NotNull Device device, final boolean applyScopeD
}
}

@SuppressWarnings("ObsoleteSdkInt")
@SuppressWarnings({"ObsoleteSdkInt", "NewApi"})
private @Nullable String getDeviceName() {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN_MR1) {
if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.JELLY_BEAN_MR1) {
return Settings.Global.getString(context.getContentResolver(), "device_name");
} else {
return null;
}
}

@SuppressWarnings("NewApi")
private TimeZone getTimeZone() {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.N) {
LocaleList locales = context.getResources().getConfiguration().getLocales();
if (!locales.isEmpty()) {
Locale locale = locales.get(0);
Expand Down Expand Up @@ -557,9 +579,9 @@ private TimeZone getTimeZone() {
}
}

@SuppressWarnings("ObsoleteSdkInt")
@SuppressWarnings({"ObsoleteSdkInt", "NewApi"})
private long getBlockSizeLong(final @NotNull StatFs stat) {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN_MR2) {
if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.JELLY_BEAN_MR2) {
return stat.getBlockSizeLong();
}
return getBlockSizeDep(stat);
Expand All @@ -570,9 +592,9 @@ private int getBlockSizeDep(final @NotNull StatFs stat) {
return stat.getBlockSize();
}

@SuppressWarnings("ObsoleteSdkInt")
@SuppressWarnings({"ObsoleteSdkInt", "NewApi"})
private long getBlockCountLong(final @NotNull StatFs stat) {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN_MR2) {
if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.JELLY_BEAN_MR2) {
return stat.getBlockCountLong();
}
return getBlockCountDep(stat);
Expand All @@ -583,9 +605,9 @@ private int getBlockCountDep(final @NotNull StatFs stat) {
return stat.getBlockCount();
}

@SuppressWarnings("ObsoleteSdkInt")
@SuppressWarnings({"ObsoleteSdkInt", "NewApi"})
private long getAvailableBlocksLong(final @NotNull StatFs stat) {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN_MR2) {
if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.JELLY_BEAN_MR2) {
return stat.getAvailableBlocksLong();
}
return getAvailableBlocksDep(stat);
Expand Down Expand Up @@ -627,9 +649,9 @@ private int getAvailableBlocksDep(final @NotNull StatFs stat) {
return null;
}

@SuppressWarnings("ObsoleteSdkInt")
@SuppressWarnings({"ObsoleteSdkInt", "NewApi"})
private @Nullable File[] getExternalFilesDirs() {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT) {
if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.KITKAT) {
return context.getExternalFilesDirs(null);
} else {
File single = context.getExternalFilesDir(null);
Expand Down Expand Up @@ -907,7 +929,7 @@ private void setSideLoadedInfo(final @NotNull SentryBaseEvent event) {
final boolean applyScopeData = shouldApplyScopeData(transaction, hint);

if (applyScopeData) {
processNonCachedEvent(transaction);
processNonCachedEvent(transaction, hint);
}

setCommons(transaction, false, applyScopeData);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ final class LifecycleWatcher implements DefaultLifecycleObserver {
public void onStart(final @NotNull LifecycleOwner owner) {
startSession();
addAppBreadcrumb("foreground");

// Consider using owner.getLifecycle().getCurrentState().isAtLeast(Lifecycle.State.RESUMED);
// in the future.
AppState.getInstance().setInBackground(false);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package io.sentry.android.core

import io.sentry.hints.ApplyScopeData
import io.sentry.hints.Cached

class CustomCachedApplyScopeDataHint : Cached, ApplyScopeData
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import io.sentry.SentryEvent
import io.sentry.SentryLevel
import io.sentry.SentryTracer
import io.sentry.TransactionContext
import io.sentry.TypeCheckHint.SENTRY_DART_SDK_NAME
import io.sentry.android.core.DefaultAndroidEventProcessor.EMULATOR
import io.sentry.android.core.DefaultAndroidEventProcessor.KERNEL_VERSION
import io.sentry.android.core.DefaultAndroidEventProcessor.ROOTED
Expand Down Expand Up @@ -56,7 +57,7 @@ class DefaultAndroidEventProcessorTest {
private class Fixture {
val buildInfo = mock<BuildInfoProvider>()
val options = SentryAndroidOptions().apply {
setDebug(true)
isDebug = true
setLogger(mock())
sdkVersion = SdkVersion("test", "1.2.3")
}
Expand All @@ -77,6 +78,7 @@ class DefaultAndroidEventProcessorTest {
@BeforeTest
fun `set up`() {
context = ApplicationProvider.getApplicationContext()
AppState.getInstance().resetInstance()
}

@Test
Expand Down Expand Up @@ -161,7 +163,7 @@ class DefaultAndroidEventProcessorTest {
}

@Test
fun `Current should be true if it comes from main thread`() {
fun `Current and Main should be true if it comes from main thread`() {
val sut = fixture.getSut(context)

val sentryThread = SentryThread().apply {
Expand All @@ -174,6 +176,7 @@ class DefaultAndroidEventProcessorTest {
assertNotNull(sut.process(event, Hint())) {
assertNotNull(it.threads) { threads ->
assertTrue(threads.first().isCurrent == true)
assertTrue(threads.first().isMain == true)
}
}
}
Expand All @@ -193,6 +196,7 @@ class DefaultAndroidEventProcessorTest {
assertNotNull(sut.process(event, Hint())) {
assertNotNull(it.threads) { threads ->
assertFalse(threads.first().isCurrent == true)
assertFalse(threads.first().isMain == true)
}
}
}
Expand Down Expand Up @@ -497,4 +501,55 @@ class DefaultAndroidEventProcessorTest {
assertEquals("en_US", device.locale)
}
}

@Test
fun `Event sets InForeground to true if not in the background`() {
val sut = fixture.getSut(context)

AppState.getInstance().setInBackground(false)

assertNotNull(sut.process(SentryEvent(), Hint())) {
val app = it.contexts.app!!
assertTrue(app.inForeground!!)
}
}

@Test
fun `Event sets InForeground to false if in the background`() {
val sut = fixture.getSut(context)

AppState.getInstance().setInBackground(true)

assertNotNull(sut.process(SentryEvent(), Hint())) {
val app = it.contexts.app!!
assertFalse(app.inForeground!!)
}
}

@Test
fun `Events from HybridSDKs don't set main thread and in foreground context`() {
val sut = fixture.getSut(context)

val cachedHint = CustomCachedApplyScopeDataHint()
val hint = HintUtils.createWithTypeCheckHint(cachedHint)

val sdkVersion = SdkVersion(SENTRY_DART_SDK_NAME, "1.0.0")
val event = SentryEvent().apply {
sdk = sdkVersion
threads = mutableListOf(
SentryThread().apply {
id = 10L
}
)
}
// set by OutboxSender during event deserialization
HintUtils.setIsFromHybridSdk(hint, sdkVersion.name)

assertNotNull(sut.process(event, hint)) {
val app = it.contexts.app!!
assertNull(app.inForeground)
val thread = it.threads!!.first()
assertNull(thread.isMain)
}
}
}
6 changes: 6 additions & 0 deletions sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -2510,6 +2510,7 @@ public final class io/sentry/protocol/App : io/sentry/JsonSerializable, io/sentr
public fun getAppVersion ()Ljava/lang/String;
public fun getBuildType ()Ljava/lang/String;
public fun getDeviceAppHash ()Ljava/lang/String;
public fun getInForeground ()Ljava/lang/Boolean;
public fun getPermissions ()Ljava/util/Map;
public fun getUnknown ()Ljava/util/Map;
public fun serialize (Lio/sentry/JsonObjectWriter;Lio/sentry/ILogger;)V
Expand All @@ -2520,6 +2521,7 @@ public final class io/sentry/protocol/App : io/sentry/JsonSerializable, io/sentr
public fun setAppVersion (Ljava/lang/String;)V
public fun setBuildType (Ljava/lang/String;)V
public fun setDeviceAppHash (Ljava/lang/String;)V
public fun setInForeground (Ljava/lang/Boolean;)V
public fun setPermissions (Ljava/util/Map;)V
public fun setUnknown (Ljava/util/Map;)V
}
Expand All @@ -2539,6 +2541,7 @@ public final class io/sentry/protocol/App$JsonKeys {
public static final field APP_VERSION Ljava/lang/String;
public static final field BUILD_TYPE Ljava/lang/String;
public static final field DEVICE_APP_HASH Ljava/lang/String;
public static final field IN_FOREGROUND Ljava/lang/String;
public fun <init> ()V
}

Expand Down Expand Up @@ -3344,11 +3347,13 @@ public final class io/sentry/protocol/SentryThread : io/sentry/JsonSerializable,
public fun isCrashed ()Ljava/lang/Boolean;
public fun isCurrent ()Ljava/lang/Boolean;
public fun isDaemon ()Ljava/lang/Boolean;
public fun isMain ()Ljava/lang/Boolean;
public fun serialize (Lio/sentry/JsonObjectWriter;Lio/sentry/ILogger;)V
public fun setCrashed (Ljava/lang/Boolean;)V
public fun setCurrent (Ljava/lang/Boolean;)V
public fun setDaemon (Ljava/lang/Boolean;)V
public fun setId (Ljava/lang/Long;)V
public fun setMain (Ljava/lang/Boolean;)V
public fun setName (Ljava/lang/String;)V
public fun setPriority (Ljava/lang/Integer;)V
public fun setStacktrace (Lio/sentry/protocol/SentryStackTrace;)V
Expand All @@ -3367,6 +3372,7 @@ public final class io/sentry/protocol/SentryThread$JsonKeys {
public static final field CURRENT Ljava/lang/String;
public static final field DAEMON Ljava/lang/String;
public static final field ID Ljava/lang/String;
public static final field MAIN Ljava/lang/String;
public static final field NAME Ljava/lang/String;
public static final field PRIORITY Ljava/lang/String;
public static final field STACKTRACE Ljava/lang/String;
Expand Down
Loading