From 315a62f9a22e5668a01406b76653fb4f064fa16f Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Mon, 22 Apr 2024 11:57:00 +0200 Subject: [PATCH 1/5] Add start_type to app context --- .../PerformanceAndroidEventProcessor.java | 50 +++++++++------ .../PerformanceAndroidEventProcessorTest.kt | 62 +++++++++++++++++++ sentry/api/sentry.api | 3 + .../src/main/java/io/sentry/protocol/App.java | 27 +++++++- .../sentry/protocol/AppSerializationTest.kt | 1 + .../test/java/io/sentry/protocol/AppTest.kt | 3 + sentry/src/test/resources/json/app.json | 3 +- sentry/src/test/resources/json/contexts.json | 3 +- 8 files changed, 128 insertions(+), 24 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java index 2502722f85..db8814a91b 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java @@ -16,6 +16,7 @@ import io.sentry.android.core.performance.ActivityLifecycleTimeSpan; import io.sentry.android.core.performance.AppStartMetrics; import io.sentry.android.core.performance.TimeSpan; +import io.sentry.protocol.App; import io.sentry.protocol.MeasurementValue; import io.sentry.protocol.SentryId; import io.sentry.protocol.SentrySpan; @@ -79,26 +80,37 @@ public SentryEvent process(@NotNull SentryEvent event, @NotNull Hint hint) { // the app start measurement is only sent once and only if the transaction has // the app.start span, which is automatically created by the SDK. - if (!sentStartMeasurement && hasAppStartSpan(transaction)) { - final @NotNull TimeSpan appStartTimeSpan = - AppStartMetrics.getInstance().getAppStartTimeSpanWithFallback(options); - final long appStartUpDurationMs = appStartTimeSpan.getDurationMs(); - - // if appStartUpDurationMs is 0, metrics are not ready to be sent - if (appStartUpDurationMs != 0) { - final MeasurementValue value = - new MeasurementValue( - (float) appStartUpDurationMs, MeasurementUnit.Duration.MILLISECOND.apiName()); - - final String appStartKey = - AppStartMetrics.getInstance().getAppStartType() == AppStartMetrics.AppStartType.COLD - ? MeasurementValue.KEY_APP_START_COLD - : MeasurementValue.KEY_APP_START_WARM; - - transaction.getMeasurements().put(appStartKey, value); + if (hasAppStartSpan(transaction)) { + if (!sentStartMeasurement) { + final @NotNull TimeSpan appStartTimeSpan = + AppStartMetrics.getInstance().getAppStartTimeSpanWithFallback(options); + final long appStartUpDurationMs = appStartTimeSpan.getDurationMs(); + + // if appStartUpDurationMs is 0, metrics are not ready to be sent + if (appStartUpDurationMs != 0) { + final MeasurementValue value = + new MeasurementValue( + (float) appStartUpDurationMs, MeasurementUnit.Duration.MILLISECOND.apiName()); + + final String appStartKey = + AppStartMetrics.getInstance().getAppStartType() == AppStartMetrics.AppStartType.COLD + ? MeasurementValue.KEY_APP_START_COLD + : MeasurementValue.KEY_APP_START_WARM; + + transaction.getMeasurements().put(appStartKey, value); + + attachColdAppStartSpans(AppStartMetrics.getInstance(), transaction); + sentStartMeasurement = true; + } + } - attachColdAppStartSpans(AppStartMetrics.getInstance(), transaction); - sentStartMeasurement = true; + final @Nullable App appContext = transaction.getContexts().getApp(); + if (appContext != null) { + final String appStartType = + AppStartMetrics.getInstance().getAppStartType() == AppStartMetrics.AppStartType.COLD + ? "cold" + : "warm"; + appContext.setStartType(appStartType); } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt index 4c23691e63..dce5cb3998 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt @@ -16,6 +16,7 @@ import io.sentry.android.core.ActivityLifecycleIntegration.UI_LOAD_OP import io.sentry.android.core.performance.ActivityLifecycleTimeSpan import io.sentry.android.core.performance.AppStartMetrics import io.sentry.android.core.performance.AppStartMetrics.AppStartType +import io.sentry.protocol.App import io.sentry.protocol.MeasurementValue import io.sentry.protocol.SentrySpan import io.sentry.protocol.SentryTransaction @@ -27,6 +28,7 @@ import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse +import kotlin.test.assertNull import kotlin.test.assertTrue @RunWith(AndroidJUnit4::class) @@ -464,6 +466,66 @@ class PerformanceAndroidEventProcessorTest { } } + @Test + fun `does not set start_type field for txns without app start span`() { + // given some cold app start metrics + // where class loaded happened way before app start + setAppStart(fixture.options, coldStart = true) + + val sut = fixture.getSut(enablePerformanceV2 = true) + val context = TransactionContext("Activity", UI_LOAD_OP) + val tracer = SentryTracer(context, fixture.hub) + var tr = SentryTransaction(tracer) + // usually set by DefaultAndroidEventProcessor + tr.contexts.setApp(App()) + + // when the processor attaches the app start spans + tr = sut.process(tr, Hint()) + + // start_type should not be set + assertNull(tr.contexts.app!!.startType) + } + + @Test + fun `sets start_type field for app context`() { + // given some cold app start metrics + // where class loaded happened way before app start + setAppStart(fixture.options, coldStart = true) + + val sut = fixture.getSut(enablePerformanceV2 = true) + val context = TransactionContext("Activity", UI_LOAD_OP) + val tracer = SentryTracer(context, fixture.hub) + var tr = SentryTransaction(tracer) + // usually set by DefaultAndroidEventProcessor + tr.contexts.setApp(App()) + + val appStartSpan = SentrySpan( + 0.0, + 1.0, + tr.contexts.trace!!.traceId, + SpanId(), + null, + APP_START_COLD, + "App Start", + SpanStatus.OK, + null, + emptyMap(), + emptyMap(), + null, + null + ) + tr.spans.add(appStartSpan) + + // when the processor attaches the app start spans + tr = sut.process(tr, Hint()) + + // start_type should be set + assertEquals( + "cold", + tr.contexts.app!!.startType + ) + } + private fun setAppStart(options: SentryAndroidOptions, coldStart: Boolean = true) { AppStartMetrics.getInstance().apply { appStartType = when (coldStart) { diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 84fa3a4751..827bd1a955 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -3643,6 +3643,7 @@ public final class io/sentry/protocol/App : io/sentry/JsonSerializable, io/sentr public fun getDeviceAppHash ()Ljava/lang/String; public fun getInForeground ()Ljava/lang/Boolean; public fun getPermissions ()Ljava/util/Map; + public fun getStartType ()Ljava/lang/String; public fun getUnknown ()Ljava/util/Map; public fun getViewNames ()Ljava/util/List; public fun hashCode ()I @@ -3656,6 +3657,7 @@ public final class io/sentry/protocol/App : io/sentry/JsonSerializable, io/sentr public fun setDeviceAppHash (Ljava/lang/String;)V public fun setInForeground (Ljava/lang/Boolean;)V public fun setPermissions (Ljava/util/Map;)V + public fun setStartType (Ljava/lang/String;)V public fun setUnknown (Ljava/util/Map;)V public fun setViewNames (Ljava/util/List;)V } @@ -3676,6 +3678,7 @@ public final class io/sentry/protocol/App$JsonKeys { 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 static final field START_TYPE Ljava/lang/String; public static final field VIEW_NAMES Ljava/lang/String; public fun ()V } diff --git a/sentry/src/main/java/io/sentry/protocol/App.java b/sentry/src/main/java/io/sentry/protocol/App.java index b7b41638db..bec57d22f3 100644 --- a/sentry/src/main/java/io/sentry/protocol/App.java +++ b/sentry/src/main/java/io/sentry/protocol/App.java @@ -40,8 +40,10 @@ 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; - /** The list of the visibile UI screens * */ + /** The list of the visible UI screens * */ private @Nullable List viewNames; + /** the app start type */ + private @Nullable String startType; /** * A flag indicating whether the app is in foreground or not. An app is in foreground when it's * visible to the user. @@ -61,6 +63,7 @@ public App() {} this.permissions = CollectionUtils.newConcurrentHashMap(app.permissions); this.inForeground = app.inForeground; this.viewNames = CollectionUtils.newArrayList(app.viewNames); + this.startType = app.startType; this.unknown = CollectionUtils.newConcurrentHashMap(app.unknown); } @@ -151,6 +154,15 @@ public void setViewNames(final @Nullable List viewNames) { this.viewNames = viewNames; } + @Nullable + public String getStartType() { + return startType; + } + + public void setStartType(final @Nullable String startType) { + this.startType = startType; + } + @Override public boolean equals(Object o) { if (this == o) return true; @@ -165,7 +177,8 @@ public boolean equals(Object o) { && Objects.equals(appBuild, app.appBuild) && Objects.equals(permissions, app.permissions) && Objects.equals(inForeground, app.inForeground) - && Objects.equals(viewNames, app.viewNames); + && Objects.equals(viewNames, app.viewNames) + && Objects.equals(startType, app.startType); } @Override @@ -180,7 +193,8 @@ public int hashCode() { appBuild, permissions, inForeground, - viewNames); + viewNames, + startType); } // region json @@ -207,6 +221,7 @@ public static final class JsonKeys { public static final String APP_PERMISSIONS = "permissions"; public static final String IN_FOREGROUND = "in_foreground"; public static final String VIEW_NAMES = "view_names"; + public static final String START_TYPE = "start_type"; } @Override @@ -243,6 +258,9 @@ public void serialize(final @NotNull ObjectWriter writer, final @NotNull ILogger if (viewNames != null) { writer.name(JsonKeys.VIEW_NAMES).value(logger, viewNames); } + if (startType != null) { + writer.name(JsonKeys.START_TYPE).value(startType); + } if (unknown != null) { for (String key : unknown.keySet()) { Object value = unknown.get(key); @@ -298,6 +316,9 @@ public static final class Deserializer implements JsonDeserializer { app.setViewNames(viewNames); } break; + case JsonKeys.START_TYPE: + app.startType = reader.nextStringOrNull(); + break; default: if (unknown == null) { unknown = new ConcurrentHashMap<>(); diff --git a/sentry/src/test/java/io/sentry/protocol/AppSerializationTest.kt b/sentry/src/test/java/io/sentry/protocol/AppSerializationTest.kt index c39f93643a..85716fe913 100644 --- a/sentry/src/test/java/io/sentry/protocol/AppSerializationTest.kt +++ b/sentry/src/test/java/io/sentry/protocol/AppSerializationTest.kt @@ -31,6 +31,7 @@ class AppSerializationTest { ) inForeground = true viewNames = listOf("MainActivity", "SidebarActivity") + startType = "cold" } } 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 3f51594fff..5c36f148c0 100644 --- a/sentry/src/test/java/io/sentry/protocol/AppTest.kt +++ b/sentry/src/test/java/io/sentry/protocol/AppTest.kt @@ -21,6 +21,7 @@ class AppTest { app.permissions = mapOf(Pair("internet", "granted")) app.viewNames = listOf("MainActivity") app.inForeground = true + app.startType = "cold" val unknown = mapOf(Pair("unknown", "unknown")) app.unknown = unknown @@ -49,6 +50,7 @@ class AppTest { app.permissions = mapOf(Pair("internet", "granted")) app.viewNames = listOf("MainActivity") app.inForeground = true + app.startType = "cold" val unknown = mapOf(Pair("unknown", "unknown")) app.unknown = unknown @@ -67,6 +69,7 @@ class AppTest { assertEquals(listOf("MainActivity"), clone.viewNames) assertEquals(true, clone.inForeground) + assertEquals("cold", clone.startType) assertNotNull(clone.unknown) { assertEquals("unknown", it["unknown"]) } diff --git a/sentry/src/test/resources/json/app.json b/sentry/src/test/resources/json/app.json index e835258efa..76eab3b545 100644 --- a/sentry/src/test/resources/json/app.json +++ b/sentry/src/test/resources/json/app.json @@ -12,5 +12,6 @@ "CAMERA": "granted" }, "in_foreground": true, - "view_names": ["MainActivity", "SidebarActivity"] + "view_names": ["MainActivity", "SidebarActivity"], + "start_type": "cold" } diff --git a/sentry/src/test/resources/json/contexts.json b/sentry/src/test/resources/json/contexts.json index 97f06e0be6..574bb01921 100644 --- a/sentry/src/test/resources/json/contexts.json +++ b/sentry/src/test/resources/json/contexts.json @@ -14,7 +14,8 @@ "CAMERA": "granted" }, "in_foreground": true, - "view_names": ["MainActivity", "SidebarActivity"] + "view_names": ["MainActivity", "SidebarActivity"], + "start_type": "cold" }, "browser": { From 9308c2adfdc39306a258240f99d194ceb36c3fbc Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Tue, 23 Apr 2024 09:12:29 +0200 Subject: [PATCH 2/5] Create app contexst in case it doesn't exist --- .../core/PerformanceAndroidEventProcessor.java | 16 +++++++++------- .../PerformanceAndroidEventProcessorTest.kt | 17 +++++------------ 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java index db8814a91b..5e80f91839 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java @@ -104,14 +104,16 @@ public SentryEvent process(@NotNull SentryEvent event, @NotNull Hint hint) { } } - final @Nullable App appContext = transaction.getContexts().getApp(); - if (appContext != null) { - final String appStartType = - AppStartMetrics.getInstance().getAppStartType() == AppStartMetrics.AppStartType.COLD - ? "cold" - : "warm"; - appContext.setStartType(appStartType); + @Nullable App appContext = transaction.getContexts().getApp(); + if (appContext == null) { + appContext = new App(); + transaction.getContexts().setApp(appContext); } + final String appStartType = + AppStartMetrics.getInstance().getAppStartType() == AppStartMetrics.AppStartType.COLD + ? "cold" + : "warm"; + appContext.setStartType(appStartType); } final SentryId eventId = transaction.getEventId(); diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt index dce5cb3998..d7bef488c2 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt @@ -16,7 +16,6 @@ import io.sentry.android.core.ActivityLifecycleIntegration.UI_LOAD_OP import io.sentry.android.core.performance.ActivityLifecycleTimeSpan import io.sentry.android.core.performance.AppStartMetrics import io.sentry.android.core.performance.AppStartMetrics.AppStartType -import io.sentry.protocol.App import io.sentry.protocol.MeasurementValue import io.sentry.protocol.SentrySpan import io.sentry.protocol.SentryTransaction @@ -468,36 +467,30 @@ class PerformanceAndroidEventProcessorTest { @Test fun `does not set start_type field for txns without app start span`() { - // given some cold app start metrics - // where class loaded happened way before app start + // given some ui.load txn setAppStart(fixture.options, coldStart = true) val sut = fixture.getSut(enablePerformanceV2 = true) val context = TransactionContext("Activity", UI_LOAD_OP) val tracer = SentryTracer(context, fixture.hub) var tr = SentryTransaction(tracer) - // usually set by DefaultAndroidEventProcessor - tr.contexts.setApp(App()) - // when the processor attaches the app start spans + // when it contains no app start span and is processed tr = sut.process(tr, Hint()) // start_type should not be set - assertNull(tr.contexts.app!!.startType) + assertNull(tr.contexts.app?.startType) } @Test fun `sets start_type field for app context`() { - // given some cold app start metrics - // where class loaded happened way before app start + // given some cold app start setAppStart(fixture.options, coldStart = true) val sut = fixture.getSut(enablePerformanceV2 = true) val context = TransactionContext("Activity", UI_LOAD_OP) val tracer = SentryTracer(context, fixture.hub) var tr = SentryTransaction(tracer) - // usually set by DefaultAndroidEventProcessor - tr.contexts.setApp(App()) val appStartSpan = SentrySpan( 0.0, @@ -519,7 +512,7 @@ class PerformanceAndroidEventProcessorTest { // when the processor attaches the app start spans tr = sut.process(tr, Hint()) - // start_type should be set + // start_type should be set as well assertEquals( "cold", tr.contexts.app!!.startType From 834e50cd1a04d4d5b0228acd156e0eff7e5d1656 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Tue, 23 Apr 2024 09:13:29 +0200 Subject: [PATCH 3/5] Add changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 34b429a9c4..402e177555 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Features + +- Add start_type to app context ([#3379](https://github.com/getsentry/sentry-java/pull/3379)) + ### Fixes - Fix timing metric value different from span duration ([#3368](https://github.com/getsentry/sentry-java/pull/3368)) From 1579b21c53dbfbc333b229b7db63d77eb6508a75 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Tue, 23 Apr 2024 11:12:43 +0200 Subject: [PATCH 4/5] Fix test fixtures --- sentry/src/test/resources/json/sentry_base_event.json | 3 ++- .../test/resources/json/sentry_base_event_with_null_extra.json | 3 ++- sentry/src/test/resources/json/sentry_event.json | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/sentry/src/test/resources/json/sentry_base_event.json b/sentry/src/test/resources/json/sentry_base_event.json index 48b7c1ccb9..afaf040863 100644 --- a/sentry/src/test/resources/json/sentry_base_event.json +++ b/sentry/src/test/resources/json/sentry_base_event.json @@ -17,7 +17,8 @@ "CAMERA": "granted" }, "in_foreground": true, - "view_names": ["MainActivity", "SidebarActivity"] + "view_names": ["MainActivity", "SidebarActivity"], + "start_type": "cold" }, "browser": { diff --git a/sentry/src/test/resources/json/sentry_base_event_with_null_extra.json b/sentry/src/test/resources/json/sentry_base_event_with_null_extra.json index 773eb99e2f..018cc7aae7 100644 --- a/sentry/src/test/resources/json/sentry_base_event_with_null_extra.json +++ b/sentry/src/test/resources/json/sentry_base_event_with_null_extra.json @@ -17,7 +17,8 @@ "CAMERA": "granted" }, "in_foreground": true, - "view_names": ["MainActivity", "SidebarActivity"] + "view_names": ["MainActivity", "SidebarActivity"], + "start_type": "cold" }, "browser": { diff --git a/sentry/src/test/resources/json/sentry_event.json b/sentry/src/test/resources/json/sentry_event.json index 5d57960db9..6d0b351ed4 100644 --- a/sentry/src/test/resources/json/sentry_event.json +++ b/sentry/src/test/resources/json/sentry_event.json @@ -152,7 +152,8 @@ "CAMERA": "granted" }, "in_foreground": true, - "view_names": ["MainActivity", "SidebarActivity"] + "view_names": ["MainActivity", "SidebarActivity"], + "start_type": "cold" }, "browser": { From 31a106cd12e167402f5f88ef5fa6f44e8b2356cb Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Tue, 23 Apr 2024 15:21:39 +0200 Subject: [PATCH 5/5] Fix tests --- sentry/src/test/resources/json/sentry_transaction.json | 3 ++- .../resources/json/sentry_transaction_legacy_date_format.json | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/sentry/src/test/resources/json/sentry_transaction.json b/sentry/src/test/resources/json/sentry_transaction.json index 93503569b1..944a0bfe92 100644 --- a/sentry/src/test/resources/json/sentry_transaction.json +++ b/sentry/src/test/resources/json/sentry_transaction.json @@ -100,7 +100,8 @@ "CAMERA": "granted" }, "in_foreground": true, - "view_names": ["MainActivity", "SidebarActivity"] + "view_names": ["MainActivity", "SidebarActivity"], + "start_type": "cold" }, "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 6e54117084..789c4fe2a9 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 @@ -100,7 +100,8 @@ "CAMERA": "granted" }, "in_foreground": true, - "view_names": ["MainActivity", "SidebarActivity"] + "view_names": ["MainActivity", "SidebarActivity"], + "start_type": "cold" }, "browser": {