From 7da2a31f00a1d9a4f1ffe2af69510402c14e35d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kry=C5=A1tof=20Wold=C5=99ich?= <31292499+krystofwoldrich@users.noreply.github.com> Date: Fri, 3 Feb 2023 11:11:20 +0100 Subject: [PATCH 1/7] fix(internal): Do not attach view hierarchy to hybrid sdks events (#2513) --- .../core/ViewHierarchyEventProcessor.java | 5 +++++ .../core/ViewHierarchyEventProcessorTest.kt | 21 +++++++++++++++++-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java index b2b79b1742..675715a383 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java @@ -14,6 +14,7 @@ import io.sentry.android.core.internal.gestures.ViewUtils; import io.sentry.protocol.ViewHierarchy; import io.sentry.protocol.ViewHierarchyNode; +import io.sentry.util.HintUtils; import io.sentry.util.JsonSerializationUtils; import io.sentry.util.Objects; import java.util.ArrayList; @@ -43,6 +44,10 @@ public ViewHierarchyEventProcessor(final @NotNull SentryAndroidOptions options) return event; } + if (HintUtils.isFromHybridSdk(hint)) { + return event; + } + final @Nullable Activity activity = CurrentActivityHolder.getInstance().getActivity(); final @Nullable ViewHierarchy viewHierarchy = snapshotViewHierarchy(activity, options.getLogger()); diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ViewHierarchyEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ViewHierarchyEventProcessorTest.kt index b06e9c8cb3..a74e83858a 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ViewHierarchyEventProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ViewHierarchyEventProcessorTest.kt @@ -9,6 +9,7 @@ import io.sentry.Hint import io.sentry.JsonSerializable import io.sentry.JsonSerializer import io.sentry.SentryEvent +import io.sentry.TypeCheckHint import io.sentry.protocol.SentryException import org.junit.runner.RunWith import org.mockito.invocation.InvocationOnMock @@ -64,10 +65,10 @@ class ViewHierarchyEventProcessorTest { fun process( attachViewHierarchy: Boolean, - event: SentryEvent + event: SentryEvent, + hint: Hint = Hint() ): Pair { val processor = getSut(attachViewHierarchy) - val hint = Hint() processor.process(event, hint) return Pair(event, hint) @@ -104,6 +105,22 @@ class ViewHierarchyEventProcessorTest { assertNull(viewHierarchy) } + @Test + fun `when an event errored, the view hierarchy should not attached if the event is from hybrid sdk`() { + val hintFromHybridSdk = Hint() + hintFromHybridSdk.set(TypeCheckHint.SENTRY_IS_FROM_HYBRID_SDK, true) + val (event, hint) = fixture.process( + true, + SentryEvent().apply { + exceptions = listOf(SentryException()) + }, + hintFromHybridSdk + ) + + assertNotNull(event) + assertNull(hint.viewHierarchy) + } + @Test fun `when an event errored, the view hierarchy should not attached if the feature is disabled`() { val (event, hint) = fixture.process( From f6a135d1ef81f9cd70d72608e8beb8c2103f69bd Mon Sep 17 00:00:00 2001 From: Stefano Date: Fri, 3 Feb 2023 12:12:06 +0100 Subject: [PATCH 2/7] Fix transaction performance collector oom (#2505) * moved synchronized() bit later in stop() method in DefaultTransactionPerformanceCollector * PerformanceCollectionData now contains single values instead of lists and removed the commit() method * TransactionPerformanceCollector: - is now set in Android options - a NoOpTransactionPerformanceCollector is set in Java options - deletes data after 30 seconds - collect data only if a profile is sampled --- CHANGELOG.md | 1 + .../api/sentry-android-core.api | 4 +- .../android/core/AndroidCpuCollector.java | 7 +-- .../android/core/AndroidMemoryCollector.java | 6 +- .../core/AndroidOptionsInitializer.java | 2 + .../core/AndroidTransactionProfiler.java | 34 ++++++----- .../android/core/AndroidCpuCollectorTest.kt | 23 +++----- .../core/AndroidMemoryCollectorTest.kt | 6 +- .../core/AndroidOptionsInitializerTest.kt | 9 +++ .../core/AndroidTransactionProfilerTest.kt | 18 +++--- .../android/core/SentryAndroidOptionsTest.kt | 2 +- sentry/api/sentry.api | 20 +++---- ...efaultTransactionPerformanceCollector.java | 57 ++++++++++--------- .../src/main/java/io/sentry/ICollector.java | 2 +- .../java/io/sentry/ITransactionProfiler.java | 4 +- .../java/io/sentry/JavaMemoryCollector.java | 6 +- .../NoOpTransactionPerformanceCollector.java | 3 +- .../io/sentry/NoOpTransactionProfiler.java | 4 +- .../io/sentry/PerformanceCollectionData.java | 39 +++---------- .../main/java/io/sentry/SentryOptions.java | 13 ++++- .../src/main/java/io/sentry/SentryTracer.java | 9 ++- .../TransactionPerformanceCollector.java | 3 +- ...ultTransactionPerformanceCollectorTest.kt} | 39 +++++++------ .../java/io/sentry/JavaMemoryCollectorTest.kt | 16 +++--- .../sentry/PerformanceCollectionDataTest.kt | 51 +++-------------- .../test/java/io/sentry/SentryOptionsTest.kt | 14 +++++ sentry/src/test/java/io/sentry/SentryTest.kt | 2 +- .../test/java/io/sentry/SentryTracerTest.kt | 29 +++++++++- 28 files changed, 218 insertions(+), 205 deletions(-) rename sentry/src/test/java/io/sentry/{TransactionPerformanceCollectorTest.kt => DefaultTransactionPerformanceCollectorTest.kt} (88%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 18d48c036c..739bd707b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Fixes +- Fix transaction performance collector oom ([#2505](https://github.com/getsentry/sentry-java/pull/2505)) - Remove authority from URLs sent to Sentry ([#2366](https://github.com/getsentry/sentry-java/pull/2366)) - Fix `sentry-bom` containing incorrect artifacts ([#2504](https://github.com/getsentry/sentry-java/pull/2504)) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index f406706e92..20ea1e97d9 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -25,7 +25,7 @@ public final class io/sentry/android/core/ActivityLifecycleIntegration : android public final class io/sentry/android/core/AndroidCpuCollector : io/sentry/ICollector { public fun (Lio/sentry/ILogger;Lio/sentry/android/core/BuildInfoProvider;)V - public fun collect (Ljava/lang/Iterable;)V + public fun collect (Lio/sentry/PerformanceCollectionData;)V public fun setup ()V } @@ -45,7 +45,7 @@ public final class io/sentry/android/core/AndroidLogger : io/sentry/ILogger { public class io/sentry/android/core/AndroidMemoryCollector : io/sentry/ICollector { public fun ()V - public fun collect (Ljava/lang/Iterable;)V + public fun collect (Lio/sentry/PerformanceCollectionData;)V public fun setup ()V } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidCpuCollector.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidCpuCollector.java index c3d41f0aac..0992fec96d 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidCpuCollector.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidCpuCollector.java @@ -66,8 +66,7 @@ public void setup() { @SuppressLint("NewApi") @Override - public void collect( - @NotNull final Iterable performanceCollectionData) { + public void collect(final @NotNull PerformanceCollectionData performanceCollectionData) { if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.LOLLIPOP || !isEnabled) { return; } @@ -86,9 +85,7 @@ public void collect( new CpuCollectionData( System.currentTimeMillis(), (cpuUsagePercentage / (double) numCores) * 100.0); - for (PerformanceCollectionData data : performanceCollectionData) { - data.addCpuData(cpuData); - } + performanceCollectionData.addCpuData(cpuData); } /** Read the /proc/self/stat file and parses the result. */ diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidMemoryCollector.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidMemoryCollector.java index 2b03ef8b87..e43823b8d0 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidMemoryCollector.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidMemoryCollector.java @@ -14,13 +14,11 @@ public class AndroidMemoryCollector implements ICollector { public void setup() {} @Override - public void collect(@NotNull Iterable performanceCollectionData) { + public void collect(final @NotNull PerformanceCollectionData performanceCollectionData) { long now = System.currentTimeMillis(); long usedMemory = Runtime.getRuntime().totalMemory() - Runtime.getRuntime().freeMemory(); long usedNativeMemory = Debug.getNativeHeapSize() - Debug.getNativeHeapFreeSize(); MemoryCollectionData memoryData = new MemoryCollectionData(now, usedMemory, usedNativeMemory); - for (PerformanceCollectionData data : performanceCollectionData) { - data.addMemoryData(memoryData); - } + performanceCollectionData.addMemoryData(memoryData); } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index 358c6fb904..16465c8b78 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -7,6 +7,7 @@ import android.content.pm.PackageInfo; import android.content.res.AssetManager; import android.os.Build; +import io.sentry.DefaultTransactionPerformanceCollector; import io.sentry.ILogger; import io.sentry.SendFireAndForgetEnvelopeSender; import io.sentry.SendFireAndForgetOutboxSender; @@ -168,6 +169,7 @@ static void initializeIntegrationsAndProcessors( options.addCollector(new AndroidMemoryCollector()); options.addCollector(new AndroidCpuCollector(options.getLogger(), buildInfoProvider)); } + options.setTransactionPerformanceCollector(new DefaultTransactionPerformanceCollector(options)); } private static void installDefaultIntegrations( diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java index 57cd6dd329..7ec5297975 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java @@ -251,7 +251,7 @@ public void onFrameMetricCollected( @Override public @Nullable synchronized ProfilingTraceData onTransactionFinish( final @NotNull ITransaction transaction, - final @Nullable PerformanceCollectionData performanceCollectionData) { + final @Nullable List performanceCollectionData) { try { return options .getExecutorService() @@ -269,7 +269,7 @@ public void onFrameMetricCollected( private @Nullable ProfilingTraceData onTransactionFinish( final @NotNull ITransaction transaction, final boolean isTimeout, - final @Nullable PerformanceCollectionData performanceCollectionData) { + final @Nullable List performanceCollectionData) { // onTransactionStart() is only available since Lollipop // and SystemClock.elapsedRealtimeNanos() since Jelly Bean @@ -416,28 +416,32 @@ public void onFrameMetricCollected( } private void putPerformanceCollectionDataInMeasurements( - final @Nullable PerformanceCollectionData performanceCollectionData) { + final @Nullable List performanceCollectionData) { if (performanceCollectionData != null) { final @NotNull ArrayDeque memoryUsageMeasurements = - new ArrayDeque<>(); + new ArrayDeque<>(performanceCollectionData.size()); final @NotNull ArrayDeque nativeMemoryUsageMeasurements = - new ArrayDeque<>(); - final @NotNull ArrayDeque cpuUsageMeasurements = new ArrayDeque<>(); - for (CpuCollectionData cpuData : performanceCollectionData.getCpuData()) { - cpuUsageMeasurements.add( - new ProfileMeasurementValue( - TimeUnit.MILLISECONDS.toNanos(cpuData.getTimestampMillis()) - transactionStartNanos, - cpuData.getCpuUsagePercentage())); - } - for (MemoryCollectionData memoryData : performanceCollectionData.getMemoryData()) { - if (memoryData.getUsedHeapMemory() > -1) { + new ArrayDeque<>(performanceCollectionData.size()); + final @NotNull ArrayDeque cpuUsageMeasurements = + new ArrayDeque<>(performanceCollectionData.size()); + for (PerformanceCollectionData performanceData : performanceCollectionData) { + CpuCollectionData cpuData = performanceData.getCpuData(); + MemoryCollectionData memoryData = performanceData.getMemoryData(); + if (cpuData != null) { + cpuUsageMeasurements.add( + new ProfileMeasurementValue( + TimeUnit.MILLISECONDS.toNanos(cpuData.getTimestampMillis()) + - transactionStartNanos, + cpuData.getCpuUsagePercentage())); + } + if (memoryData != null && memoryData.getUsedHeapMemory() > -1) { memoryUsageMeasurements.add( new ProfileMeasurementValue( TimeUnit.MILLISECONDS.toNanos(memoryData.getTimestampMillis()) - transactionStartNanos, memoryData.getUsedHeapMemory())); } - if (memoryData.getUsedNativeMemory() > -1) { + if (memoryData != null && memoryData.getUsedNativeMemory() > -1) { nativeMemoryUsageMeasurements.add( new ProfileMeasurementValue( TimeUnit.MILLISECONDS.toNanos(memoryData.getTimestampMillis()) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidCpuCollectorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidCpuCollectorTest.kt index 671093ddf0..45010255e8 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidCpuCollectorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidCpuCollectorTest.kt @@ -8,10 +8,9 @@ import org.mockito.kotlin.mock import org.mockito.kotlin.whenever import kotlin.test.Test import kotlin.test.assertFailsWith -import kotlin.test.assertFalse import kotlin.test.assertNotEquals import kotlin.test.assertNotNull -import kotlin.test.assertTrue +import kotlin.test.assertNull class AndroidCpuCollectorTest { @@ -43,9 +42,8 @@ class AndroidCpuCollectorTest { @Test fun `collect works only after setup`() { val data = PerformanceCollectionData() - fixture.getSut().collect(listOf(data)) - data.commitData() - assertTrue(data.cpuData.isEmpty()) + fixture.getSut().collect(data) + assertNull(data.cpuData) } @Test @@ -53,13 +51,11 @@ class AndroidCpuCollectorTest { val data = PerformanceCollectionData() val collector = fixture.getSut() collector.setup() - collector.collect(listOf(data)) - data.commitData() + collector.collect(data) val cpuData = data.cpuData - assertNotNull(data.cpuData) - assertFalse(data.cpuData.isEmpty()) - assertNotEquals(0.0, cpuData[0].cpuUsagePercentage) - assertNotEquals(0, cpuData[0].timestampMillis) + assertNotNull(cpuData) + assertNotEquals(0.0, cpuData.cpuUsagePercentage) + assertNotEquals(0, cpuData.timestampMillis) } @Test @@ -69,8 +65,7 @@ class AndroidCpuCollectorTest { whenever(mockBuildInfoProvider.sdkInfoVersion).thenReturn(Build.VERSION_CODES.KITKAT) val collector = fixture.getSut(mockBuildInfoProvider) collector.setup() - collector.collect(listOf(data)) - data.commitData() - assertTrue(data.cpuData.isEmpty()) + collector.collect(data) + assertNull(data.cpuData) } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidMemoryCollectorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidMemoryCollectorTest.kt index dad91d1206..7879c2daf5 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidMemoryCollectorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidMemoryCollectorTest.kt @@ -18,13 +18,11 @@ class AndroidMemoryCollectorTest { @Test fun `when collect, both native and heap memory are collected`() { - val performanceCollectionData = PerformanceCollectionData() - val data = listOf(performanceCollectionData) + val data = PerformanceCollectionData() val usedNativeMemory = Debug.getNativeHeapSize() - Debug.getNativeHeapFreeSize() val usedMemory = fixture.runtime.totalMemory() - fixture.runtime.freeMemory() fixture.collector.collect(data) - performanceCollectionData.commitData() - val memoryData = performanceCollectionData.memoryData.firstOrNull() + val memoryData = data.memoryData assertNotNull(memoryData) assertNotEquals(-1, memoryData.usedNativeMemory) assertEquals(usedNativeMemory, memoryData.usedNativeMemory) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt index adc2e88b04..114646985a 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt @@ -4,6 +4,7 @@ import android.content.Context import android.os.Bundle import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 +import io.sentry.DefaultTransactionPerformanceCollector import io.sentry.ILogger import io.sentry.MainEventProcessor import io.sentry.SentryOptions @@ -24,6 +25,7 @@ import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse +import kotlin.test.assertIs import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue @@ -525,4 +527,11 @@ class AndroidOptionsInitializerTest { assertTrue { fixture.sentryOptions.collectors.any { it is AndroidCpuCollector } } } + + @Test + fun `DefaultTransactionPerformanceCollector is set to options`() { + fixture.initSut() + + assertIs(fixture.sentryOptions.transactionPerformanceCollector) + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt index 52cc697f18..ab5bc2fb29 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt @@ -376,14 +376,18 @@ class AndroidTransactionProfilerTest { @Test fun `profiler includes performance measurements when passed on transaction finish`() { val profiler = fixture.getSut(context) - val memoryCollectionData = PerformanceCollectionData() - memoryCollectionData.addMemoryData(MemoryCollectionData(1, 2, 3)) - memoryCollectionData.addCpuData(CpuCollectionData(1, 1.4)) - memoryCollectionData.commitData() - memoryCollectionData.addMemoryData(MemoryCollectionData(2, 3, 4)) - memoryCollectionData.commitData() + val performanceCollectionData = ArrayList() + var singleData = PerformanceCollectionData() + singleData.addMemoryData(MemoryCollectionData(1, 2, 3)) + singleData.addCpuData(CpuCollectionData(1, 1.4)) + performanceCollectionData.add(singleData) + + singleData = PerformanceCollectionData() + singleData.addMemoryData(MemoryCollectionData(2, 3, 4)) + performanceCollectionData.add(singleData) + profiler.onTransactionStart(fixture.transaction1) - val data = profiler.onTransactionFinish(fixture.transaction1, memoryCollectionData) + val data = profiler.onTransactionFinish(fixture.transaction1, performanceCollectionData) assertContentEquals( listOf(1.4), data!!.measurementsMap[ProfileMeasurement.ID_CPU_USAGE]!!.values.map { it.value } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidOptionsTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidOptionsTest.kt index be11c4ca41..f5068c7762 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidOptionsTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidOptionsTest.kt @@ -113,7 +113,7 @@ class SentryAndroidOptionsTest { override fun onTransactionStart(transaction: ITransaction) {} override fun onTransactionFinish( transaction: ITransaction, - memoryCollectionData: PerformanceCollectionData? + performanceCollectionData: List? ): ProfilingTraceData? = null } } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index bd4e9ef9ff..8692d59010 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -186,7 +186,7 @@ public final class io/sentry/DateUtils { public final class io/sentry/DefaultTransactionPerformanceCollector : io/sentry/TransactionPerformanceCollector { public fun (Lio/sentry/SentryOptions;)V public fun start (Lio/sentry/ITransaction;)V - public fun stop (Lio/sentry/ITransaction;)Lio/sentry/PerformanceCollectionData; + public fun stop (Lio/sentry/ITransaction;)Ljava/util/List; } public final class io/sentry/DiagnosticLogger : io/sentry/ILogger { @@ -391,7 +391,7 @@ public final class io/sentry/HubAdapter : io/sentry/IHub { } public abstract interface class io/sentry/ICollector { - public abstract fun collect (Ljava/lang/Iterable;)V + public abstract fun collect (Lio/sentry/PerformanceCollectionData;)V public abstract fun setup ()V } @@ -571,7 +571,7 @@ public abstract interface class io/sentry/ITransaction : io/sentry/ISpan { } public abstract interface class io/sentry/ITransactionProfiler { - public abstract fun onTransactionFinish (Lio/sentry/ITransaction;Lio/sentry/PerformanceCollectionData;)Lio/sentry/ProfilingTraceData; + public abstract fun onTransactionFinish (Lio/sentry/ITransaction;Ljava/util/List;)Lio/sentry/ProfilingTraceData; public abstract fun onTransactionStart (Lio/sentry/ITransaction;)V } @@ -596,7 +596,7 @@ public final class io/sentry/IpAddressUtils { public final class io/sentry/JavaMemoryCollector : io/sentry/ICollector { public fun ()V - public fun collect (Ljava/lang/Iterable;)V + public fun collect (Lio/sentry/PerformanceCollectionData;)V public fun setup ()V } @@ -862,12 +862,12 @@ public final class io/sentry/NoOpTransaction : io/sentry/ITransaction { public final class io/sentry/NoOpTransactionPerformanceCollector : io/sentry/TransactionPerformanceCollector { public static fun getInstance ()Lio/sentry/NoOpTransactionPerformanceCollector; public fun start (Lio/sentry/ITransaction;)V - public fun stop (Lio/sentry/ITransaction;)Lio/sentry/PerformanceCollectionData; + public fun stop (Lio/sentry/ITransaction;)Ljava/util/List; } public final class io/sentry/NoOpTransactionProfiler : io/sentry/ITransactionProfiler { public static fun getInstance ()Lio/sentry/NoOpTransactionProfiler; - public fun onTransactionFinish (Lio/sentry/ITransaction;Lio/sentry/PerformanceCollectionData;)Lio/sentry/ProfilingTraceData; + public fun onTransactionFinish (Lio/sentry/ITransaction;Ljava/util/List;)Lio/sentry/ProfilingTraceData; public fun onTransactionStart (Lio/sentry/ITransaction;)V } @@ -891,9 +891,8 @@ public final class io/sentry/PerformanceCollectionData { public fun ()V public fun addCpuData (Lio/sentry/CpuCollectionData;)V public fun addMemoryData (Lio/sentry/MemoryCollectionData;)V - public fun commitData ()V - public fun getCpuData ()Ljava/util/List; - public fun getMemoryData ()Ljava/util/List; + public fun getCpuData ()Lio/sentry/CpuCollectionData; + public fun getMemoryData ()Lio/sentry/MemoryCollectionData; } public final class io/sentry/ProfilingTraceData : io/sentry/JsonSerializable, io/sentry/JsonUnknown { @@ -1646,6 +1645,7 @@ public class io/sentry/SentryOptions { public fun setTracesSampleRate (Ljava/lang/Double;)V public fun setTracesSampler (Lio/sentry/SentryOptions$TracesSamplerCallback;)V public fun setTracingOrigins (Ljava/util/List;)V + public fun setTransactionPerformanceCollector (Lio/sentry/TransactionPerformanceCollector;)V public fun setTransactionProfiler (Lio/sentry/ITransactionProfiler;)V public fun setTransportFactory (Lio/sentry/ITransportFactory;)V public fun setTransportGate (Lio/sentry/transport/ITransportGate;)V @@ -2069,7 +2069,7 @@ public final class io/sentry/TransactionOptions { public abstract interface class io/sentry/TransactionPerformanceCollector { public abstract fun start (Lio/sentry/ITransaction;)V - public abstract fun stop (Lio/sentry/ITransaction;)Lio/sentry/PerformanceCollectionData; + public abstract fun stop (Lio/sentry/ITransaction;)Ljava/util/List; } public final class io/sentry/TypeCheckHint { diff --git a/sentry/src/main/java/io/sentry/DefaultTransactionPerformanceCollector.java b/sentry/src/main/java/io/sentry/DefaultTransactionPerformanceCollector.java index a9284e4f21..b4ffec0872 100644 --- a/sentry/src/main/java/io/sentry/DefaultTransactionPerformanceCollector.java +++ b/sentry/src/main/java/io/sentry/DefaultTransactionPerformanceCollector.java @@ -1,6 +1,7 @@ package io.sentry; import io.sentry.util.Objects; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Timer; @@ -18,7 +19,7 @@ public final class DefaultTransactionPerformanceCollector private static final long TRANSACTION_COLLECTION_TIMEOUT_MILLIS = 30000; private final @NotNull Object timerLock = new Object(); private volatile @Nullable Timer timer = null; - private final @NotNull Map performanceDataMap = + private final @NotNull Map> performanceDataMap = new ConcurrentHashMap<>(); private final @NotNull List collectors; private final @NotNull SentryOptions options; @@ -42,17 +43,11 @@ public void start(final @NotNull ITransaction transaction) { } if (!performanceDataMap.containsKey(transaction.getEventId().toString())) { - performanceDataMap.put(transaction.getEventId().toString(), new PerformanceCollectionData()); + performanceDataMap.put(transaction.getEventId().toString(), new ArrayList<>()); + // We schedule deletion of collected performance data after a timeout options .getExecutorService() - .schedule( - () -> { - PerformanceCollectionData data = stop(transaction); - if (data != null) { - performanceDataMap.put(transaction.getEventId().toString(), data); - } - }, - TRANSACTION_COLLECTION_TIMEOUT_MILLIS); + .schedule(() -> stop(transaction), TRANSACTION_COLLECTION_TIMEOUT_MILLIS); } if (!isStarted.getAndSet(true)) { synchronized (timerLock) { @@ -74,22 +69,23 @@ public void run() { // and collect() calls. // This way ICollectors that collect average stats based on time intervals, like // AndroidCpuCollector, can have an actual time interval to evaluate. - timer.scheduleAtFixedRate( + TimerTask timerTask = new TimerTask() { @Override public void run() { - synchronized (timerLock) { - for (ICollector collector : collectors) { - collector.collect(performanceDataMap.values()); - } - // We commit data after calling all collectors. - // This way we avoid issues caused by having multiple cpu or memory collectors. - for (PerformanceCollectionData data : performanceDataMap.values()) { - data.commitData(); - } + final @NotNull PerformanceCollectionData tempData = new PerformanceCollectionData(); + + for (ICollector collector : collectors) { + collector.collect(tempData); + } + + for (List data : performanceDataMap.values()) { + data.add(tempData); } } - }, + }; + timer.scheduleAtFixedRate( + timerTask, TRANSACTION_COLLECTION_INTERVAL_MILLIS, TRANSACTION_COLLECTION_INTERVAL_MILLIS); } @@ -97,17 +93,24 @@ public void run() { } @Override - public @Nullable PerformanceCollectionData stop(final @NotNull ITransaction transaction) { - synchronized (timerLock) { - PerformanceCollectionData data = - performanceDataMap.remove(transaction.getEventId().toString()); - if (performanceDataMap.isEmpty() && isStarted.getAndSet(false)) { + public @Nullable List stop(final @NotNull ITransaction transaction) { + List data = + performanceDataMap.remove(transaction.getEventId().toString()); + options + .getLogger() + .log( + SentryLevel.DEBUG, + "stop collecting performance info for transactions %s (%s)", + transaction.getName(), + transaction.getSpanContext().getTraceId().toString()); + if (performanceDataMap.isEmpty() && isStarted.getAndSet(false)) { + synchronized (timerLock) { if (timer != null) { timer.cancel(); timer = null; } } - return data; } + return data; } } diff --git a/sentry/src/main/java/io/sentry/ICollector.java b/sentry/src/main/java/io/sentry/ICollector.java index cae520e063..945c113d69 100644 --- a/sentry/src/main/java/io/sentry/ICollector.java +++ b/sentry/src/main/java/io/sentry/ICollector.java @@ -9,5 +9,5 @@ public interface ICollector { void setup(); - void collect(@NotNull final Iterable performanceCollectionData); + void collect(final @NotNull PerformanceCollectionData performanceCollectionData); } diff --git a/sentry/src/main/java/io/sentry/ITransactionProfiler.java b/sentry/src/main/java/io/sentry/ITransactionProfiler.java index a53cba859e..44676e6f72 100644 --- a/sentry/src/main/java/io/sentry/ITransactionProfiler.java +++ b/sentry/src/main/java/io/sentry/ITransactionProfiler.java @@ -1,5 +1,6 @@ package io.sentry; +import java.util.List; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -11,5 +12,6 @@ public interface ITransactionProfiler { @Nullable ProfilingTraceData onTransactionFinish( - @NotNull ITransaction transaction, @Nullable PerformanceCollectionData memoryCollectionData); + @NotNull ITransaction transaction, + @Nullable List performanceCollectionData); } diff --git a/sentry/src/main/java/io/sentry/JavaMemoryCollector.java b/sentry/src/main/java/io/sentry/JavaMemoryCollector.java index b25117ad9a..36e6f075de 100644 --- a/sentry/src/main/java/io/sentry/JavaMemoryCollector.java +++ b/sentry/src/main/java/io/sentry/JavaMemoryCollector.java @@ -12,12 +12,10 @@ public final class JavaMemoryCollector implements ICollector { public void setup() {} @Override - public void collect(@NotNull Iterable performanceCollectionData) { + public void collect(final @NotNull PerformanceCollectionData performanceCollectionData) { final long now = System.currentTimeMillis(); final long usedMemory = runtime.totalMemory() - runtime.freeMemory(); MemoryCollectionData memoryData = new MemoryCollectionData(now, usedMemory); - for (PerformanceCollectionData data : performanceCollectionData) { - data.addMemoryData(memoryData); - } + performanceCollectionData.addMemoryData(memoryData); } } diff --git a/sentry/src/main/java/io/sentry/NoOpTransactionPerformanceCollector.java b/sentry/src/main/java/io/sentry/NoOpTransactionPerformanceCollector.java index 8a0bd40b59..02edeb1094 100644 --- a/sentry/src/main/java/io/sentry/NoOpTransactionPerformanceCollector.java +++ b/sentry/src/main/java/io/sentry/NoOpTransactionPerformanceCollector.java @@ -1,5 +1,6 @@ package io.sentry; +import java.util.List; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -18,7 +19,7 @@ private NoOpTransactionPerformanceCollector() {} public void start(@NotNull ITransaction transaction) {} @Override - public @Nullable PerformanceCollectionData stop(@NotNull ITransaction transaction) { + public @Nullable List stop(@NotNull ITransaction transaction) { return null; } } diff --git a/sentry/src/main/java/io/sentry/NoOpTransactionProfiler.java b/sentry/src/main/java/io/sentry/NoOpTransactionProfiler.java index c0dffb42f5..9f51ea33a9 100644 --- a/sentry/src/main/java/io/sentry/NoOpTransactionProfiler.java +++ b/sentry/src/main/java/io/sentry/NoOpTransactionProfiler.java @@ -1,5 +1,6 @@ package io.sentry; +import java.util.List; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -18,7 +19,8 @@ public void onTransactionStart(@NotNull ITransaction transaction) {} @Override public @Nullable ProfilingTraceData onTransactionFinish( - @NotNull ITransaction transaction, @Nullable PerformanceCollectionData memoryCollectionData) { + @NotNull ITransaction transaction, + @Nullable List performanceCollectionData) { return null; } } diff --git a/sentry/src/main/java/io/sentry/PerformanceCollectionData.java b/sentry/src/main/java/io/sentry/PerformanceCollectionData.java index 1c5b79656e..af8837386a 100644 --- a/sentry/src/main/java/io/sentry/PerformanceCollectionData.java +++ b/sentry/src/main/java/io/sentry/PerformanceCollectionData.java @@ -1,55 +1,32 @@ package io.sentry; -import java.util.ArrayList; -import java.util.List; import org.jetbrains.annotations.ApiStatus; -import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @ApiStatus.Internal public final class PerformanceCollectionData { - private final @NotNull List memoryData = new ArrayList<>(); - private final @NotNull List cpuData = new ArrayList<>(); - private @Nullable MemoryCollectionData uncommittedMemoryData = null; - private @Nullable CpuCollectionData uncommittedCpuData = null; + private @Nullable MemoryCollectionData memoryData = null; + private @Nullable CpuCollectionData cpuData = null; - /** - * Add a {@link io.sentry.MemoryCollectionData} to internal uncommitted data. To save the data - * call {@code commitData}. Only the last uncommitted memory data will be retained. - */ + /** Store a {@link io.sentry.MemoryCollectionData}, if not null. */ public void addMemoryData(final @Nullable MemoryCollectionData memoryCollectionData) { if (memoryCollectionData != null) { - uncommittedMemoryData = memoryCollectionData; + memoryData = memoryCollectionData; } } - /** - * Add a {@link io.sentry.CpuCollectionData} to internal uncommitted data. To save the data call - * {@code commitData()}. Only the last uncommitted cpu data will be retained. - */ + /** Store a {@link io.sentry.CpuCollectionData}, if not null. */ public void addCpuData(final @Nullable CpuCollectionData cpuCollectionData) { if (cpuCollectionData != null) { - uncommittedCpuData = cpuCollectionData; + cpuData = cpuCollectionData; } } - /** Save any uncommitted data. */ - public void commitData() { - if (uncommittedMemoryData != null) { - memoryData.add(uncommittedMemoryData); - uncommittedMemoryData = null; - } - if (uncommittedCpuData != null) { - cpuData.add(uncommittedCpuData); - uncommittedCpuData = null; - } - } - - public @NotNull List getCpuData() { + public @Nullable CpuCollectionData getCpuData() { return cpuData; } - public @NotNull List getMemoryData() { + public @Nullable MemoryCollectionData getMemoryData() { return memoryData; } } diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index 5427f0749b..2ce67f06b3 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -396,7 +396,7 @@ public class SentryOptions { private final @NotNull List collectors = new ArrayList<>(); /** Performance collector that collect performance stats while transactions run. */ - private final @NotNull TransactionPerformanceCollector transactionPerformanceCollector = + private @NotNull TransactionPerformanceCollector transactionPerformanceCollector = NoOpTransactionPerformanceCollector.getInstance(); /** @@ -1900,6 +1900,17 @@ public void setMainThreadChecker(final @NotNull IMainThreadChecker mainThreadChe return transactionPerformanceCollector; } + /** + * Sets the performance collector used to collect performance stats while transactions run. + * + * @param transactionPerformanceCollector the performance collector. + */ + @ApiStatus.Internal + public void setTransactionPerformanceCollector( + final @NotNull TransactionPerformanceCollector transactionPerformanceCollector) { + this.transactionPerformanceCollector = transactionPerformanceCollector; + } + /** * Whether OPTIONS requests should be traced. * diff --git a/sentry/src/main/java/io/sentry/SentryTracer.java b/sentry/src/main/java/io/sentry/SentryTracer.java index 10c774a839..08574ce942 100644 --- a/sentry/src/main/java/io/sentry/SentryTracer.java +++ b/sentry/src/main/java/io/sentry/SentryTracer.java @@ -141,7 +141,9 @@ public SentryTracer( this.baggage = new Baggage(hub.getOptions().getLogger()); } - if (transactionPerformanceCollector != null) { + // We are currently sending the performance data only in profiles, so there's no point in + // collecting them if a profile is not sampled + if (transactionPerformanceCollector != null && Boolean.TRUE.equals(isProfileSampled())) { transactionPerformanceCollector.start(this); } @@ -345,7 +347,7 @@ public void finish(@Nullable SpanStatus status) { public void finish(@Nullable SpanStatus status, @Nullable SentryDate finishDate) { this.finishStatus = FinishStatus.finishing(status); if (!root.isFinished() && (!waitForChildren || hasAllChildrenFinished())) { - PerformanceCollectionData performanceCollectionData = null; + List performanceCollectionData = null; if (transactionPerformanceCollector != null) { performanceCollectionData = transactionPerformanceCollector.stop(this); } @@ -357,6 +359,9 @@ public void finish(@Nullable SpanStatus status, @Nullable SentryDate finishDate) .getTransactionProfiler() .onTransactionFinish(this, performanceCollectionData); } + if (performanceCollectionData != null) { + performanceCollectionData.clear(); + } // try to get the high precision timestamp from the root span SentryDate finishTimestamp = root.getFinishDate(); diff --git a/sentry/src/main/java/io/sentry/TransactionPerformanceCollector.java b/sentry/src/main/java/io/sentry/TransactionPerformanceCollector.java index 81a6803be8..013667d2cf 100644 --- a/sentry/src/main/java/io/sentry/TransactionPerformanceCollector.java +++ b/sentry/src/main/java/io/sentry/TransactionPerformanceCollector.java @@ -1,5 +1,6 @@ package io.sentry; +import java.util.List; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -8,5 +9,5 @@ public interface TransactionPerformanceCollector { void start(@NotNull ITransaction transaction); @Nullable - PerformanceCollectionData stop(@NotNull ITransaction transaction); + List stop(@NotNull ITransaction transaction); } diff --git a/sentry/src/test/java/io/sentry/TransactionPerformanceCollectorTest.kt b/sentry/src/test/java/io/sentry/DefaultTransactionPerformanceCollectorTest.kt similarity index 88% rename from sentry/src/test/java/io/sentry/TransactionPerformanceCollectorTest.kt rename to sentry/src/test/java/io/sentry/DefaultTransactionPerformanceCollectorTest.kt index 5d7b5b7718..b87de67f68 100644 --- a/sentry/src/test/java/io/sentry/TransactionPerformanceCollectorTest.kt +++ b/sentry/src/test/java/io/sentry/DefaultTransactionPerformanceCollectorTest.kt @@ -24,7 +24,7 @@ import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue -class TransactionPerformanceCollectorTest { +class DefaultTransactionPerformanceCollectorTest { private val className = "io.sentry.DefaultTransactionPerformanceCollector" private val ctorTypes: Array> = arrayOf(SentryOptions::class.java) @@ -51,10 +51,8 @@ class TransactionPerformanceCollectorTest { val mockCpuCollector: ICollector = object : ICollector { override fun setup() {} - override fun collect(performanceCollectionData: Iterable) { - performanceCollectionData.forEach { - it.addCpuData(mock()) - } + override fun collect(performanceCollectionData: PerformanceCollectionData) { + performanceCollectionData.addCpuData(mock()) } } @@ -150,11 +148,18 @@ class TransactionPerformanceCollectorTest { // There are no more transactions running: the time should stop now verify(fixture.mockTimer)!!.cancel() + assertNotNull(data1) + assertNotNull(data2) + val memoryData1 = data1.map { it.memoryData } + val cpuData1 = data1.map { it.cpuData } + val memoryData2 = data2.map { it.memoryData } + val cpuData2 = data2.map { it.cpuData } + // The data returned by the collector is not empty - assertFalse(data1!!.memoryData.isEmpty()) - assertFalse(data1.cpuData.isEmpty()) - assertFalse(data2!!.memoryData.isEmpty()) - assertFalse(data2.cpuData.isEmpty()) + assertFalse(memoryData1.isEmpty()) + assertFalse(cpuData1.isEmpty()) + assertFalse(memoryData2.isEmpty()) + assertFalse(cpuData2.isEmpty()) } @Test @@ -169,10 +174,9 @@ class TransactionPerformanceCollectorTest { fixture.lastScheduledRunnable?.run() verify(fixture.mockTimer)!!.cancel() - // Data is returned even after the collector times out + // Data is deleted after the collector times out val data1 = collector.stop(fixture.transaction1) - assertFalse(data1!!.memoryData.isEmpty()) - assertFalse(data1.cpuData.isEmpty()) + assertNull(data1) } @Test @@ -193,13 +197,16 @@ class TransactionPerformanceCollectorTest { // Let's sleep to make the collector get values Thread.sleep(300) val data1 = collector.stop(fixture.transaction1) + assertNotNull(data1) + val memoryData = data1.map { it.memoryData } + val cpuData = data1.map { it.cpuData } // The data returned by the collector is not empty - assertFalse(data1!!.memoryData.isEmpty()) - assertFalse(data1.cpuData.isEmpty()) + assertFalse(memoryData.isEmpty()) + assertFalse(cpuData.isEmpty()) // We have the same number of memory and cpu data, even if we have 2 memory collectors and 1 cpu collector - assertEquals(data1.memoryData.size, data1.cpuData.size) + assertEquals(memoryData.size, cpuData.size) } @Test @@ -226,7 +233,7 @@ class TransactionPerformanceCollectorTest { } } - override fun collect(performanceCollectionData: MutableIterable) { + override fun collect(performanceCollectionData: PerformanceCollectionData) { if (mainThreadChecker.isMainThread) { throw AssertionError("collect() was called in the main thread") } diff --git a/sentry/src/test/java/io/sentry/JavaMemoryCollectorTest.kt b/sentry/src/test/java/io/sentry/JavaMemoryCollectorTest.kt index 8af6b2ab4d..e2914edff6 100644 --- a/sentry/src/test/java/io/sentry/JavaMemoryCollectorTest.kt +++ b/sentry/src/test/java/io/sentry/JavaMemoryCollectorTest.kt @@ -2,8 +2,8 @@ package io.sentry import kotlin.test.Test import kotlin.test.assertEquals -import kotlin.test.assertFalse import kotlin.test.assertNotEquals +import kotlin.test.assertNotNull class JavaMemoryCollectorTest { @@ -16,15 +16,13 @@ class JavaMemoryCollectorTest { @Test fun `when collect, only heap memory is collected`() { - val performanceCollectionData = PerformanceCollectionData() - val data = listOf(performanceCollectionData) + val data = PerformanceCollectionData() val usedMemory = fixture.runtime.totalMemory() - fixture.runtime.freeMemory() fixture.collector.collect(data) - performanceCollectionData.commitData() - val memoryData = performanceCollectionData.memoryData - assertFalse(memoryData.isEmpty()) - assertEquals(-1, memoryData.first().usedNativeMemory) - assertEquals(usedMemory, memoryData.first().usedHeapMemory) - assertNotEquals(0, memoryData.first().timestampMillis) + val memoryData = data.memoryData + assertNotNull(memoryData) + assertEquals(-1, memoryData.usedNativeMemory) + assertEquals(usedMemory, memoryData.usedHeapMemory) + assertNotEquals(0, memoryData.timestampMillis) } } diff --git a/sentry/src/test/java/io/sentry/PerformanceCollectionDataTest.kt b/sentry/src/test/java/io/sentry/PerformanceCollectionDataTest.kt index d0db2807bd..e105e105c6 100644 --- a/sentry/src/test/java/io/sentry/PerformanceCollectionDataTest.kt +++ b/sentry/src/test/java/io/sentry/PerformanceCollectionDataTest.kt @@ -1,11 +1,9 @@ package io.sentry -import org.mockito.kotlin.mock import kotlin.test.Test import kotlin.test.assertEquals -import kotlin.test.assertFalse import kotlin.test.assertNotEquals -import kotlin.test.assertTrue +import kotlin.test.assertNull class PerformanceCollectionDataTest { @@ -16,45 +14,25 @@ class PerformanceCollectionDataTest { } @Test - fun `memory data is saved only after commitData`() { - val data = fixture.getSut() - data.addMemoryData(mock()) - assertTrue(data.memoryData.isEmpty()) - data.commitData() - assertFalse(data.memoryData.isEmpty()) - } - - @Test - fun `cpu data is saved only after commitData`() { - val data = fixture.getSut() - data.addCpuData(mock()) - assertTrue(data.cpuData.isEmpty()) - data.commitData() - assertFalse(data.cpuData.isEmpty()) - } - - @Test - fun `only the last of multiple memory data is saved on commit`() { + fun `only the last of multiple memory data is saved`() { val data = fixture.getSut() val memData1 = MemoryCollectionData(0, 0, 0) val memData2 = MemoryCollectionData(1, 1, 1) data.addMemoryData(memData1) data.addMemoryData(memData2) - data.commitData() - val savedMemoryData = data.memoryData.first() + val savedMemoryData = data.memoryData assertNotEquals(memData1, savedMemoryData) assertEquals(memData2, savedMemoryData) } @Test - fun `only the last of multiple cpu data is saved on commit`() { + fun `only the last of multiple cpu data is saved`() { val data = fixture.getSut() val cpuData1 = CpuCollectionData(0, 0.0) val cpuData2 = CpuCollectionData(1, 1.0) data.addCpuData(cpuData1) data.addCpuData(cpuData2) - data.commitData() - val savedCpuData = data.cpuData.first() + val savedCpuData = data.cpuData assertNotEquals(cpuData1, savedCpuData) assertEquals(cpuData2, savedCpuData) } @@ -66,23 +44,8 @@ class PerformanceCollectionDataTest { data.addCpuData(cpuData1) data.addCpuData(null) data.addMemoryData(null) - data.commitData() - assertEquals(1, data.cpuData.size) - assertTrue(data.memoryData.isEmpty()) - val savedCpuData = data.cpuData.first() + assertNull(data.memoryData) + val savedCpuData = data.cpuData assertEquals(cpuData1, savedCpuData) } - - @Test - fun `committing multiple times does not duplicate values`() { - val data = fixture.getSut() - data.addCpuData(mock()) - data.addMemoryData(mock()) - data.commitData() - assertEquals(1, data.cpuData.size) - assertEquals(1, data.memoryData.size) - data.commitData() - assertEquals(1, data.cpuData.size) - assertEquals(1, data.memoryData.size) - } } diff --git a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt index 1906bb679a..32a2420a78 100644 --- a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt @@ -388,7 +388,21 @@ class SentryOptionsTest { assertEquals("${File.separator}test${File.separator}${hash}${File.separator}profiling_traces", options.profilingTracesDirPath) } + @Test fun `when options are initialized, idleTimeout is 3000`() { assertEquals(3000L, SentryOptions().idleTimeout) } + + @Test + fun `when options are initialized, TransactionPerformanceCollector is a NoOp`() { + assertEquals(SentryOptions().transactionPerformanceCollector, NoOpTransactionPerformanceCollector.getInstance()) + } + + @Test + fun `when setTransactionPerformanceCollector is called, overrides default`() { + val performanceCollector = mock() + val options = SentryOptions() + options.transactionPerformanceCollector = performanceCollector + assertEquals(performanceCollector, options.transactionPerformanceCollector) + } } diff --git a/sentry/src/test/java/io/sentry/SentryTest.kt b/sentry/src/test/java/io/sentry/SentryTest.kt index d0c01aad40..f6258fcf2b 100644 --- a/sentry/src/test/java/io/sentry/SentryTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTest.kt @@ -381,7 +381,7 @@ class SentryTest { private class CustomMemoryCollector : ICollector { override fun setup() {} - override fun collect(performanceCollectionData: MutableIterable) {} + override fun collect(performanceCollectionData: PerformanceCollectionData) {} } private class CustomModulesLoader : IModulesLoader { diff --git a/sentry/src/test/java/io/sentry/SentryTracerTest.kt b/sentry/src/test/java/io/sentry/SentryTracerTest.kt index d25631a5a3..a3cf806e2e 100644 --- a/sentry/src/test/java/io/sentry/SentryTracerTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTracerTest.kt @@ -45,10 +45,11 @@ class SentryTracerTest { idleTimeout: Long? = null, trimEnd: Boolean = false, transactionFinishedCallback: TransactionFinishedCallback? = null, - samplingDecision: TracesSamplingDecision? = null + samplingDecision: TracesSamplingDecision? = null, + performanceCollector: TransactionPerformanceCollector? = transactionPerformanceCollector ): SentryTracer { optionsConfiguration.configure(options) - return SentryTracer(TransactionContext("name", "op", samplingDecision), hub, startTimestamp, waitForChildren, idleTimeout, trimEnd, transactionFinishedCallback, transactionPerformanceCollector) + return SentryTracer(TransactionContext("name", "op", samplingDecision), hub, startTimestamp, waitForChildren, idleTimeout, trimEnd, transactionFinishedCallback, performanceCollector) } } @@ -888,8 +889,16 @@ class SentryTracerTest { } @Test - fun `when transaction is created, transactionPerformanceCollector is started`() { + fun `when transaction is created, but not profiled, transactionPerformanceCollector is not started`() { val transaction = fixture.getSut() + verify(fixture.transactionPerformanceCollector, never()).start(anyOrNull()) + } + + @Test + fun `when transaction is created and profiled transactionPerformanceCollector is started`() { + val transaction = fixture.getSut(optionsConfiguration = { + it.profilesSampleRate = 1.0 + }, samplingDecision = TracesSamplingDecision(true, null, true, null)) verify(fixture.transactionPerformanceCollector).start(check { assertEquals(transaction, it) }) } @@ -909,4 +918,18 @@ class SentryTracerTest { assertEquals("new-name-2", transaction.name) assertEquals(TransactionNameSource.CUSTOM, transaction.transactionNameSource) } + + @Test + fun `when transaction is finished, collected performance data is cleared`() { + val data = mutableListOf(mock(), mock()) + val mockPerformanceCollector = object : TransactionPerformanceCollector { + override fun start(transaction: ITransaction) {} + override fun stop(transaction: ITransaction): MutableList = data + } + val transaction = fixture.getSut(optionsConfiguration = { + it.profilesSampleRate = 1.0 + }, performanceCollector = mockPerformanceCollector) + transaction.finish() + assertTrue(data.isEmpty()) + } } From be4a0bafafc66ec974590ddd62fb3f1be7cda31c Mon Sep 17 00:00:00 2001 From: getsentry-bot Date: Mon, 6 Feb 2023 15:00:49 +0000 Subject: [PATCH 3/7] release: 6.13.1 --- CHANGELOG.md | 2 +- gradle.properties | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 739bd707b2..137a738287 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## Unreleased +## 6.13.1 ### Fixes diff --git a/gradle.properties b/gradle.properties index 87ea904298..955c6be9fc 100644 --- a/gradle.properties +++ b/gradle.properties @@ -10,7 +10,7 @@ android.useAndroidX=true android.defaults.buildfeatures.buildconfig=true # Release information -versionName=6.13.0 +versionName=6.13.1 # Override the SDK name on native crashes on Android sentryAndroidSdkName=sentry.native.android From 754021cd6c6cb9de539fc3eeca8695872d8fd52a Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Tue, 7 Feb 2023 16:43:01 +0100 Subject: [PATCH 4/7] Ignore Shutdown in progress when closing ShutdownHookIntegration (#2521) Co-authored-by: Markus Hintersteiner --- CHANGELOG.md | 6 +++++ .../io/sentry/ShutdownHookIntegration.java | 12 ++++++++- .../io/sentry/ShutdownHookIntegrationTest.kt | 27 +++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 137a738287..f7cafab10f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Fixes + +- Ignore Shutdown in progress when closing ShutdownHookIntegration ([#2521](https://github.com/getsentry/sentry-java/pull/2521)) + ## 6.13.1 ### Fixes diff --git a/sentry/src/main/java/io/sentry/ShutdownHookIntegration.java b/sentry/src/main/java/io/sentry/ShutdownHookIntegration.java index 17e93ab891..e3c7a29f9b 100644 --- a/sentry/src/main/java/io/sentry/ShutdownHookIntegration.java +++ b/sentry/src/main/java/io/sentry/ShutdownHookIntegration.java @@ -41,7 +41,17 @@ public void register(final @NotNull IHub hub, final @NotNull SentryOptions optio @Override public void close() throws IOException { if (thread != null) { - runtime.removeShutdownHook(thread); + try { + runtime.removeShutdownHook(thread); + } catch (IllegalStateException e) { + @Nullable final String message = e.getMessage(); + // https://github.com/openjdk/jdk/blob/09b8a1959771213cb982d062f0a913285e4a0c6e/src/java.base/share/classes/java/lang/ApplicationShutdownHooks.java#L83 + if (message != null && message.equals("Shutdown in progress")) { + // ignore + } else { + throw e; + } + } } } diff --git a/sentry/src/test/java/io/sentry/ShutdownHookIntegrationTest.kt b/sentry/src/test/java/io/sentry/ShutdownHookIntegrationTest.kt index aef8d50723..f7e3848cad 100644 --- a/sentry/src/test/java/io/sentry/ShutdownHookIntegrationTest.kt +++ b/sentry/src/test/java/io/sentry/ShutdownHookIntegrationTest.kt @@ -5,7 +5,9 @@ import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever import kotlin.test.Test +import kotlin.test.assertFails import kotlin.test.assertNotNull class ShutdownHookIntegrationTest { @@ -77,4 +79,29 @@ class ShutdownHookIntegrationTest { verify(fixture.hub).flush(eq(10000)) } + + @Test + fun `shutdown in progress is handled gracefully`() { + val integration = fixture.getSut() + whenever(fixture.runtime.removeShutdownHook(any())).thenThrow(java.lang.IllegalStateException("Shutdown in progress")) + + integration.register(fixture.hub, fixture.options) + integration.close() + + verify(fixture.runtime).removeShutdownHook(any()) + } + + @Test + fun `non shutdown in progress during removeShutdownHook is rethrown`() { + val integration = fixture.getSut() + whenever(fixture.runtime.removeShutdownHook(any())).thenThrow(java.lang.IllegalStateException()) + + integration.register(fixture.hub, fixture.options) + + assertFails { + integration.close() + } + + verify(fixture.runtime).removeShutdownHook(any()) + } } From b8fb344d14cb8a23c3101daeeab7cb4c01cdac1e Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Wed, 8 Feb 2023 08:11:47 +0100 Subject: [PATCH 5/7] Fix app start span end-time is wrong if SDK init is deferred (#2519) --- CHANGELOG.md | 1 + .../api/sentry-android-core.api | 1 + .../core/ActivityLifecycleIntegration.java | 50 ++++--- .../io/sentry/android/core/AppStartState.java | 16 +++ .../core/SentryPerformanceProvider.java | 16 ++- .../core/ActivityLifecycleIntegrationTest.kt | 127 ++++++++++++++---- .../core/SentryPerformanceProviderTest.kt | 24 ++++ 7 files changed, 184 insertions(+), 51 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f7cafab10f..45d8b0873b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixes - Ignore Shutdown in progress when closing ShutdownHookIntegration ([#2521](https://github.com/getsentry/sentry-java/pull/2521)) +- Fix app start span end-time is wrong if SDK init is deferred ([#2519](https://github.com/getsentry/sentry-java/pull/2519)) ## 6.13.1 diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 20ea1e97d9..8aa2350d0d 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -71,6 +71,7 @@ public final class io/sentry/android/core/AppLifecycleIntegration : io/sentry/In } public final class io/sentry/android/core/AppStartState { + public fun getAppStartEndTime ()Lio/sentry/SentryDate; public fun getAppStartInterval ()Ljava/lang/Long; public fun getAppStartMillis ()Ljava/lang/Long; public fun getAppStartTime ()Lio/sentry/SentryDate; diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java index d448c92b21..4e00390771 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java @@ -56,8 +56,7 @@ public final class ActivityLifecycleIntegration private boolean isAllActivityCallbacksAvailable; private boolean firstActivityCreated = false; - private boolean firstActivityResumed = false; - private boolean foregroundImportance = false; + private final boolean foregroundImportance; private @Nullable ISpan appStartSpan; private final @NotNull WeakHashMap ttidSpanMap = new WeakHashMap<>(); @@ -210,6 +209,11 @@ private void startTracing(final @NotNull Activity activity) { getAppStartDesc(coldStart), appStartTime, Instrumenter.SENTRY); + + // in case there's already an end time (e.g. due to deferred SDK init) + // we can finish the app-start span + finishAppStartSpan(); + // The first activity ttidSpan should start at the same time as the app start time ttidSpanMap.put( activity, @@ -328,28 +332,17 @@ public synchronized void onActivityStarted(final @NotNull Activity activity) { @SuppressLint("NewApi") @Override public synchronized void onActivityResumed(final @NotNull Activity activity) { - if (!firstActivityResumed) { - - // we only finish the app start if the process is of foregroundImportance - if (foregroundImportance) { - // sets App start as finished when the very first activity calls onResume - AppStartState.getInstance().setAppStartEnd(); - } else { - if (options != null) { - options - .getLogger() - .log( - SentryLevel.DEBUG, - "App Start won't be reported because Process wasn't of foregroundImportance."); - } - } - // finishes app start span - if (performanceEnabled && appStartSpan != null) { - appStartSpan.finish(); - } - firstActivityResumed = true; + // app start span + @Nullable final SentryDate appStartStartTime = AppStartState.getInstance().getAppStartTime(); + @Nullable final SentryDate appStartEndTime = AppStartState.getInstance().getAppStartEndTime(); + // in case the SentryPerformanceProvider is disabled it does not set the app start times, + // and we need to set the end time manually here, + // the start time gets set manually in SentryAndroid.init() + if (appStartStartTime != null && appStartEndTime == null) { + AppStartState.getInstance().setAppStartEnd(); } + finishAppStartSpan(); final ISpan ttidSpan = ttidSpanMap.get(activity); final View rootView = activity.findViewById(android.R.id.content); @@ -507,4 +500,17 @@ private void setColdStart(final @Nullable Bundle savedInstanceState) { return APP_START_WARM; } } + + private void finishAppStartSpan() { + final @Nullable SentryDate appStartEndTime = AppStartState.getInstance().getAppStartEndTime(); + if (appStartSpan != null + && !appStartSpan.isFinished() + && performanceEnabled + && appStartEndTime != null) { + + final SpanStatus status = + appStartSpan.getStatus() != null ? appStartSpan.getStatus() : SpanStatus.OK; + appStartSpan.finish(status, appStartEndTime); + } + } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java b/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java index 27e2581e28..de690aa668 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java @@ -1,7 +1,9 @@ package io.sentry.android.core; import android.os.SystemClock; +import io.sentry.DateUtils; import io.sentry.SentryDate; +import io.sentry.SentryLongDate; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -84,6 +86,20 @@ public SentryDate getAppStartTime() { return appStartTime; } + @Nullable + public SentryDate getAppStartEndTime() { + @Nullable final SentryDate start = getAppStartTime(); + if (start != null) { + @Nullable final Long durationMillis = getAppStartInterval(); + if (durationMillis != null) { + final long startNanos = start.nanoTimestamp(); + final long endNanos = startNanos + DateUtils.millisToNanos(durationMillis); + return new SentryLongDate(endNanos); + } + } + return null; + } + @Nullable public Long getAppStartMillis() { return appStartMillis; diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java index ff94c61d36..7ba270351b 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java @@ -29,6 +29,8 @@ public final class SentryPerformanceProvider extends EmptySecureContentProvider private static long appStartMillis = SystemClock.uptimeMillis(); private boolean firstActivityCreated = false; + private boolean firstActivityResumed = false; + private @Nullable Application application; public SentryPerformanceProvider() { @@ -91,9 +93,6 @@ public void onActivityCreated(@NotNull Activity activity, @Nullable Bundle saved final boolean coldStart = savedInstanceState == null; AppStartState.getInstance().setColdStart(coldStart); - if (application != null) { - application.unregisterActivityLifecycleCallbacks(this); - } firstActivityCreated = true; } } @@ -102,7 +101,16 @@ public void onActivityCreated(@NotNull Activity activity, @Nullable Bundle saved public void onActivityStarted(@NotNull Activity activity) {} @Override - public void onActivityResumed(@NotNull Activity activity) {} + public void onActivityResumed(@NotNull Activity activity) { + if (!firstActivityResumed) { + // sets App start as finished when the very first activity calls onResume + firstActivityResumed = true; + AppStartState.getInstance().setAppStartEnd(); + } + if (application != null) { + application.unregisterActivityLifecycleCallbacks(this); + } + } @Override public void onActivityPaused(@NotNull Activity activity) {} diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt index 68acc87fd5..1f3005a5fe 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt @@ -6,6 +6,7 @@ import android.app.ActivityManager.RunningAppProcessInfo import android.app.Application import android.os.Bundle import io.sentry.Breadcrumb +import io.sentry.DateUtils import io.sentry.Hub import io.sentry.Scope import io.sentry.SentryDate @@ -650,66 +651,142 @@ class ActivityLifecycleIntegrationTest { } @Test - fun `App start end time is set`() { - val sut = fixture.getSut(14) + fun `When firstActivityCreated is true, start transaction with given appStartTime`() { + val sut = fixture.getSut() fixture.options.tracesSampleRate = 1.0 sut.register(fixture.hub, fixture.options) - setAppStartTime() + val date = SentryNanotimeDate(Date(0), 0) + setAppStartTime(date) val activity = mock() - sut.onActivityCreated(activity, null) - sut.onActivityResumed(activity) + sut.onActivityCreated(activity, fixture.bundle) - // SystemClock.uptimeMillis() always returns 0, can't assert real values - assertNotNull(AppStartState.getInstance().appStartInterval) + // call only once + verify(fixture.hub).startTransaction(any(), check { assertEquals(date, it.startTimestamp) }) } @Test - fun `App start end time isnt set if not foregroundImportance`() { - val sut = fixture.getSut(14, importance = RunningAppProcessInfo.IMPORTANCE_BACKGROUND) + fun `When firstActivityCreated is true, do not create app start span if not foregroundImportance`() { + val sut = fixture.getSut(importance = RunningAppProcessInfo.IMPORTANCE_BACKGROUND) fixture.options.tracesSampleRate = 1.0 sut.register(fixture.hub, fixture.options) - setAppStartTime() + // usually set by SentryPerformanceProvider + val date = SentryNanotimeDate(Date(0), 0) + setAppStartTime(date) + AppStartState.getInstance().setAppStartEnd(1) val activity = mock() - sut.onActivityCreated(activity, null) - sut.onActivityResumed(activity) + sut.onActivityCreated(activity, fixture.bundle) - assertNull(AppStartState.getInstance().appStartInterval) + // call only once + verify(fixture.hub).startTransaction(any(), check { assertNull(it.startTimestamp) }) } @Test - fun `When firstActivityCreated is true, start transaction with given appStartTime`() { - val sut = fixture.getSut() + fun `Create and finish app start span immediately in case SDK init is deferred`() { + val sut = fixture.getSut(importance = RunningAppProcessInfo.IMPORTANCE_FOREGROUND) fixture.options.tracesSampleRate = 1.0 sut.register(fixture.hub, fixture.options) - val date = SentryNanotimeDate(Date(0), 0) - setAppStartTime(date) + // usually set by SentryPerformanceProvider + val startDate = SentryNanotimeDate(Date(0), 0) + setAppStartTime(startDate) + AppStartState.getInstance().setColdStart(false) + AppStartState.getInstance().setAppStartEnd(1) + + val endDate = AppStartState.getInstance().appStartEndTime!! val activity = mock() sut.onActivityCreated(activity, fixture.bundle) - // call only once - verify(fixture.hub).startTransaction(any(), check { assertEquals(date, it.startTimestamp) }) + val appStartSpanCount = fixture.transaction.children.count { + it.spanContext.operation.startsWith("app.start.warm") && + it.startDate.nanoTimestamp() == startDate.nanoTimestamp() && + it.finishDate!!.nanoTimestamp() == endDate.nanoTimestamp() + } + assertEquals(1, appStartSpanCount) } @Test - fun `When firstActivityCreated is true, do not use appStartTime if not foregroundImportance`() { - val sut = fixture.getSut(importance = RunningAppProcessInfo.IMPORTANCE_BACKGROUND) + fun `When SentryPerformanceProvider is disabled, app start time span is still created`() { + val sut = fixture.getSut(importance = RunningAppProcessInfo.IMPORTANCE_FOREGROUND) fixture.options.tracesSampleRate = 1.0 sut.register(fixture.hub, fixture.options) - val date = SentryNanotimeDate(Date(0), 0) - setAppStartTime(date) + // usually done by SentryPerformanceProvider, if disabled it's done by + // SentryAndroid.init + val startDate = SentryNanotimeDate(Date(0), 0) + setAppStartTime(startDate) + AppStartState.getInstance().setColdStart(false) + // when activity is created val activity = mock() sut.onActivityCreated(activity, fixture.bundle) + // then app-start end time should still be null + assertNull(AppStartState.getInstance().appStartEndTime) - // call only once - verify(fixture.hub).startTransaction(any(), check { assertNull(it.startTimestamp) }) + // when activity is resumed + sut.onActivityResumed(activity) + // end-time should be set + assertNotNull(AppStartState.getInstance().appStartEndTime) + } + + @Test + fun `When app-start end time is already set, it should not be overwritten`() { + val sut = fixture.getSut(importance = RunningAppProcessInfo.IMPORTANCE_FOREGROUND) + fixture.options.tracesSampleRate = 1.0 + sut.register(fixture.hub, fixture.options) + + // usually done by SentryPerformanceProvider + val startDate = SentryNanotimeDate(Date(0), 0) + setAppStartTime(startDate) + AppStartState.getInstance().setColdStart(false) + AppStartState.getInstance().setAppStartEnd(1234) + + // when activity is created and resumed + val activity = mock() + sut.onActivityCreated(activity, fixture.bundle) + sut.onActivityResumed(activity) + + // then the end time should not be overwritten + assertEquals( + DateUtils.millisToNanos(1234), + AppStartState.getInstance().appStartEndTime!!.nanoTimestamp() + ) + } + + @Test + fun `When activity lifecycle happens multiple times, app-start end time should not be overwritten`() { + val sut = fixture.getSut(importance = RunningAppProcessInfo.IMPORTANCE_FOREGROUND) + fixture.options.tracesSampleRate = 1.0 + sut.register(fixture.hub, fixture.options) + + // usually done by SentryPerformanceProvider + val startDate = SentryNanotimeDate(Date(0), 0) + setAppStartTime(startDate) + AppStartState.getInstance().setColdStart(false) + + // when activity is created, started and resumed multiple times + val activity = mock() + sut.onActivityCreated(activity, fixture.bundle) + sut.onActivityStarted(activity) + sut.onActivityResumed(activity) + + val firstAppStartEndTime = AppStartState.getInstance().appStartEndTime + + Thread.sleep(1) + sut.onActivityPaused(activity) + sut.onActivityStopped(activity) + sut.onActivityStarted(activity) + sut.onActivityResumed(activity) + + // then the end time should not be overwritten + assertEquals( + firstAppStartEndTime!!.nanoTimestamp(), + AppStartState.getInstance().appStartEndTime!!.nanoTimestamp() + ) } @Test diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SentryPerformanceProviderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SentryPerformanceProviderTest.kt index 611c087dd1..cf3ca7c2ea 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SentryPerformanceProviderTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SentryPerformanceProviderTest.kt @@ -1,16 +1,20 @@ package io.sentry.android.core +import android.app.Application import android.content.pm.ProviderInfo import android.os.Bundle import androidx.test.ext.junit.runners.AndroidJUnit4 import io.sentry.SentryNanotimeDate import org.junit.runner.RunWith +import org.mockito.kotlin.any import org.mockito.kotlin.mock +import org.mockito.kotlin.verify import java.util.Date import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse +import kotlin.test.assertNotNull import kotlin.test.assertTrue @RunWith(AndroidJUnit4::class) @@ -73,6 +77,26 @@ class SentryPerformanceProviderTest { assertFalse(AppStartState.getInstance().isColdStart!!) } + @Test + fun `provider sets app start end on first activity resume, and unregisters afterwards`() { + val providerInfo = ProviderInfo() + + val mockContext = ContextUtilsTest.createMockContext(true) + providerInfo.authority = AUTHORITY + + val provider = SentryPerformanceProvider() + provider.attachInfo(mockContext, providerInfo) + + provider.onActivityCreated(mock(), Bundle()) + provider.onActivityResumed(mock()) + + assertNotNull(AppStartState.getInstance().appStartInterval) + assertNotNull(AppStartState.getInstance().appStartEndTime) + + verify((mockContext.applicationContext as Application)) + .unregisterActivityLifecycleCallbacks(any()) + } + companion object { private const val AUTHORITY = "io.sentry.sample.SentryPerformanceProvider" } From 03c5163db3ac3eaba6db990641ab13728d772ca9 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos <6349682+vaind@users.noreply.github.com> Date: Wed, 8 Feb 2023 09:12:01 +0100 Subject: [PATCH 6/7] chore: use github token with saucectl-run-action to avoid GH API rate limits (#2527) --- .github/workflows/integration-tests-benchmarks.yml | 4 ++++ .github/workflows/integration-tests-ui.yml | 2 ++ 2 files changed, 6 insertions(+) diff --git a/.github/workflows/integration-tests-benchmarks.yml b/.github/workflows/integration-tests-benchmarks.yml index a7e05c3570..eb9e25bcb8 100644 --- a/.github/workflows/integration-tests-benchmarks.yml +++ b/.github/workflows/integration-tests-benchmarks.yml @@ -42,6 +42,8 @@ jobs: - name: Run All Tests in SauceLab uses: saucelabs/saucectl-run-action@889cc2382b05b47e4a78bd35516603acc6c15fad # pin@v2 if: github.event_name != 'pull_request' && env.SAUCE_USERNAME != null + env: + GITHUB_TOKEN: ${{ github.token }} with: sauce-username: ${{ secrets.SAUCE_USERNAME }} sauce-access-key: ${{ secrets.SAUCE_ACCESS_KEY }} @@ -50,6 +52,8 @@ jobs: - name: Run one test in SauceLab uses: saucelabs/saucectl-run-action@889cc2382b05b47e4a78bd35516603acc6c15fad # pin@v2 if: github.event_name == 'pull_request' && env.SAUCE_USERNAME != null + env: + GITHUB_TOKEN: ${{ github.token }} with: sauce-username: ${{ secrets.SAUCE_USERNAME }} sauce-access-key: ${{ secrets.SAUCE_ACCESS_KEY }} diff --git a/.github/workflows/integration-tests-ui.yml b/.github/workflows/integration-tests-ui.yml index ff7313583d..9ad6b837d7 100644 --- a/.github/workflows/integration-tests-ui.yml +++ b/.github/workflows/integration-tests-ui.yml @@ -36,6 +36,8 @@ jobs: - name: Run Tests in SauceLab uses: saucelabs/saucectl-run-action@889cc2382b05b47e4a78bd35516603acc6c15fad # pin@v2 + env: + GITHUB_TOKEN: ${{ github.token }} with: sauce-username: ${{ secrets.SAUCE_USERNAME }} sauce-access-key: ${{ secrets.SAUCE_ACCESS_KEY }} From fe30606689aa4c20db711bb91bfd369169ee18f5 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com> Date: Wed, 8 Feb 2023 13:01:49 +0100 Subject: [PATCH 7/7] Add `main` flag to threads and `in_foreground` flag for app contexts (#2516) --- CHANGELOG.md | 4 + .../core/DefaultAndroidEventProcessor.java | 80 ++++++++++++------- .../sentry/android/core/LifecycleWatcher.java | 3 + .../core/CustomCachedApplyScopeDataHint.kt | 6 ++ .../core/DefaultAndroidEventProcessorTest.kt | 59 +++++++++++++- sentry/api/sentry.api | 6 ++ .../src/main/java/io/sentry/protocol/App.java | 22 +++++ .../java/io/sentry/protocol/SentryThread.java | 31 +++++++ .../sentry/protocol/AppSerializationTest.kt | 1 + .../test/java/io/sentry/protocol/AppTest.kt | 7 +- .../protocol/SentryThreadSerializationTest.kt | 1 + sentry/src/test/resources/json/app.json | 3 +- sentry/src/test/resources/json/contexts.json | 3 +- .../resources/json/sentry_base_event.json | 3 +- .../src/test/resources/json/sentry_event.json | 4 +- .../test/resources/json/sentry_thread.json | 1 + .../resources/json/sentry_transaction.json | 3 +- ...sentry_transaction_legacy_date_format.json | 3 +- 18 files changed, 201 insertions(+), 39 deletions(-) create mode 100644 sentry-android-core/src/test/java/io/sentry/android/core/CustomCachedApplyScopeDataHint.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index 45d8b0873b..ef593e197c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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)) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java index 93e634e154..6a50b66577 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java @@ -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()); @@ -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); @@ -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) { + thread.setMain(isMainThread); } } } @@ -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) { + // 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") @@ -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 @@ -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); @@ -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); @@ -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); @@ -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); @@ -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); @@ -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); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java b/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java index 6229a0798c..9789393580 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java @@ -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); } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/CustomCachedApplyScopeDataHint.kt b/sentry-android-core/src/test/java/io/sentry/android/core/CustomCachedApplyScopeDataHint.kt new file mode 100644 index 0000000000..b9e83420ff --- /dev/null +++ b/sentry-android-core/src/test/java/io/sentry/android/core/CustomCachedApplyScopeDataHint.kt @@ -0,0 +1,6 @@ +package io.sentry.android.core + +import io.sentry.hints.ApplyScopeData +import io.sentry.hints.Cached + +class CustomCachedApplyScopeDataHint : Cached, ApplyScopeData diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt index 7a8ef41498..a1ef274a8f 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt @@ -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 @@ -56,7 +57,7 @@ class DefaultAndroidEventProcessorTest { private class Fixture { val buildInfo = mock() val options = SentryAndroidOptions().apply { - setDebug(true) + isDebug = true setLogger(mock()) sdkVersion = SdkVersion("test", "1.2.3") } @@ -77,6 +78,7 @@ class DefaultAndroidEventProcessorTest { @BeforeTest fun `set up`() { context = ApplicationProvider.getApplicationContext() + AppState.getInstance().resetInstance() } @Test @@ -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 { @@ -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) } } } @@ -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) } } } @@ -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) + } + } } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 8692d59010..ea163bd357 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -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 @@ -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 } @@ -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 ()V } @@ -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 @@ -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; diff --git a/sentry/src/main/java/io/sentry/protocol/App.java b/sentry/src/main/java/io/sentry/protocol/App.java index a88067c3ea..aa76f567fc 100644 --- a/sentry/src/main/java/io/sentry/protocol/App.java +++ b/sentry/src/main/java/io/sentry/protocol/App.java @@ -38,6 +38,11 @@ public final class App implements JsonUnknown, JsonSerializable { private @Nullable String appBuild; /** Application permissions in the form of "permission_name" : "granted|not_granted" */ private @Nullable Map permissions; + /** + * A flag indicating whether the app is in foreground or not. An app is in foreground when it's + * visible to the user. + */ + private @Nullable Boolean inForeground; public App() {} @@ -50,6 +55,7 @@ public App() {} this.buildType = app.buildType; this.deviceAppHash = app.deviceAppHash; this.permissions = CollectionUtils.newConcurrentHashMap(app.permissions); + this.inForeground = app.inForeground; this.unknown = CollectionUtils.newConcurrentHashMap(app.unknown); } @@ -122,6 +128,15 @@ public void setPermissions(@Nullable Map permissions) { this.permissions = permissions; } + @Nullable + public Boolean getInForeground() { + return inForeground; + } + + public void setInForeground(final @Nullable Boolean inForeground) { + this.inForeground = inForeground; + } + // region json @Nullable @@ -144,6 +159,7 @@ public static final class JsonKeys { public static final String APP_VERSION = "app_version"; public static final String APP_BUILD = "app_build"; public static final String APP_PERMISSIONS = "permissions"; + public static final String IN_FOREGROUND = "in_foreground"; } @Override @@ -174,6 +190,9 @@ public void serialize(@NotNull JsonObjectWriter writer, @NotNull ILogger logger) if (permissions != null && !permissions.isEmpty()) { writer.name(JsonKeys.APP_PERMISSIONS).value(logger, permissions); } + if (inForeground != null) { + writer.name(JsonKeys.IN_FOREGROUND).value(inForeground); + } if (unknown != null) { for (String key : unknown.keySet()) { Object value = unknown.get(key); @@ -220,6 +239,9 @@ public static final class Deserializer implements JsonDeserializer { CollectionUtils.newConcurrentHashMap( (Map) reader.nextObjectOrNull()); break; + case JsonKeys.IN_FOREGROUND: + app.inForeground = reader.nextBooleanOrNull(); + break; default: if (unknown == null) { unknown = new ConcurrentHashMap<>(); diff --git a/sentry/src/main/java/io/sentry/protocol/SentryThread.java b/sentry/src/main/java/io/sentry/protocol/SentryThread.java index f120611264..1c955064e3 100644 --- a/sentry/src/main/java/io/sentry/protocol/SentryThread.java +++ b/sentry/src/main/java/io/sentry/protocol/SentryThread.java @@ -35,6 +35,7 @@ public final class SentryThread implements JsonUnknown, JsonSerializable { private @Nullable Boolean crashed; private @Nullable Boolean current; private @Nullable Boolean daemon; + private @Nullable Boolean main; private @Nullable SentryStackTrace stacktrace; @SuppressWarnings("unused") @@ -166,6 +167,29 @@ public void setDaemon(final @Nullable Boolean daemon) { this.daemon = daemon; } + /** + * If applicable, a flag indicating whether the thread was responsible for rendering the user + * interface. On mobile platforms this is oftentimes referred to as the "main thread" or "ui + * thread". + * + * @return if its the main thread or not + */ + @Nullable + public Boolean isMain() { + return main; + } + + /** + * If applicable, a flag indicating whether the thread was responsible for rendering the user + * interface. On mobile platforms this is oftentimes referred to as the "main thread" or "ui + * thread". + * + * @param main if its the main thread or not + */ + public void setMain(final @Nullable Boolean main) { + this.main = main; + } + /** * Gets the state of the thread. * @@ -205,6 +229,7 @@ public static final class JsonKeys { public static final String CRASHED = "crashed"; public static final String CURRENT = "current"; public static final String DAEMON = "daemon"; + public static final String MAIN = "main"; public static final String STACKTRACE = "stacktrace"; } @@ -233,6 +258,9 @@ public void serialize(@NotNull JsonObjectWriter writer, @NotNull ILogger logger) if (daemon != null) { writer.name(JsonKeys.DAEMON).value(daemon); } + if (main != null) { + writer.name(JsonKeys.MAIN).value(main); + } if (stacktrace != null) { writer.name(JsonKeys.STACKTRACE).value(logger, stacktrace); } @@ -278,6 +306,9 @@ public static final class Deserializer implements JsonDeserializer case JsonKeys.DAEMON: sentryThread.daemon = reader.nextBooleanOrNull(); break; + case JsonKeys.MAIN: + sentryThread.main = reader.nextBooleanOrNull(); + break; case JsonKeys.STACKTRACE: sentryThread.stacktrace = reader.nextOrNull(logger, new SentryStackTrace.Deserializer()); diff --git a/sentry/src/test/java/io/sentry/protocol/AppSerializationTest.kt b/sentry/src/test/java/io/sentry/protocol/AppSerializationTest.kt index 9eef2a6eb7..f07938d4c5 100644 --- a/sentry/src/test/java/io/sentry/protocol/AppSerializationTest.kt +++ b/sentry/src/test/java/io/sentry/protocol/AppSerializationTest.kt @@ -29,6 +29,7 @@ class AppSerializationTest { "WRITE_EXTERNAL_STORAGE" to "not_granted", "CAMERA" to "granted" ) + inForeground = true } } private val fixture = Fixture() diff --git a/sentry/src/test/java/io/sentry/protocol/AppTest.kt b/sentry/src/test/java/io/sentry/protocol/AppTest.kt index a40f5adf36..0a2c301615 100644 --- a/sentry/src/test/java/io/sentry/protocol/AppTest.kt +++ b/sentry/src/test/java/io/sentry/protocol/AppTest.kt @@ -18,8 +18,9 @@ class AppTest { app.appVersion = "app version" app.buildType = "build type" app.deviceAppHash = "device app hash" + app.inForeground = true val unknown = mapOf(Pair("unknown", "unknown")) - app.setUnknown(unknown) + app.unknown = unknown val clone = App(app) @@ -41,8 +42,9 @@ class AppTest { app.appVersion = "app version" app.buildType = "build type" app.deviceAppHash = "device app hash" + app.inForeground = true val unknown = mapOf(Pair("unknown", "unknown")) - app.setUnknown(unknown) + app.unknown = unknown val clone = App(app) @@ -55,6 +57,7 @@ class AppTest { assertEquals("app version", clone.appVersion) assertEquals("build type", clone.buildType) assertEquals("device app hash", clone.deviceAppHash) + assertEquals(true, clone.inForeground) assertNotNull(clone.unknown) { assertEquals("unknown", it["unknown"]) } diff --git a/sentry/src/test/java/io/sentry/protocol/SentryThreadSerializationTest.kt b/sentry/src/test/java/io/sentry/protocol/SentryThreadSerializationTest.kt index fd1494e216..c3c5597ae3 100644 --- a/sentry/src/test/java/io/sentry/protocol/SentryThreadSerializationTest.kt +++ b/sentry/src/test/java/io/sentry/protocol/SentryThreadSerializationTest.kt @@ -24,6 +24,7 @@ class SentryThreadSerializationTest { isCrashed = false isCurrent = false isDaemon = true + isMain = true stacktrace = SentryStackTrace().apply { frames = listOf( SentryStackFrame().apply { diff --git a/sentry/src/test/resources/json/app.json b/sentry/src/test/resources/json/app.json index 753d6b7580..5294b48e15 100644 --- a/sentry/src/test/resources/json/app.json +++ b/sentry/src/test/resources/json/app.json @@ -10,5 +10,6 @@ { "WRITE_EXTERNAL_STORAGE": "not_granted", "CAMERA": "granted" - } + }, + "in_foreground": true } diff --git a/sentry/src/test/resources/json/contexts.json b/sentry/src/test/resources/json/contexts.json index 153bd67c66..6f945a6834 100644 --- a/sentry/src/test/resources/json/contexts.json +++ b/sentry/src/test/resources/json/contexts.json @@ -12,7 +12,8 @@ { "WRITE_EXTERNAL_STORAGE": "not_granted", "CAMERA": "granted" - } + }, + "in_foreground": true }, "browser": { diff --git a/sentry/src/test/resources/json/sentry_base_event.json b/sentry/src/test/resources/json/sentry_base_event.json index bd75bc6ffa..b207cabc9f 100644 --- a/sentry/src/test/resources/json/sentry_base_event.json +++ b/sentry/src/test/resources/json/sentry_base_event.json @@ -15,7 +15,8 @@ { "WRITE_EXTERNAL_STORAGE": "not_granted", "CAMERA": "granted" - } + }, + "in_foreground": true }, "browser": { diff --git a/sentry/src/test/resources/json/sentry_event.json b/sentry/src/test/resources/json/sentry_event.json index 0d2cb76f33..bb637d1e30 100644 --- a/sentry/src/test/resources/json/sentry_event.json +++ b/sentry/src/test/resources/json/sentry_event.json @@ -23,6 +23,7 @@ "crashed": false, "current": false, "daemon": true, + "main": true, "stacktrace": { "frames": @@ -138,7 +139,8 @@ { "WRITE_EXTERNAL_STORAGE": "not_granted", "CAMERA": "granted" - } + }, + "in_foreground": true }, "browser": { diff --git a/sentry/src/test/resources/json/sentry_thread.json b/sentry/src/test/resources/json/sentry_thread.json index 5879c15578..da11f6f788 100644 --- a/sentry/src/test/resources/json/sentry_thread.json +++ b/sentry/src/test/resources/json/sentry_thread.json @@ -6,6 +6,7 @@ "crashed": false, "current": false, "daemon": true, + "main": true, "stacktrace": { "frames": diff --git a/sentry/src/test/resources/json/sentry_transaction.json b/sentry/src/test/resources/json/sentry_transaction.json index fd37108f85..38018b2481 100644 --- a/sentry/src/test/resources/json/sentry_transaction.json +++ b/sentry/src/test/resources/json/sentry_transaction.json @@ -58,7 +58,8 @@ { "WRITE_EXTERNAL_STORAGE": "not_granted", "CAMERA": "granted" - } + }, + "in_foreground": true }, "browser": { diff --git a/sentry/src/test/resources/json/sentry_transaction_legacy_date_format.json b/sentry/src/test/resources/json/sentry_transaction_legacy_date_format.json index 6fba7f71ce..8681cb9fc3 100644 --- a/sentry/src/test/resources/json/sentry_transaction_legacy_date_format.json +++ b/sentry/src/test/resources/json/sentry_transaction_legacy_date_format.json @@ -58,7 +58,8 @@ { "WRITE_EXTERNAL_STORAGE": "not_granted", "CAMERA": "granted" - } + }, + "in_foreground": true }, "browser": {