From c7202577c85ef18381f303c35c60cf6b23d90088 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Thu, 26 Jan 2023 12:30:37 +0100 Subject: [PATCH 01/19] Add initial poc for tracking compose rendering time --- .../io/sentry/compose/SentryComposeTracing.kt | 46 +++++++++++++++++++ .../android/compose/ComposeActivity.kt | 21 ++++++--- 2 files changed, 61 insertions(+), 6 deletions(-) create mode 100644 sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt diff --git a/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt b/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt new file mode 100644 index 0000000000..dc0d15a2ca --- /dev/null +++ b/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt @@ -0,0 +1,46 @@ +package io.sentry.compose + +import androidx.compose.runtime.Stable +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.ui.ExperimentalComposeUiApi +import androidx.compose.ui.Modifier +import androidx.compose.ui.composed +import androidx.compose.ui.draw.drawWithContent +import androidx.compose.ui.modifier.ProvidableModifierLocal +import androidx.compose.ui.modifier.modifierLocalConsumer +import androidx.compose.ui.modifier.modifierLocalProvider +import androidx.compose.ui.platform.testTag +import io.sentry.ISpan +import io.sentry.Sentry + +@Stable +private class StableHolder(val item: T) { + operator fun component1(): T = item +} + +private val localSentrySpanModifier = ProvidableModifierLocal { + StableHolder(Sentry.getSpan()) +} + +@ExperimentalComposeUiApi +public fun Modifier.sentryTraced(tag: String): Modifier = Modifier.composed { + val span = remember { mutableStateOf>(StableHolder(null)) } + this + .testTag(tag) + .modifierLocalConsumer { + span.value = + StableHolder(localSentrySpanModifier.current.item?.startChild("ui.load", tag)) + } + .drawWithContent { + drawContent() + span.value.apply { + if (item?.isFinished == false) { + item.finish() + } + } + } + .modifierLocalProvider(localSentrySpanModifier) { + span.value + } +} diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt index 259947399a..1a4d14ee85 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt @@ -1,3 +1,5 @@ +@file:OptIn(ExperimentalComposeUiApi::class) + package io.sentry.samples.android.compose import android.os.Bundle @@ -18,6 +20,7 @@ import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment +import androidx.compose.ui.ExperimentalComposeUiApi import androidx.compose.ui.Modifier import androidx.compose.ui.platform.testTag import androidx.compose.ui.text.input.TextFieldValue @@ -29,6 +32,7 @@ import androidx.navigation.compose.composable import androidx.navigation.compose.rememberNavController import androidx.navigation.navArgument import io.sentry.Sentry +import io.sentry.compose.sentryTraced import io.sentry.compose.withSentryObservableEffect import io.sentry.samples.android.GithubAPI import kotlinx.coroutines.launch @@ -58,14 +62,16 @@ fun Landing( Column( verticalArrangement = Arrangement.Center, horizontalAlignment = Alignment.CenterHorizontally, - modifier = Modifier.fillMaxSize() + modifier = Modifier + .sentryTraced("landing") + .fillMaxSize() ) { Button( onClick = { navigateGithub() }, modifier = Modifier - .testTag("button_nav_github") + .sentryTraced("button_nav_github") .padding(top = 32.dp) ) { Text("Navigate to Github Page") @@ -73,7 +79,7 @@ fun Landing( Button( onClick = { navigateGithubWithArgs() }, modifier = Modifier - .testTag("button_nav_github_args") + .sentryTraced("button_nav_github_args") .padding(top = 32.dp) ) { Text("Navigate to Github Page With Args") @@ -81,7 +87,7 @@ fun Landing( Button( onClick = { throw RuntimeException("Crash from Compose") }, modifier = Modifier - .testTag("button_crash") + .sentryTraced("button_crash") .padding(top = 32.dp) ) { Text("Crash from Compose") @@ -105,7 +111,9 @@ fun Github( Column( verticalArrangement = Arrangement.Center, horizontalAlignment = Alignment.CenterHorizontally, - modifier = Modifier.fillMaxSize() + modifier = Modifier + .sentryTraced("github-$user") + .fillMaxSize() ) { TextField( value = user, @@ -133,7 +141,8 @@ fun Github( fun SampleNavigation(navController: NavHostController) { NavHost( navController = navController, - startDestination = Destination.Landing.route + startDestination = Destination.Landing.route, + // modifier = Modifier.sentryTraced("app") ) { composable(Destination.Landing.route) { Landing( From b28318bf91c26760f59a9227c51a1ca6ef901d5c Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Wed, 1 Feb 2023 13:19:01 +0100 Subject: [PATCH 02/19] Add SpanOptions to auto-remove, trim and finish spans --- sentry/api/sentry.api | 30 ++++- sentry/src/main/java/io/sentry/Hub.java | 13 ++ .../src/main/java/io/sentry/HubAdapter.java | 5 + sentry/src/main/java/io/sentry/IHub.java | 3 + sentry/src/main/java/io/sentry/ISpan.java | 14 ++ sentry/src/main/java/io/sentry/NoOpHub.java | 5 + sentry/src/main/java/io/sentry/NoOpSpan.java | 16 +++ .../main/java/io/sentry/NoOpTransaction.java | 16 +++ sentry/src/main/java/io/sentry/Scope.java | 10 ++ sentry/src/main/java/io/sentry/Sentry.java | 9 ++ .../src/main/java/io/sentry/SentryTracer.java | 125 ++++++++++++++++-- sentry/src/main/java/io/sentry/Span.java | 50 ++++++- .../src/main/java/io/sentry/SpanOptions.java | 43 ++++++ 13 files changed, 318 insertions(+), 21 deletions(-) create mode 100644 sentry/src/main/java/io/sentry/SpanOptions.java diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 81bbb428ae..19f17bb362 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -325,6 +325,7 @@ public final class io/sentry/Hub : io/sentry/IHub { public fun flush (J)V public fun getLastEventId ()Lio/sentry/protocol/SentryId; public fun getOptions ()Lio/sentry/SentryOptions; + public fun getRootSpan ()Lio/sentry/ISpan; public fun getSpan ()Lio/sentry/ISpan; public fun isCrashedLastRun ()Ljava/lang/Boolean; public fun isEnabled ()Z @@ -368,6 +369,7 @@ public final class io/sentry/HubAdapter : io/sentry/IHub { public static fun getInstance ()Lio/sentry/HubAdapter; public fun getLastEventId ()Lio/sentry/protocol/SentryId; public fun getOptions ()Lio/sentry/SentryOptions; + public fun getRootSpan ()Lio/sentry/ISpan; public fun getSpan ()Lio/sentry/ISpan; public fun isCrashedLastRun ()Ljava/lang/Boolean; public fun isEnabled ()Z @@ -436,6 +438,7 @@ public abstract interface class io/sentry/IHub { public abstract fun flush (J)V public abstract fun getLastEventId ()Lio/sentry/protocol/SentryId; public abstract fun getOptions ()Lio/sentry/SentryOptions; + public abstract fun getRootSpan ()Lio/sentry/ISpan; public abstract fun getSpan ()Lio/sentry/ISpan; public abstract fun isCrashedLastRun ()Ljava/lang/Boolean; public abstract fun isEnabled ()Z @@ -549,6 +552,8 @@ public abstract interface class io/sentry/ISpan { public abstract fun startChild (Ljava/lang/String;)Lio/sentry/ISpan; public abstract fun startChild (Ljava/lang/String;Ljava/lang/String;)Lio/sentry/ISpan; public abstract fun startChild (Ljava/lang/String;Ljava/lang/String;Lio/sentry/SentryDate;Lio/sentry/Instrumenter;)Lio/sentry/ISpan; + public abstract fun startChild (Ljava/lang/String;Ljava/lang/String;Lio/sentry/SentryDate;Lio/sentry/Instrumenter;Lio/sentry/SpanOptions;)Lio/sentry/ISpan; + public abstract fun startChild (Ljava/lang/String;Ljava/lang/String;Lio/sentry/SpanOptions;)Lio/sentry/ISpan; public abstract fun toBaggageHeader (Ljava/util/List;)Lio/sentry/BaggageHeader; public abstract fun toSentryTrace ()Lio/sentry/SentryTraceHeader; public abstract fun traceContext ()Lio/sentry/TraceContext; @@ -756,6 +761,7 @@ public final class io/sentry/NoOpHub : io/sentry/IHub { public static fun getInstance ()Lio/sentry/NoOpHub; public fun getLastEventId ()Lio/sentry/protocol/SentryId; public fun getOptions ()Lio/sentry/SentryOptions; + public fun getRootSpan ()Lio/sentry/ISpan; public fun getSpan ()Lio/sentry/ISpan; public fun isCrashedLastRun ()Ljava/lang/Boolean; public fun isEnabled ()Z @@ -811,6 +817,8 @@ public final class io/sentry/NoOpSpan : io/sentry/ISpan { public fun startChild (Ljava/lang/String;)Lio/sentry/ISpan; public fun startChild (Ljava/lang/String;Ljava/lang/String;)Lio/sentry/ISpan; public fun startChild (Ljava/lang/String;Ljava/lang/String;Lio/sentry/SentryDate;Lio/sentry/Instrumenter;)Lio/sentry/ISpan; + public fun startChild (Ljava/lang/String;Ljava/lang/String;Lio/sentry/SentryDate;Lio/sentry/Instrumenter;Lio/sentry/SpanOptions;)Lio/sentry/ISpan; + public fun startChild (Ljava/lang/String;Ljava/lang/String;Lio/sentry/SpanOptions;)Lio/sentry/ISpan; public fun toBaggageHeader (Ljava/util/List;)Lio/sentry/BaggageHeader; public fun toSentryTrace ()Lio/sentry/SentryTraceHeader; public fun traceContext ()Lio/sentry/TraceContext; @@ -854,6 +862,8 @@ public final class io/sentry/NoOpTransaction : io/sentry/ITransaction { public fun startChild (Ljava/lang/String;)Lio/sentry/ISpan; public fun startChild (Ljava/lang/String;Ljava/lang/String;)Lio/sentry/ISpan; public fun startChild (Ljava/lang/String;Ljava/lang/String;Lio/sentry/SentryDate;Lio/sentry/Instrumenter;)Lio/sentry/ISpan; + public fun startChild (Ljava/lang/String;Ljava/lang/String;Lio/sentry/SentryDate;Lio/sentry/Instrumenter;Lio/sentry/SpanOptions;)Lio/sentry/ISpan; + public fun startChild (Ljava/lang/String;Ljava/lang/String;Lio/sentry/SpanOptions;)Lio/sentry/ISpan; public fun toBaggageHeader (Ljava/util/List;)Lio/sentry/BaggageHeader; public fun toSentryTrace ()Lio/sentry/SentryTraceHeader; public fun traceContext ()Lio/sentry/TraceContext; @@ -1054,6 +1064,7 @@ public final class io/sentry/Scope { public fun getContexts ()Lio/sentry/protocol/Contexts; public fun getLevel ()Lio/sentry/SentryLevel; public fun getRequest ()Lio/sentry/protocol/Request; + public fun getRootSpan ()Lio/sentry/ISpan; public fun getSession ()Lio/sentry/Session; public fun getSpan ()Lio/sentry/ISpan; public fun getTags ()Ljava/util/Map; @@ -1144,6 +1155,7 @@ public final class io/sentry/Sentry { public static fun flush (J)V public static fun getCurrentHub ()Lio/sentry/IHub; public static fun getLastEventId ()Lio/sentry/protocol/SentryId; + public static fun getRootSpan ()Lio/sentry/ISpan; public static fun getSpan ()Lio/sentry/ISpan; public static fun init ()V public static fun init (Lio/sentry/OptionsContainer;Lio/sentry/Sentry$OptionsConfiguration;)V @@ -1761,6 +1773,8 @@ public final class io/sentry/SentryTracer : io/sentry/ITransaction { public fun startChild (Ljava/lang/String;)Lio/sentry/ISpan; public fun startChild (Ljava/lang/String;Ljava/lang/String;)Lio/sentry/ISpan; public fun startChild (Ljava/lang/String;Ljava/lang/String;Lio/sentry/SentryDate;Lio/sentry/Instrumenter;)Lio/sentry/ISpan; + public fun startChild (Ljava/lang/String;Ljava/lang/String;Lio/sentry/SentryDate;Lio/sentry/Instrumenter;Lio/sentry/SpanOptions;)Lio/sentry/ISpan; + public fun startChild (Ljava/lang/String;Ljava/lang/String;Lio/sentry/SpanOptions;)Lio/sentry/ISpan; public fun toBaggageHeader (Ljava/util/List;)Lio/sentry/BaggageHeader; public fun toSentryTrace ()Lio/sentry/SentryTraceHeader; public fun traceContext ()Lio/sentry/TraceContext; @@ -1837,7 +1851,7 @@ public final class io/sentry/ShutdownHookIntegration : io/sentry/Integration, ja } public final class io/sentry/Span : io/sentry/ISpan { - public fun (Lio/sentry/TransactionContext;Lio/sentry/SentryTracer;Lio/sentry/IHub;Lio/sentry/SentryDate;)V + public fun (Lio/sentry/TransactionContext;Lio/sentry/SentryTracer;Lio/sentry/IHub;Lio/sentry/SentryDate;Lio/sentry/SpanOptions;)V public fun finish ()V public fun finish (Lio/sentry/SpanStatus;)V public fun finish (Lio/sentry/SpanStatus;Lio/sentry/SentryDate;)V @@ -1846,6 +1860,7 @@ public final class io/sentry/Span : io/sentry/ISpan { public fun getDescription ()Ljava/lang/String; public fun getFinishDate ()Lio/sentry/SentryDate; public fun getOperation ()Ljava/lang/String; + public fun getOptions ()Lio/sentry/SpanOptions; public fun getParentSpanId ()Lio/sentry/SpanId; public fun getSamplingDecision ()Lio/sentry/TracesSamplingDecision; public fun getSpanContext ()Lio/sentry/SpanContext; @@ -1862,15 +1877,19 @@ public final class io/sentry/Span : io/sentry/ISpan { public fun isSampled ()Ljava/lang/Boolean; public fun setData (Ljava/lang/String;Ljava/lang/Object;)V public fun setDescription (Ljava/lang/String;)V + public fun setEndDate (Lio/sentry/SentryDate;)V public fun setMeasurement (Ljava/lang/String;Ljava/lang/Number;)V public fun setMeasurement (Ljava/lang/String;Ljava/lang/Number;Lio/sentry/MeasurementUnit;)V public fun setOperation (Ljava/lang/String;)V + public fun setStartDate (Lio/sentry/SentryDate;)V public fun setStatus (Lio/sentry/SpanStatus;)V public fun setTag (Ljava/lang/String;Ljava/lang/String;)V public fun setThrowable (Ljava/lang/Throwable;)V public fun startChild (Ljava/lang/String;)Lio/sentry/ISpan; public fun startChild (Ljava/lang/String;Ljava/lang/String;)Lio/sentry/ISpan; public fun startChild (Ljava/lang/String;Ljava/lang/String;Lio/sentry/SentryDate;Lio/sentry/Instrumenter;)Lio/sentry/ISpan; + public fun startChild (Ljava/lang/String;Ljava/lang/String;Lio/sentry/SentryDate;Lio/sentry/Instrumenter;Lio/sentry/SpanOptions;)Lio/sentry/ISpan; + public fun startChild (Ljava/lang/String;Ljava/lang/String;Lio/sentry/SpanOptions;)Lio/sentry/ISpan; public fun toBaggageHeader (Ljava/util/List;)Lio/sentry/BaggageHeader; public fun toSentryTrace ()Lio/sentry/SentryTraceHeader; public fun traceContext ()Lio/sentry/TraceContext; @@ -1942,6 +1961,15 @@ public final class io/sentry/SpanId$Deserializer : io/sentry/JsonDeserializer { public synthetic fun deserialize (Lio/sentry/JsonObjectReader;Lio/sentry/ILogger;)Ljava/lang/Object; } +public final class io/sentry/SpanOptions { + public fun ()V + public fun (ZZZZ)V + public fun isAutoFinish ()Z + public fun isRemoveIfNoChildren ()Z + public fun isTrimEnd ()Z + public fun isTrimStart ()Z +} + public final class io/sentry/SpanStatus : java/lang/Enum, io/sentry/JsonSerializable { public static final field ABORTED Lio/sentry/SpanStatus; public static final field ALREADY_EXISTS Lio/sentry/SpanStatus; diff --git a/sentry/src/main/java/io/sentry/Hub.java b/sentry/src/main/java/io/sentry/Hub.java index bcfa9b87f7..7228341d2a 100644 --- a/sentry/src/main/java/io/sentry/Hub.java +++ b/sentry/src/main/java/io/sentry/Hub.java @@ -777,6 +777,19 @@ public void flush(long timeoutMillis) { return span; } + @Override + public @Nullable ISpan getRootSpan() { + ISpan span = null; + if (!isEnabled()) { + options + .getLogger() + .log(SentryLevel.WARNING, "Instance is disabled and this 'getRootSpan' call is a no-op."); + } else { + span = stack.peek().getScope().getRootSpan(); + } + return span; + } + @Override @ApiStatus.Internal public void setSpanContext( diff --git a/sentry/src/main/java/io/sentry/HubAdapter.java b/sentry/src/main/java/io/sentry/HubAdapter.java index 0a7756863a..413362304c 100644 --- a/sentry/src/main/java/io/sentry/HubAdapter.java +++ b/sentry/src/main/java/io/sentry/HubAdapter.java @@ -220,6 +220,11 @@ public void setSpanContext( return Sentry.getCurrentHub().getSpan(); } + @Override + public @Nullable ISpan getRootSpan() { + return Sentry.getCurrentHub().getRootSpan(); + } + @Override public @NotNull SentryOptions getOptions() { return Sentry.getCurrentHub().getOptions(); diff --git a/sentry/src/main/java/io/sentry/IHub.java b/sentry/src/main/java/io/sentry/IHub.java index 01d8546d84..547bb05fef 100644 --- a/sentry/src/main/java/io/sentry/IHub.java +++ b/sentry/src/main/java/io/sentry/IHub.java @@ -543,6 +543,9 @@ void setSpanContext( @Nullable ISpan getSpan(); + @Nullable + ISpan getRootSpan(); + /** * Gets the {@link SentryOptions} attached to current scope. * diff --git a/sentry/src/main/java/io/sentry/ISpan.java b/sentry/src/main/java/io/sentry/ISpan.java index 0b10fdee25..5c27fe4388 100644 --- a/sentry/src/main/java/io/sentry/ISpan.java +++ b/sentry/src/main/java/io/sentry/ISpan.java @@ -16,6 +16,11 @@ public interface ISpan { @NotNull ISpan startChild(@NotNull String operation); + @ApiStatus.Internal + @NotNull + ISpan startChild( + @NotNull String operation, @Nullable String description, @NotNull SpanOptions spanOptions); + @ApiStatus.Internal @NotNull ISpan startChild( @@ -24,6 +29,15 @@ ISpan startChild( @Nullable SentryDate timestamp, @NotNull Instrumenter instrumenter); + @ApiStatus.Internal + @NotNull + ISpan startChild( + @NotNull String operation, + @Nullable String description, + @Nullable SentryDate timestamp, + @NotNull Instrumenter instrumenter, + @NotNull SpanOptions spanOptions); + /** * Starts a child Span. * diff --git a/sentry/src/main/java/io/sentry/NoOpHub.java b/sentry/src/main/java/io/sentry/NoOpHub.java index 52bef0ff43..a4056e502d 100644 --- a/sentry/src/main/java/io/sentry/NoOpHub.java +++ b/sentry/src/main/java/io/sentry/NoOpHub.java @@ -177,6 +177,11 @@ public void setSpanContext( return null; } + @Override + public @Nullable ISpan getRootSpan() { + return null; + } + @Override public @NotNull SentryOptions getOptions() { return emptyOptions; diff --git a/sentry/src/main/java/io/sentry/NoOpSpan.java b/sentry/src/main/java/io/sentry/NoOpSpan.java index e89d49b799..fbfc28d77f 100644 --- a/sentry/src/main/java/io/sentry/NoOpSpan.java +++ b/sentry/src/main/java/io/sentry/NoOpSpan.java @@ -20,6 +20,12 @@ public static NoOpSpan getInstance() { return NoOpSpan.getInstance(); } + @Override + public @NotNull ISpan startChild( + @NotNull String operation, @Nullable String description, @NotNull SpanOptions spanOptions) { + return NoOpSpan.getInstance(); + } + @Override public @NotNull ISpan startChild( @NotNull String operation, @@ -29,6 +35,16 @@ public static NoOpSpan getInstance() { return NoOpSpan.getInstance(); } + @Override + public @NotNull ISpan startChild( + @NotNull String operation, + @Nullable String description, + @Nullable SentryDate timestamp, + @NotNull Instrumenter instrumenter, + @NotNull SpanOptions spanOptions) { + return NoOpSpan.getInstance(); + } + @Override public @NotNull ISpan startChild( final @NotNull String operation, final @Nullable String description) { diff --git a/sentry/src/main/java/io/sentry/NoOpTransaction.java b/sentry/src/main/java/io/sentry/NoOpTransaction.java index dd04a4f6f3..30344eeb01 100644 --- a/sentry/src/main/java/io/sentry/NoOpTransaction.java +++ b/sentry/src/main/java/io/sentry/NoOpTransaction.java @@ -41,6 +41,12 @@ public void setName(@NotNull String name, @NotNull TransactionNameSource transac return NoOpSpan.getInstance(); } + @Override + public @NotNull ISpan startChild( + @NotNull String operation, @Nullable String description, @NotNull SpanOptions spanOptions) { + return NoOpSpan.getInstance(); + } + @Override public @NotNull ISpan startChild( @NotNull String operation, @@ -50,6 +56,16 @@ public void setName(@NotNull String name, @NotNull TransactionNameSource transac return NoOpSpan.getInstance(); } + @Override + public @NotNull ISpan startChild( + @NotNull String operation, + @Nullable String description, + @Nullable SentryDate timestamp, + @NotNull Instrumenter instrumenter, + @NotNull SpanOptions spanOptions) { + return NoOpSpan.getInstance(); + } + @Override public @NotNull ISpan startChild( final @NotNull String operation, final @Nullable String description) { diff --git a/sentry/src/main/java/io/sentry/Scope.java b/sentry/src/main/java/io/sentry/Scope.java index ac27668611..0397214f0b 100644 --- a/sentry/src/main/java/io/sentry/Scope.java +++ b/sentry/src/main/java/io/sentry/Scope.java @@ -199,6 +199,16 @@ public ISpan getSpan() { return tx; } + /** + * Returns current active root Span, aka the Transaction. + * + * @return current active Span or Transaction or null if transaction has not been set. + */ + @Nullable + public ISpan getRootSpan() { + return transaction; + } + /** * Sets the current active transaction * diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index 4abcba8b1c..18a9946343 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -830,6 +830,15 @@ public static void endSession() { return getCurrentHub().getSpan(); } + /** + * Gets the current active transaction or span. + * + * @return the active span or null when no active transaction is running + */ + public static @Nullable ISpan getRootSpan() { + return getCurrentHub().getRootSpan(); + } + /** * Returns if the App has crashed (Process has terminated) during the last run. It only returns * true or false if offline caching {{@link SentryOptions#getCacheDirPath()} } is set with a valid diff --git a/sentry/src/main/java/io/sentry/SentryTracer.java b/sentry/src/main/java/io/sentry/SentryTracer.java index 10c774a839..ce5d3717e8 100644 --- a/sentry/src/main/java/io/sentry/SentryTracer.java +++ b/sentry/src/main/java/io/sentry/SentryTracer.java @@ -10,6 +10,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Timer; @@ -124,7 +125,7 @@ public SentryTracer( Objects.requireNonNull(context, "context is required"); Objects.requireNonNull(hub, "hub is required"); this.measurements = new ConcurrentHashMap<>(); - this.root = new Span(context, this, hub, startTimestamp); + this.root = new Span(context, this, hub, startTimestamp, new SpanOptions()); this.name = context.getName(); this.instrumenter = context.getInstrumenter(); this.hub = hub; @@ -207,9 +208,25 @@ ISpan startChild( final @NotNull SpanId parentSpanId, final @NotNull String operation, final @Nullable String description) { - final ISpan span = createChild(parentSpanId, operation); - span.setDescription(description); - return span; + return startChild(parentSpanId, operation, description, new SpanOptions()); + } + + /** + * Starts a child Span with given trace id and parent span id. + * + * @param parentSpanId - parent span id + * @param operation - span operation name + * @param description - span description + * @param spanOptions - span options + * @return a new transaction span + */ + @NotNull + ISpan startChild( + final @NotNull SpanId parentSpanId, + final @NotNull String operation, + final @Nullable String description, + final @NotNull SpanOptions spanOptions) { + return createChild(parentSpanId, operation, description, spanOptions); } @NotNull @@ -219,18 +236,37 @@ ISpan startChild( final @Nullable String description, final @Nullable SentryDate timestamp, final @NotNull Instrumenter instrumenter) { - return createChild(parentSpanId, operation, description, timestamp, instrumenter); + return createChild( + parentSpanId, operation, description, timestamp, instrumenter, new SpanOptions()); + } + + @NotNull + ISpan startChild( + final @NotNull SpanId parentSpanId, + final @NotNull String operation, + final @Nullable String description, + final @Nullable SentryDate timestamp, + final @NotNull Instrumenter instrumenter, + final @NotNull SpanOptions spanOptions) { + return createChild(parentSpanId, operation, description, timestamp, instrumenter, spanOptions); } /** * Starts a child Span with given trace id and parent span id. * * @param parentSpanId - parent span id + * @param operation - the span operation + * @param description - the optional span description + * @param options - span options * @return a new transaction span */ @NotNull - private ISpan createChild(final @NotNull SpanId parentSpanId, final @NotNull String operation) { - return createChild(parentSpanId, operation, null, null, Instrumenter.SENTRY); + private ISpan createChild( + final @NotNull SpanId parentSpanId, + final @NotNull String operation, + final @Nullable String description, + final @NotNull SpanOptions options) { + return createChild(parentSpanId, operation, description, null, Instrumenter.SENTRY, options); } @NotNull @@ -239,7 +275,8 @@ private ISpan createChild( final @NotNull String operation, final @Nullable String description, @Nullable SentryDate timestamp, - final @NotNull Instrumenter instrumenter) { + final @NotNull Instrumenter instrumenter, + final @NotNull SpanOptions spanOptions) { if (root.isFinished()) { return NoOpSpan.getInstance(); } @@ -259,6 +296,7 @@ private ISpan createChild( operation, this.hub, timestamp, + spanOptions, __ -> { final FinishStatus finishStatus = this.finishStatus; if (idleTimeout != null) { @@ -284,24 +322,41 @@ private ISpan createChild( @Override public @NotNull ISpan startChild( - final @NotNull String operation, + @NotNull String operation, @Nullable String description, @Nullable SentryDate timestamp, @NotNull Instrumenter instrumenter) { - return createChild(operation, description, timestamp, instrumenter); + return startChild(operation, description, timestamp, instrumenter, new SpanOptions()); + } + + @Override + public @NotNull ISpan startChild( + final @NotNull String operation, + @Nullable String description, + @Nullable SentryDate timestamp, + @NotNull Instrumenter instrumenter, + @NotNull SpanOptions spanOptions) { + return createChild(operation, description, timestamp, instrumenter, spanOptions); } @Override public @NotNull ISpan startChild( final @NotNull String operation, final @Nullable String description) { - return createChild(operation, description, null, Instrumenter.SENTRY); + return startChild(operation, description, null, Instrumenter.SENTRY, new SpanOptions()); + } + + @Override + public @NotNull ISpan startChild( + @NotNull String operation, @Nullable String description, @NotNull SpanOptions spanOptions) { + return createChild(operation, description, null, Instrumenter.SENTRY, spanOptions); } private @NotNull ISpan createChild( final @NotNull String operation, final @Nullable String description, @Nullable SentryDate timestamp, - final @NotNull Instrumenter instrumenter) { + final @NotNull Instrumenter instrumenter, + final @NotNull SpanOptions spanOptions) { if (root.isFinished()) { return NoOpSpan.getInstance(); } @@ -311,7 +366,7 @@ private ISpan createChild( } if (children.size() < hub.getOptions().getMaxSpans()) { - return root.startChild(operation, description, timestamp, instrumenter); + return root.startChild(operation, description, timestamp, instrumenter, spanOptions); } else { hub.getOptions() .getLogger() @@ -343,6 +398,50 @@ public void finish(@Nullable SpanStatus status) { @Override @ApiStatus.Internal public void finish(@Nullable SpanStatus status, @Nullable SentryDate finishDate) { + + // auto-finish any open spans + for (Span span : getSpans()) { + if (span.getOptions().isAutoFinish()) { + if (finishDate != null) { + span.finish(getSpanContext().status, finishDate); + } else { + span.finish(); + } + } + } + + // remove spans with no children + // trim if enabled + final Iterator iterator = children.iterator(); + while (iterator.hasNext()) { + final Span span = iterator.next(); + int childCount = 0; + @Nullable SentryDate minChildStart = null; + @Nullable SentryDate maxChildEnd = null; + for (Span child : children) { + if (child.getParentSpanId() != null && child.getParentSpanId().equals(span.getSpanId())) { + childCount++; + if (minChildStart == null || minChildStart.diff(child.getStartDate()) < 0) { + minChildStart = child.getStartDate(); + } + if (maxChildEnd == null + || (child.getFinishDate() != null && maxChildEnd.diff(child.getFinishDate()) < 0)) { + maxChildEnd = child.getFinishDate(); + } + } + } + if (span.getOptions().isRemoveIfNoChildren() && childCount == 0) { + iterator.remove(); + } else { + if (span.getOptions().isTrimStart() && minChildStart != null) { + span.setStartDate(minChildStart); + } + if (span.getOptions().isTrimEnd() && maxChildEnd != null) { + span.setEndDate(maxChildEnd); + } + } + } + this.finishStatus = FinishStatus.finishing(status); if (!root.isFinished() && (!waitForChildren || hasAllChildrenFinished())) { PerformanceCollectionData performanceCollectionData = null; diff --git a/sentry/src/main/java/io/sentry/Span.java b/sentry/src/main/java/io/sentry/Span.java index 428e73cb9a..826e59fe45 100644 --- a/sentry/src/main/java/io/sentry/Span.java +++ b/sentry/src/main/java/io/sentry/Span.java @@ -9,13 +9,12 @@ import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import org.jetbrains.annotations.VisibleForTesting; @ApiStatus.Internal public final class Span implements ISpan { /** The moment in time when span was started. */ - private final @NotNull SentryDate startTimestamp; + private @NotNull SentryDate startTimestamp; /** The moment in time when span has ended. */ private @Nullable SentryDate timestamp; @@ -35,6 +34,8 @@ public final class Span implements ISpan { private final @NotNull AtomicBoolean finished = new AtomicBoolean(false); + private final @NotNull SpanOptions options; + private @Nullable SpanFinishedCallback spanFinishedCallback; private final @NotNull Map data = new ConcurrentHashMap<>(); @@ -45,7 +46,7 @@ public final class Span implements ISpan { final @NotNull SentryTracer transaction, final @NotNull String operation, final @NotNull IHub hub) { - this(traceId, parentSpanId, transaction, operation, hub, null, null); + this(traceId, parentSpanId, transaction, operation, hub, null, new SpanOptions(), null); } Span( @@ -55,12 +56,14 @@ public final class Span implements ISpan { final @NotNull String operation, final @NotNull IHub hub, final @Nullable SentryDate startTimestamp, + final @NotNull SpanOptions options, final @Nullable SpanFinishedCallback spanFinishedCallback) { this.context = new SpanContext( traceId, new SpanId(), operation, parentSpanId, transaction.getSamplingDecision()); this.transaction = Objects.requireNonNull(transaction, "transaction is required"); this.hub = Objects.requireNonNull(hub, "hub is required"); + this.options = options; this.spanFinishedCallback = spanFinishedCallback; if (startTimestamp != null) { this.startTimestamp = startTimestamp; @@ -69,12 +72,12 @@ public final class Span implements ISpan { } } - @VisibleForTesting public Span( final @NotNull TransactionContext context, final @NotNull SentryTracer sentryTracer, final @NotNull IHub hub, - final @Nullable SentryDate startTimestamp) { + final @Nullable SentryDate startTimestamp, + final @NotNull SpanOptions options) { this.context = Objects.requireNonNull(context, "context is required"); this.transaction = Objects.requireNonNull(sentryTracer, "sentryTracer is required"); this.hub = Objects.requireNonNull(hub, "hub is required"); @@ -84,6 +87,7 @@ public Span( } else { this.startTimestamp = hub.getOptions().getDateProvider().now(); } + this.options = options; } public @NotNull SentryDate getStartDate() { @@ -104,13 +108,14 @@ public Span( final @NotNull String operation, final @Nullable String description, final @Nullable SentryDate timestamp, - final @NotNull Instrumenter instrumenter) { + final @NotNull Instrumenter instrumenter, + @NotNull SpanOptions spanOptions) { if (finished.get()) { return NoOpSpan.getInstance(); } return transaction.startChild( - context.getSpanId(), operation, description, timestamp, instrumenter); + context.getSpanId(), operation, description, timestamp, instrumenter, spanOptions); } @Override @@ -123,6 +128,24 @@ public Span( return transaction.startChild(context.getSpanId(), operation, description); } + @Override + public @NotNull ISpan startChild( + @NotNull String operation, @Nullable String description, @NotNull SpanOptions spanOptions) { + if (finished.get()) { + return NoOpSpan.getInstance(); + } + return transaction.startChild(context.getSpanId(), operation, description, spanOptions); + } + + @Override + public @NotNull ISpan startChild( + @NotNull String operation, + @Nullable String description, + @Nullable SentryDate timestamp, + @NotNull Instrumenter instrumenter) { + return startChild(operation, description, timestamp, instrumenter, new SpanOptions()); + } + @Override public @NotNull SentryTraceHeader toSentryTrace() { return new SentryTraceHeader(context.getTraceId(), context.getSpanId(), context.getSampled()); @@ -317,4 +340,17 @@ public boolean isNoOp() { void setSpanFinishedCallback(final @Nullable SpanFinishedCallback callback) { this.spanFinishedCallback = callback; } + + public void setStartDate(SentryDate date) { + this.startTimestamp = date; + } + + public void setEndDate(SentryDate date) { + this.timestamp = date; + } + + @NotNull + public SpanOptions getOptions() { + return options; + } } diff --git a/sentry/src/main/java/io/sentry/SpanOptions.java b/sentry/src/main/java/io/sentry/SpanOptions.java new file mode 100644 index 0000000000..86cce5c441 --- /dev/null +++ b/sentry/src/main/java/io/sentry/SpanOptions.java @@ -0,0 +1,43 @@ +package io.sentry; + +import org.jetbrains.annotations.ApiStatus; + +@ApiStatus.Internal +public final class SpanOptions { + + private final boolean trimStart; + private final boolean trimEnd; + private final boolean removeIfNoChildren; + private final boolean autoFinish; + + public SpanOptions() { + this(false, false, false, false); + } + + public SpanOptions( + final boolean trimStart, + final boolean trimEnd, + final boolean removeIfNoChildren, + final boolean autoFinish) { + this.trimStart = trimStart; + this.trimEnd = trimEnd; + this.removeIfNoChildren = removeIfNoChildren; + this.autoFinish = autoFinish; + } + + public boolean isTrimStart() { + return trimStart; + } + + public boolean isTrimEnd() { + return trimEnd; + } + + public boolean isRemoveIfNoChildren() { + return removeIfNoChildren; + } + + public boolean isAutoFinish() { + return autoFinish; + } +} From cd8e3e4b8aac306a37e63f35c4354c4a6de2ff51 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Wed, 1 Feb 2023 13:19:43 +0100 Subject: [PATCH 03/19] Improve Compose tracing, only first composition/rendering is tracked --- sentry-compose/api/android/sentry-compose.api | 4 + .../io/sentry/compose/SentryComposeTracing.kt | 83 ++++++--- .../android/compose/ComposeActivity.kt | 172 +++++++++--------- 3 files changed, 143 insertions(+), 116 deletions(-) diff --git a/sentry-compose/api/android/sentry-compose.api b/sentry-compose/api/android/sentry-compose.api index e30b863f6b..e951ebe14e 100644 --- a/sentry-compose/api/android/sentry-compose.api +++ b/sentry-compose/api/android/sentry-compose.api @@ -6,6 +6,10 @@ public final class io/sentry/compose/BuildConfig { public fun ()V } +public final class io/sentry/compose/SentryComposeTracingKt { + public static final fun SentryTraced (Ljava/lang/String;Landroidx/compose/ui/Modifier;Lkotlin/jvm/functions/Function3;Landroidx/compose/runtime/Composer;II)V +} + public final class io/sentry/compose/SentryNavigationIntegrationKt { public static final fun withSentryObservableEffect (Landroidx/navigation/NavHostController;ZZLandroidx/compose/runtime/Composer;II)Landroidx/navigation/NavHostController; } diff --git a/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt b/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt index dc0d15a2ca..48370733a1 100644 --- a/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt +++ b/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt @@ -1,46 +1,71 @@ package io.sentry.compose -import androidx.compose.runtime.Stable -import androidx.compose.runtime.mutableStateOf +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.BoxScope +import androidx.compose.runtime.Composable +import androidx.compose.runtime.Immutable +import androidx.compose.runtime.compositionLocalOf import androidx.compose.runtime.remember import androidx.compose.ui.ExperimentalComposeUiApi import androidx.compose.ui.Modifier -import androidx.compose.ui.composed import androidx.compose.ui.draw.drawWithContent -import androidx.compose.ui.modifier.ProvidableModifierLocal -import androidx.compose.ui.modifier.modifierLocalConsumer -import androidx.compose.ui.modifier.modifierLocalProvider import androidx.compose.ui.platform.testTag -import io.sentry.ISpan import io.sentry.Sentry +import io.sentry.SpanOptions -@Stable -private class StableHolder(val item: T) { - operator fun component1(): T = item +private const val OP_PARENT_COMPOSITION = "compose.composition" +private const val OP_COMPOSE = "compose" + +private const val OP_PARENT_RENDER = "compose.rendering" +private const val OP_RENDER = "render" + +@Immutable +private class ImmutableHolder(var item: T) + +private val localSentryCompositionParentSpan = compositionLocalOf { + ImmutableHolder( + Sentry.getRootSpan() + ?.startChild(OP_PARENT_COMPOSITION, null, SpanOptions(true, true, true, true)) + ) } -private val localSentrySpanModifier = ProvidableModifierLocal { - StableHolder(Sentry.getSpan()) +private val localSentryRenderingParentSpan = compositionLocalOf { + ImmutableHolder( + Sentry.getRootSpan() + ?.startChild(OP_PARENT_RENDER, null, SpanOptions(true, true, true, true)) + ) } @ExperimentalComposeUiApi -public fun Modifier.sentryTraced(tag: String): Modifier = Modifier.composed { - val span = remember { mutableStateOf>(StableHolder(null)) } - this - .testTag(tag) - .modifierLocalConsumer { - span.value = - StableHolder(localSentrySpanModifier.current.item?.startChild("ui.load", tag)) - } - .drawWithContent { - drawContent() - span.value.apply { - if (item?.isFinished == false) { - item.finish() +@Composable +public fun SentryTraced( + tag: String, + modifier: Modifier = Modifier, + content: @Composable BoxScope.() -> Unit +) { + val parentCompositionSpan = localSentryCompositionParentSpan.current + val parentRenderingSpan = localSentryRenderingParentSpan.current + val compositionSpan = parentCompositionSpan.item?.startChild(OP_COMPOSE, tag) + val firstRendered = remember { ImmutableHolder(false) } + + Box( + modifier = modifier + .testTag(tag) + .drawWithContent { + val renderSpan = if (!firstRendered.item) { + parentRenderingSpan.item?.startChild( + OP_RENDER, + tag + ) + } else { + null } + drawContent() + firstRendered.item = true + renderSpan?.finish() } - } - .modifierLocalProvider(localSentrySpanModifier) { - span.value - } + ) { + content() + } + compositionSpan?.finish() } diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt index 1a4d14ee85..fca9c00220 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt @@ -31,9 +31,7 @@ import androidx.navigation.compose.NavHost import androidx.navigation.compose.composable import androidx.navigation.compose.rememberNavController import androidx.navigation.navArgument -import io.sentry.Sentry -import io.sentry.compose.sentryTraced -import io.sentry.compose.withSentryObservableEffect +import io.sentry.compose.SentryTraced import io.sentry.samples.android.GithubAPI import kotlinx.coroutines.launch @@ -43,15 +41,10 @@ class ComposeActivity : ComponentActivity() { super.onCreate(savedInstanceState) setContent { - val navController = rememberNavController().withSentryObservableEffect() + val navController = rememberNavController() SampleNavigation(navController) } } - - override fun onResume() { - super.onResume() - Sentry.getSpan()?.finish() - } } @Composable @@ -59,38 +52,38 @@ fun Landing( navigateGithub: () -> Unit, navigateGithubWithArgs: () -> Unit ) { - Column( - verticalArrangement = Arrangement.Center, - horizontalAlignment = Alignment.CenterHorizontally, - modifier = Modifier - .sentryTraced("landing") - .fillMaxSize() - ) { - Button( - onClick = { - navigateGithub() - }, - modifier = Modifier - .sentryTraced("button_nav_github") - .padding(top = 32.dp) - ) { - Text("Navigate to Github Page") - } - Button( - onClick = { navigateGithubWithArgs() }, - modifier = Modifier - .sentryTraced("button_nav_github_args") - .padding(top = 32.dp) - ) { - Text("Navigate to Github Page With Args") - } - Button( - onClick = { throw RuntimeException("Crash from Compose") }, - modifier = Modifier - .sentryTraced("button_crash") - .padding(top = 32.dp) + SentryTraced(tag = "buttons_page") { + Column( + verticalArrangement = Arrangement.Center, + horizontalAlignment = Alignment.CenterHorizontally, + modifier = Modifier.fillMaxSize() ) { - Text("Crash from Compose") + SentryTraced(tag = "button_nav_github") { + Button( + onClick = { + navigateGithub() + }, + modifier = Modifier.padding(top = 32.dp) + ) { + Text("Navigate to Github") + } + } + SentryTraced(tag = "button_nav_github_args") { + Button( + onClick = { navigateGithubWithArgs() }, + modifier = Modifier.padding(top = 32.dp) + ) { + Text("Navigate to Github Page With Args") + } + } + SentryTraced(tag = "button_crash") { + Button( + onClick = { throw RuntimeException("Crash from Compose") }, + modifier = Modifier.padding(top = 32.dp) + ) { + Text("Crash from Compose") + } + } } } } @@ -108,62 +101,67 @@ fun Github( result = GithubAPI.service.listReposAsync(user.text, perPage).random().full_name } - Column( - verticalArrangement = Arrangement.Center, - horizontalAlignment = Alignment.CenterHorizontally, - modifier = Modifier - .sentryTraced("github-$user") - .fillMaxSize() - ) { - TextField( - value = user, - onValueChange = { newText -> - user = newText - } - ) - Text("Random repo $result") - Button( - onClick = { - scope.launch { - result = GithubAPI.service.listReposAsync(user.text, perPage).random().full_name - } - }, + SentryTraced("github-$user") { + Column( + verticalArrangement = Arrangement.Center, + horizontalAlignment = Alignment.CenterHorizontally, modifier = Modifier - .testTag("button_list_repos_async") - .padding(top = 32.dp) + .fillMaxSize() ) { - Text("Make Request") + TextField( + value = user, + onValueChange = { newText -> + user = newText + } + ) + Text("Random repo $result") + Button( + onClick = { + scope.launch { + result = + GithubAPI.service.listReposAsync(user.text, perPage).random().full_name + } + }, + modifier = Modifier + .testTag("button_list_repos_async") + .padding(top = 32.dp) + ) { + Text("Make Request") + } } } } @Composable fun SampleNavigation(navController: NavHostController) { - NavHost( - navController = navController, - startDestination = Destination.Landing.route, - // modifier = Modifier.sentryTraced("app") - ) { - composable(Destination.Landing.route) { - Landing( - navigateGithub = { navController.navigate("github") }, - navigateGithubWithArgs = { navController.navigate("github/spotify?per_page=10") } - ) - } - composable(Destination.Github.route) { - Github() - } - composable( - Destination.GithubWithArgs.route, - arguments = listOf( - navArgument(Destination.USER_ARG) { type = NavType.StringType }, - navArgument(Destination.PER_PAGE_ARG) { type = NavType.IntType; defaultValue = 10 } - ) + SentryTraced(tag = "navhost") { + NavHost( + navController = navController, + startDestination = Destination.Landing.route ) { - Github( - it.arguments?.getString(Destination.USER_ARG) ?: "getsentry", - it.arguments?.getInt(Destination.PER_PAGE_ARG) ?: 10 - ) + composable(Destination.Landing.route) { + Landing( + navigateGithub = { navController.navigate("github") }, + navigateGithubWithArgs = { navController.navigate("github/spotify?per_page=10") } + ) + } + composable(Destination.Github.route) { + Github() + } + composable( + Destination.GithubWithArgs.route, + arguments = listOf( + navArgument(Destination.USER_ARG) { type = NavType.StringType }, + navArgument(Destination.PER_PAGE_ARG) { + type = NavType.IntType; defaultValue = 10 + } + ) + ) { + Github( + it.arguments?.getString(Destination.USER_ARG) ?: "getsentry", + it.arguments?.getInt(Destination.PER_PAGE_ARG) ?: 10 + ) + } } } } From 81f1ba51640d889733ab6d02bd73af712cb95a75 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Wed, 1 Feb 2023 13:30:14 +0100 Subject: [PATCH 04/19] Fix span trimming logic --- sentry/src/main/java/io/sentry/SentryDate.java | 8 ++++++++ sentry/src/main/java/io/sentry/SentryTracer.java | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/sentry/src/main/java/io/sentry/SentryDate.java b/sentry/src/main/java/io/sentry/SentryDate.java index 097c3dca9a..ed44720a3f 100644 --- a/sentry/src/main/java/io/sentry/SentryDate.java +++ b/sentry/src/main/java/io/sentry/SentryDate.java @@ -37,6 +37,14 @@ public long diff(final @NotNull SentryDate otherDate) { return nanoTimestamp() - otherDate.nanoTimestamp(); } + public boolean beforeOrEqual(final @NotNull SentryDate otherDate) { + return diff(otherDate) <= 0; + } + + public boolean afterOrEqual(final @NotNull SentryDate otherDate) { + return diff(otherDate) >= 0; + } + @Override public int compareTo(@NotNull SentryDate otherDate) { return Long.valueOf(nanoTimestamp()).compareTo(otherDate.nanoTimestamp()); diff --git a/sentry/src/main/java/io/sentry/SentryTracer.java b/sentry/src/main/java/io/sentry/SentryTracer.java index ce5d3717e8..b75de57cca 100644 --- a/sentry/src/main/java/io/sentry/SentryTracer.java +++ b/sentry/src/main/java/io/sentry/SentryTracer.java @@ -421,11 +421,11 @@ public void finish(@Nullable SpanStatus status, @Nullable SentryDate finishDate) for (Span child : children) { if (child.getParentSpanId() != null && child.getParentSpanId().equals(span.getSpanId())) { childCount++; - if (minChildStart == null || minChildStart.diff(child.getStartDate()) < 0) { + if (minChildStart == null || child.getStartDate().beforeOrEqual(minChildStart)) { minChildStart = child.getStartDate(); } if (maxChildEnd == null - || (child.getFinishDate() != null && maxChildEnd.diff(child.getFinishDate()) < 0)) { + || (child.getFinishDate() != null && child.getFinishDate().afterOrEqual(maxChildEnd))) { maxChildEnd = child.getFinishDate(); } } From c43fcd40e349da9530595abb3363e257e9d3f429 Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Wed, 1 Feb 2023 12:33:45 +0000 Subject: [PATCH 05/19] Format code --- sentry/src/main/java/io/sentry/SentryTracer.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sentry/src/main/java/io/sentry/SentryTracer.java b/sentry/src/main/java/io/sentry/SentryTracer.java index b75de57cca..180719a574 100644 --- a/sentry/src/main/java/io/sentry/SentryTracer.java +++ b/sentry/src/main/java/io/sentry/SentryTracer.java @@ -425,7 +425,8 @@ public void finish(@Nullable SpanStatus status, @Nullable SentryDate finishDate) minChildStart = child.getStartDate(); } if (maxChildEnd == null - || (child.getFinishDate() != null && child.getFinishDate().afterOrEqual(maxChildEnd))) { + || (child.getFinishDate() != null + && child.getFinishDate().afterOrEqual(maxChildEnd))) { maxChildEnd = child.getFinishDate(); } } From a105676ebe0aad493863aff4c2e1dab13426ad4e Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Fri, 3 Feb 2023 09:28:59 +0100 Subject: [PATCH 06/19] Add tests, remove optional span --- .../io/sentry/compose/SentryComposeTracing.kt | 4 +- sentry/api/sentry.api | 5 +- .../src/main/java/io/sentry/SentryDate.java | 8 +- .../src/main/java/io/sentry/SentryTracer.java | 33 ++--- .../src/main/java/io/sentry/SpanOptions.java | 21 ++- .../test/java/io/sentry/SentryLongDateTest.kt | 46 +++++++ .../test/java/io/sentry/SentryTracerTest.kt | 126 ++++++++++++++++++ 7 files changed, 203 insertions(+), 40 deletions(-) diff --git a/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt b/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt index 48370733a1..467ab236f8 100644 --- a/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt +++ b/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt @@ -25,14 +25,14 @@ private class ImmutableHolder(var item: T) private val localSentryCompositionParentSpan = compositionLocalOf { ImmutableHolder( Sentry.getRootSpan() - ?.startChild(OP_PARENT_COMPOSITION, null, SpanOptions(true, true, true, true)) + ?.startChild(OP_PARENT_COMPOSITION, null, SpanOptions(true, true, true)) ) } private val localSentryRenderingParentSpan = compositionLocalOf { ImmutableHolder( Sentry.getRootSpan() - ?.startChild(OP_PARENT_RENDER, null, SpanOptions(true, true, true, true)) + ?.startChild(OP_PARENT_RENDER, null, SpanOptions(true, true, true)) ) } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 074fc07f9f..ac29323880 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -1297,6 +1297,8 @@ public abstract class io/sentry/SentryDate : java/lang/Comparable { public fun compareTo (Lio/sentry/SentryDate;)I public synthetic fun compareTo (Ljava/lang/Object;)I public fun diff (Lio/sentry/SentryDate;)J + public final fun isAfter (Lio/sentry/SentryDate;)Z + public final fun isBefore (Lio/sentry/SentryDate;)Z public fun laterDateNanosTimestampByDiff (Lio/sentry/SentryDate;)J public abstract fun nanoTimestamp ()J } @@ -1963,9 +1965,8 @@ public final class io/sentry/SpanId$Deserializer : io/sentry/JsonDeserializer { public final class io/sentry/SpanOptions { public fun ()V - public fun (ZZZZ)V + public fun (ZZZ)V public fun isAutoFinish ()Z - public fun isRemoveIfNoChildren ()Z public fun isTrimEnd ()Z public fun isTrimStart ()Z } diff --git a/sentry/src/main/java/io/sentry/SentryDate.java b/sentry/src/main/java/io/sentry/SentryDate.java index ed44720a3f..d2620ab302 100644 --- a/sentry/src/main/java/io/sentry/SentryDate.java +++ b/sentry/src/main/java/io/sentry/SentryDate.java @@ -37,12 +37,12 @@ public long diff(final @NotNull SentryDate otherDate) { return nanoTimestamp() - otherDate.nanoTimestamp(); } - public boolean beforeOrEqual(final @NotNull SentryDate otherDate) { - return diff(otherDate) <= 0; + public final boolean isBefore(final @NotNull SentryDate otherDate) { + return diff(otherDate) < 0; } - public boolean afterOrEqual(final @NotNull SentryDate otherDate) { - return diff(otherDate) >= 0; + public final boolean isAfter(final @NotNull SentryDate otherDate) { + return diff(otherDate) > 0; } @Override diff --git a/sentry/src/main/java/io/sentry/SentryTracer.java b/sentry/src/main/java/io/sentry/SentryTracer.java index b75de57cca..e4bbfb7e89 100644 --- a/sentry/src/main/java/io/sentry/SentryTracer.java +++ b/sentry/src/main/java/io/sentry/SentryTracer.java @@ -10,7 +10,6 @@ import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Timer; @@ -410,29 +409,23 @@ public void finish(@Nullable SpanStatus status, @Nullable SentryDate finishDate) } } - // remove spans with no children - // trim if enabled - final Iterator iterator = children.iterator(); - while (iterator.hasNext()) { - final Span span = iterator.next(); - int childCount = 0; + // trim span to children if enabled + for (Span span : children) { @Nullable SentryDate minChildStart = null; @Nullable SentryDate maxChildEnd = null; - for (Span child : children) { - if (child.getParentSpanId() != null && child.getParentSpanId().equals(span.getSpanId())) { - childCount++; - if (minChildStart == null || child.getStartDate().beforeOrEqual(minChildStart)) { - minChildStart = child.getStartDate(); - } - if (maxChildEnd == null - || (child.getFinishDate() != null && child.getFinishDate().afterOrEqual(maxChildEnd))) { - maxChildEnd = child.getFinishDate(); + + if (span.getOptions().isTrimStart() || span.getOptions().isTrimEnd()) { + for (Span child : children) { + if (child.getParentSpanId() != null && child.getParentSpanId().equals(span.getSpanId())) { + if (minChildStart == null || child.getStartDate().isBefore(minChildStart)) { + minChildStart = child.getStartDate(); + } + if (maxChildEnd == null + || (child.getFinishDate() != null && child.getFinishDate().isAfter(maxChildEnd))) { + maxChildEnd = child.getFinishDate(); + } } } - } - if (span.getOptions().isRemoveIfNoChildren() && childCount == 0) { - iterator.remove(); - } else { if (span.getOptions().isTrimStart() && minChildStart != null) { span.setStartDate(minChildStart); } diff --git a/sentry/src/main/java/io/sentry/SpanOptions.java b/sentry/src/main/java/io/sentry/SpanOptions.java index 86cce5c441..2218148dd1 100644 --- a/sentry/src/main/java/io/sentry/SpanOptions.java +++ b/sentry/src/main/java/io/sentry/SpanOptions.java @@ -7,21 +7,22 @@ public final class SpanOptions { private final boolean trimStart; private final boolean trimEnd; - private final boolean removeIfNoChildren; private final boolean autoFinish; public SpanOptions() { - this(false, false, false, false); + this(false, false, false); } - public SpanOptions( - final boolean trimStart, - final boolean trimEnd, - final boolean removeIfNoChildren, - final boolean autoFinish) { + /** + * @param trimStart true if the start time should be trimmed to the minimum start time of it's + * children + * @param trimEnd true if the end time should be trimmed to the maximum end time of it's children + * @param autoFinish true if this span should be finished whenever the root transaction gets + * finished + */ + public SpanOptions(final boolean trimStart, final boolean trimEnd, final boolean autoFinish) { this.trimStart = trimStart; this.trimEnd = trimEnd; - this.removeIfNoChildren = removeIfNoChildren; this.autoFinish = autoFinish; } @@ -33,10 +34,6 @@ public boolean isTrimEnd() { return trimEnd; } - public boolean isRemoveIfNoChildren() { - return removeIfNoChildren; - } - public boolean isAutoFinish() { return autoFinish; } diff --git a/sentry/src/test/java/io/sentry/SentryLongDateTest.kt b/sentry/src/test/java/io/sentry/SentryLongDateTest.kt index a1b7f81fa5..1f97d3c852 100644 --- a/sentry/src/test/java/io/sentry/SentryLongDateTest.kt +++ b/sentry/src/test/java/io/sentry/SentryLongDateTest.kt @@ -2,6 +2,8 @@ package io.sentry import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue class SentryLongDateTest { @@ -50,4 +52,48 @@ class SentryLongDateTest { assertEquals(1, 1672742031123456789.compareTo(1672742031123456788)) assertEquals(1, date1.compareTo(date2)) } + + // isBefore() + @Test + fun `isBefore() returns true if for an earlier date`() { + val date1 = SentryLongDate(0) + val date2 = SentryLongDate(1) + assertTrue(date1.isBefore(date2)) + } + + @Test + fun `isBefore() returns false for the same date`() { + val date1 = SentryLongDate(1) + val date2 = SentryLongDate(1) + assertFalse(date1.isBefore(date2)) + } + + @Test + fun `isBefore() returns false for a later date`() { + val date1 = SentryLongDate(2) + val date2 = SentryLongDate(1) + assertFalse(date1.isBefore(date2)) + } + + // isAfter() + @Test + fun `isAfter() returns true if for a later date`() { + val date1 = SentryLongDate(2) + val date2 = SentryLongDate(1) + assertTrue(date1.isAfter(date2)) + } + + @Test + fun `isAfter() returns false for the same date`() { + val date1 = SentryLongDate(1) + val date2 = SentryLongDate(1) + assertFalse(date1.isAfter(date2)) + } + + @Test + fun `isAfter() returns false for a sooner date`() { + val date1 = SentryLongDate(1) + val date2 = SentryLongDate(2) + assertFalse(date1.isAfter(date2)) + } } diff --git a/sentry/src/test/java/io/sentry/SentryTracerTest.kt b/sentry/src/test/java/io/sentry/SentryTracerTest.kt index d25631a5a3..ba22ff8da6 100644 --- a/sentry/src/test/java/io/sentry/SentryTracerTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTracerTest.kt @@ -909,4 +909,130 @@ class SentryTracerTest { assertEquals("new-name-2", transaction.name) assertEquals(TransactionNameSource.CUSTOM, transaction.transactionNameSource) } + + @Test + fun `when spans have auto-finish enabled, finish them on transaction finish`() { + val transaction = fixture.getSut(waitForChildren = true, idleTimeout = 50, trimEnd = true, samplingDecision = TracesSamplingDecision(true)) + + // when a span is created with auto-finish + val span = transaction.startChild("composition", null, SpanOptions(false, false, true)) as Span + + // and the transaction is finished + transaction.finish() + + // then the span should be finished as well + assertTrue(span.isFinished) + + // and the transaction should be captured + verify(fixture.hub).captureTransaction( + check { + assertEquals(1, it.spans.size) + assertEquals(transaction.root.finishDate, span.finishDate) + }, + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + } + + @Test + fun `when spans have auto-finish enabled, finish them on transaction finish using the transaction finish date`() { + val transaction = fixture.getSut(waitForChildren = true, idleTimeout = 50, trimEnd = true, samplingDecision = TracesSamplingDecision(true)) + + // when a span is created with auto-finish + val span = transaction.startChild("composition", null, SpanOptions(false, false, true)) as Span + + // and the transaction is finished + Thread.sleep(1) + val transactionFinishDate = SentryAutoDateProvider().now() + transaction.finish(SpanStatus.OK, transactionFinishDate) + + // then the span should be finished as well + assertTrue(span.isFinished) + + // and the transaction should be captured + verify(fixture.hub).captureTransaction( + check { + assertEquals(1, it.spans.size) + assertEquals(transactionFinishDate, span.finishDate) + }, + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + } + + @Test + fun `when spans have trim-start enabled, trim them on transaction finish`() { + val transaction = fixture.getSut(waitForChildren = true, idleTimeout = 50, trimEnd = true, samplingDecision = TracesSamplingDecision(true)) + + // when a parent span is created + val parentSpan = transaction.startChild("composition", null, SpanOptions(true, false, true)) as Span + + // with a child which starts later + Thread.sleep(5) + val child1 = parentSpan.startChild("child1") as Span + child1.finish() + + val child2 = parentSpan.startChild("child2") as Span + Thread.sleep(5) + child2.finish() + + parentSpan.finish() + + val expectedParentStartDate = child1.startDate + val expectedParentEndDate = parentSpan.finishDate + + transaction.finish() + + assertTrue(parentSpan.isFinished) + assertEquals(expectedParentStartDate, parentSpan.startDate) + assertEquals(expectedParentEndDate, parentSpan.finishDate) + + verify(fixture.hub).captureTransaction( + check { + assertEquals(3, it.spans.size) + }, + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + } + + @Test + fun `when spans have trim-end enabled, trim them on transaction finish`() { + val transaction = fixture.getSut(waitForChildren = true, idleTimeout = 50, trimEnd = true, samplingDecision = TracesSamplingDecision(true)) + + // when a parent span is created + val parentSpan = transaction.startChild("composition", null, SpanOptions(false, true, true)) as Span + + // with a child which starts later + Thread.sleep(5) + val child1 = parentSpan.startChild("child1") as Span + child1.finish() + + val child2 = parentSpan.startChild("child2") as Span + Thread.sleep(5) + child2.finish() + + parentSpan.finish() + + val expectedParentStartDate = parentSpan.startDate + val expectedParentEndDate = child2.finishDate + + transaction.finish() + + assertTrue(parentSpan.isFinished) + assertEquals(expectedParentStartDate, parentSpan.startDate) + assertEquals(expectedParentEndDate, parentSpan.finishDate) + + verify(fixture.hub).captureTransaction( + check { + assertEquals(3, it.spans.size) + }, + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + } } From 17500bd098522d711cfbaff8d21d4ea955645824 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Fri, 3 Feb 2023 10:36:53 +0100 Subject: [PATCH 07/19] Update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 18d48c036c..45f2fdca37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Features + +- Add capabilities to track compose composition/rendering time ([#2507](https://github.com/getsentry/sentry-java/pull/2507)) + ### Fixes - Remove authority from URLs sent to Sentry ([#2366](https://github.com/getsentry/sentry-java/pull/2366)) From 78c7345752705be16dc63b73c52eef17e3f6e5f9 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Thu, 9 Feb 2023 10:34:57 +0100 Subject: [PATCH 08/19] Apply suggestions from code review Co-authored-by: Roman Zavarnitsyn --- CHANGELOG.md | 2 +- .../kotlin/io/sentry/compose/SentryComposeTracing.kt | 8 ++++---- .../io/sentry/samples/android/compose/ComposeActivity.kt | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 45f2fdca37..c44ffae70c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Features -- Add capabilities to track compose composition/rendering time ([#2507](https://github.com/getsentry/sentry-java/pull/2507)) +- Add capabilities to track Jetpack Compose composition/rendering time ([#2507](https://github.com/getsentry/sentry-java/pull/2507)) ### Fixes diff --git a/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt b/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt index 467ab236f8..b829ac3f5d 100644 --- a/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt +++ b/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt @@ -13,11 +13,11 @@ import androidx.compose.ui.platform.testTag import io.sentry.Sentry import io.sentry.SpanOptions -private const val OP_PARENT_COMPOSITION = "compose.composition" -private const val OP_COMPOSE = "compose" +private const val OP_PARENT_COMPOSITION = "ui.compose.composition" +private const val OP_COMPOSE = "ui.compose" -private const val OP_PARENT_RENDER = "compose.rendering" -private const val OP_RENDER = "render" +private const val OP_PARENT_RENDER = "ui.compose.rendering" +private const val OP_RENDER = "ui.render" @Immutable private class ImmutableHolder(var item: T) diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt index fca9c00220..f4d64f1418 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt @@ -41,7 +41,7 @@ class ComposeActivity : ComponentActivity() { super.onCreate(savedInstanceState) setContent { - val navController = rememberNavController() + val navController = rememberNavController().withSentryObservableEffect() SampleNavigation(navController) } } From 49f13100a1a6fd5de94315888cb010700f89b9b0 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Thu, 16 Feb 2023 14:34:21 +0100 Subject: [PATCH 09/19] Update based on PR comments --- .../io/sentry/compose/SentryComposeTracing.kt | 17 ++- sentry/api/sentry.api | 14 +- sentry/src/main/java/io/sentry/Hub.java | 13 -- .../src/main/java/io/sentry/HubAdapter.java | 5 - sentry/src/main/java/io/sentry/IHub.java | 3 - sentry/src/main/java/io/sentry/NoOpHub.java | 5 - sentry/src/main/java/io/sentry/Scope.java | 10 -- sentry/src/main/java/io/sentry/Sentry.java | 9 -- .../src/main/java/io/sentry/SentryTracer.java | 137 ++++-------------- sentry/src/main/java/io/sentry/Span.java | 57 +++++++- .../src/main/java/io/sentry/SpanOptions.java | 71 ++++++++- .../test/java/io/sentry/SentryTracerTest.kt | 10 +- 12 files changed, 169 insertions(+), 182 deletions(-) diff --git a/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt b/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt index b829ac3f5d..3687a777e5 100644 --- a/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt +++ b/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt @@ -10,6 +10,7 @@ import androidx.compose.ui.ExperimentalComposeUiApi import androidx.compose.ui.Modifier import androidx.compose.ui.draw.drawWithContent import androidx.compose.ui.platform.testTag +import io.sentry.ISpan import io.sentry.Sentry import io.sentry.SpanOptions @@ -22,17 +23,25 @@ private const val OP_RENDER = "ui.render" @Immutable private class ImmutableHolder(var item: T) +private fun getRootSpan(): ISpan? { + var rootSpan: ISpan? = null + Sentry.configureScope { + rootSpan = it.transaction + } + return rootSpan +} + private val localSentryCompositionParentSpan = compositionLocalOf { ImmutableHolder( - Sentry.getRootSpan() - ?.startChild(OP_PARENT_COMPOSITION, null, SpanOptions(true, true, true)) + getRootSpan() + ?.startChild(OP_PARENT_COMPOSITION, null, SpanOptions(true, true, true, false, null)) ) } private val localSentryRenderingParentSpan = compositionLocalOf { ImmutableHolder( - Sentry.getRootSpan() - ?.startChild(OP_PARENT_RENDER, null, SpanOptions(true, true, true)) + getRootSpan() + ?.startChild(OP_PARENT_RENDER, null, SpanOptions(true, true, true, false, null)) ) } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index e72c06beb4..ffda14741e 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -325,7 +325,6 @@ public final class io/sentry/Hub : io/sentry/IHub { public fun flush (J)V public fun getLastEventId ()Lio/sentry/protocol/SentryId; public fun getOptions ()Lio/sentry/SentryOptions; - public fun getRootSpan ()Lio/sentry/ISpan; public fun getSpan ()Lio/sentry/ISpan; public fun isCrashedLastRun ()Ljava/lang/Boolean; public fun isEnabled ()Z @@ -369,7 +368,6 @@ public final class io/sentry/HubAdapter : io/sentry/IHub { public static fun getInstance ()Lio/sentry/HubAdapter; public fun getLastEventId ()Lio/sentry/protocol/SentryId; public fun getOptions ()Lio/sentry/SentryOptions; - public fun getRootSpan ()Lio/sentry/ISpan; public fun getSpan ()Lio/sentry/ISpan; public fun isCrashedLastRun ()Ljava/lang/Boolean; public fun isEnabled ()Z @@ -438,7 +436,6 @@ public abstract interface class io/sentry/IHub { public abstract fun flush (J)V public abstract fun getLastEventId ()Lio/sentry/protocol/SentryId; public abstract fun getOptions ()Lio/sentry/SentryOptions; - public abstract fun getRootSpan ()Lio/sentry/ISpan; public abstract fun getSpan ()Lio/sentry/ISpan; public abstract fun isCrashedLastRun ()Ljava/lang/Boolean; public abstract fun isEnabled ()Z @@ -761,7 +758,6 @@ public final class io/sentry/NoOpHub : io/sentry/IHub { public static fun getInstance ()Lio/sentry/NoOpHub; public fun getLastEventId ()Lio/sentry/protocol/SentryId; public fun getOptions ()Lio/sentry/SentryOptions; - public fun getRootSpan ()Lio/sentry/ISpan; public fun getSpan ()Lio/sentry/ISpan; public fun isCrashedLastRun ()Ljava/lang/Boolean; public fun isEnabled ()Z @@ -1063,7 +1059,6 @@ public final class io/sentry/Scope { public fun getContexts ()Lio/sentry/protocol/Contexts; public fun getLevel ()Lio/sentry/SentryLevel; public fun getRequest ()Lio/sentry/protocol/Request; - public fun getRootSpan ()Lio/sentry/ISpan; public fun getSession ()Lio/sentry/Session; public fun getSpan ()Lio/sentry/ISpan; public fun getTags ()Ljava/util/Map; @@ -1154,7 +1149,6 @@ public final class io/sentry/Sentry { public static fun flush (J)V public static fun getCurrentHub ()Lio/sentry/IHub; public static fun getLastEventId ()Lio/sentry/protocol/SentryId; - public static fun getRootSpan ()Lio/sentry/ISpan; public static fun getSpan ()Lio/sentry/ISpan; public static fun init ()V public static fun init (Lio/sentry/OptionsContainer;Lio/sentry/Sentry$OptionsConfiguration;)V @@ -1879,7 +1873,6 @@ public final class io/sentry/Span : io/sentry/ISpan { public fun isSampled ()Ljava/lang/Boolean; public fun setData (Ljava/lang/String;Ljava/lang/Object;)V public fun setDescription (Ljava/lang/String;)V - public fun setEndDate (Lio/sentry/SentryDate;)V public fun setMeasurement (Ljava/lang/String;Ljava/lang/Number;)V public fun setMeasurement (Ljava/lang/String;Ljava/lang/Number;Lio/sentry/MeasurementUnit;)V public fun setOperation (Ljava/lang/String;)V @@ -1895,6 +1888,7 @@ public final class io/sentry/Span : io/sentry/ISpan { public fun toBaggageHeader (Ljava/util/List;)Lio/sentry/BaggageHeader; public fun toSentryTrace ()Lio/sentry/SentryTraceHeader; public fun traceContext ()Lio/sentry/TraceContext; + public fun updateEndDate (Lio/sentry/SentryDate;)Z } public class io/sentry/SpanContext : io/sentry/JsonSerializable, io/sentry/JsonUnknown { @@ -1965,10 +1959,12 @@ public final class io/sentry/SpanId$Deserializer : io/sentry/JsonDeserializer { public final class io/sentry/SpanOptions { public fun ()V - public fun (ZZZ)V - public fun isAutoFinish ()Z + public fun (ZZZZLjava/lang/Long;)V + public fun getIdleTimeout ()Ljava/lang/Long; + public fun isIdle ()Z public fun isTrimEnd ()Z public fun isTrimStart ()Z + public fun isWaitForChildren ()Z } public final class io/sentry/SpanStatus : java/lang/Enum, io/sentry/JsonSerializable { diff --git a/sentry/src/main/java/io/sentry/Hub.java b/sentry/src/main/java/io/sentry/Hub.java index 7228341d2a..bcfa9b87f7 100644 --- a/sentry/src/main/java/io/sentry/Hub.java +++ b/sentry/src/main/java/io/sentry/Hub.java @@ -777,19 +777,6 @@ public void flush(long timeoutMillis) { return span; } - @Override - public @Nullable ISpan getRootSpan() { - ISpan span = null; - if (!isEnabled()) { - options - .getLogger() - .log(SentryLevel.WARNING, "Instance is disabled and this 'getRootSpan' call is a no-op."); - } else { - span = stack.peek().getScope().getRootSpan(); - } - return span; - } - @Override @ApiStatus.Internal public void setSpanContext( diff --git a/sentry/src/main/java/io/sentry/HubAdapter.java b/sentry/src/main/java/io/sentry/HubAdapter.java index 413362304c..0a7756863a 100644 --- a/sentry/src/main/java/io/sentry/HubAdapter.java +++ b/sentry/src/main/java/io/sentry/HubAdapter.java @@ -220,11 +220,6 @@ public void setSpanContext( return Sentry.getCurrentHub().getSpan(); } - @Override - public @Nullable ISpan getRootSpan() { - return Sentry.getCurrentHub().getRootSpan(); - } - @Override public @NotNull SentryOptions getOptions() { return Sentry.getCurrentHub().getOptions(); diff --git a/sentry/src/main/java/io/sentry/IHub.java b/sentry/src/main/java/io/sentry/IHub.java index 547bb05fef..01d8546d84 100644 --- a/sentry/src/main/java/io/sentry/IHub.java +++ b/sentry/src/main/java/io/sentry/IHub.java @@ -543,9 +543,6 @@ void setSpanContext( @Nullable ISpan getSpan(); - @Nullable - ISpan getRootSpan(); - /** * Gets the {@link SentryOptions} attached to current scope. * diff --git a/sentry/src/main/java/io/sentry/NoOpHub.java b/sentry/src/main/java/io/sentry/NoOpHub.java index a4056e502d..52bef0ff43 100644 --- a/sentry/src/main/java/io/sentry/NoOpHub.java +++ b/sentry/src/main/java/io/sentry/NoOpHub.java @@ -177,11 +177,6 @@ public void setSpanContext( return null; } - @Override - public @Nullable ISpan getRootSpan() { - return null; - } - @Override public @NotNull SentryOptions getOptions() { return emptyOptions; diff --git a/sentry/src/main/java/io/sentry/Scope.java b/sentry/src/main/java/io/sentry/Scope.java index 0397214f0b..ac27668611 100644 --- a/sentry/src/main/java/io/sentry/Scope.java +++ b/sentry/src/main/java/io/sentry/Scope.java @@ -199,16 +199,6 @@ public ISpan getSpan() { return tx; } - /** - * Returns current active root Span, aka the Transaction. - * - * @return current active Span or Transaction or null if transaction has not been set. - */ - @Nullable - public ISpan getRootSpan() { - return transaction; - } - /** * Sets the current active transaction * diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index 18a9946343..4abcba8b1c 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -830,15 +830,6 @@ public static void endSession() { return getCurrentHub().getSpan(); } - /** - * Gets the current active transaction or span. - * - * @return the active span or null when no active transaction is running - */ - public static @Nullable ISpan getRootSpan() { - return getCurrentHub().getRootSpan(); - } - /** * Returns if the App has crashed (Process has terminated) during the last run. It only returns * true or false if offline caching {{@link SentryOptions#getCacheDirPath()} } is set with a valid diff --git a/sentry/src/main/java/io/sentry/SentryTracer.java b/sentry/src/main/java/io/sentry/SentryTracer.java index 4b1a3b7c83..4c4ec32b0c 100644 --- a/sentry/src/main/java/io/sentry/SentryTracer.java +++ b/sentry/src/main/java/io/sentry/SentryTracer.java @@ -8,8 +8,6 @@ import io.sentry.protocol.User; import io.sentry.util.Objects; import java.util.ArrayList; -import java.util.Collections; -import java.util.Comparator; import java.util.List; import java.util.Map; import java.util.Timer; @@ -30,12 +28,6 @@ public final class SentryTracer implements ITransaction { private final @NotNull List children = new CopyOnWriteArrayList<>(); private final @NotNull IHub hub; private @NotNull String name; - /** - * When `waitForChildren` is set to `true`, tracer will finish only when both conditions are met - * (the order of meeting condition does not matter): - tracer itself is finished - all child spans - * are finished - */ - private final boolean waitForChildren; /** * Holds the status for finished tracer. Tracer can have finishedStatus set, but not be finished @@ -50,28 +42,9 @@ public final class SentryTracer implements ITransaction { */ private final @Nullable TransactionFinishedCallback transactionFinishedCallback; - /** - * If `trimEnd` is true, sets the end timestamp of the transaction to the highest timestamp of - * child spans, trimming the duration of the transaction. This is useful to discard extra time in - * the idle transactions to trim their duration to children' duration. - */ - private final boolean trimEnd; - - /** - * The idle time, measured in ms, to wait until the transaction will be finished. The transaction - * will use the end timestamp of the last finished span as the endtime for the transaction. - * - *

When set to {@code null} the transaction must be finished manually. - * - *

The default is 3 seconds. - */ - private final @Nullable Long idleTimeout; - private volatile @Nullable TimerTask timerTask; private volatile @Nullable Timer timer = null; private final @NotNull Object timerLock = new Object(); - private final @NotNull SpanByTimestampComparator spanByTimestampComparator = - new SpanByTimestampComparator(); private final @NotNull AtomicBoolean isFinishTimerRunning = new AtomicBoolean(false); private final @NotNull Baggage baggage; @@ -124,13 +97,16 @@ public SentryTracer( Objects.requireNonNull(context, "context is required"); Objects.requireNonNull(hub, "hub is required"); this.measurements = new ConcurrentHashMap<>(); - this.root = new Span(context, this, hub, startTimestamp, new SpanOptions()); + this.root = + new Span( + context, + this, + hub, + startTimestamp, + new SpanOptions(false, trimEnd, false, waitForChildren, idleTimeout)); this.name = context.getName(); this.instrumenter = context.getInstrumenter(); this.hub = hub; - this.waitForChildren = waitForChildren; - this.idleTimeout = idleTimeout; - this.trimEnd = trimEnd; this.transactionFinishedCallback = transactionFinishedCallback; this.transactionPerformanceCollector = transactionPerformanceCollector; this.transactionNameSource = context.getTransactionNameSource(); @@ -169,7 +145,7 @@ public void run() { } }; - timer.schedule(timerTask, idleTimeout); + timer.schedule(timerTask, root.getOptions().getIdleTimeout()); } } } @@ -300,11 +276,11 @@ private ISpan createChild( spanOptions, __ -> { final FinishStatus finishStatus = this.finishStatus; - if (idleTimeout != null) { + if (root.getOptions().getIdleTimeout() != null) { // if it's an idle transaction, no matter the status, we'll reset the timeout here // so the transaction will either idle and finish itself, or a new child will be // added and we'll wait for it again - if (!waitForChildren || hasAllChildrenFinished()) { + if (!root.getOptions().isWaitForChildren() || hasAllChildrenFinished()) { scheduleFinish(); } } else if (finishStatus.isFinishing) { @@ -400,45 +376,29 @@ public void finish(@Nullable SpanStatus status) { @ApiStatus.Internal public void finish(@Nullable SpanStatus status, @Nullable SentryDate finishDate) { - // auto-finish any open spans - for (Span span : getSpans()) { - if (span.getOptions().isAutoFinish()) { - if (finishDate != null) { - span.finish(getSpanContext().status, finishDate); - } else { - span.finish(); - } - } + // try to get the high precision timestamp from the root span + SentryDate finishTimestamp = root.getFinishDate(); + + // if a finishDate was passed in, use that instead + if (finishDate != null) { + finishTimestamp = finishDate; } - // trim span to children if enabled - for (Span span : children) { - @Nullable SentryDate minChildStart = null; - @Nullable SentryDate maxChildEnd = null; - - if (span.getOptions().isTrimStart() || span.getOptions().isTrimEnd()) { - for (Span child : children) { - if (child.getParentSpanId() != null && child.getParentSpanId().equals(span.getSpanId())) { - if (minChildStart == null || child.getStartDate().isBefore(minChildStart)) { - minChildStart = child.getStartDate(); - } - if (maxChildEnd == null - || (child.getFinishDate() != null && child.getFinishDate().isAfter(maxChildEnd))) { - maxChildEnd = child.getFinishDate(); - } - } - } - if (span.getOptions().isTrimStart() && minChildStart != null) { - span.setStartDate(minChildStart); - } - if (span.getOptions().isTrimEnd() && maxChildEnd != null) { - span.setEndDate(maxChildEnd); - } + // if it's not set -> fallback to the current time + if (finishTimestamp == null) { + finishTimestamp = hub.getOptions().getDateProvider().now(); + } + + // auto-finish any idle spans first + for (final Span span : children) { + if (span.getOptions().isIdle()) { + span.finish(getSpanContext().status, finishTimestamp); } } this.finishStatus = FinishStatus.finishing(status); - if (!root.isFinished() && (!waitForChildren || hasAllChildrenFinished())) { + if (!root.isFinished() + && (!root.getOptions().isWaitForChildren() || hasAllChildrenFinished())) { List performanceCollectionData = null; if (transactionPerformanceCollector != null) { performanceCollectionData = transactionPerformanceCollector.stop(this); @@ -455,18 +415,6 @@ public void finish(@Nullable SpanStatus status, @Nullable SentryDate finishDate) performanceCollectionData.clear(); } - // try to get the high precision timestamp from the root span - SentryDate finishTimestamp = root.getFinishDate(); - - // if a finishDate was passed in, use that instead - if (finishDate != null) { - finishTimestamp = finishDate; - } - - // if it's not set -> fallback to the current time - if (finishTimestamp == null) { - finishTimestamp = hub.getOptions().getDateProvider().now(); - } // finish unfinished children for (final Span child : children) { if (!child.isFinished()) { @@ -475,16 +423,6 @@ public void finish(@Nullable SpanStatus status, @Nullable SentryDate finishDate) child.finish(SpanStatus.DEADLINE_EXCEEDED, finishTimestamp); } } - - // set the transaction finish timestamp to the latest child timestamp, if the transaction - // is an idle transaction - if (!children.isEmpty() && trimEnd) { - final @NotNull Span oldestChild = Collections.max(children, spanByTimestampComparator); - final @Nullable SentryDate oldestChildTimestamp = oldestChild.getFinishDate(); - if (oldestChildTimestamp != null && finishTimestamp.compareTo(oldestChildTimestamp) > 0) { - finishTimestamp = oldestChildTimestamp; - } - } root.finish(finishStatus.spanStatus, finishTimestamp); hub.configureScope( @@ -496,7 +434,7 @@ public void finish(@Nullable SpanStatus status, @Nullable SentryDate finishDate) } }); }); - SentryTransaction transaction = new SentryTransaction(this); + final SentryTransaction transaction = new SentryTransaction(this); if (transactionFinishedCallback != null) { transactionFinishedCallback.execute(this); } @@ -510,7 +448,7 @@ public void finish(@Nullable SpanStatus status, @Nullable SentryDate finishDate) } } - if (children.isEmpty() && idleTimeout != null) { + if (children.isEmpty() && root.getOptions().getIdleTimeout() != null) { // if it's an idle transaction which has no children, we drop it to save user's quota return; } @@ -816,21 +754,4 @@ private FinishStatus(final boolean isFinishing, final @Nullable SpanStatus spanS this.spanStatus = spanStatus; } } - - private static final class SpanByTimestampComparator implements Comparator { - - @SuppressWarnings({"JdkObsolete", "JavaUtilDate"}) - @Override - public int compare(final @NotNull Span o1, final @NotNull Span o2) { - final @Nullable SentryDate first = o1.getFinishDate(); - final @Nullable SentryDate second = o2.getFinishDate(); - if (first == null) { - return -1; - } else if (second == null) { - return 1; - } else { - return first.compareTo(second); - } - } - } } diff --git a/sentry/src/main/java/io/sentry/Span.java b/sentry/src/main/java/io/sentry/Span.java index 826e59fe45..3d4495ccde 100644 --- a/sentry/src/main/java/io/sentry/Span.java +++ b/sentry/src/main/java/io/sentry/Span.java @@ -2,6 +2,8 @@ import io.sentry.protocol.SentryId; import io.sentry.util.Objects; +import java.util.ArrayList; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -186,6 +188,30 @@ public void finish(final @Nullable SpanStatus status, final @Nullable SentryDate this.context.setStatus(status); this.timestamp = timestamp == null ? hub.getOptions().getDateProvider().now() : timestamp; + if (options.isTrimStart() || options.isTrimEnd()) { + @Nullable SentryDate minChildStart = null; + @Nullable SentryDate maxChildEnd = null; + + final @NotNull List children = getChildren(); + for (final Span child : children) { + if (child.getParentSpanId() != null && child.getParentSpanId().equals(getSpanId())) { + if (minChildStart == null || child.getStartDate().isBefore(minChildStart)) { + minChildStart = child.getStartDate(); + } + if (maxChildEnd == null + || (child.getFinishDate() != null && child.getFinishDate().isAfter(maxChildEnd))) { + maxChildEnd = child.getFinishDate(); + } + } + } + if (options.isTrimStart() && minChildStart != null) { + setStartDate(minChildStart); + } + if (options.isTrimEnd() && maxChildEnd != null) { + updateEndDate(maxChildEnd); + } + } + if (throwable != null) { hub.setSpanContext(throwable, this, this.transaction.getName()); } @@ -341,16 +367,41 @@ void setSpanFinishedCallback(final @Nullable SpanFinishedCallback callback) { this.spanFinishedCallback = callback; } - public void setStartDate(SentryDate date) { + public void setStartDate(@NotNull SentryDate date) { this.startTimestamp = date; } - public void setEndDate(SentryDate date) { - this.timestamp = date; + /** + * Updates the end date of the span. Note: This will only update the end date if the span is + * already finished. + * + * @param date the end date to set + * @return true if the end date was updated, false otherwise + */ + public boolean updateEndDate(@NotNull SentryDate date) { + if (this.timestamp != null) { + this.timestamp = date; + return true; + } + return false; } @NotNull public SpanOptions getOptions() { return options; } + + @NotNull + private List getChildren() { + final List children = new ArrayList<>(); + final Iterator iterator = transaction.getSpans().iterator(); + + while (iterator.hasNext()) { + final Span span = iterator.next(); + if (span.getParentSpanId() != null && span.getParentSpanId().equals(getSpanId())) { + children.add(span); + } + } + return children; + } } diff --git a/sentry/src/main/java/io/sentry/SpanOptions.java b/sentry/src/main/java/io/sentry/SpanOptions.java index 2218148dd1..e3c04c9998 100644 --- a/sentry/src/main/java/io/sentry/SpanOptions.java +++ b/sentry/src/main/java/io/sentry/SpanOptions.java @@ -1,29 +1,75 @@ package io.sentry; import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.Nullable; @ApiStatus.Internal public final class SpanOptions { + /** + * If `trimStart` is true, sets the start timestamp of the transaction to the lowest start + * timestamp of child spans. + * + *

Currently only relevant for non-root spans. + */ private final boolean trimStart; + /** + * If `trimEnd` is true, sets the end timestamp of the transaction to the highest timestamp of + * child spans, trimming the duration of the transaction. This is useful to discard extra time in + * the idle transactions to trim their duration to children' duration. + */ private final boolean trimEnd; - private final boolean autoFinish; + + /** + * true if the span is considered idle and should be automatically finished when it's parent gets + * finished + * + *

Currently only relevant for non-root spans. + */ + private final boolean isIdle; + + /** + * The idle time, measured in ms, to wait until the transaction will be finished. The span will + * use the end timestamp of the last finished span as the endtime for the transaction. + * + *

When set to {@code null} the transaction must be finished manually. + * + *

The default is 3 seconds. + * + *

Currently only relevant for transactions. + */ + private final @Nullable Long idleTimeout; + + /** + * When `waitForChildren` is set to `true`, tracer will finish only when both conditions are met + * (the order of meeting condition does not matter): - tracer itself is finished - all child spans + * are finished. + * + *

Currently only relevant for transactions. + */ + private final boolean waitForChildren; public SpanOptions() { - this(false, false, false); + this(false, false, false, false, null); } /** * @param trimStart true if the start time should be trimmed to the minimum start time of it's * children * @param trimEnd true if the end time should be trimmed to the maximum end time of it's children - * @param autoFinish true if this span should be finished whenever the root transaction gets - * finished + * @param isIdle true if this span can be finished automatically */ - public SpanOptions(final boolean trimStart, final boolean trimEnd, final boolean autoFinish) { + public SpanOptions( + final boolean trimStart, + final boolean trimEnd, + final boolean isIdle, + final boolean waitForChildren, + @Nullable Long idleTimeout) { this.trimStart = trimStart; this.trimEnd = trimEnd; - this.autoFinish = autoFinish; + this.isIdle = isIdle; + this.waitForChildren = waitForChildren; + this.idleTimeout = idleTimeout; } public boolean isTrimStart() { @@ -34,7 +80,16 @@ public boolean isTrimEnd() { return trimEnd; } - public boolean isAutoFinish() { - return autoFinish; + public boolean isIdle() { + return isIdle; + } + + @Nullable + public Long getIdleTimeout() { + return idleTimeout; + } + + public boolean isWaitForChildren() { + return waitForChildren; } } diff --git a/sentry/src/test/java/io/sentry/SentryTracerTest.kt b/sentry/src/test/java/io/sentry/SentryTracerTest.kt index 95bb825e70..09a3892343 100644 --- a/sentry/src/test/java/io/sentry/SentryTracerTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTracerTest.kt @@ -924,7 +924,7 @@ class SentryTracerTest { val transaction = fixture.getSut(waitForChildren = true, idleTimeout = 50, trimEnd = true, samplingDecision = TracesSamplingDecision(true)) // when a span is created with auto-finish - val span = transaction.startChild("composition", null, SpanOptions(false, false, true)) as Span + val span = transaction.startChild("composition", null, SpanOptions(false, false, true, false, null)) as Span // and the transaction is finished transaction.finish() @@ -936,7 +936,7 @@ class SentryTracerTest { verify(fixture.hub).captureTransaction( check { assertEquals(1, it.spans.size) - assertEquals(transaction.root.finishDate, span.finishDate) + assertEquals(transaction.root.finishDate!!.nanoTimestamp(), span.finishDate!!.nanoTimestamp()) }, anyOrNull(), anyOrNull(), @@ -949,7 +949,7 @@ class SentryTracerTest { val transaction = fixture.getSut(waitForChildren = true, idleTimeout = 50, trimEnd = true, samplingDecision = TracesSamplingDecision(true)) // when a span is created with auto-finish - val span = transaction.startChild("composition", null, SpanOptions(false, false, true)) as Span + val span = transaction.startChild("composition", null, SpanOptions(false, false, true, false, null)) as Span // and the transaction is finished Thread.sleep(1) @@ -976,7 +976,7 @@ class SentryTracerTest { val transaction = fixture.getSut(waitForChildren = true, idleTimeout = 50, trimEnd = true, samplingDecision = TracesSamplingDecision(true)) // when a parent span is created - val parentSpan = transaction.startChild("composition", null, SpanOptions(true, false, true)) as Span + val parentSpan = transaction.startChild("composition", null, SpanOptions(true, false, true, false, null)) as Span // with a child which starts later Thread.sleep(5) @@ -1013,7 +1013,7 @@ class SentryTracerTest { val transaction = fixture.getSut(waitForChildren = true, idleTimeout = 50, trimEnd = true, samplingDecision = TracesSamplingDecision(true)) // when a parent span is created - val parentSpan = transaction.startChild("composition", null, SpanOptions(false, true, true)) as Span + val parentSpan = transaction.startChild("composition", null, SpanOptions(false, true, true, false, null)) as Span // with a child which starts later Thread.sleep(5) From 4098d79cd69c27afa921fe3448479798af6e311f Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Tue, 21 Feb 2023 08:30:12 +0100 Subject: [PATCH 10/19] Let TransactionOptions inherit from SpanOptions --- .../core/ActivityLifecycleIntegrationTest.kt | 5 +- .../io/sentry/compose/SentryComposeTracing.kt | 20 +++++- .../android/compose/ComposeActivity.kt | 1 + sentry/api/sentry.api | 14 ++-- sentry/src/main/java/io/sentry/Hub.java | 39 +++------- .../src/main/java/io/sentry/SentryTracer.java | 53 ++++---------- .../src/main/java/io/sentry/SpanOptions.java | 72 ++++--------------- .../java/io/sentry/TransactionOptions.java | 27 ++++--- .../test/java/io/sentry/SentryTracerTest.kt | 28 ++++++-- 9 files changed, 109 insertions(+), 150 deletions(-) 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 4d70c0e335..ebbfac87d3 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 @@ -64,7 +64,10 @@ class ActivityLifecycleIntegrationTest { fun getSut(apiVersion: Int = 29, importance: Int = RunningAppProcessInfo.IMPORTANCE_FOREGROUND): ActivityLifecycleIntegration { whenever(hub.options).thenReturn(options) - transaction = SentryTracer(context, hub, true, transactionFinishedCallback) + val transactionOptions = TransactionOptions().apply { + isWaitForChildren = true + } + transaction = SentryTracer(context, hub, transactionOptions, transactionFinishedCallback) whenever(hub.startTransaction(any(), any())).thenReturn(transaction) whenever(buildInfo.sdkInfoVersion).thenReturn(apiVersion) diff --git a/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt b/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt index 3687a777e5..16b7b6200e 100644 --- a/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt +++ b/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt @@ -34,14 +34,30 @@ private fun getRootSpan(): ISpan? { private val localSentryCompositionParentSpan = compositionLocalOf { ImmutableHolder( getRootSpan() - ?.startChild(OP_PARENT_COMPOSITION, null, SpanOptions(true, true, true, false, null)) + ?.startChild( + OP_PARENT_COMPOSITION, + null, + SpanOptions().apply { + isTrimStart = true + isTrimEnd = true + isIdle = true + } + ) ) } private val localSentryRenderingParentSpan = compositionLocalOf { ImmutableHolder( getRootSpan() - ?.startChild(OP_PARENT_RENDER, null, SpanOptions(true, true, true, false, null)) + ?.startChild( + OP_PARENT_RENDER, + null, + SpanOptions().apply { + isTrimStart = true + isTrimEnd = true + isIdle = true + } + ) ) } diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt index f4d64f1418..1a4929b0b7 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt @@ -32,6 +32,7 @@ import androidx.navigation.compose.composable import androidx.navigation.compose.rememberNavController import androidx.navigation.navArgument import io.sentry.compose.SentryTraced +import io.sentry.compose.withSentryObservableEffect import io.sentry.samples.android.GithubAPI import kotlinx.coroutines.launch diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index b3eeb38934..5feec36c9d 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -1750,7 +1750,7 @@ public final class io/sentry/SentryTraceHeader { public final class io/sentry/SentryTracer : io/sentry/ITransaction { public fun (Lio/sentry/TransactionContext;Lio/sentry/IHub;)V - public fun (Lio/sentry/TransactionContext;Lio/sentry/IHub;ZLio/sentry/TransactionFinishedCallback;)V + public fun (Lio/sentry/TransactionContext;Lio/sentry/IHub;Lio/sentry/TransactionOptions;Lio/sentry/TransactionFinishedCallback;)V public fun finish ()V public fun finish (Lio/sentry/SpanStatus;)V public fun finish (Lio/sentry/SpanStatus;Lio/sentry/SentryDate;)V @@ -1979,14 +1979,14 @@ public final class io/sentry/SpanId$Deserializer : io/sentry/JsonDeserializer { public synthetic fun deserialize (Lio/sentry/JsonObjectReader;Lio/sentry/ILogger;)Ljava/lang/Object; } -public final class io/sentry/SpanOptions { +public class io/sentry/SpanOptions { public fun ()V - public fun (ZZZZLjava/lang/Long;)V - public fun getIdleTimeout ()Ljava/lang/Long; public fun isIdle ()Z public fun isTrimEnd ()Z public fun isTrimStart ()Z - public fun isWaitForChildren ()Z + public fun setIdle (Z)V + public fun setTrimEnd (Z)V + public fun setTrimStart (Z)V } public final class io/sentry/SpanStatus : java/lang/Enum, io/sentry/JsonSerializable { @@ -2096,21 +2096,19 @@ public abstract interface class io/sentry/TransactionFinishedCallback { public abstract fun execute (Lio/sentry/ITransaction;)V } -public final class io/sentry/TransactionOptions { +public final class io/sentry/TransactionOptions : io/sentry/SpanOptions { public fun ()V public fun getCustomSamplingContext ()Lio/sentry/CustomSamplingContext; public fun getIdleTimeout ()Ljava/lang/Long; public fun getStartTimestamp ()Lio/sentry/SentryDate; public fun getTransactionFinishedCallback ()Lio/sentry/TransactionFinishedCallback; public fun isBindToScope ()Z - public fun isTrimEnd ()Z public fun isWaitForChildren ()Z public fun setBindToScope (Z)V public fun setCustomSamplingContext (Lio/sentry/CustomSamplingContext;)V public fun setIdleTimeout (Ljava/lang/Long;)V public fun setStartTimestamp (Lio/sentry/SentryDate;)V public fun setTransactionFinishedCallback (Lio/sentry/TransactionFinishedCallback;)V - public fun setTrimEnd (Z)V public fun setWaitForChildren (Z)V } diff --git a/sentry/src/main/java/io/sentry/Hub.java b/sentry/src/main/java/io/sentry/Hub.java index bf4fbe3afd..11d6adb249 100644 --- a/sentry/src/main/java/io/sentry/Hub.java +++ b/sentry/src/main/java/io/sentry/Hub.java @@ -670,15 +670,7 @@ public void flush(long timeoutMillis) { public @NotNull ITransaction startTransaction( final @NotNull TransactionContext transactionContext, final @NotNull TransactionOptions transactionOptions) { - return createTransaction( - transactionContext, - transactionOptions.getCustomSamplingContext(), - transactionOptions.isBindToScope(), - transactionOptions.getStartTimestamp(), - transactionOptions.isWaitForChildren(), - transactionOptions.getIdleTimeout(), - transactionOptions.isTrimEnd(), - transactionOptions.getTransactionFinishedCallback()); + return createTransaction(transactionContext, transactionOptions); } @Override @@ -686,19 +678,17 @@ public void flush(long timeoutMillis) { final @NotNull TransactionContext transactionContext, final @Nullable CustomSamplingContext customSamplingContext, final boolean bindToScope) { - return createTransaction( - transactionContext, customSamplingContext, bindToScope, null, false, null, false, null); + + final TransactionOptions transactionOptions = new TransactionOptions(); + transactionOptions.setCustomSamplingContext(customSamplingContext); + transactionOptions.setBindToScope(bindToScope); + + return createTransaction(transactionContext, transactionOptions); } private @NotNull ITransaction createTransaction( final @NotNull TransactionContext transactionContext, - final @Nullable CustomSamplingContext customSamplingContext, - final boolean bindToScope, - final @Nullable SentryDate startTimestamp, - final boolean waitForChildren, - final @Nullable Long idleTimeout, - final boolean trimEnd, - final @Nullable TransactionFinishedCallback transactionFinishedCallback) { + final @NotNull TransactionOptions transactionOptions) { Objects.requireNonNull(transactionContext, "transactionContext is required"); ITransaction transaction; @@ -726,20 +716,13 @@ public void flush(long timeoutMillis) { transaction = NoOpTransaction.getInstance(); } else { final SamplingContext samplingContext = - new SamplingContext(transactionContext, customSamplingContext); + new SamplingContext(transactionContext, transactionOptions.getCustomSamplingContext()); @NotNull TracesSamplingDecision samplingDecision = tracesSampler.sample(samplingContext); transactionContext.setSamplingDecision(samplingDecision); transaction = new SentryTracer( - transactionContext, - this, - startTimestamp, - waitForChildren, - idleTimeout, - trimEnd, - transactionFinishedCallback, - transactionPerformanceCollector); + transactionContext, this, transactionOptions, null, transactionPerformanceCollector); // The listener is called only if the transaction exists, as the transaction is needed to // stop it @@ -748,7 +731,7 @@ public void flush(long timeoutMillis) { transactionProfiler.onTransactionStart(transaction); } } - if (bindToScope) { + if (transactionOptions.isBindToScope()) { configureScope(scope -> scope.setTransaction(transaction)); } return transaction; diff --git a/sentry/src/main/java/io/sentry/SentryTracer.java b/sentry/src/main/java/io/sentry/SentryTracer.java index 4c4ec32b0c..7d769df375 100644 --- a/sentry/src/main/java/io/sentry/SentryTracer.java +++ b/sentry/src/main/java/io/sentry/SentryTracer.java @@ -53,63 +53,40 @@ public final class SentryTracer implements ITransaction { private final @NotNull Instrumenter instrumenter; private final @NotNull Contexts contexts = new Contexts(); private final @Nullable TransactionPerformanceCollector transactionPerformanceCollector; + private final @NotNull TransactionOptions transactionOptions; public SentryTracer(final @NotNull TransactionContext context, final @NotNull IHub hub) { - this(context, hub, null, false, null, false, null); + this(context, hub, new TransactionOptions(), null, null); } public SentryTracer( final @NotNull TransactionContext context, final @NotNull IHub hub, - final boolean waitForChildren, + final @NotNull TransactionOptions transactionOptions, final @Nullable TransactionFinishedCallback transactionFinishedCallback) { - this(context, hub, null, waitForChildren, null, false, transactionFinishedCallback); + this(context, hub, transactionOptions, transactionFinishedCallback, null); } SentryTracer( final @NotNull TransactionContext context, final @NotNull IHub hub, - final @Nullable SentryDate startTimestamp, - final boolean waitForChildren, - final @Nullable Long idleTimeout, - final boolean trimEnd, - final @Nullable TransactionFinishedCallback transactionFinishedCallback) { - this( - context, - hub, - startTimestamp, - waitForChildren, - idleTimeout, - trimEnd, - transactionFinishedCallback, - null); - } - - SentryTracer( - final @NotNull TransactionContext context, - final @NotNull IHub hub, - final @Nullable SentryDate startTimestamp, - final boolean waitForChildren, - final @Nullable Long idleTimeout, - final boolean trimEnd, + final @NotNull TransactionOptions transactionOptions, final @Nullable TransactionFinishedCallback transactionFinishedCallback, final @Nullable TransactionPerformanceCollector transactionPerformanceCollector) { Objects.requireNonNull(context, "context is required"); Objects.requireNonNull(hub, "hub is required"); this.measurements = new ConcurrentHashMap<>(); + this.root = - new Span( - context, - this, - hub, - startTimestamp, - new SpanOptions(false, trimEnd, false, waitForChildren, idleTimeout)); + new Span(context, this, hub, transactionOptions.getStartTimestamp(), transactionOptions); + this.name = context.getName(); this.instrumenter = context.getInstrumenter(); this.hub = hub; this.transactionFinishedCallback = transactionFinishedCallback; this.transactionPerformanceCollector = transactionPerformanceCollector; this.transactionNameSource = context.getTransactionNameSource(); + this.transactionOptions = transactionOptions; if (context.getBaggage() != null) { this.baggage = context.getBaggage(); @@ -123,7 +100,7 @@ public SentryTracer( transactionPerformanceCollector.start(this); } - if (idleTimeout != null) { + if (transactionOptions.getIdleTimeout() != null) { timer = new Timer(true); scheduleFinish(); } @@ -145,7 +122,7 @@ public void run() { } }; - timer.schedule(timerTask, root.getOptions().getIdleTimeout()); + timer.schedule(timerTask, transactionOptions.getIdleTimeout()); } } } @@ -276,11 +253,11 @@ private ISpan createChild( spanOptions, __ -> { final FinishStatus finishStatus = this.finishStatus; - if (root.getOptions().getIdleTimeout() != null) { + if (transactionOptions.getIdleTimeout() != null) { // if it's an idle transaction, no matter the status, we'll reset the timeout here // so the transaction will either idle and finish itself, or a new child will be // added and we'll wait for it again - if (!root.getOptions().isWaitForChildren() || hasAllChildrenFinished()) { + if (!transactionOptions.isWaitForChildren() || hasAllChildrenFinished()) { scheduleFinish(); } } else if (finishStatus.isFinishing) { @@ -398,7 +375,7 @@ public void finish(@Nullable SpanStatus status, @Nullable SentryDate finishDate) this.finishStatus = FinishStatus.finishing(status); if (!root.isFinished() - && (!root.getOptions().isWaitForChildren() || hasAllChildrenFinished())) { + && (!transactionOptions.isWaitForChildren() || hasAllChildrenFinished())) { List performanceCollectionData = null; if (transactionPerformanceCollector != null) { performanceCollectionData = transactionPerformanceCollector.stop(this); @@ -448,7 +425,7 @@ public void finish(@Nullable SpanStatus status, @Nullable SentryDate finishDate) } } - if (children.isEmpty() && root.getOptions().getIdleTimeout() != null) { + if (children.isEmpty() && transactionOptions.getIdleTimeout() != null) { // if it's an idle transaction which has no children, we drop it to save user's quota return; } diff --git a/sentry/src/main/java/io/sentry/SpanOptions.java b/sentry/src/main/java/io/sentry/SpanOptions.java index e3c04c9998..42fc9906a3 100644 --- a/sentry/src/main/java/io/sentry/SpanOptions.java +++ b/sentry/src/main/java/io/sentry/SpanOptions.java @@ -1,10 +1,11 @@ package io.sentry; +import com.jakewharton.nopen.annotation.Open; import org.jetbrains.annotations.ApiStatus; -import org.jetbrains.annotations.Nullable; @ApiStatus.Internal -public final class SpanOptions { +@Open +public class SpanOptions { /** * If `trimStart` is true, sets the start timestamp of the transaction to the lowest start @@ -12,65 +13,19 @@ public final class SpanOptions { * *

Currently only relevant for non-root spans. */ - private final boolean trimStart; + private boolean trimStart = false; /** * If `trimEnd` is true, sets the end timestamp of the transaction to the highest timestamp of * child spans, trimming the duration of the transaction. This is useful to discard extra time in * the idle transactions to trim their duration to children' duration. */ - private final boolean trimEnd; + private boolean trimEnd = false; /** * true if the span is considered idle and should be automatically finished when it's parent gets - * finished - * - *

Currently only relevant for non-root spans. - */ - private final boolean isIdle; - - /** - * The idle time, measured in ms, to wait until the transaction will be finished. The span will - * use the end timestamp of the last finished span as the endtime for the transaction. - * - *

When set to {@code null} the transaction must be finished manually. - * - *

The default is 3 seconds. - * - *

Currently only relevant for transactions. - */ - private final @Nullable Long idleTimeout; - - /** - * When `waitForChildren` is set to `true`, tracer will finish only when both conditions are met - * (the order of meeting condition does not matter): - tracer itself is finished - all child spans - * are finished. - * - *

Currently only relevant for transactions. + * finished. */ - private final boolean waitForChildren; - - public SpanOptions() { - this(false, false, false, false, null); - } - - /** - * @param trimStart true if the start time should be trimmed to the minimum start time of it's - * children - * @param trimEnd true if the end time should be trimmed to the maximum end time of it's children - * @param isIdle true if this span can be finished automatically - */ - public SpanOptions( - final boolean trimStart, - final boolean trimEnd, - final boolean isIdle, - final boolean waitForChildren, - @Nullable Long idleTimeout) { - this.trimStart = trimStart; - this.trimEnd = trimEnd; - this.isIdle = isIdle; - this.waitForChildren = waitForChildren; - this.idleTimeout = idleTimeout; - } + private boolean isIdle = false; public boolean isTrimStart() { return trimStart; @@ -84,12 +39,15 @@ public boolean isIdle() { return isIdle; } - @Nullable - public Long getIdleTimeout() { - return idleTimeout; + public void setTrimStart(boolean trimStart) { + this.trimStart = trimStart; + } + + public void setTrimEnd(boolean trimEnd) { + this.trimEnd = trimEnd; } - public boolean isWaitForChildren() { - return waitForChildren; + public void setIdle(boolean idle) { + isIdle = idle; } } diff --git a/sentry/src/main/java/io/sentry/TransactionOptions.java b/sentry/src/main/java/io/sentry/TransactionOptions.java index e71974a2b3..a3b1450d74 100644 --- a/sentry/src/main/java/io/sentry/TransactionOptions.java +++ b/sentry/src/main/java/io/sentry/TransactionOptions.java @@ -4,21 +4,21 @@ import org.jetbrains.annotations.Nullable; @ApiStatus.Internal -public final class TransactionOptions { +public final class TransactionOptions extends SpanOptions { private @Nullable CustomSamplingContext customSamplingContext = null; private boolean bindToScope = false; private @Nullable SentryDate startTimestamp = null; private boolean waitForChildren = false; + private @Nullable Long idleTimeout = null; - private boolean trimEnd = false; private @Nullable TransactionFinishedCallback transactionFinishedCallback = null; public @Nullable CustomSamplingContext getCustomSamplingContext() { return customSamplingContext; } - public void setCustomSamplingContext(CustomSamplingContext customSamplingContext) { + public void setCustomSamplingContext(@Nullable CustomSamplingContext customSamplingContext) { this.customSamplingContext = customSamplingContext; } @@ -38,6 +38,11 @@ public void setStartTimestamp(@Nullable SentryDate startTimestamp) { this.startTimestamp = startTimestamp; } + /** + * When `waitForChildren` is set to `true`, tracer will finish only when both conditions are met + * (the order of meeting condition does not matter): - tracer itself is finished - all child spans + * are finished. + */ public boolean isWaitForChildren() { return waitForChildren; } @@ -46,6 +51,14 @@ public void setWaitForChildren(boolean waitForChildren) { this.waitForChildren = waitForChildren; } + /** + * The idle time, measured in ms, to wait until the transaction will be finished. The span will + * use the end timestamp of the last finished span as the endtime for the transaction. + * + *

When set to {@code null} the transaction must be finished manually. + * + *

The default is 3 seconds. + */ public @Nullable Long getIdleTimeout() { return idleTimeout; } @@ -54,14 +67,6 @@ public void setIdleTimeout(@Nullable Long idleTimeout) { this.idleTimeout = idleTimeout; } - public boolean isTrimEnd() { - return trimEnd; - } - - public void setTrimEnd(boolean trimEnd) { - this.trimEnd = trimEnd; - } - public @Nullable TransactionFinishedCallback getTransactionFinishedCallback() { return transactionFinishedCallback; } diff --git a/sentry/src/test/java/io/sentry/SentryTracerTest.kt b/sentry/src/test/java/io/sentry/SentryTracerTest.kt index 09a3892343..893dbd038b 100644 --- a/sentry/src/test/java/io/sentry/SentryTracerTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTracerTest.kt @@ -49,7 +49,13 @@ class SentryTracerTest { performanceCollector: TransactionPerformanceCollector? = transactionPerformanceCollector ): SentryTracer { optionsConfiguration.configure(options) - return SentryTracer(TransactionContext("name", "op", samplingDecision), hub, startTimestamp, waitForChildren, idleTimeout, trimEnd, transactionFinishedCallback, performanceCollector) + + val transactionOptions = TransactionOptions() + transactionOptions.startTimestamp = startTimestamp + transactionOptions.isWaitForChildren = waitForChildren + transactionOptions.idleTimeout = idleTimeout + transactionOptions.isTrimEnd = trimEnd + return SentryTracer(TransactionContext("name", "op", samplingDecision), hub, transactionOptions, transactionFinishedCallback, performanceCollector) } } @@ -924,7 +930,10 @@ class SentryTracerTest { val transaction = fixture.getSut(waitForChildren = true, idleTimeout = 50, trimEnd = true, samplingDecision = TracesSamplingDecision(true)) // when a span is created with auto-finish - val span = transaction.startChild("composition", null, SpanOptions(false, false, true, false, null)) as Span + val spanOptions = SpanOptions().apply { + isIdle = true + } + val span = transaction.startChild("composition", null, spanOptions) as Span // and the transaction is finished transaction.finish() @@ -949,7 +958,10 @@ class SentryTracerTest { val transaction = fixture.getSut(waitForChildren = true, idleTimeout = 50, trimEnd = true, samplingDecision = TracesSamplingDecision(true)) // when a span is created with auto-finish - val span = transaction.startChild("composition", null, SpanOptions(false, false, true, false, null)) as Span + val spanOptions = SpanOptions().apply { + isIdle = true + } + val span = transaction.startChild("composition", null, spanOptions) as Span // and the transaction is finished Thread.sleep(1) @@ -976,7 +988,10 @@ class SentryTracerTest { val transaction = fixture.getSut(waitForChildren = true, idleTimeout = 50, trimEnd = true, samplingDecision = TracesSamplingDecision(true)) // when a parent span is created - val parentSpan = transaction.startChild("composition", null, SpanOptions(true, false, true, false, null)) as Span + val spanOptions = SpanOptions().apply { + isTrimStart = true + } + val parentSpan = transaction.startChild("composition", null, spanOptions) as Span // with a child which starts later Thread.sleep(5) @@ -1013,7 +1028,10 @@ class SentryTracerTest { val transaction = fixture.getSut(waitForChildren = true, idleTimeout = 50, trimEnd = true, samplingDecision = TracesSamplingDecision(true)) // when a parent span is created - val parentSpan = transaction.startChild("composition", null, SpanOptions(false, true, true, false, null)) as Span + val spanOptions = SpanOptions().apply { + isTrimEnd = true + } + val parentSpan = transaction.startChild("composition", null, spanOptions) as Span // with a child which starts later Thread.sleep(5) From 5929c5a123638cfe73c32a8076f9d5749873c34b Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Tue, 21 Feb 2023 08:32:59 +0100 Subject: [PATCH 11/19] Fix Changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a2f9e89ca9..929109a8b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - If set to `true`, performance is enabled, even if no `tracesSampleRate` or `tracesSampler` have been configured. - If set to `false` performance is disabled, regardless of `tracesSampleRate` and `tracesSampler` options. - Detect dependencies by listing MANIFEST.MF files at runtime ([#2538](https://github.com/getsentry/sentry-java/pull/2538)) +- Add capabilities to track Jetpack Compose composition/rendering time ([#2507](https://github.com/getsentry/sentry-java/pull/2507)) ### Fixes @@ -20,7 +21,6 @@ - Add time-to-full-display span to Activity auto-instrumentation ([#2432](https://github.com/getsentry/sentry-java/pull/2432)) - Add `main` flag to threads and `in_foreground` flag for app contexts ([#2516](https://github.com/getsentry/sentry-java/pull/2516)) -- Add capabilities to track Jetpack Compose composition/rendering time ([#2507](https://github.com/getsentry/sentry-java/pull/2507)) ### Fixes From b87364ed81da0914cfa656ff354ffa339691671b Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Mon, 27 Feb 2023 08:54:40 +0100 Subject: [PATCH 12/19] Added enableUserInteractionTracing to SentryTraced --- sentry-compose/api/android/sentry-compose.api | 2 +- .../kotlin/io/sentry/compose/SentryComposeTracing.kt | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/sentry-compose/api/android/sentry-compose.api b/sentry-compose/api/android/sentry-compose.api index e951ebe14e..21fb2160ff 100644 --- a/sentry-compose/api/android/sentry-compose.api +++ b/sentry-compose/api/android/sentry-compose.api @@ -7,7 +7,7 @@ public final class io/sentry/compose/BuildConfig { } public final class io/sentry/compose/SentryComposeTracingKt { - public static final fun SentryTraced (Ljava/lang/String;Landroidx/compose/ui/Modifier;Lkotlin/jvm/functions/Function3;Landroidx/compose/runtime/Composer;II)V + public static final fun SentryTraced (Ljava/lang/String;Landroidx/compose/ui/Modifier;ZLkotlin/jvm/functions/Function3;Landroidx/compose/runtime/Composer;II)V } public final class io/sentry/compose/SentryNavigationIntegrationKt { diff --git a/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt b/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt index 16b7b6200e..aa2746a6fb 100644 --- a/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt +++ b/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt @@ -66,6 +66,7 @@ private val localSentryRenderingParentSpan = compositionLocalOf { public fun SentryTraced( tag: String, modifier: Modifier = Modifier, + enableUserInteractionTracing: Boolean = true, content: @Composable BoxScope.() -> Unit ) { val parentCompositionSpan = localSentryCompositionParentSpan.current @@ -73,9 +74,10 @@ public fun SentryTraced( val compositionSpan = parentCompositionSpan.item?.startChild(OP_COMPOSE, tag) val firstRendered = remember { ImmutableHolder(false) } + val baseModifier = if (enableUserInteractionTracing) modifier.testTag(tag) else modifier + Box( - modifier = modifier - .testTag(tag) + modifier = baseModifier .drawWithContent { val renderSpan = if (!firstRendered.item) { parentRenderingSpan.item?.startChild( From be159b43750965175709b8c48c5f297777d1916a Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Mon, 27 Feb 2023 09:19:57 +0100 Subject: [PATCH 13/19] Add more tests for span trimming --- sentry/api/sentry.api | 1 - sentry/src/main/java/io/sentry/Span.java | 16 ++--- sentry/src/test/java/io/sentry/SpanTest.kt | 81 +++++++++++++++++++++- 3 files changed, 86 insertions(+), 12 deletions(-) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 5feec36c9d..6ffd181ff5 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -1878,7 +1878,6 @@ public final class io/sentry/Span : io/sentry/ISpan { public fun getDescription ()Ljava/lang/String; public fun getFinishDate ()Lio/sentry/SentryDate; public fun getOperation ()Ljava/lang/String; - public fun getOptions ()Lio/sentry/SpanOptions; public fun getParentSpanId ()Lio/sentry/SpanId; public fun getSamplingDecision ()Lio/sentry/TracesSamplingDecision; public fun getSpanContext ()Lio/sentry/SpanContext; diff --git a/sentry/src/main/java/io/sentry/Span.java b/sentry/src/main/java/io/sentry/Span.java index 3d4495ccde..da14817971 100644 --- a/sentry/src/main/java/io/sentry/Span.java +++ b/sentry/src/main/java/io/sentry/Span.java @@ -194,14 +194,12 @@ public void finish(final @Nullable SpanStatus status, final @Nullable SentryDate final @NotNull List children = getChildren(); for (final Span child : children) { - if (child.getParentSpanId() != null && child.getParentSpanId().equals(getSpanId())) { - if (minChildStart == null || child.getStartDate().isBefore(minChildStart)) { - minChildStart = child.getStartDate(); - } - if (maxChildEnd == null - || (child.getFinishDate() != null && child.getFinishDate().isAfter(maxChildEnd))) { - maxChildEnd = child.getFinishDate(); - } + if (minChildStart == null || child.getStartDate().isBefore(minChildStart)) { + minChildStart = child.getStartDate(); + } + if (maxChildEnd == null + || (child.getFinishDate() != null && child.getFinishDate().isAfter(maxChildEnd))) { + maxChildEnd = child.getFinishDate(); } } if (options.isTrimStart() && minChildStart != null) { @@ -387,7 +385,7 @@ public boolean updateEndDate(@NotNull SentryDate date) { } @NotNull - public SpanOptions getOptions() { + SpanOptions getOptions() { return options; } diff --git a/sentry/src/test/java/io/sentry/SpanTest.kt b/sentry/src/test/java/io/sentry/SpanTest.kt index f4a4ea6796..2f3c806fc4 100644 --- a/sentry/src/test/java/io/sentry/SpanTest.kt +++ b/sentry/src/test/java/io/sentry/SpanTest.kt @@ -25,13 +25,16 @@ class SpanTest { ) } - fun getSut(): Span { + fun getSut(options: SpanOptions = SpanOptions()): Span { return Span( SentryId(), SpanId(), SentryTracer(TransactionContext("name", "op"), hub), "op", - hub + hub, + null, + options, + null ) } } @@ -210,6 +213,80 @@ class SpanTest { assertEquals(ex, span.throwable) } + @Test + fun `when span trim-start is enabled, trim to start of child span`() { + // when trim start is enabled + val span = fixture.getSut( + SpanOptions().apply { + isTrimStart = true + } + ) + + // and a child span is created + Thread.sleep(1) + val child1 = span.startChild("op1") as Span + child1.finish() + + val finishDate = SentryInstantDateProvider().now() + span.finish(SpanStatus.OK, finishDate) + + // then the span start should match the child + // but the finish date should be kept the same + assertEquals(child1.startDate, span.startDate) + assertEquals(finishDate, span.finishDate) + } + + @Test + fun `when span trim-end is enabled, trim to end of child span`() { + // when trim end is enabled + val span = fixture.getSut( + SpanOptions().apply { + isTrimEnd = true + } + ) + + val startDate = span.startDate + + // and a child span is created + Thread.sleep(1) + val child1 = span.startChild("op1") as Span + child1.finish() + + span.finish(SpanStatus.OK) + + // then the start should be left unchanged + // but the end should match the child + assertEquals(startDate, span.startDate) + assertEquals(child1.finishDate, span.finishDate) + } + + @Test + fun `when span trimming is enabled, trim to child spans`() { + // when a span with start+end trimming is enabled + val span = fixture.getSut( + SpanOptions().apply { + isTrimStart = true + isTrimEnd = true + } + ) + + // and two child spans are started + val child1 = span.startChild("op1") as Span + Thread.sleep(1) + child1.finish() + + val child2 = span.startChild("op2") as Span + Thread.sleep(1) + child2.finish() + + span.finish(SpanStatus.OK) + assertTrue(span.isFinished) + + // then the span should match start/finish should match it's children + assertEquals(child1.startDate, span.startDate) + assertEquals(child2.finishDate, span.finishDate) + } + @Test fun `child trace state is equal to transaction trace state`() { val transaction = getTransaction() From ef3702f7b5554650ad431e9defdd754e40c88714 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Mon, 27 Feb 2023 09:44:41 +0100 Subject: [PATCH 14/19] Fix use proper status for finishing idle spans --- sentry/src/main/java/io/sentry/SentryTracer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry/src/main/java/io/sentry/SentryTracer.java b/sentry/src/main/java/io/sentry/SentryTracer.java index 7d769df375..70a5ec5132 100644 --- a/sentry/src/main/java/io/sentry/SentryTracer.java +++ b/sentry/src/main/java/io/sentry/SentryTracer.java @@ -369,7 +369,7 @@ public void finish(@Nullable SpanStatus status, @Nullable SentryDate finishDate) // auto-finish any idle spans first for (final Span span : children) { if (span.getOptions().isIdle()) { - span.finish(getSpanContext().status, finishTimestamp); + span.finish((status != null) ? status : getSpanContext().status, finishTimestamp); } } From babea52b1f76913aa86403cd1a143b779283051c Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Mon, 27 Feb 2023 09:50:47 +0100 Subject: [PATCH 15/19] Fix merge conflict --- sentry/src/main/java/io/sentry/Span.java | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/sentry/src/main/java/io/sentry/Span.java b/sentry/src/main/java/io/sentry/Span.java index 3e86ff74c1..1a656de2ae 100644 --- a/sentry/src/main/java/io/sentry/Span.java +++ b/sentry/src/main/java/io/sentry/Span.java @@ -378,21 +378,6 @@ public void setStartDate(@NotNull SentryDate date) { this.startTimestamp = date; } - /** - * Updates the end date of the span. Note: This will only update the end date if the span is - * already finished. - * - * @param date the end date to set - * @return true if the end date was updated, false otherwise - */ - public boolean updateEndDate(@NotNull SentryDate date) { - if (this.timestamp != null) { - this.timestamp = date; - return true; - } - return false; - } - @NotNull SpanOptions getOptions() { return options; From 83e5a6fec9b4cc81b94213c55c0804e3f8ed8713 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Mon, 27 Feb 2023 10:14:22 +0100 Subject: [PATCH 16/19] Change visibility of span start date setter --- sentry/api/sentry.api | 1 - sentry/src/main/java/io/sentry/Span.java | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 5fe51fe241..6a391864d4 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -1915,7 +1915,6 @@ public final class io/sentry/Span : io/sentry/ISpan { public fun setMeasurement (Ljava/lang/String;Ljava/lang/Number;)V public fun setMeasurement (Ljava/lang/String;Ljava/lang/Number;Lio/sentry/MeasurementUnit;)V public fun setOperation (Ljava/lang/String;)V - public fun setStartDate (Lio/sentry/SentryDate;)V public fun setStatus (Lio/sentry/SpanStatus;)V public fun setTag (Ljava/lang/String;Ljava/lang/String;)V public fun setThrowable (Ljava/lang/Throwable;)V diff --git a/sentry/src/main/java/io/sentry/Span.java b/sentry/src/main/java/io/sentry/Span.java index 1a656de2ae..36a239d5d5 100644 --- a/sentry/src/main/java/io/sentry/Span.java +++ b/sentry/src/main/java/io/sentry/Span.java @@ -203,7 +203,7 @@ public void finish(final @Nullable SpanStatus status, final @Nullable SentryDate } } if (options.isTrimStart() && minChildStart != null) { - setStartDate(minChildStart); + updateStartDate(minChildStart); } if (options.isTrimEnd() && maxChildEnd != null) { updateEndDate(maxChildEnd); @@ -374,7 +374,7 @@ void setSpanFinishedCallback(final @Nullable SpanFinishedCallback callback) { this.spanFinishedCallback = callback; } - public void setStartDate(@NotNull SentryDate date) { + private void updateStartDate(@NotNull SentryDate date) { this.startTimestamp = date; } From 672fb224eec332f56763f72d20dc172732dc483d Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Fri, 10 Mar 2023 08:51:44 +0100 Subject: [PATCH 17/19] Make activity transactions idle Instead of immediatelly finishing the transaction in onPostResume, this way delayed JPC spans can be properly created --- .../core/ActivityLifecycleIntegration.java | 19 ++--- .../core/ActivityLifecycleIntegrationTest.kt | 74 +++++++++++++------ .../io/sentry/compose/SentryComposeTracing.kt | 4 +- .../src/main/java/io/sentry/SentryTracer.java | 3 + 4 files changed, 61 insertions(+), 39 deletions(-) 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 94190d0f78..39e3ceccce 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 @@ -182,7 +182,10 @@ private void startTracing(final @NotNull Activity activity) { final Boolean coldStart = AppStartState.getInstance().isColdStart(); final TransactionOptions transactionOptions = new TransactionOptions(); - + if (options.isEnableActivityLifecycleTracingAutoFinish()) { + transactionOptions.setIdleTimeout(options.getIdleTimeout()); + transactionOptions.setTrimEnd(true); + } transactionOptions.setWaitForChildren(true); transactionOptions.setTransactionFinishedCallback( (finishingTransaction) -> { @@ -392,21 +395,11 @@ public synchronized void onActivityResumed(final @NotNull Activity activity) { mainHandler.post(() -> onFirstFrameDrawn(ttidSpan)); } addBreadcrumb(activity, "resumed"); - - // fallback call for API < 29 compatibility, otherwise it happens on onActivityPostResumed - if (!isAllActivityCallbacksAvailable && options != null) { - stopTracing(activity, options.isEnableActivityLifecycleTracingAutoFinish()); - } } @Override - public synchronized void onActivityPostResumed(final @NotNull Activity activity) { - // only executed if API >= 29 otherwise it happens on onActivityResumed - if (isAllActivityCallbacksAvailable && options != null) { - // this should be called only when onResume has been executed already, which means - // the UI is responsive at this moment. - stopTracing(activity, options.isEnableActivityLifecycleTracingAutoFinish()); - } + public void onActivityPostResumed(@NonNull Activity activity) { + // empty override, required to avoid a api-level breaking super.onActivityPostResumed() calls } @Override 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 57bcee58db..8a81124aab 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 @@ -16,6 +16,7 @@ import io.sentry.FullyDisplayedReporter import io.sentry.Hub import io.sentry.ISentryExecutorService import io.sentry.Scope +import io.sentry.Sentry import io.sentry.SentryDate import io.sentry.SentryLevel import io.sentry.SentryNanotimeDate @@ -71,10 +72,21 @@ class ActivityLifecycleIntegrationTest { lateinit var transaction: SentryTracer val buildInfo = mock() - fun getSut(apiVersion: Int = 29, importance: Int = RunningAppProcessInfo.IMPORTANCE_FOREGROUND): ActivityLifecycleIntegration { + fun getSut( + apiVersion: Int = 29, + importance: Int = RunningAppProcessInfo.IMPORTANCE_FOREGROUND, + initializer: Sentry.OptionsConfiguration? = null + ): ActivityLifecycleIntegration { + initializer?.configure(options) + whenever(hub.options).thenReturn(options) + + // TODO: we should let the ActivityLifecycleIntegration create the proper transaction here val transactionOptions = TransactionOptions().apply { isWaitForChildren = true + if (options.isEnableActivityLifecycleTracingAutoFinish) { + idleTimeout = options.idleTimeout + } } transaction = SentryTracer(context, hub, transactionOptions, transactionFinishedCallback) whenever(hub.startTransaction(any(), any())).thenReturn(transaction) @@ -420,18 +432,32 @@ class ActivityLifecycleIntegrationTest { } @Test - fun `When tracing auto finish is enabled and ttid and ttfd spans are finished, it stops the transaction on onActivityPostResumed`() { - val sut = fixture.getSut() - fixture.options.tracesSampleRate = 1.0 - fixture.options.isEnableTimeToFullDisplayTracing = true - sut.register(fixture.hub, fixture.options) - + fun `When tracing auto finish is enabled and ttid and ttfd spans are finished, it schedules the transaction finish`() { val activity = mock() + val sut = fixture.getSut(initializer = { + it.tracesSampleRate = 1.0 + it.isEnableTimeToFullDisplayTracing = true + it.idleTimeout = 200 + }) + sut.register(fixture.hub, fixture.options) sut.onActivityCreated(activity, fixture.bundle) + sut.ttidSpanMap.values.first().finish() sut.ttfdSpan?.finish() - sut.onActivityPostResumed(activity) + // then transaction should not be immediatelly finished + verify(fixture.hub, never()) + .captureTransaction( + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + + // but when idle timeout has passed + Thread.sleep(400) + + // then the transaction should be finished verify(fixture.hub).captureTransaction( check { assertEquals(SpanStatus.OK, it.status) @@ -488,16 +514,16 @@ class ActivityLifecycleIntegrationTest { @Test fun `When tracing auto finish is disabled, do not finish transaction`() { - val sut = fixture.getSut() - fixture.options.tracesSampleRate = 1.0 - fixture.options.isEnableActivityLifecycleTracingAutoFinish = false + val sut = fixture.getSut(initializer = { + it.tracesSampleRate = 1.0 + it.isEnableActivityLifecycleTracingAutoFinish = false + }) sut.register(fixture.hub, fixture.options) - val activity = mock() sut.onActivityCreated(activity, fixture.bundle) sut.onActivityPostResumed(activity) - verify(fixture.hub, never()).captureTransaction(any(), anyOrNull(), anyOrNull(), anyOrNull()) + verify(fixture.hub, never()).captureTransaction(any(), anyOrNull(), anyOrNull(), anyOrNull()) } @Test @@ -671,37 +697,37 @@ class ActivityLifecycleIntegrationTest { sut.onActivityCreated(activity, mock()) sut.onActivityResumed(activity) - verify(fixture.hub, never()).captureTransaction(any(), any(), anyOrNull(), anyOrNull()) + verify(fixture.hub, never()).captureTransaction(any(), any(), anyOrNull(), anyOrNull()) } @Test - fun `start transaction on created if API less than 29`() { + fun `do not stop transaction on resumed if API less than 29 and ttid and ttfd are finished`() { val sut = fixture.getSut(14) fixture.options.tracesSampleRate = 1.0 + fixture.options.isEnableTimeToFullDisplayTracing = true sut.register(fixture.hub, fixture.options) - setAppStartTime() - val activity = mock() sut.onActivityCreated(activity, mock()) + sut.ttidSpanMap.values.first().finish() + sut.ttfdSpan?.finish() + sut.onActivityResumed(activity) - verify(fixture.hub).startTransaction(any(), any()) + verify(fixture.hub, never()).captureTransaction(any(), any(), anyOrNull(), anyOrNull()) } @Test - fun `stop transaction on resumed if API 29 less than 29 and ttid and ttfd are finished`() { + fun `start transaction on created if API less than 29`() { val sut = fixture.getSut(14) fixture.options.tracesSampleRate = 1.0 - fixture.options.isEnableTimeToFullDisplayTracing = true sut.register(fixture.hub, fixture.options) + setAppStartTime() + val activity = mock() sut.onActivityCreated(activity, mock()) - sut.ttidSpanMap.values.first().finish() - sut.ttfdSpan?.finish() - sut.onActivityResumed(activity) - verify(fixture.hub).captureTransaction(any(), anyOrNull(), anyOrNull(), anyOrNull()) + verify(fixture.hub).startTransaction(any(), any()) } @Test diff --git a/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt b/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt index aa2746a6fb..e76fdeb52b 100644 --- a/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt +++ b/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt @@ -36,7 +36,7 @@ private val localSentryCompositionParentSpan = compositionLocalOf { getRootSpan() ?.startChild( OP_PARENT_COMPOSITION, - null, + "Jetpack Compose Initial Composition", SpanOptions().apply { isTrimStart = true isTrimEnd = true @@ -51,7 +51,7 @@ private val localSentryRenderingParentSpan = compositionLocalOf { getRootSpan() ?.startChild( OP_PARENT_RENDER, - null, + "Jetpack Compose Initial Render", SpanOptions().apply { isTrimStart = true isTrimEnd = true diff --git a/sentry/src/main/java/io/sentry/SentryTracer.java b/sentry/src/main/java/io/sentry/SentryTracer.java index 7e8ef7320d..7995ab8927 100644 --- a/sentry/src/main/java/io/sentry/SentryTracer.java +++ b/sentry/src/main/java/io/sentry/SentryTracer.java @@ -427,6 +427,9 @@ public void finish(@Nullable SpanStatus status, @Nullable SentryDate finishDate) if (children.isEmpty() && transactionOptions.getIdleTimeout() != null) { // if it's an idle transaction which has no children, we drop it to save user's quota + hub.getOptions() + .getLogger() + .log(SentryLevel.DEBUG, "Dropping idle transaction because it has no child spans"); return; } From 8750da77a603a8acfe9db5b05b1aac11f99e3cb1 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Tue, 14 Mar 2023 15:04:24 +0100 Subject: [PATCH 18/19] Fix span trim start/end logic --- sentry/src/main/java/io/sentry/Span.java | 6 ++- sentry/src/test/java/io/sentry/SpanTest.kt | 50 ++++++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/sentry/src/main/java/io/sentry/Span.java b/sentry/src/main/java/io/sentry/Span.java index 36a239d5d5..9f247b9863 100644 --- a/sentry/src/main/java/io/sentry/Span.java +++ b/sentry/src/main/java/io/sentry/Span.java @@ -202,10 +202,12 @@ public void finish(final @Nullable SpanStatus status, final @Nullable SentryDate maxChildEnd = child.getFinishDate(); } } - if (options.isTrimStart() && minChildStart != null) { + if (options.isTrimStart() && minChildStart != null && startTimestamp.isBefore(minChildStart)) { updateStartDate(minChildStart); } - if (options.isTrimEnd() && maxChildEnd != null) { + if (options.isTrimEnd() && + maxChildEnd != null && + (this.timestamp == null || this.timestamp.isAfter(maxChildEnd))) { updateEndDate(maxChildEnd); } } diff --git a/sentry/src/test/java/io/sentry/SpanTest.kt b/sentry/src/test/java/io/sentry/SpanTest.kt index b2ee9092d5..d635853751 100644 --- a/sentry/src/test/java/io/sentry/SpanTest.kt +++ b/sentry/src/test/java/io/sentry/SpanTest.kt @@ -237,6 +237,30 @@ class SpanTest { assertEquals(finishDate, span.finishDate) } + @Test + fun `when span trim-start is enabled, do not trim to start of child span if it started earlier`() { + // when trim start is enabled + val span = fixture.getSut( + SpanOptions().apply { + isTrimStart = true + } + ) + val startDate = span.startDate + + // and a child span is created but has an earlier timestamp + val child1 = span.startChild( + "op1", "desc", + SentryLongDate(span.startDate.nanoTimestamp() - 1000L), + Instrumenter.SENTRY, + SpanOptions() + ) as Span + child1.finish() + span.finish(SpanStatus.OK) + + // then the span start should remain unchanged + assertEquals(startDate, span.startDate) + } + @Test fun `when span trim-end is enabled, trim to end of child span`() { // when trim end is enabled @@ -261,6 +285,32 @@ class SpanTest { assertEquals(child1.finishDate, span.finishDate) } + @Test + fun `when span trim-end is enabled, do not trim to end of child span if parent already finishes earlier`() { + // when trim end is enabled + val span = fixture.getSut( + SpanOptions().apply { + isTrimEnd = true + } + ) + + val startDate = span.startDate + + // and a child span is created, but finished later than the parent + val child1 = span.startChild("op1") as Span + span.finish(SpanStatus.OK) + + val finishDate = span.finishDate!! + + Thread.sleep(1) + child1.finish() + + + // then both start and finish date should be left unchanged + assertEquals(startDate, span.startDate) + assertEquals(finishDate, span.finishDate) + } + @Test fun `when span trimming is enabled, trim to child spans`() { // when a span with start+end trimming is enabled From f3463dbd3cd81b1937337495ca5d899c539690ce Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Tue, 14 Mar 2023 17:03:06 +0100 Subject: [PATCH 19/19] Add e2e UI test to ensure ttid/ttfd spans are created Also: reformat code --- .../uitest/android/AutomaticSpansTest.kt | 57 +++++++++++++++++++ sentry/src/main/java/io/sentry/Span.java | 10 ++-- sentry/src/test/java/io/sentry/SpanTest.kt | 4 +- 3 files changed, 65 insertions(+), 6 deletions(-) create mode 100644 sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/AutomaticSpansTest.kt diff --git a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/AutomaticSpansTest.kt b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/AutomaticSpansTest.kt new file mode 100644 index 0000000000..a2524c44a8 --- /dev/null +++ b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/AutomaticSpansTest.kt @@ -0,0 +1,57 @@ +package io.sentry.uitest.android + +import androidx.lifecycle.Lifecycle +import androidx.test.core.app.launchActivity +import androidx.test.ext.junit.runners.AndroidJUnit4 +import io.sentry.Sentry +import io.sentry.SentryLevel +import io.sentry.SentryOptions +import io.sentry.android.core.SentryAndroidOptions +import io.sentry.protocol.SentryTransaction +import org.junit.runner.RunWith +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +@RunWith(AndroidJUnit4::class) +class AutomaticSpansTest : BaseUiTest() { + + @Test + fun ttidTtfdSpans() { + val transactions = mutableListOf() + + initSentry(false) { options: SentryAndroidOptions -> + options.isDebug = true + options.setDiagnosticLevel(SentryLevel.DEBUG) + options.tracesSampleRate = 1.0 + options.profilesSampleRate = 1.0 + options.isEnableAutoActivityLifecycleTracing = true + options.beforeSendTransaction + options.isEnableTimeToFullDisplayTracing = true + options.beforeSendTransaction = + SentryOptions.BeforeSendTransactionCallback { transaction, _ -> + transactions.add(transaction) + transaction + } + } + + val activity = launchActivity() + activity.moveToState(Lifecycle.State.RESUMED) + activity.onActivity { + Sentry.reportFullyDisplayed() + } + activity.moveToState(Lifecycle.State.DESTROYED) + + assertEquals(1, transactions.size) + assertTrue("TTID span missing") { + transactions.first().spans.any { + it.op == "ui.load.initial_display" + } + } + assertTrue("TTFD span missing") { + transactions.first().spans.any { + it.op == "ui.load.full_display" + } + } + } +} diff --git a/sentry/src/main/java/io/sentry/Span.java b/sentry/src/main/java/io/sentry/Span.java index 9f247b9863..f32a71e875 100644 --- a/sentry/src/main/java/io/sentry/Span.java +++ b/sentry/src/main/java/io/sentry/Span.java @@ -202,12 +202,14 @@ public void finish(final @Nullable SpanStatus status, final @Nullable SentryDate maxChildEnd = child.getFinishDate(); } } - if (options.isTrimStart() && minChildStart != null && startTimestamp.isBefore(minChildStart)) { + if (options.isTrimStart() + && minChildStart != null + && startTimestamp.isBefore(minChildStart)) { updateStartDate(minChildStart); } - if (options.isTrimEnd() && - maxChildEnd != null && - (this.timestamp == null || this.timestamp.isAfter(maxChildEnd))) { + if (options.isTrimEnd() + && maxChildEnd != null + && (this.timestamp == null || this.timestamp.isAfter(maxChildEnd))) { updateEndDate(maxChildEnd); } } diff --git a/sentry/src/test/java/io/sentry/SpanTest.kt b/sentry/src/test/java/io/sentry/SpanTest.kt index d635853751..17af239171 100644 --- a/sentry/src/test/java/io/sentry/SpanTest.kt +++ b/sentry/src/test/java/io/sentry/SpanTest.kt @@ -249,7 +249,8 @@ class SpanTest { // and a child span is created but has an earlier timestamp val child1 = span.startChild( - "op1", "desc", + "op1", + "desc", SentryLongDate(span.startDate.nanoTimestamp() - 1000L), Instrumenter.SENTRY, SpanOptions() @@ -305,7 +306,6 @@ class SpanTest { Thread.sleep(1) child1.finish() - // then both start and finish date should be left unchanged assertEquals(startDate, span.startDate) assertEquals(finishDate, span.finishDate)