Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add start_type to app context #3379

Merged
merged 5 commits into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Copy link
Member

Choose a reason for hiding this comment

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

l: I know it should be set by the DefaultAndroidEventProcessor which is running before this one, but should we fall back to creating a new instance of App in case it's null?

final String appStartType =
AppStartMetrics.getInstance().getAppStartType() == AppStartMetrics.AppStartType.COLD
? "cold"
: "warm";
appContext.setStartType(appStartType);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down
3 changes: 3 additions & 0 deletions sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand All @@ -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 <init> ()V
}
Expand Down
27 changes: 24 additions & 3 deletions sentry/src/main/java/io/sentry/protocol/App.java
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, String> permissions;
/** The list of the visibile UI screens * */
/** The list of the visible UI screens * */
private @Nullable List<String> 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.
Expand All @@ -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);
}

Expand Down Expand Up @@ -151,6 +154,15 @@ public void setViewNames(final @Nullable List<String> 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;
Expand All @@ -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
Expand All @@ -180,7 +193,8 @@ public int hashCode() {
appBuild,
permissions,
inForeground,
viewNames);
viewNames,
startType);
}

// region json
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -298,6 +316,9 @@ public static final class Deserializer implements JsonDeserializer<App> {
app.setViewNames(viewNames);
}
break;
case JsonKeys.START_TYPE:
app.startType = reader.nextStringOrNull();
break;
default:
if (unknown == null) {
unknown = new ConcurrentHashMap<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ class AppSerializationTest {
)
inForeground = true
viewNames = listOf("MainActivity", "SidebarActivity")
startType = "cold"
}
}
private val fixture = Fixture()
Expand Down
3 changes: 3 additions & 0 deletions sentry/src/test/java/io/sentry/protocol/AppTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand All @@ -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"])
}
Expand Down
3 changes: 2 additions & 1 deletion sentry/src/test/resources/json/app.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,6 @@
"CAMERA": "granted"
},
"in_foreground": true,
"view_names": ["MainActivity", "SidebarActivity"]
"view_names": ["MainActivity", "SidebarActivity"],
"start_type": "cold"
}
3 changes: 2 additions & 1 deletion sentry/src/test/resources/json/contexts.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
"CAMERA": "granted"
},
"in_foreground": true,
"view_names": ["MainActivity", "SidebarActivity"]
"view_names": ["MainActivity", "SidebarActivity"],
"start_type": "cold"
},
"browser":
{
Expand Down
Loading