From 341b8328f5330ffdd3561cb032ef09f58a2d1db3 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Tue, 25 May 2021 12:51:47 +0200 Subject: [PATCH 01/21] Feat: App start metric --- CHANGELOG.md | 1 + .../api/sentry-android-core.api | 16 ++++ .../src/main/AndroidManifest.xml | 6 ++ .../core/ActivityLifecycleIntegration.java | 13 +++ .../core/AndroidOptionsInitializer.java | 1 + .../io/sentry/android/core/AppStartState.java | 56 ++++++++++++ .../core/DefaultAndroidEventProcessor.java | 5 +- .../PerformanceAndroidEventProcessor.java | 35 ++++++++ .../io/sentry/android/core/SentryAndroid.java | 13 ++- .../core/SentryPerformanceProvider.java | 86 +++++++++++++++++++ sentry/api/sentry.api | 5 ++ .../io/sentry/protocol/MeasurementValue.java | 13 +++ .../io/sentry/protocol/SentryTransaction.java | 7 ++ 13 files changed, 252 insertions(+), 5 deletions(-) create mode 100644 sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java create mode 100644 sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java create mode 100644 sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java create mode 100644 sentry/src/main/java/io/sentry/protocol/MeasurementValue.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 83c246756c..6bc7557597 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ * Feat: OkHttp callback for Customising the Span (#1478) * Feat: Add breadcrumb in Spring RestTemplate integration (#1481) * Fix: Cloning Stack (#1483) +* Feat: App start up metrics (#) # 5.0.0-beta.4 diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 12d0d12306..10216109ff 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -73,6 +73,11 @@ public final class io/sentry/android/core/NdkIntegration : io/sentry/Integration public final fun register (Lio/sentry/IHub;Lio/sentry/SentryOptions;)V } +public final class io/sentry/android/core/PerformanceAndroidEventProcessor : io/sentry/EventProcessor { + public fun ()V + public fun process (Lio/sentry/protocol/SentryTransaction;Ljava/lang/Object;)Lio/sentry/protocol/SentryTransaction; +} + public final class io/sentry/android/core/PhoneStateBreadcrumbsIntegration : io/sentry/Integration, java/io/Closeable { public fun (Landroid/content/Context;)V public fun close ()V @@ -123,6 +128,17 @@ public final class io/sentry/android/core/SentryInitProvider : android/content/C public fun update (Landroid/net/Uri;Landroid/content/ContentValues;Ljava/lang/String;[Ljava/lang/String;)I } +public final class io/sentry/android/core/SentryPerformanceProvider : android/content/ContentProvider { + public fun ()V + public fun attachInfo (Landroid/content/Context;Landroid/content/pm/ProviderInfo;)V + public fun delete (Landroid/net/Uri;Ljava/lang/String;[Ljava/lang/String;)I + public fun getType (Landroid/net/Uri;)Ljava/lang/String; + public fun insert (Landroid/net/Uri;Landroid/content/ContentValues;)Landroid/net/Uri; + public fun onCreate ()Z + public fun query (Landroid/net/Uri;[Ljava/lang/String;Ljava/lang/String;[Ljava/lang/String;Ljava/lang/String;)Landroid/database/Cursor; + public fun update (Landroid/net/Uri;Landroid/content/ContentValues;Ljava/lang/String;[Ljava/lang/String;)I +} + public final class io/sentry/android/core/SystemEventsBreadcrumbsIntegration : io/sentry/Integration, java/io/Closeable { public fun (Landroid/content/Context;)V public fun (Landroid/content/Context;Ljava/util/List;)V diff --git a/sentry-android-core/src/main/AndroidManifest.xml b/sentry-android-core/src/main/AndroidManifest.xml index facbc0d012..d0aa1fbe4d 100644 --- a/sentry-android-core/src/main/AndroidManifest.xml +++ b/sentry-android-core/src/main/AndroidManifest.xml @@ -8,5 +8,11 @@ android:name=".SentryInitProvider" android:authorities="${applicationId}.SentryInitProvider" android:exported="false"/> + + 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 1fe07871d0..4080ffc7f4 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 @@ -34,6 +34,9 @@ public final class ActivityLifecycleIntegration private boolean isAllActivityCallbacksAvailable; + private boolean firstActivityCreated = false; + private boolean firstActivityResumed = false; + // WeakHashMap isn't thread safe but ActivityLifecycleCallbacks is only called from the // main-thread private final @NotNull WeakHashMap activitiesWithOngoingTransactions = @@ -186,6 +189,11 @@ public synchronized void onActivityPreCreated( @Override public synchronized void onActivityCreated( final @NonNull Activity activity, final @Nullable Bundle savedInstanceState) { + if (!firstActivityCreated) { + AppStartState.getInstance().setColdStart(savedInstanceState == null); + firstActivityCreated = true; + } + addBreadcrumb(activity, "created"); // fallback call for API < 29 compatibility, otherwise it happens on onActivityPreCreated @@ -201,6 +209,11 @@ public synchronized void onActivityStarted(final @NonNull Activity activity) { @Override public synchronized void onActivityResumed(final @NonNull Activity activity) { + if (!firstActivityResumed) { + AppStartState.getInstance().setAppStartEnd(); + firstActivityResumed = true; + } + addBreadcrumb(activity, "resumed"); // fallback call for API < 29 compatibility, otherwise it happens on onActivityPostResumed diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index 052bd91572..f98a8df6d9 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -105,6 +105,7 @@ static void init( readDefaultOptionValues(options, context); options.addEventProcessor(new DefaultAndroidEventProcessor(context, logger, buildInfoProvider)); + options.addEventProcessor(new PerformanceAndroidEventProcessor()); options.setTransportGate(new AndroidTransportGate(context, options.getLogger())); } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java b/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java new file mode 100644 index 0000000000..d1c1a55860 --- /dev/null +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java @@ -0,0 +1,56 @@ +package io.sentry.android.core; + +import android.os.SystemClock; +import java.util.Date; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +final class AppStartState { + + private static final @NotNull AppStartState instance = new AppStartState(); + + private @Nullable Long appStart; + private @Nullable Long appStartEnd; + private @Nullable Boolean coldStart; + private @Nullable Date appStartTime; + + private AppStartState() {} + + static @NotNull AppStartState getInstance() { + return instance; + } + + void setAppStartEnd() { + appStartEnd = SystemClock.uptimeMillis(); + } + + @Nullable + Long getAppStartInterval() { + if (appStart == null || appStartEnd == null || coldStart == null) { + return null; + } + return appStartEnd - appStart; + } + + boolean getColdStart() { + return Boolean.TRUE.equals(coldStart); + } + + void setColdStart(final boolean coldStart) { + this.coldStart = coldStart; + } + + @Nullable + Date getAppStartTime() { + return appStartTime; + } + + synchronized void setAppStartTime(final long appStart, final @NotNull Date appStartTime) { + // method is synchronized because the SDK may by init. on a background thread. + if (this.appStartTime != null && this.appStart != null) { + return; + } + this.appStartTime = appStartTime; + this.appStart = appStart; + } +} diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java index 2b02a7dbeb..7b8f83798d 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java @@ -70,9 +70,6 @@ final class DefaultAndroidEventProcessor implements EventProcessor { @TestOnly static final String EMULATOR = "emulator"; @TestOnly static final String SIDE_LOADED = "sideLoaded"; - // it could also be a parameter and get from Sentry.init(...) - private static final @Nullable Date appStartTime = DateUtils.getCurrentDateTime(); - @TestOnly final Context context; @TestOnly final Future> contextData; @@ -299,7 +296,7 @@ private void mergeDebugImages(final @NotNull SentryEvent event) { private void setAppExtras(final @NotNull App app) { app.setAppName(getApplicationName()); - app.setAppStartTime(appStartTime); + app.setAppStartTime(AppStartState.getInstance().getAppStartTime()); } @SuppressWarnings("deprecation") diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java new file mode 100644 index 0000000000..7c6ede0d78 --- /dev/null +++ b/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java @@ -0,0 +1,35 @@ +package io.sentry.android.core; + +import io.sentry.EventProcessor; +import io.sentry.protocol.MeasurementValue; +import io.sentry.protocol.SentryTransaction; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +public final class PerformanceAndroidEventProcessor implements EventProcessor { + + // transactions may be started in parallel, making this field volatile instead of a lock + // to avoid contention. + private volatile boolean sentStartMeasurement = false; + + @Override + public @NotNull SentryTransaction process( + @NotNull SentryTransaction transaction, @Nullable Object hint) { + // the app start metric is sent only once when the 1st transaction happens + // after the app start is collected. + if (!sentStartMeasurement) { + Long appStartUpInterval = AppStartState.getInstance().getAppStartInterval(); + if (appStartUpInterval != null) { + MeasurementValue value = new MeasurementValue((float) appStartUpInterval); + + String appStartKey = + AppStartState.getInstance().getColdStart() ? "app_start_cold" : "app_start_warm"; + + transaction.getMeasurements().put(appStartKey, value); + sentStartMeasurement = true; + } + } + + return transaction; + } +} diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java index 0bf9fcb99b..f6305654ee 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java @@ -1,16 +1,23 @@ package io.sentry.android.core; import android.content.Context; +import android.os.SystemClock; +import io.sentry.DateUtils; import io.sentry.ILogger; import io.sentry.OptionsContainer; import io.sentry.Sentry; import io.sentry.SentryLevel; import java.lang.reflect.InvocationTargetException; +import java.util.Date; import org.jetbrains.annotations.NotNull; /** Sentry initialization class */ public final class SentryAndroid { + // static to rely on Class load + private static final @NotNull Date appStartTime = DateUtils.getCurrentDateTime(); + private static final long appStart = SystemClock.uptimeMillis(); + private SentryAndroid() {} /** @@ -51,10 +58,14 @@ public static void init( * @param logger your custom logger that implements ILogger * @param configuration Sentry.OptionsConfiguration configuration handler */ - public static void init( + public static synchronized void init( @NotNull final Context context, @NotNull ILogger logger, @NotNull Sentry.OptionsConfiguration configuration) { + // if SentryPerformanceProvider was disabled or removed, we set the App Start when + // the SDK is called. + AppStartState.getInstance().setAppStartTime(appStart, appStartTime); + try { Sentry.init( OptionsContainer.create(SentryAndroidOptions.class), diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java new file mode 100644 index 0000000000..138d97b937 --- /dev/null +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java @@ -0,0 +1,86 @@ +package io.sentry.android.core; + +import android.content.ContentProvider; +import android.content.ContentValues; +import android.content.Context; +import android.content.pm.ProviderInfo; +import android.database.Cursor; +import android.net.Uri; +import android.os.SystemClock; +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; +import io.sentry.DateUtils; +import java.util.Date; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; + +/** + * SentryPerformanceProvider is responsible for collecting data (eg appStart) as early as possible + * as ContentProvider is the only reliable hook for libraries that works across all the supported + * SDK versions. When minSDK is >= 24, we could use Process.getStartUptimeMillis() + */ +@ApiStatus.Internal +public final class SentryPerformanceProvider extends ContentProvider { + + // static to rely on Class load + private static final @NotNull Date appStartTime = DateUtils.getCurrentDateTime(); + // SystemClock.uptimeMillis() isn't affected by phone provider or clock changes. + private static final long appStart = SystemClock.uptimeMillis(); + + public SentryPerformanceProvider() { + AppStartState.getInstance().setAppStartTime(appStart, appStartTime); + } + + @Override + public boolean onCreate() { + return true; + } + + @Override + public void attachInfo(Context context, ProviderInfo info) { + // applicationId is expected to be prepended. See AndroidManifest.xml + if (SentryPerformanceProvider.class.getName().equals(info.authority)) { + throw new IllegalStateException( + "An applicationId is required to fulfill the manifest placeholder."); + } + super.attachInfo(context, info); + } + + @Nullable + @Override + public Cursor query( + @NonNull Uri uri, + @Nullable String[] projection, + @Nullable String selection, + @Nullable String[] selectionArgs, + @Nullable String sortOrder) { + return null; + } + + @Nullable + @Override + public String getType(@NonNull Uri uri) { + return null; + } + + @Nullable + @Override + public Uri insert(@NonNull Uri uri, @Nullable ContentValues values) { + return null; + } + + @Override + public int delete( + @NonNull Uri uri, @Nullable String selection, @Nullable String[] selectionArgs) { + return 0; + } + + @Override + public int update( + @NonNull Uri uri, + @Nullable ContentValues values, + @Nullable String selection, + @Nullable String[] selectionArgs) { + return 0; + } +} diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 6ce0a5b88b..2bc68fa5d3 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -1496,6 +1496,10 @@ public final class io/sentry/protocol/Gpu : io/sentry/IUnknownPropertiesConsumer public fun setVersion (Ljava/lang/String;)V } +public final class io/sentry/protocol/MeasurementValue { + public fun (F)V +} + public final class io/sentry/protocol/Mechanism : io/sentry/IUnknownPropertiesConsumer { public fun ()V public fun (Ljava/lang/Thread;)V @@ -1741,6 +1745,7 @@ public final class io/sentry/protocol/SentryThread : io/sentry/IUnknownPropertie public final class io/sentry/protocol/SentryTransaction : io/sentry/SentryBaseEvent { public fun (Lio/sentry/SentryTracer;)V + public fun getMeasurements ()Ljava/util/Map; public fun getSpans ()Ljava/util/List; public fun getStartTimestamp ()Ljava/util/Date; public fun getStatus ()Lio/sentry/SpanStatus; diff --git a/sentry/src/main/java/io/sentry/protocol/MeasurementValue.java b/sentry/src/main/java/io/sentry/protocol/MeasurementValue.java new file mode 100644 index 0000000000..36ddc15f6c --- /dev/null +++ b/sentry/src/main/java/io/sentry/protocol/MeasurementValue.java @@ -0,0 +1,13 @@ +package io.sentry.protocol; + +import org.jetbrains.annotations.ApiStatus; + +@ApiStatus.Internal +public final class MeasurementValue { + @SuppressWarnings("UnusedVariable") + private final float value; + + public MeasurementValue(final float value) { + this.value = value; + } +} diff --git a/sentry/src/main/java/io/sentry/protocol/SentryTransaction.java b/sentry/src/main/java/io/sentry/protocol/SentryTransaction.java index 0004ee899f..4abc9f7f91 100644 --- a/sentry/src/main/java/io/sentry/protocol/SentryTransaction.java +++ b/sentry/src/main/java/io/sentry/protocol/SentryTransaction.java @@ -9,6 +9,7 @@ import io.sentry.util.Objects; import java.util.ArrayList; import java.util.Date; +import java.util.HashMap; import java.util.List; import java.util.Map; import org.jetbrains.annotations.ApiStatus; @@ -34,6 +35,8 @@ public final class SentryTransaction extends SentryBaseEvent { @SuppressWarnings("UnusedVariable") private @NotNull final String type = "transaction"; + private @NotNull final Map measurements = new HashMap<>(); + @SuppressWarnings("deprecation") public SentryTransaction(final @NotNull SentryTracer sentryTracer) { super(sentryTracer.getEventId()); @@ -85,4 +88,8 @@ public boolean isSampled() { final SpanContext trace = this.getContexts().getTrace(); return trace != null && Boolean.TRUE.equals(trace.getSampled()); } + + public @NotNull Map getMeasurements() { + return measurements; + } } From e91becde00eae668c57f4b584aa319b34605a290 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com> Date: Tue, 25 May 2021 12:52:18 +0200 Subject: [PATCH 02/21] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6bc7557597..778157dfc8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ * Feat: OkHttp callback for Customising the Span (#1478) * Feat: Add breadcrumb in Spring RestTemplate integration (#1481) * Fix: Cloning Stack (#1483) -* Feat: App start up metrics (#) +* Feat: App start up metrics (#1487) # 5.0.0-beta.4 From 7c38b2c283ce5f6491f881c81f15b302d30ceaa0 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Tue, 25 May 2021 13:10:31 +0200 Subject: [PATCH 03/21] draft --- sentry-android-core/api/sentry-android-core.api | 5 ----- .../android/core/ActivityLifecycleIntegration.java | 8 ++++++-- .../android/core/AndroidOptionsInitializer.java | 2 +- .../java/io/sentry/android/core/AppStartState.java | 11 +++++++++++ .../core/PerformanceAndroidEventProcessor.java | 12 ++++++++++-- .../java/io/sentry/android/core/SentryAndroid.java | 3 ++- 6 files changed, 30 insertions(+), 11 deletions(-) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 10216109ff..a2fcab865b 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -73,11 +73,6 @@ public final class io/sentry/android/core/NdkIntegration : io/sentry/Integration public final fun register (Lio/sentry/IHub;Lio/sentry/SentryOptions;)V } -public final class io/sentry/android/core/PerformanceAndroidEventProcessor : io/sentry/EventProcessor { - public fun ()V - public fun process (Lio/sentry/protocol/SentryTransaction;Ljava/lang/Object;)Lio/sentry/protocol/SentryTransaction; -} - public final class io/sentry/android/core/PhoneStateBreadcrumbsIntegration : io/sentry/Integration, java/io/Closeable { public fun (Landroid/content/Context;)V public fun close ()V 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 4080ffc7f4..9c141766d7 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 @@ -34,7 +34,9 @@ public final class ActivityLifecycleIntegration private boolean isAllActivityCallbacksAvailable; + /** if the very first Activity has been already created */ private boolean firstActivityCreated = false; + /** if the very first Activity has been already resumed */ private boolean firstActivityResumed = false; // WeakHashMap isn't thread safe but ActivityLifecycleCallbacks is only called from the @@ -189,7 +191,8 @@ public synchronized void onActivityPreCreated( @Override public synchronized void onActivityCreated( final @NonNull Activity activity, final @Nullable Bundle savedInstanceState) { - if (!firstActivityCreated) { + if (!firstActivityCreated && performanceEnabled) { + // if Activity has savedInstanceState then its a warm start AppStartState.getInstance().setColdStart(savedInstanceState == null); firstActivityCreated = true; } @@ -209,7 +212,8 @@ public synchronized void onActivityStarted(final @NonNull Activity activity) { @Override public synchronized void onActivityResumed(final @NonNull Activity activity) { - if (!firstActivityResumed) { + if (!firstActivityResumed && performanceEnabled) { + // sets App start as finished when the very first activity calls onResume AppStartState.getInstance().setAppStartEnd(); firstActivityResumed = true; } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index f98a8df6d9..de740b57f6 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -105,7 +105,7 @@ static void init( readDefaultOptionValues(options, context); options.addEventProcessor(new DefaultAndroidEventProcessor(context, logger, buildInfoProvider)); - options.addEventProcessor(new PerformanceAndroidEventProcessor()); + options.addEventProcessor(new PerformanceAndroidEventProcessor(options)); options.setTransportGate(new AndroidTransportGate(context, options.getLogger())); } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java b/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java index d1c1a55860..aab1245279 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java @@ -5,13 +5,24 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +/** AppStartState holds the state of the App Start metric and appStartTime */ final class AppStartState { private static final @NotNull AppStartState instance = new AppStartState(); + /** appStart in milliseconds */ private @Nullable Long appStart; + + /** appStartEnd in milliseconds */ private @Nullable Long appStartEnd; + + /** + * The type of App start coldStart=true -> Cold start coldStart=false -> Warm start coldStart=null + * -> unknown yet + */ private @Nullable Boolean coldStart; + + /** appStart as a Date used in the App's Context */ private @Nullable Date appStartTime; private AppStartState() {} diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java index 7c6ede0d78..457c5fa4e2 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java @@ -6,7 +6,14 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -public final class PerformanceAndroidEventProcessor implements EventProcessor { +/** Event Processor responsable for adding Android metrics to transactions */ +final class PerformanceAndroidEventProcessor implements EventProcessor { + + final @NotNull SentryAndroidOptions options; + + PerformanceAndroidEventProcessor(final @NotNull SentryAndroidOptions options) { + this.options = options; + } // transactions may be started in parallel, making this field volatile instead of a lock // to avoid contention. @@ -17,8 +24,9 @@ public final class PerformanceAndroidEventProcessor implements EventProcessor { @NotNull SentryTransaction transaction, @Nullable Object hint) { // the app start metric is sent only once when the 1st transaction happens // after the app start is collected. - if (!sentStartMeasurement) { + if (!sentStartMeasurement && options.isTracingEnabled()) { Long appStartUpInterval = AppStartState.getInstance().getAppStartInterval(); + // if appStartUpInterval is null, metrics are not ready to be sent if (appStartUpInterval != null) { MeasurementValue value = new MeasurementValue((float) appStartUpInterval); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java index f6305654ee..a90229e370 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java @@ -14,8 +14,9 @@ /** Sentry initialization class */ public final class SentryAndroid { - // static to rely on Class load + // static to rely on Class load init. private static final @NotNull Date appStartTime = DateUtils.getCurrentDateTime(); + // SystemClock.uptimeMillis() isn't affected by phone provider or clock changes. private static final long appStart = SystemClock.uptimeMillis(); private SentryAndroid() {} From 13a44ca7c2d29cf1db9d99e057152bf5db45da59 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Tue, 25 May 2021 14:20:51 +0200 Subject: [PATCH 04/21] add tests --- .../io/sentry/android/core/AppStartState.java | 19 ++-- .../core/ActivityLifecycleIntegrationTest.kt | 57 ++++++++++ .../core/AndroidOptionsInitializerTest.kt | 10 ++ .../sentry/android/core/AppStartStateTest.kt | 44 ++++++++ .../PerformanceAndroidEventProcessorTest.kt | 100 ++++++++++++++++++ .../sentry/android/core/SentryAndroidTest.kt | 16 +++ .../core/SentryPerformanceProviderTest.kt | 37 +++++++ 7 files changed, 275 insertions(+), 8 deletions(-) create mode 100644 sentry-android-core/src/test/java/io/sentry/android/core/AppStartStateTest.kt create mode 100644 sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt create mode 100644 sentry-android-core/src/test/java/io/sentry/android/core/SentryPerformanceProviderTest.kt diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java b/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java index aab1245279..9f6d8f4a1b 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java @@ -4,11 +4,12 @@ import java.util.Date; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.TestOnly; /** AppStartState holds the state of the App Start metric and appStartTime */ final class AppStartState { - private static final @NotNull AppStartState instance = new AppStartState(); + private static @NotNull AppStartState instance = new AppStartState(); /** appStart in milliseconds */ private @Nullable Long appStart; @@ -16,11 +17,8 @@ final class AppStartState { /** appStartEnd in milliseconds */ private @Nullable Long appStartEnd; - /** - * The type of App start coldStart=true -> Cold start coldStart=false -> Warm start coldStart=null - * -> unknown yet - */ - private @Nullable Boolean coldStart; + /** The type of App start coldStart=true -> Cold start, coldStart=false -> Warm start */ + private boolean coldStart; /** appStart as a Date used in the App's Context */ private @Nullable Date appStartTime; @@ -31,20 +29,25 @@ private AppStartState() {} return instance; } + @TestOnly + void resetInstance() { + instance = new AppStartState(); + } + void setAppStartEnd() { appStartEnd = SystemClock.uptimeMillis(); } @Nullable Long getAppStartInterval() { - if (appStart == null || appStartEnd == null || coldStart == null) { + if (appStart == null || appStartEnd == null) { return null; } return appStartEnd - appStart; } boolean getColdStart() { - return Boolean.TRUE.equals(coldStart); + return coldStart; } void setColdStart(final boolean coldStart) { 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 e0e2258afe..e6c1df2a75 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 @@ -17,6 +17,7 @@ import io.sentry.SentryOptions import io.sentry.SentryTracer import io.sentry.SpanStatus import io.sentry.TransactionContext +import java.util.Date import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse @@ -420,4 +421,60 @@ class ActivityLifecycleIntegrationTest { verify(fixture.hub).captureTransaction(any()) } + + @Test + fun `App start is Cold when savedInstanceState is null`() { + val sut = fixture.getSut(14) + fixture.options.tracesSampleRate = 1.0 + sut.register(fixture.hub, fixture.options) + + val activity = mock() + sut.onActivityCreated(activity, null) + + assertTrue(AppStartState.getInstance().coldStart) + } + + @Test + fun `App start is Warm when savedInstanceState is not null`() { + val sut = fixture.getSut(14) + fixture.options.tracesSampleRate = 1.0 + sut.register(fixture.hub, fixture.options) + + val activity = mock() + val bundle = Bundle() + sut.onActivityCreated(activity, bundle) + + assertFalse(AppStartState.getInstance().coldStart) + } + + @Test + fun `Do not overwrite App start type after set`() { + val sut = fixture.getSut(14) + fixture.options.tracesSampleRate = 1.0 + sut.register(fixture.hub, fixture.options) + + val activity = mock() + val bundle = Bundle() + sut.onActivityCreated(activity, bundle) + sut.onActivityCreated(activity, null) + + assertFalse(AppStartState.getInstance().coldStart) + } + + @Test + fun `App start end time is set`() { + val sut = fixture.getSut(14) + fixture.options.tracesSampleRate = 1.0 + sut.register(fixture.hub, fixture.options) + + // set by SentryPerformanceProvider so forcing it here + AppStartState.getInstance().setAppStartTime(0, Date()) + + val activity = mock() + sut.onActivityCreated(activity, null) + sut.onActivityResumed(activity) + + // SystemClock.uptimeMillis() always returns 0, can't assert real values + assertNotNull(AppStartState.getInstance().appStartInterval) + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt index 68c94d2a51..9622c2d602 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt @@ -62,6 +62,16 @@ class AndroidOptionsInitializerTest { assertNotNull(actual) } + @Test + fun `PerformanceAndroidEventProcessor added to processors list`() { + val sentryOptions = SentryAndroidOptions() + val mockContext = createMockContext() + + AndroidOptionsInitializer.init(sentryOptions, mockContext) + val actual = sentryOptions.eventProcessors.any { it is PerformanceAndroidEventProcessor } + assertNotNull(actual) + } + @Test fun `MainEventProcessor added to processors list and its the 1st`() { val sentryOptions = SentryAndroidOptions() diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AppStartStateTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AppStartStateTest.kt new file mode 100644 index 0000000000..30e19d0c47 --- /dev/null +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AppStartStateTest.kt @@ -0,0 +1,44 @@ +package io.sentry.android.core + +import java.util.Date +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertNull +import kotlin.test.assertSame + +class AppStartStateTest { + + @BeforeTest + fun `reset instance`() { + AppStartState.getInstance().resetInstance() + } + + @Test + fun `appStartInterval returns null if end time is not set`() { + val sut = AppStartState.getInstance() + + sut.setAppStartTime(0, Date()) + + assertNull(sut.appStartInterval) + } + + @Test + fun `appStartInterval returns null if start time is not set`() { + val sut = AppStartState.getInstance() + + sut.setAppStartEnd() + + assertNull(sut.appStartInterval) + } + + @Test + fun `do not overwrite app start values if already set`() { + val sut = AppStartState.getInstance() + + val date = Date() + sut.setAppStartTime(0, date) + sut.setAppStartTime(1, Date()) + + assertSame(date, sut.appStartTime) + } +} diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt new file mode 100644 index 0000000000..355f52c6b2 --- /dev/null +++ b/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt @@ -0,0 +1,100 @@ +package io.sentry.android.core + +import com.nhaarman.mockitokotlin2.mock +import io.sentry.SentryTracer +import io.sentry.protocol.SentryTransaction +import java.util.Date +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertTrue + +class PerformanceAndroidEventProcessorTest { + + private class Fixture { + val options = SentryAndroidOptions().apply { + tracesSampleRate = 1.0 + } + + val tracer = SentryTracer(mock(), mock()) + + fun getSut(): PerformanceAndroidEventProcessor { + return PerformanceAndroidEventProcessor(options) + } + } + + private val fixture = Fixture() + + @BeforeTest + fun `reset instance`() { + AppStartState.getInstance().resetInstance() + } + + @Test + fun `add cold start measurement`() { + val sut = fixture.getSut() + + var tr = SentryTransaction(fixture.tracer) + setAppStart() + + tr = sut.process(tr, null) + + assertTrue(tr.measurements.containsKey("app_start_cold")) + } + + @Test + fun `add warm start measurement`() { + val sut = fixture.getSut() + + var tr = SentryTransaction(fixture.tracer) + setAppStart(false) + + tr = sut.process(tr, null) + + assertTrue(tr.measurements.containsKey("app_start_warm")) + } + + @Test + fun `do not add app start metric twice`() { + val sut = fixture.getSut() + + var tr1 = SentryTransaction(fixture.tracer) + setAppStart(false) + + tr1 = sut.process(tr1, null) + + var tr2 = SentryTransaction(fixture.tracer) + tr2 = sut.process(tr2, null) + + assertTrue(tr1.measurements.containsKey("app_start_warm")) + assertTrue(tr2.measurements.isEmpty()) + } + + @Test + fun `do not add app start metric if its not ready`() { + val sut = fixture.getSut() + + var tr = SentryTransaction(fixture.tracer) + + tr = sut.process(tr, null) + + assertTrue(tr.measurements.isEmpty()) + } + + @Test + fun `do not add app start metric if performance is disabled`() { + val sut = fixture.getSut() + sut.options.tracesSampleRate = null + + var tr = SentryTransaction(fixture.tracer) + + tr = sut.process(tr, null) + + assertTrue(tr.measurements.isEmpty()) + } + + private fun setAppStart(coldStart: Boolean = true) { + AppStartState.getInstance().coldStart = coldStart + AppStartState.getInstance().setAppStartTime(0, Date()) + AppStartState.getInstance().setAppStartEnd() + } +} diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt index ecf41c5385..9907c4fc8e 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt @@ -14,6 +14,7 @@ import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse +import kotlin.test.assertNotNull import kotlin.test.assertTrue import org.junit.runner.RunWith @@ -23,6 +24,7 @@ class SentryAndroidTest { @BeforeTest fun `set up`() { Sentry.close() + AppStartState.getInstance().resetInstance() } @Test @@ -88,4 +90,18 @@ class SentryAndroidTest { SentryAndroid.init(mockContext, logger) verify(logger, never()).log(eq(SentryLevel.FATAL), any(), any()) } + + @Test + fun `set app start if provider is disabled`() { + val metaData = Bundle() + val mockContext = ContextUtilsTest.mockMetaData(metaData = metaData) + metaData.putString(ManifestMetadataReader.DSN, "https://key@sentry.io/123") + + SentryAndroid.init(mockContext, mock()) + + // done by ActivityLifecycleIntegration so forcing it here + AppStartState.getInstance().setAppStartEnd() + + assertNotNull(AppStartState.getInstance().appStartInterval) + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SentryPerformanceProviderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SentryPerformanceProviderTest.kt new file mode 100644 index 0000000000..6aa2ca6ad4 --- /dev/null +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SentryPerformanceProviderTest.kt @@ -0,0 +1,37 @@ +package io.sentry.android.core + +import android.content.pm.ProviderInfo +import androidx.test.ext.junit.runners.AndroidJUnit4 +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertNotNull +import org.junit.runner.RunWith + +@RunWith(AndroidJUnit4::class) +class SentryPerformanceProviderTest { + + @BeforeTest + fun `set up`() { + AppStartState.getInstance().resetInstance() + } + + @Test + fun `provider sets app start`() { + val providerInfo = ProviderInfo() + + val mockContext = ContextUtilsTest.createMockContext() + providerInfo.authority = AUTHORITY + + val provider = SentryPerformanceProvider() + provider.attachInfo(mockContext, providerInfo) + + // done by ActivityLifecycleIntegration so forcing it here + AppStartState.getInstance().setAppStartEnd() + + assertNotNull(AppStartState.getInstance().appStartInterval) + } + + companion object { + private const val AUTHORITY = "io.sentry.sample.SentryPerformanceProvider" + } +} From f2afe1b85d87b3c7054541e4c35d4fc05488534c Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Tue, 25 May 2021 14:25:17 +0200 Subject: [PATCH 05/21] fix --- .../android/core/PerformanceAndroidEventProcessor.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java index 457c5fa4e2..f9ad38f78a 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java @@ -25,12 +25,12 @@ final class PerformanceAndroidEventProcessor implements EventProcessor { // the app start metric is sent only once when the 1st transaction happens // after the app start is collected. if (!sentStartMeasurement && options.isTracingEnabled()) { - Long appStartUpInterval = AppStartState.getInstance().getAppStartInterval(); + final Long appStartUpInterval = AppStartState.getInstance().getAppStartInterval(); // if appStartUpInterval is null, metrics are not ready to be sent if (appStartUpInterval != null) { - MeasurementValue value = new MeasurementValue((float) appStartUpInterval); + final MeasurementValue value = new MeasurementValue((float) appStartUpInterval); - String appStartKey = + final String appStartKey = AppStartState.getInstance().getColdStart() ? "app_start_cold" : "app_start_warm"; transaction.getMeasurements().put(appStartKey, value); From 7480dfb31f2bd140364f927576a668b32ed3c060 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Tue, 8 Jun 2021 14:52:36 +0200 Subject: [PATCH 06/21] review --- .../core/ActivityLifecycleIntegration.java | 3 +- .../io/sentry/android/core/AppStartState.java | 25 ++++++------ .../PerformanceAndroidEventProcessor.java | 39 ++++++++++--------- .../core/SentryPerformanceProvider.java | 4 +- .../core/ActivityLifecycleIntegrationTest.kt | 6 +-- .../sentry/android/core/AppStartStateTest.kt | 14 ++++++- .../PerformanceAndroidEventProcessorTest.kt | 12 +++--- 7 files changed, 59 insertions(+), 44 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 9c141766d7..c7459f8cfa 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 @@ -34,9 +34,7 @@ public final class ActivityLifecycleIntegration private boolean isAllActivityCallbacksAvailable; - /** if the very first Activity has been already created */ private boolean firstActivityCreated = false; - /** if the very first Activity has been already resumed */ private boolean firstActivityResumed = false; // WeakHashMap isn't thread safe but ActivityLifecycleCallbacks is only called from the @@ -193,6 +191,7 @@ public synchronized void onActivityCreated( final @NonNull Activity activity, final @Nullable Bundle savedInstanceState) { if (!firstActivityCreated && performanceEnabled) { // if Activity has savedInstanceState then its a warm start + // https://developer.android.com/topic/performance/vitals/launch-time#warm AppStartState.getInstance().setColdStart(savedInstanceState == null); firstActivityCreated = true; } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java b/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java index 9f6d8f4a1b..8e50c691c8 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java @@ -11,11 +11,9 @@ final class AppStartState { private static @NotNull AppStartState instance = new AppStartState(); - /** appStart in milliseconds */ - private @Nullable Long appStart; + private @Nullable Long appStartMillis; - /** appStartEnd in milliseconds */ - private @Nullable Long appStartEnd; + private @Nullable Long appStartEndMillis; /** The type of App start coldStart=true -> Cold start, coldStart=false -> Warm start */ private boolean coldStart; @@ -35,18 +33,23 @@ void resetInstance() { } void setAppStartEnd() { - appStartEnd = SystemClock.uptimeMillis(); + setAppStartEnd(SystemClock.uptimeMillis()); + } + + @TestOnly + void setAppStartEnd(final long appStartEndMillis) { + this.appStartEndMillis = appStartEndMillis; } @Nullable Long getAppStartInterval() { - if (appStart == null || appStartEnd == null) { + if (appStartMillis == null || appStartEndMillis == null) { return null; } - return appStartEnd - appStart; + return appStartEndMillis - appStartMillis; } - boolean getColdStart() { + boolean isColdStart() { return coldStart; } @@ -59,12 +62,12 @@ Date getAppStartTime() { return appStartTime; } - synchronized void setAppStartTime(final long appStart, final @NotNull Date appStartTime) { + synchronized void setAppStartTime(final long appStartMillis, final @NotNull Date appStartTime) { // method is synchronized because the SDK may by init. on a background thread. - if (this.appStartTime != null && this.appStart != null) { + if (this.appStartTime != null && this.appStartMillis != null) { return; } this.appStartTime = appStartTime; - this.appStart = appStart; + this.appStartMillis = appStartMillis; } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java index f9ad38f78a..72910173b5 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java @@ -6,35 +6,38 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -/** Event Processor responsable for adding Android metrics to transactions */ +/** Event Processor responsible for adding Android metrics to transactions */ final class PerformanceAndroidEventProcessor implements EventProcessor { - final @NotNull SentryAndroidOptions options; + private final boolean tracingEnabled; + + private boolean sentStartMeasurement = false; + + // transactions may be started in parallel, locking it so sentStartMeasurement + private final @NotNull Object measurementLock = new Object(); PerformanceAndroidEventProcessor(final @NotNull SentryAndroidOptions options) { - this.options = options; + tracingEnabled = options.isTracingEnabled(); } - // transactions may be started in parallel, making this field volatile instead of a lock - // to avoid contention. - private volatile boolean sentStartMeasurement = false; - @Override public @NotNull SentryTransaction process( @NotNull SentryTransaction transaction, @Nullable Object hint) { // the app start metric is sent only once when the 1st transaction happens // after the app start is collected. - if (!sentStartMeasurement && options.isTracingEnabled()) { - final Long appStartUpInterval = AppStartState.getInstance().getAppStartInterval(); - // if appStartUpInterval is null, metrics are not ready to be sent - if (appStartUpInterval != null) { - final MeasurementValue value = new MeasurementValue((float) appStartUpInterval); - - final String appStartKey = - AppStartState.getInstance().getColdStart() ? "app_start_cold" : "app_start_warm"; - - transaction.getMeasurements().put(appStartKey, value); - sentStartMeasurement = true; + synchronized (measurementLock) { + if (!sentStartMeasurement && tracingEnabled) { + final Long appStartUpInterval = AppStartState.getInstance().getAppStartInterval(); + // if appStartUpInterval is null, metrics are not ready to be sent + if (appStartUpInterval != null) { + final MeasurementValue value = new MeasurementValue((float) appStartUpInterval); + + final String appStartKey = + AppStartState.getInstance().isColdStart() ? "app_start_cold" : "app_start_warm"; + + transaction.getMeasurements().put(appStartKey, value); + sentStartMeasurement = true; + } } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java index 138d97b937..96648f158b 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java @@ -25,10 +25,10 @@ public final class SentryPerformanceProvider extends ContentProvider { // static to rely on Class load private static final @NotNull Date appStartTime = DateUtils.getCurrentDateTime(); // SystemClock.uptimeMillis() isn't affected by phone provider or clock changes. - private static final long appStart = SystemClock.uptimeMillis(); + private static final long appStartMillis = SystemClock.uptimeMillis(); public SentryPerformanceProvider() { - AppStartState.getInstance().setAppStartTime(appStart, appStartTime); + AppStartState.getInstance().setAppStartTime(appStartMillis, appStartTime); } @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 e6c1df2a75..f180bb689c 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 @@ -431,7 +431,7 @@ class ActivityLifecycleIntegrationTest { val activity = mock() sut.onActivityCreated(activity, null) - assertTrue(AppStartState.getInstance().coldStart) + assertTrue(AppStartState.getInstance().isColdStart) } @Test @@ -444,7 +444,7 @@ class ActivityLifecycleIntegrationTest { val bundle = Bundle() sut.onActivityCreated(activity, bundle) - assertFalse(AppStartState.getInstance().coldStart) + assertFalse(AppStartState.getInstance().isColdStart) } @Test @@ -458,7 +458,7 @@ class ActivityLifecycleIntegrationTest { sut.onActivityCreated(activity, bundle) sut.onActivityCreated(activity, null) - assertFalse(AppStartState.getInstance().coldStart) + assertFalse(AppStartState.getInstance().isColdStart) } @Test diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AppStartStateTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AppStartStateTest.kt index 30e19d0c47..aa6e077ddc 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AppStartStateTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AppStartStateTest.kt @@ -3,6 +3,7 @@ package io.sentry.android.core import java.util.Date import kotlin.test.BeforeTest import kotlin.test.Test +import kotlin.test.assertEquals import kotlin.test.assertNull import kotlin.test.assertSame @@ -17,7 +18,7 @@ class AppStartStateTest { fun `appStartInterval returns null if end time is not set`() { val sut = AppStartState.getInstance() - sut.setAppStartTime(0, Date()) + sut.setAppStartTime(0, Date(0)) assertNull(sut.appStartInterval) } @@ -41,4 +42,15 @@ class AppStartStateTest { assertSame(date, sut.appStartTime) } + + @Test + fun `getAppStartInterval returns right calculation`() { + val sut = AppStartState.getInstance() + + val date = Date() + sut.setAppStartTime(100, date) + sut.setAppStartEnd(500) + + assertEquals(400, sut.appStartInterval) + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt index 355f52c6b2..1cd8ba377f 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt @@ -11,13 +11,12 @@ import kotlin.test.assertTrue class PerformanceAndroidEventProcessorTest { private class Fixture { - val options = SentryAndroidOptions().apply { - tracesSampleRate = 1.0 - } + val options = SentryAndroidOptions() val tracer = SentryTracer(mock(), mock()) - fun getSut(): PerformanceAndroidEventProcessor { + fun getSut(tracesSampleRate: Double? = 1.0): PerformanceAndroidEventProcessor { + options.tracesSampleRate = tracesSampleRate return PerformanceAndroidEventProcessor(options) } } @@ -82,8 +81,7 @@ class PerformanceAndroidEventProcessorTest { @Test fun `do not add app start metric if performance is disabled`() { - val sut = fixture.getSut() - sut.options.tracesSampleRate = null + val sut = fixture.getSut(tracesSampleRate = null) var tr = SentryTransaction(fixture.tracer) @@ -93,7 +91,7 @@ class PerformanceAndroidEventProcessorTest { } private fun setAppStart(coldStart: Boolean = true) { - AppStartState.getInstance().coldStart = coldStart + AppStartState.getInstance().isColdStart = coldStart AppStartState.getInstance().setAppStartTime(0, Date()) AppStartState.getInstance().setAppStartEnd() } From 121876abe5e2e685fab774106bf3d4b009781b4b Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Wed, 9 Jun 2021 14:29:09 +0200 Subject: [PATCH 07/21] timestamp --- .../core/ActivityLifecycleIntegration.java | 21 +++++++-- sentry/src/main/java/io/sentry/Hub.java | 22 ++++++++- .../src/main/java/io/sentry/HubAdapter.java | 8 ++++ sentry/src/main/java/io/sentry/IHub.java | 17 ++++++- sentry/src/main/java/io/sentry/ISpan.java | 7 +++ sentry/src/main/java/io/sentry/NoOpHub.java | 7 +++ sentry/src/main/java/io/sentry/NoOpSpan.java | 7 +++ .../main/java/io/sentry/NoOpTransaction.java | 6 +++ sentry/src/main/java/io/sentry/Sentry.java | 11 +++++ .../src/main/java/io/sentry/SentryTracer.java | 45 ++++++++++++++++--- sentry/src/main/java/io/sentry/Span.java | 26 ++++++++--- 11 files changed, 162 insertions(+), 15 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 c7459f8cfa..efe9c8338c 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 @@ -8,6 +8,7 @@ import androidx.annotation.Nullable; import io.sentry.Breadcrumb; import io.sentry.IHub; +import io.sentry.ISpan; import io.sentry.ITransaction; import io.sentry.Integration; import io.sentry.Scope; @@ -17,6 +18,7 @@ import io.sentry.util.Objects; import java.io.Closeable; import java.io.IOException; +import java.util.Date; import java.util.Map; import java.util.WeakHashMap; import org.jetbrains.annotations.NotNull; @@ -37,6 +39,8 @@ public final class ActivityLifecycleIntegration private boolean firstActivityCreated = false; private boolean firstActivityResumed = false; + private @Nullable ISpan appStartSpan; + // WeakHashMap isn't thread safe but ActivityLifecycleCallbacks is only called from the // main-thread private final @NotNull WeakHashMap activitiesWithOngoingTransactions = @@ -119,8 +123,16 @@ private void startTracing(final @NonNull Activity activity) { stopPreviousTransactions(); // we can only bind to the scope if there's no running transaction - final ITransaction transaction = - hub.startTransaction(getActivityName(activity), "navigation"); + ITransaction transaction; + final String activityName = getActivityName(activity); + final String op = "navigation"; + final Date appStartTime = AppStartState.getInstance().getAppStartTime(); + if (firstActivityCreated || appStartTime == null) { + transaction = hub.startTransaction(activityName, op); + } else { + transaction = hub.startTransaction(activityName, op, appStartTime); + appStartSpan = transaction.startChild("app.start", appStartTime); + } // lets bind to the scope so other integrations can pick it up hub.configureScope( @@ -193,7 +205,6 @@ public synchronized void onActivityCreated( // if Activity has savedInstanceState then its a warm start // https://developer.android.com/topic/performance/vitals/launch-time#warm AppStartState.getInstance().setColdStart(savedInstanceState == null); - firstActivityCreated = true; } addBreadcrumb(activity, "created"); @@ -202,6 +213,7 @@ public synchronized void onActivityCreated( if (!isAllActivityCallbacksAvailable) { startTracing(activity); } + firstActivityCreated = true; } @Override @@ -214,6 +226,9 @@ public synchronized void onActivityResumed(final @NonNull Activity activity) { if (!firstActivityResumed && performanceEnabled) { // sets App start as finished when the very first activity calls onResume AppStartState.getInstance().setAppStartEnd(); + if (appStartSpan != null) { + appStartSpan.finish(); + } firstActivityResumed = true; } diff --git a/sentry/src/main/java/io/sentry/Hub.java b/sentry/src/main/java/io/sentry/Hub.java index ca4273bb11..1276e7cb65 100644 --- a/sentry/src/main/java/io/sentry/Hub.java +++ b/sentry/src/main/java/io/sentry/Hub.java @@ -11,6 +11,7 @@ import io.sentry.util.Pair; import java.io.Closeable; import java.util.Collections; +import java.util.Date; import java.util.List; import java.util.Map; import java.util.WeakHashMap; @@ -580,6 +581,25 @@ public void flush(long timeoutMillis) { final @NotNull TransactionContext transactionContext, final @Nullable CustomSamplingContext customSamplingContext, final boolean bindToScope) { + return createTransaction(transactionContext, customSamplingContext, bindToScope, null); + } + + @ApiStatus.Internal + @Override + public @NotNull ITransaction startTransaction( + @NotNull TransactionContext transactionContext, + @Nullable CustomSamplingContext customSamplingContext, + boolean bindToScope, + @NotNull Date startTimestamp) { + return createTransaction( + transactionContext, customSamplingContext, bindToScope, startTimestamp); + } + + private @NotNull ITransaction createTransaction( + final @NotNull TransactionContext transactionContext, + final @Nullable CustomSamplingContext customSamplingContext, + final boolean bindToScope, + final @Nullable Date startTimestamp) { Objects.requireNonNull(transactionContext, "transactionContext is required"); ITransaction transaction; @@ -602,7 +622,7 @@ public void flush(long timeoutMillis) { boolean samplingDecision = tracesSampler.sample(samplingContext); transactionContext.setSampled(samplingDecision); - transaction = new SentryTracer(transactionContext, this); + transaction = new SentryTracer(transactionContext, this, startTimestamp); } if (bindToScope) { configureScope(scope -> scope.setTransaction(transaction)); diff --git a/sentry/src/main/java/io/sentry/HubAdapter.java b/sentry/src/main/java/io/sentry/HubAdapter.java index a6e03ae7ef..bf5adf3847 100644 --- a/sentry/src/main/java/io/sentry/HubAdapter.java +++ b/sentry/src/main/java/io/sentry/HubAdapter.java @@ -3,6 +3,8 @@ import io.sentry.protocol.SentryId; import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.User; + +import java.util.Date; import java.util.List; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; @@ -174,6 +176,12 @@ public void flush(long timeoutMillis) { return Sentry.startTransaction(transactionContexts, customSamplingContext, bindToScope); } + @ApiStatus.Internal + @Override + public @NotNull ITransaction startTransaction(@NotNull TransactionContext transactionContexts, @Nullable CustomSamplingContext customSamplingContext, boolean bindToScope, @NotNull Date startTimestamp) { + return Sentry.startTransaction(transactionContexts, customSamplingContext, bindToScope, startTimestamp); + } + @Override public @Nullable SentryTraceHeader traceHeaders() { return Sentry.traceHeaders(); diff --git a/sentry/src/main/java/io/sentry/IHub.java b/sentry/src/main/java/io/sentry/IHub.java index 0f88661eba..0245554818 100644 --- a/sentry/src/main/java/io/sentry/IHub.java +++ b/sentry/src/main/java/io/sentry/IHub.java @@ -3,6 +3,7 @@ import io.sentry.protocol.SentryId; import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.User; +import java.util.Date; import java.util.List; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; @@ -373,6 +374,14 @@ ITransaction startTransaction( @Nullable CustomSamplingContext customSamplingContext, boolean bindToScope); + @ApiStatus.Internal + @NotNull + ITransaction startTransaction( + @NotNull TransactionContext transactionContexts, + @Nullable CustomSamplingContext customSamplingContext, + boolean bindToScope, + @NotNull Date startTimestamp); + /** * Creates a Transaction and returns the instance. Based on the {@link * SentryOptions#getTracesSampleRate()} the decision if transaction is sampled will be taken by @@ -384,7 +393,13 @@ ITransaction startTransaction( */ default @NotNull ITransaction startTransaction( final @NotNull String name, final @NotNull String operation) { - return startTransaction(name, operation, null); + return startTransaction(name, operation, (CustomSamplingContext) null); + } + + @ApiStatus.Internal + default @NotNull ITransaction startTransaction( + final @NotNull String name, final @NotNull String operation, @NotNull Date startTimestamp) { + return startTransaction(new TransactionContext(name, operation), null, false, startTimestamp); } /** diff --git a/sentry/src/main/java/io/sentry/ISpan.java b/sentry/src/main/java/io/sentry/ISpan.java index 145423a85a..feef7a3f20 100644 --- a/sentry/src/main/java/io/sentry/ISpan.java +++ b/sentry/src/main/java/io/sentry/ISpan.java @@ -1,5 +1,8 @@ package io.sentry; +import java.util.Date; + +import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -14,6 +17,10 @@ public interface ISpan { @NotNull ISpan startChild(@NotNull String operation); + @ApiStatus.Internal + @NotNull + ISpan startChild(@NotNull String operation, @NotNull Date timestamp); + /** * 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 2ef71d60d7..4e577dbe78 100644 --- a/sentry/src/main/java/io/sentry/NoOpHub.java +++ b/sentry/src/main/java/io/sentry/NoOpHub.java @@ -3,6 +3,8 @@ import io.sentry.protocol.SentryId; import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.User; + +import java.util.Date; import java.util.List; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -134,6 +136,11 @@ public void flush(long timeoutMillis) {} return NoOpTransaction.getInstance(); } + @Override + public @NotNull ITransaction startTransaction(@NotNull TransactionContext transactionContexts, @Nullable CustomSamplingContext customSamplingContext, boolean bindToScope, @NotNull Date startTimestamp) { + return NoOpTransaction.getInstance(); + } + @Override public @NotNull SentryTraceHeader traceHeaders() { return new SentryTraceHeader(SentryId.EMPTY_ID, SpanId.EMPTY_ID, true); diff --git a/sentry/src/main/java/io/sentry/NoOpSpan.java b/sentry/src/main/java/io/sentry/NoOpSpan.java index 75f44ff085..7795cf9a26 100644 --- a/sentry/src/main/java/io/sentry/NoOpSpan.java +++ b/sentry/src/main/java/io/sentry/NoOpSpan.java @@ -4,6 +4,8 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import java.util.Date; + public final class NoOpSpan implements ISpan { private static final NoOpSpan instance = new NoOpSpan(); @@ -19,6 +21,11 @@ public static NoOpSpan getInstance() { return NoOpSpan.getInstance(); } + @Override + public @NotNull ISpan startChild(@NotNull String operation, @NotNull Date timestamp) { + 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 6d2ba8d8b5..c0d015295a 100644 --- a/sentry/src/main/java/io/sentry/NoOpTransaction.java +++ b/sentry/src/main/java/io/sentry/NoOpTransaction.java @@ -4,6 +4,7 @@ import io.sentry.protocol.Request; import io.sentry.protocol.SentryId; import java.util.Collections; +import java.util.Date; import java.util.List; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; @@ -32,6 +33,11 @@ public void setName(@NotNull String name) {} return NoOpSpan.getInstance(); } + @Override + public @NotNull ISpan startChild(@NotNull String operation, @NotNull Date timestamp) { + 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/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index 635bf86113..f8c31f2af1 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -6,6 +6,7 @@ import io.sentry.protocol.User; import java.io.File; import java.lang.reflect.InvocationTargetException; +import java.util.Date; import java.util.List; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; @@ -652,6 +653,16 @@ public static void endSession() { .startTransaction(transactionContexts, customSamplingContext, bindToScope); } + @ApiStatus.Internal + public static @NotNull ITransaction startTransaction( + final @NotNull TransactionContext transactionContexts, + final @Nullable CustomSamplingContext customSamplingContext, + final boolean bindToScope, + final @NotNull Date startTimestamp) { + return getCurrentHub() + .startTransaction(transactionContexts, customSamplingContext, bindToScope, startTimestamp); + } + /** * Returns trace header of active transaction or {@code null} if no transaction is active. * diff --git a/sentry/src/main/java/io/sentry/SentryTracer.java b/sentry/src/main/java/io/sentry/SentryTracer.java index 8e1a1bb879..0fc16a2c87 100644 --- a/sentry/src/main/java/io/sentry/SentryTracer.java +++ b/sentry/src/main/java/io/sentry/SentryTracer.java @@ -24,9 +24,16 @@ public final class SentryTracer implements ITransaction { private @NotNull String name; public SentryTracer(final @NotNull TransactionContext context, final @NotNull IHub hub) { + this(context, hub, null); + } + + SentryTracer( + final @NotNull TransactionContext context, + final @NotNull IHub hub, + final @Nullable Date startTimestamp) { Objects.requireNonNull(context, "context is required"); Objects.requireNonNull(hub, "hub is required"); - this.root = new Span(context, this, hub); + this.root = new Span(context, this, hub, startTimestamp); this.name = context.getName(); this.hub = hub; } @@ -56,11 +63,19 @@ ISpan startChild( final @NotNull SpanId parentSpanId, final @NotNull String operation, final @Nullable String description) { - final ISpan span = startChild(parentSpanId, operation); + final ISpan span = createChild(parentSpanId, operation); span.setDescription(description); return span; } + @NotNull + ISpan startChild( + final @NotNull SpanId parentSpanId, + final @NotNull String operation, + final @NotNull Date timestamp) { + return createChild(parentSpanId, operation, timestamp); + } + /** * Starts a child Span with given trace id and parent span id. * @@ -68,17 +83,37 @@ ISpan startChild( * @return a new transaction span */ @NotNull - private ISpan startChild(final @NotNull SpanId parentSpanId, final @NotNull String operation) { + private ISpan createChild(final @NotNull SpanId parentSpanId, final @NotNull String operation) { + return createChild(parentSpanId, operation, null); + } + + @NotNull + private ISpan createChild(final @NotNull SpanId parentSpanId, final @NotNull String operation, @Nullable Date timestamp) { Objects.requireNonNull(parentSpanId, "parentSpanId is required"); Objects.requireNonNull(operation, "operation is required"); - final Span span = new Span(root.getTraceId(), parentSpanId, this, operation, this.hub); + final Span span = new Span(root.getTraceId(), parentSpanId, this, operation, this.hub, timestamp); this.children.add(span); return span; } @Override public @NotNull ISpan startChild(final @NotNull String operation) { - return this.startChild(operation, null); + return this.startChild(operation, (String) null); + } + + @Override + public @NotNull ISpan startChild(final @NotNull String operation, @NotNull Date timestamp) { + if (children.size() < hub.getOptions().getMaxSpans()) { + return root.startChild(operation, timestamp); + } else { + hub.getOptions() + .getLogger() + .log( + SentryLevel.WARNING, + "Span operation: %s, dropped due to limit reached. Returning NoOpSpan.", + operation); + return NoOpSpan.getInstance(); + } } @Override diff --git a/sentry/src/main/java/io/sentry/Span.java b/sentry/src/main/java/io/sentry/Span.java index 18f1a202f9..171ffec7e9 100644 --- a/sentry/src/main/java/io/sentry/Span.java +++ b/sentry/src/main/java/io/sentry/Span.java @@ -38,21 +38,32 @@ 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); + } + + Span( + final @NotNull SentryId traceId, + final @Nullable SpanId parentSpanId, + final @NotNull SentryTracer transaction, + final @NotNull String operation, + final @NotNull IHub hub, + final @Nullable Date startTimestamp) { this.context = - new SpanContext(traceId, new SpanId(), operation, parentSpanId, transaction.isSampled()); + new SpanContext(traceId, new SpanId(), operation, parentSpanId, transaction.isSampled()); this.transaction = Objects.requireNonNull(transaction, "transaction is required"); - this.startTimestamp = DateUtils.getCurrentDateTime(); + this.startTimestamp = startTimestamp != null ? startTimestamp : DateUtils.getCurrentDateTime(); this.hub = Objects.requireNonNull(hub, "hub is required"); } Span( final @NotNull TransactionContext context, final @NotNull SentryTracer sentryTracer, - final @NotNull IHub hub) { + final @NotNull IHub hub, + final @Nullable Date startTimestamp) { this.context = Objects.requireNonNull(context, "context is required"); this.transaction = Objects.requireNonNull(sentryTracer, "sentryTracer is required"); this.hub = Objects.requireNonNull(hub, "hub is required"); - this.startTimestamp = DateUtils.getCurrentDateTime(); + this.startTimestamp = startTimestamp != null ? startTimestamp : DateUtils.getCurrentDateTime(); } public @NotNull Date getStartTimestamp() { @@ -65,7 +76,12 @@ public final class Span implements ISpan { @Override public @NotNull ISpan startChild(final @NotNull String operation) { - return this.startChild(operation, null); + return this.startChild(operation, (String)null); + } + + @Override + public @NotNull ISpan startChild(@NotNull String operation, @NotNull Date timestamp) { + return transaction.startChild(context.getSpanId(), operation, timestamp); } @Override From 51751e5b4c7d50a638c5c07075da56bbbfb04b2e Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Wed, 9 Jun 2021 14:50:00 +0200 Subject: [PATCH 08/21] ref --- .../core/ActivityLifecycleIntegration.java | 17 ++++++-- .../src/main/java/io/sentry/HubAdapter.java | 10 +++-- sentry/src/main/java/io/sentry/ISpan.java | 1 - sentry/src/main/java/io/sentry/NoOpHub.java | 7 +++- sentry/src/main/java/io/sentry/NoOpSpan.java | 3 +- sentry/src/main/java/io/sentry/Sentry.java | 10 ++--- .../src/main/java/io/sentry/SentryTracer.java | 40 +++++++++++-------- sentry/src/main/java/io/sentry/Span.java | 16 ++++---- 8 files changed, 62 insertions(+), 42 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 efe9c8338c..39c30b4eb7 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 @@ -28,6 +28,8 @@ public final class ActivityLifecycleIntegration implements Integration, Closeable, Application.ActivityLifecycleCallbacks { + private static final String NAV_OP = "navigation"; + private final @NotNull Application application; private @Nullable IHub hub; private @Nullable SentryAndroidOptions options; @@ -125,12 +127,17 @@ private void startTracing(final @NonNull Activity activity) { // we can only bind to the scope if there's no running transaction ITransaction transaction; final String activityName = getActivityName(activity); - final String op = "navigation"; + final Date appStartTime = AppStartState.getInstance().getAppStartTime(); + + // in case appStartTime isn't available, we don't create a span for it. if (firstActivityCreated || appStartTime == null) { - transaction = hub.startTransaction(activityName, op); + transaction = hub.startTransaction(activityName, NAV_OP); } else { - transaction = hub.startTransaction(activityName, op, appStartTime); + // start transaction with app start timestamp + transaction = hub.startTransaction(activityName, NAV_OP, appStartTime); + // start specific span for app start + // TODO: add description cold/warm appStartSpan = transaction.startChild("app.start", appStartTime); } @@ -226,8 +233,10 @@ public synchronized void onActivityResumed(final @NonNull Activity activity) { if (!firstActivityResumed && performanceEnabled) { // sets App start as finished when the very first activity calls onResume AppStartState.getInstance().setAppStartEnd(); + + // finishes app start span if (appStartSpan != null) { - appStartSpan.finish(); + appStartSpan.finish(); } firstActivityResumed = true; } diff --git a/sentry/src/main/java/io/sentry/HubAdapter.java b/sentry/src/main/java/io/sentry/HubAdapter.java index bf5adf3847..cb56bd0eb5 100644 --- a/sentry/src/main/java/io/sentry/HubAdapter.java +++ b/sentry/src/main/java/io/sentry/HubAdapter.java @@ -3,7 +3,6 @@ import io.sentry.protocol.SentryId; import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.User; - import java.util.Date; import java.util.List; import org.jetbrains.annotations.ApiStatus; @@ -178,8 +177,13 @@ public void flush(long timeoutMillis) { @ApiStatus.Internal @Override - public @NotNull ITransaction startTransaction(@NotNull TransactionContext transactionContexts, @Nullable CustomSamplingContext customSamplingContext, boolean bindToScope, @NotNull Date startTimestamp) { - return Sentry.startTransaction(transactionContexts, customSamplingContext, bindToScope, startTimestamp); + public @NotNull ITransaction startTransaction( + @NotNull TransactionContext transactionContexts, + @Nullable CustomSamplingContext customSamplingContext, + boolean bindToScope, + @NotNull Date startTimestamp) { + return Sentry.startTransaction( + transactionContexts, customSamplingContext, bindToScope, startTimestamp); } @Override diff --git a/sentry/src/main/java/io/sentry/ISpan.java b/sentry/src/main/java/io/sentry/ISpan.java index feef7a3f20..2bd5bf233a 100644 --- a/sentry/src/main/java/io/sentry/ISpan.java +++ b/sentry/src/main/java/io/sentry/ISpan.java @@ -1,7 +1,6 @@ package io.sentry; import java.util.Date; - import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; diff --git a/sentry/src/main/java/io/sentry/NoOpHub.java b/sentry/src/main/java/io/sentry/NoOpHub.java index 4e577dbe78..152f6b8c6b 100644 --- a/sentry/src/main/java/io/sentry/NoOpHub.java +++ b/sentry/src/main/java/io/sentry/NoOpHub.java @@ -3,7 +3,6 @@ import io.sentry.protocol.SentryId; import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.User; - import java.util.Date; import java.util.List; import org.jetbrains.annotations.NotNull; @@ -137,7 +136,11 @@ public void flush(long timeoutMillis) {} } @Override - public @NotNull ITransaction startTransaction(@NotNull TransactionContext transactionContexts, @Nullable CustomSamplingContext customSamplingContext, boolean bindToScope, @NotNull Date startTimestamp) { + public @NotNull ITransaction startTransaction( + @NotNull TransactionContext transactionContexts, + @Nullable CustomSamplingContext customSamplingContext, + boolean bindToScope, + @NotNull Date startTimestamp) { return NoOpTransaction.getInstance(); } diff --git a/sentry/src/main/java/io/sentry/NoOpSpan.java b/sentry/src/main/java/io/sentry/NoOpSpan.java index 7795cf9a26..c9c9871cea 100644 --- a/sentry/src/main/java/io/sentry/NoOpSpan.java +++ b/sentry/src/main/java/io/sentry/NoOpSpan.java @@ -1,11 +1,10 @@ package io.sentry; import io.sentry.protocol.SentryId; +import java.util.Date; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import java.util.Date; - public final class NoOpSpan implements ISpan { private static final NoOpSpan instance = new NoOpSpan(); diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index f8c31f2af1..6a6ea14ee9 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -655,12 +655,12 @@ public static void endSession() { @ApiStatus.Internal public static @NotNull ITransaction startTransaction( - final @NotNull TransactionContext transactionContexts, - final @Nullable CustomSamplingContext customSamplingContext, - final boolean bindToScope, - final @NotNull Date startTimestamp) { + final @NotNull TransactionContext transactionContexts, + final @Nullable CustomSamplingContext customSamplingContext, + final boolean bindToScope, + final @NotNull Date startTimestamp) { return getCurrentHub() - .startTransaction(transactionContexts, customSamplingContext, bindToScope, startTimestamp); + .startTransaction(transactionContexts, customSamplingContext, bindToScope, startTimestamp); } /** diff --git a/sentry/src/main/java/io/sentry/SentryTracer.java b/sentry/src/main/java/io/sentry/SentryTracer.java index 0fc16a2c87..7ec519829f 100644 --- a/sentry/src/main/java/io/sentry/SentryTracer.java +++ b/sentry/src/main/java/io/sentry/SentryTracer.java @@ -70,9 +70,9 @@ ISpan startChild( @NotNull ISpan startChild( - final @NotNull SpanId parentSpanId, - final @NotNull String operation, - final @NotNull Date timestamp) { + final @NotNull SpanId parentSpanId, + final @NotNull String operation, + final @NotNull Date timestamp) { return createChild(parentSpanId, operation, timestamp); } @@ -88,10 +88,14 @@ private ISpan createChild(final @NotNull SpanId parentSpanId, final @NotNull Str } @NotNull - private ISpan createChild(final @NotNull SpanId parentSpanId, final @NotNull String operation, @Nullable Date timestamp) { + private ISpan createChild( + final @NotNull SpanId parentSpanId, + final @NotNull String operation, + @Nullable Date timestamp) { Objects.requireNonNull(parentSpanId, "parentSpanId is required"); Objects.requireNonNull(operation, "operation is required"); - final Span span = new Span(root.getTraceId(), parentSpanId, this, operation, this.hub, timestamp); + final Span span = + new Span(root.getTraceId(), parentSpanId, this, operation, this.hub, timestamp); this.children.add(span); return span; } @@ -103,24 +107,26 @@ private ISpan createChild(final @NotNull SpanId parentSpanId, final @NotNull Str @Override public @NotNull ISpan startChild(final @NotNull String operation, @NotNull Date timestamp) { - if (children.size() < hub.getOptions().getMaxSpans()) { - return root.startChild(operation, timestamp); - } else { - hub.getOptions() - .getLogger() - .log( - SentryLevel.WARNING, - "Span operation: %s, dropped due to limit reached. Returning NoOpSpan.", - operation); - return NoOpSpan.getInstance(); - } + return createChild(operation, null, timestamp); } @Override public @NotNull ISpan startChild( final @NotNull String operation, final @Nullable String description) { + return createChild(operation, description, null); + } + + private @NotNull ISpan createChild( + final @NotNull String operation, + final @Nullable String description, + @Nullable Date timestamp) { if (children.size() < hub.getOptions().getMaxSpans()) { - return root.startChild(operation, description); + // TODO: overloads would be better + if (timestamp != null) { + return root.startChild(operation, timestamp); + } else { + return root.startChild(operation, description); + } } else { hub.getOptions() .getLogger() diff --git a/sentry/src/main/java/io/sentry/Span.java b/sentry/src/main/java/io/sentry/Span.java index 171ffec7e9..f54166b3ff 100644 --- a/sentry/src/main/java/io/sentry/Span.java +++ b/sentry/src/main/java/io/sentry/Span.java @@ -42,14 +42,14 @@ public final class Span implements ISpan { } Span( - final @NotNull SentryId traceId, - final @Nullable SpanId parentSpanId, - final @NotNull SentryTracer transaction, - final @NotNull String operation, - final @NotNull IHub hub, - final @Nullable Date startTimestamp) { + final @NotNull SentryId traceId, + final @Nullable SpanId parentSpanId, + final @NotNull SentryTracer transaction, + final @NotNull String operation, + final @NotNull IHub hub, + final @Nullable Date startTimestamp) { this.context = - new SpanContext(traceId, new SpanId(), operation, parentSpanId, transaction.isSampled()); + new SpanContext(traceId, new SpanId(), operation, parentSpanId, transaction.isSampled()); this.transaction = Objects.requireNonNull(transaction, "transaction is required"); this.startTimestamp = startTimestamp != null ? startTimestamp : DateUtils.getCurrentDateTime(); this.hub = Objects.requireNonNull(hub, "hub is required"); @@ -76,7 +76,7 @@ public final class Span implements ISpan { @Override public @NotNull ISpan startChild(final @NotNull String operation) { - return this.startChild(operation, (String)null); + return this.startChild(operation, (String) null); } @Override From a62f437ea142ed59f476a8abb3d804aaa4802972 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Wed, 9 Jun 2021 14:50:55 +0200 Subject: [PATCH 09/21] api dump --- sentry/api/sentry.api | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 8f1f6331b3..92c96702ab 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -134,6 +134,7 @@ public final class io/sentry/Hub : io/sentry/IHub { public fun setUser (Lio/sentry/protocol/User;)V public fun startSession ()V public fun startTransaction (Lio/sentry/TransactionContext;Lio/sentry/CustomSamplingContext;Z)Lio/sentry/ITransaction; + public fun startTransaction (Lio/sentry/TransactionContext;Lio/sentry/CustomSamplingContext;ZLjava/util/Date;)Lio/sentry/ITransaction; public fun traceHeaders ()Lio/sentry/SentryTraceHeader; public fun withScope (Lio/sentry/ScopeCallback;)V } @@ -173,6 +174,7 @@ public final class io/sentry/HubAdapter : io/sentry/IHub { public fun startSession ()V public fun startTransaction (Lio/sentry/TransactionContext;)Lio/sentry/ITransaction; public fun startTransaction (Lio/sentry/TransactionContext;Lio/sentry/CustomSamplingContext;Z)Lio/sentry/ITransaction; + public fun startTransaction (Lio/sentry/TransactionContext;Lio/sentry/CustomSamplingContext;ZLjava/util/Date;)Lio/sentry/ITransaction; public fun traceHeaders ()Lio/sentry/SentryTraceHeader; public fun withScope (Lio/sentry/ScopeCallback;)V } @@ -227,10 +229,12 @@ public abstract interface class io/sentry/IHub { public fun startTransaction (Lio/sentry/TransactionContext;)Lio/sentry/ITransaction; public fun startTransaction (Lio/sentry/TransactionContext;Lio/sentry/CustomSamplingContext;)Lio/sentry/ITransaction; public abstract fun startTransaction (Lio/sentry/TransactionContext;Lio/sentry/CustomSamplingContext;Z)Lio/sentry/ITransaction; + public abstract fun startTransaction (Lio/sentry/TransactionContext;Lio/sentry/CustomSamplingContext;ZLjava/util/Date;)Lio/sentry/ITransaction; public fun startTransaction (Lio/sentry/TransactionContext;Z)Lio/sentry/ITransaction; public fun startTransaction (Ljava/lang/String;Ljava/lang/String;)Lio/sentry/ITransaction; public fun startTransaction (Ljava/lang/String;Ljava/lang/String;Lio/sentry/CustomSamplingContext;)Lio/sentry/ITransaction; public fun startTransaction (Ljava/lang/String;Ljava/lang/String;Lio/sentry/CustomSamplingContext;Z)Lio/sentry/ITransaction; + public fun startTransaction (Ljava/lang/String;Ljava/lang/String;Ljava/util/Date;)Lio/sentry/ITransaction; public fun startTransaction (Ljava/lang/String;Ljava/lang/String;Z)Lio/sentry/ITransaction; public abstract fun traceHeaders ()Lio/sentry/SentryTraceHeader; public abstract fun withScope (Lio/sentry/ScopeCallback;)V @@ -300,6 +304,7 @@ public abstract interface class io/sentry/ISpan { public abstract fun setThrowable (Ljava/lang/Throwable;)V 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/util/Date;)Lio/sentry/ISpan; public abstract fun toSentryTrace ()Lio/sentry/SentryTraceHeader; } @@ -376,6 +381,7 @@ public final class io/sentry/NoOpHub : io/sentry/IHub { public fun startSession ()V public fun startTransaction (Lio/sentry/TransactionContext;)Lio/sentry/ITransaction; public fun startTransaction (Lio/sentry/TransactionContext;Lio/sentry/CustomSamplingContext;Z)Lio/sentry/ITransaction; + public fun startTransaction (Lio/sentry/TransactionContext;Lio/sentry/CustomSamplingContext;ZLjava/util/Date;)Lio/sentry/ITransaction; public fun traceHeaders ()Lio/sentry/SentryTraceHeader; public fun withScope (Lio/sentry/ScopeCallback;)V } @@ -406,6 +412,7 @@ public final class io/sentry/NoOpSpan : io/sentry/ISpan { 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/util/Date;)Lio/sentry/ISpan; public fun toSentryTrace ()Lio/sentry/SentryTraceHeader; } @@ -436,6 +443,7 @@ public final class io/sentry/NoOpTransaction : io/sentry/ITransaction { 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/util/Date;)Lio/sentry/ISpan; public fun toSentryTrace ()Lio/sentry/SentryTraceHeader; } @@ -583,6 +591,7 @@ public final class io/sentry/Sentry { public static fun startTransaction (Lio/sentry/TransactionContext;)Lio/sentry/ITransaction; public static fun startTransaction (Lio/sentry/TransactionContext;Lio/sentry/CustomSamplingContext;)Lio/sentry/ITransaction; public static fun startTransaction (Lio/sentry/TransactionContext;Lio/sentry/CustomSamplingContext;Z)Lio/sentry/ITransaction; + public static fun startTransaction (Lio/sentry/TransactionContext;Lio/sentry/CustomSamplingContext;ZLjava/util/Date;)Lio/sentry/ITransaction; public static fun startTransaction (Lio/sentry/TransactionContext;Z)Lio/sentry/ITransaction; public static fun startTransaction (Ljava/lang/String;Ljava/lang/String;)Lio/sentry/ITransaction; public static fun startTransaction (Ljava/lang/String;Ljava/lang/String;Lio/sentry/CustomSamplingContext;)Lio/sentry/ITransaction; @@ -942,6 +951,7 @@ public final class io/sentry/SentryTracer : io/sentry/ITransaction { 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/util/Date;)Lio/sentry/ISpan; public fun toSentryTrace ()Lio/sentry/SentryTraceHeader; } @@ -1017,6 +1027,7 @@ public final class io/sentry/Span : io/sentry/ISpan { 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/util/Date;)Lio/sentry/ISpan; public fun toSentryTrace ()Lio/sentry/SentryTraceHeader; } From 4f3001f567042c5e825f630f549aa44e9ad743b0 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Wed, 9 Jun 2021 16:39:49 +0200 Subject: [PATCH 10/21] comment --- .../io/sentry/android/core/ActivityLifecycleIntegration.java | 3 ++- sentry/src/main/java/io/sentry/Span.java | 2 +- 2 files changed, 3 insertions(+), 2 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 39c30b4eb7..545e38dff3 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 @@ -137,7 +137,8 @@ private void startTracing(final @NonNull Activity activity) { // start transaction with app start timestamp transaction = hub.startTransaction(activityName, NAV_OP, appStartTime); // start specific span for app start - // TODO: add description cold/warm + // TODO: add description cold/warm or different operation? how do we break down + // per app start cold/warm? appStartSpan = transaction.startChild("app.start", appStartTime); } diff --git a/sentry/src/main/java/io/sentry/Span.java b/sentry/src/main/java/io/sentry/Span.java index f54166b3ff..1d6e2baeb7 100644 --- a/sentry/src/main/java/io/sentry/Span.java +++ b/sentry/src/main/java/io/sentry/Span.java @@ -80,7 +80,7 @@ public final class Span implements ISpan { } @Override - public @NotNull ISpan startChild(@NotNull String operation, @NotNull Date timestamp) { + public @NotNull ISpan startChild(final @NotNull String operation, final @NotNull Date timestamp) { return transaction.startChild(context.getSpanId(), operation, timestamp); } From dd388a892f877a0b750415d4101143eccb51a4e1 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 10 Jun 2021 11:09:53 +0200 Subject: [PATCH 11/21] fix tests --- .../core/ActivityLifecycleIntegrationTest.kt | 98 ++++++++++++------- sentry/src/main/java/io/sentry/Span.java | 4 +- 2 files changed, 63 insertions(+), 39 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 f180bb689c..bbd3003996 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 @@ -18,6 +18,7 @@ import io.sentry.SentryTracer import io.sentry.SpanStatus import io.sentry.TransactionContext import java.util.Date +import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse @@ -30,13 +31,15 @@ class ActivityLifecycleIntegrationTest { val application = mock() val hub = mock() val options = SentryAndroidOptions() - val activity = mock() val bundle = mock() - val transaction = SentryTracer(TransactionContext("name", "op"), hub) + val context = TransactionContext("name", "op") + val transaction = SentryTracer(context, hub) val buildInfo = mock() fun getSut(apiVersion: Int = 29): ActivityLifecycleIntegration { whenever(hub.startTransaction(any(), any())).thenReturn(transaction) + whenever(hub.options).thenReturn(options) + whenever(hub.startTransaction(any(), any(), any())).thenReturn(transaction) whenever(buildInfo.sdkInfoVersion).thenReturn(apiVersion) return ActivityLifecycleIntegration(application, buildInfo) } @@ -44,6 +47,11 @@ class ActivityLifecycleIntegrationTest { private val fixture = Fixture() + @BeforeTest + fun `reset instance`() { + AppStartState.getInstance().resetInstance() + } + @Test fun `When activity lifecycle breadcrumb is enabled, it registers callback`() { val sut = fixture.getSut() @@ -111,7 +119,8 @@ class ActivityLifecycleIntegrationTest { val sut = fixture.getSut() sut.register(fixture.hub, fixture.options) - sut.onActivityCreated(fixture.activity, fixture.bundle) + val activity = mock() + sut.onActivityCreated(activity, fixture.bundle) verify(fixture.hub).addBreadcrumb(check { assertEquals("ui.lifecycle", it.category) assertEquals("navigation", it.type) @@ -124,7 +133,8 @@ class ActivityLifecycleIntegrationTest { fun `When activity is created, it should add a breadcrumb`() { val sut = fixture.getSut() sut.register(fixture.hub, fixture.options) - sut.onActivityCreated(fixture.activity, fixture.bundle) + val activity = mock() + sut.onActivityCreated(activity, fixture.bundle) verify(fixture.hub).addBreadcrumb(any()) } @@ -133,7 +143,8 @@ class ActivityLifecycleIntegrationTest { fun `When activity is started, it should add a breadcrumb`() { val sut = fixture.getSut() sut.register(fixture.hub, fixture.options) - sut.onActivityStarted(fixture.activity) + val activity = mock() + sut.onActivityStarted(activity) verify(fixture.hub).addBreadcrumb(any()) } @@ -142,8 +153,8 @@ class ActivityLifecycleIntegrationTest { fun `When activity is resumed, it should add a breadcrumb`() { val sut = fixture.getSut() sut.register(fixture.hub, fixture.options) - - sut.onActivityResumed(fixture.activity) + val activity = mock() + sut.onActivityResumed(activity) verify(fixture.hub).addBreadcrumb(any()) } @@ -151,8 +162,8 @@ class ActivityLifecycleIntegrationTest { fun `When activity is paused, it should add a breadcrumb`() { val sut = fixture.getSut() sut.register(fixture.hub, fixture.options) - - sut.onActivityPaused(fixture.activity) + val activity = mock() + sut.onActivityPaused(activity) verify(fixture.hub).addBreadcrumb(any()) } @@ -160,8 +171,8 @@ class ActivityLifecycleIntegrationTest { fun `When activity is stopped, it should add a breadcrumb`() { val sut = fixture.getSut() sut.register(fixture.hub, fixture.options) - - sut.onActivityStopped(fixture.activity) + val activity = mock() + sut.onActivityStopped(activity) verify(fixture.hub).addBreadcrumb(any()) } @@ -169,8 +180,8 @@ class ActivityLifecycleIntegrationTest { fun `When activity is save instance, it should add a breadcrumb`() { val sut = fixture.getSut() sut.register(fixture.hub, fixture.options) - - sut.onActivitySaveInstanceState(fixture.activity, fixture.bundle) + val activity = mock() + sut.onActivitySaveInstanceState(activity, fixture.bundle) verify(fixture.hub).addBreadcrumb(any()) } @@ -178,8 +189,8 @@ class ActivityLifecycleIntegrationTest { fun `When activity is destroyed, it should add a breadcrumb`() { val sut = fixture.getSut() sut.register(fixture.hub, fixture.options) - - sut.onActivityDestroyed(fixture.activity) + val activity = mock() + sut.onActivityDestroyed(activity) verify(fixture.hub).addBreadcrumb(any()) } @@ -187,8 +198,8 @@ class ActivityLifecycleIntegrationTest { fun `When tracing is disabled, do not start tracing`() { val sut = fixture.getSut() sut.register(fixture.hub, fixture.options) - - sut.onActivityPreCreated(fixture.activity, fixture.bundle) + val activity = mock() + sut.onActivityPreCreated(activity, fixture.bundle) verify(fixture.hub, never()).startTransaction(any(), any()) } @@ -199,8 +210,9 @@ class ActivityLifecycleIntegrationTest { fixture.options.tracesSampleRate = 1.0 sut.register(fixture.hub, fixture.options) - sut.onActivityPreCreated(fixture.activity, fixture.bundle) - sut.onActivityPreCreated(fixture.activity, fixture.bundle) + val activity = mock() + sut.onActivityPreCreated(activity, fixture.bundle) + sut.onActivityPreCreated(activity, fixture.bundle) // call only once verify(fixture.hub).startTransaction(any(), any()) @@ -212,7 +224,8 @@ class ActivityLifecycleIntegrationTest { fixture.options.tracesSampleRate = 1.0 sut.register(fixture.hub, fixture.options) - sut.onActivityPreCreated(fixture.activity, fixture.bundle) + val activity = mock() + sut.onActivityPreCreated(activity, fixture.bundle) verify(fixture.hub).startTransaction(any(), check { assertEquals("navigation", it) @@ -225,7 +238,8 @@ class ActivityLifecycleIntegrationTest { fixture.options.tracesSampleRate = 1.0 sut.register(fixture.hub, fixture.options) - sut.onActivityPreCreated(fixture.activity, fixture.bundle) + val activity = mock() + sut.onActivityPreCreated(activity, fixture.bundle) verify(fixture.hub).startTransaction(check { assertEquals("Activity", it) @@ -247,7 +261,8 @@ class ActivityLifecycleIntegrationTest { assertNotNull(scope.transaction) } - sut.onActivityPreCreated(fixture.activity, fixture.bundle) + val activity = mock() + sut.onActivityPreCreated(activity, fixture.bundle) } @Test @@ -267,7 +282,8 @@ class ActivityLifecycleIntegrationTest { assertEquals(previousTransaction, scope.transaction) } - sut.onActivityPreCreated(fixture.activity, fixture.bundle) + val activity = mock() + sut.onActivityPreCreated(activity, fixture.bundle) } @Test @@ -276,8 +292,9 @@ class ActivityLifecycleIntegrationTest { fixture.options.tracesSampleRate = 1.0 sut.register(fixture.hub, fixture.options) - sut.onActivityPreCreated(fixture.activity, fixture.bundle) - sut.onActivityPostResumed(fixture.activity) + val activity = mock() + sut.onActivityPreCreated(activity, fixture.bundle) + sut.onActivityPostResumed(activity) verify(fixture.hub).captureTransaction(check { assertEquals(SpanStatus.OK, it.status) @@ -290,11 +307,12 @@ class ActivityLifecycleIntegrationTest { fixture.options.tracesSampleRate = 1.0 sut.register(fixture.hub, fixture.options) - sut.onActivityPreCreated(fixture.activity, fixture.bundle) + val activity = mock() + sut.onActivityPreCreated(activity, fixture.bundle) fixture.transaction.status = SpanStatus.UNKNOWN_ERROR - sut.onActivityPostResumed(fixture.activity) + sut.onActivityPostResumed(activity) verify(fixture.hub).captureTransaction(check { assertEquals(SpanStatus.UNKNOWN_ERROR, it.status) @@ -308,8 +326,9 @@ class ActivityLifecycleIntegrationTest { fixture.options.isEnableActivityLifecycleTracingAutoFinish = false sut.register(fixture.hub, fixture.options) - sut.onActivityPreCreated(fixture.activity, fixture.bundle) - sut.onActivityPostResumed(fixture.activity) + val activity = mock() + sut.onActivityPreCreated(activity, fixture.bundle) + sut.onActivityPostResumed(activity) verify(fixture.hub, never()).captureTransaction(any()) } @@ -319,7 +338,8 @@ class ActivityLifecycleIntegrationTest { val sut = fixture.getSut() sut.register(fixture.hub, fixture.options) - sut.onActivityPostResumed(fixture.activity) + val activity = mock() + sut.onActivityPostResumed(activity) verify(fixture.hub, never()).captureTransaction(any()) } @@ -330,8 +350,9 @@ class ActivityLifecycleIntegrationTest { fixture.options.tracesSampleRate = 1.0 sut.register(fixture.hub, fixture.options) - sut.onActivityPreCreated(fixture.activity, fixture.bundle) - sut.onActivityDestroyed(fixture.activity) + val activity = mock() + sut.onActivityPreCreated(activity, fixture.bundle) + sut.onActivityDestroyed(activity) verify(fixture.hub).captureTransaction(any()) } @@ -342,7 +363,8 @@ class ActivityLifecycleIntegrationTest { fixture.options.tracesSampleRate = 1.0 sut.register(fixture.hub, fixture.options) - sut.onActivityPreCreated(fixture.activity, fixture.bundle) + val activity = mock() + sut.onActivityPreCreated(activity, fixture.bundle) assertFalse(sut.activitiesWithOngoingTransactions.isEmpty()) } @@ -353,8 +375,9 @@ class ActivityLifecycleIntegrationTest { fixture.options.tracesSampleRate = 1.0 sut.register(fixture.hub, fixture.options) - sut.onActivityPreCreated(fixture.activity, fixture.bundle) - sut.onActivityDestroyed(fixture.activity) + val activity = mock() + sut.onActivityPreCreated(activity, fixture.bundle) + sut.onActivityDestroyed(activity) assertTrue(sut.activitiesWithOngoingTransactions.isEmpty()) } @@ -365,10 +388,9 @@ class ActivityLifecycleIntegrationTest { fixture.options.tracesSampleRate = 1.0 sut.register(fixture.hub, fixture.options) - val activity = mock() - sut.onActivityPreCreated(activity, mock()) + sut.onActivityPreCreated(mock(), mock()) - sut.onActivityPreCreated(fixture.activity, fixture.bundle) + sut.onActivityPreCreated(mock(), fixture.bundle) verify(fixture.hub).captureTransaction(any()) } diff --git a/sentry/src/main/java/io/sentry/Span.java b/sentry/src/main/java/io/sentry/Span.java index 1d6e2baeb7..933de84d06 100644 --- a/sentry/src/main/java/io/sentry/Span.java +++ b/sentry/src/main/java/io/sentry/Span.java @@ -8,6 +8,7 @@ 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 { @@ -55,7 +56,8 @@ public final class Span implements ISpan { this.hub = Objects.requireNonNull(hub, "hub is required"); } - Span( + @VisibleForTesting + public Span( final @NotNull TransactionContext context, final @NotNull SentryTracer sentryTracer, final @NotNull IHub hub, From 30f0e00f1f5f74f5e48ba19643c49af7f2bd66c0 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 10 Jun 2021 11:48:28 +0200 Subject: [PATCH 12/21] fix --- .../core/ActivityLifecycleIntegration.java | 3 +- .../PerformanceAndroidEventProcessor.java | 42 +++++++++++-------- 2 files changed, 27 insertions(+), 18 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 545e38dff3..c6fe13adff 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 @@ -29,6 +29,7 @@ public final class ActivityLifecycleIntegration implements Integration, Closeable, Application.ActivityLifecycleCallbacks { private static final String NAV_OP = "navigation"; + static final String APP_START_OP = "app.start"; private final @NotNull Application application; private @Nullable IHub hub; @@ -139,7 +140,7 @@ private void startTracing(final @NonNull Activity activity) { // start specific span for app start // TODO: add description cold/warm or different operation? how do we break down // per app start cold/warm? - appStartSpan = transaction.startChild("app.start", appStartTime); + appStartSpan = transaction.startChild(APP_START_OP, appStartTime); } // lets bind to the scope so other integrations can pick it up diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java index 72910173b5..cabdcbf9f5 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java @@ -1,8 +1,12 @@ package io.sentry.android.core; +import static io.sentry.android.core.ActivityLifecycleIntegration.APP_START_OP; + import io.sentry.EventProcessor; import io.sentry.protocol.MeasurementValue; +import io.sentry.protocol.SentrySpan; import io.sentry.protocol.SentryTransaction; +import java.util.List; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -13,34 +17,38 @@ final class PerformanceAndroidEventProcessor implements EventProcessor { private boolean sentStartMeasurement = false; - // transactions may be started in parallel, locking it so sentStartMeasurement - private final @NotNull Object measurementLock = new Object(); - PerformanceAndroidEventProcessor(final @NotNull SentryAndroidOptions options) { tracingEnabled = options.isTracingEnabled(); } @Override - public @NotNull SentryTransaction process( + public synchronized @NotNull SentryTransaction process( @NotNull SentryTransaction transaction, @Nullable Object hint) { // the app start metric is sent only once when the 1st transaction happens // after the app start is collected. - synchronized (measurementLock) { - if (!sentStartMeasurement && tracingEnabled) { - final Long appStartUpInterval = AppStartState.getInstance().getAppStartInterval(); - // if appStartUpInterval is null, metrics are not ready to be sent - if (appStartUpInterval != null) { - final MeasurementValue value = new MeasurementValue((float) appStartUpInterval); - - final String appStartKey = - AppStartState.getInstance().isColdStart() ? "app_start_cold" : "app_start_warm"; - - transaction.getMeasurements().put(appStartKey, value); - sentStartMeasurement = true; - } + if (!sentStartMeasurement && tracingEnabled && hasAppStartSpan(transaction.getSpans())) { + final Long appStartUpInterval = AppStartState.getInstance().getAppStartInterval(); + // if appStartUpInterval is null, metrics are not ready to be sent + if (appStartUpInterval != null) { + final MeasurementValue value = new MeasurementValue((float) appStartUpInterval); + + final String appStartKey = + AppStartState.getInstance().isColdStart() ? "app_start_cold" : "app_start_warm"; + + transaction.getMeasurements().put(appStartKey, value); + sentStartMeasurement = true; } } return transaction; } + + private boolean hasAppStartSpan(final @NotNull List spans) { + for (final SentrySpan span : spans) { + if (span.getOp().contentEquals(APP_START_OP)) { + return true; + } + } + return false; + } } From b8296ef24cf37fa3d3dcb8c968ee0196f30c2a3b Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 10 Jun 2021 11:58:43 +0200 Subject: [PATCH 13/21] fix --- CHANGELOG.md | 2 +- .../src/main/java/io/sentry/android/core/AppStartState.java | 4 ++-- sentry/api/sentry.api | 1 + 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d6a9d5cd82..49c6bff068 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## Unreleased -* Feat: App start up metrics (#1487) +* Feat: Measure app start time (#1487) ## 5.0.1 diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java b/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java index 8e50c691c8..0d994d68a5 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java @@ -32,7 +32,7 @@ void resetInstance() { instance = new AppStartState(); } - void setAppStartEnd() { + synchronized void setAppStartEnd() { setAppStartEnd(SystemClock.uptimeMillis()); } @@ -53,7 +53,7 @@ boolean isColdStart() { return coldStart; } - void setColdStart(final boolean coldStart) { + synchronized void setColdStart(final boolean coldStart) { this.coldStart = coldStart; } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 92c96702ab..d6bb1d8325 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -1004,6 +1004,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;Ljava/util/Date;)V public fun finish ()V public fun finish (Lio/sentry/SpanStatus;)V public fun getDescription ()Ljava/lang/String; From 2b554bbf19335e813f6d07f18c21241e80c9070f Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 10 Jun 2021 12:41:09 +0200 Subject: [PATCH 14/21] fix --- .../PerformanceAndroidEventProcessorTest.kt | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt index 1cd8ba377f..15b83e565a 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt @@ -1,7 +1,10 @@ package io.sentry.android.core import com.nhaarman.mockitokotlin2.mock +import com.nhaarman.mockitokotlin2.whenever +import io.sentry.IHub import io.sentry.SentryTracer +import io.sentry.TransactionContext import io.sentry.protocol.SentryTransaction import java.util.Date import kotlin.test.BeforeTest @@ -13,10 +16,13 @@ class PerformanceAndroidEventProcessorTest { private class Fixture { val options = SentryAndroidOptions() - val tracer = SentryTracer(mock(), mock()) + val hub = mock() + val context = TransactionContext("name", "op") + val tracer = SentryTracer(context, hub) fun getSut(tracesSampleRate: Double? = 1.0): PerformanceAndroidEventProcessor { options.tracesSampleRate = tracesSampleRate + whenever(hub.options).thenReturn(options) return PerformanceAndroidEventProcessor(options) } } @@ -32,7 +38,7 @@ class PerformanceAndroidEventProcessorTest { fun `add cold start measurement`() { val sut = fixture.getSut() - var tr = SentryTransaction(fixture.tracer) + var tr = getTransaction() setAppStart() tr = sut.process(tr, null) @@ -44,7 +50,7 @@ class PerformanceAndroidEventProcessorTest { fun `add warm start measurement`() { val sut = fixture.getSut() - var tr = SentryTransaction(fixture.tracer) + var tr = getTransaction() setAppStart(false) tr = sut.process(tr, null) @@ -56,12 +62,12 @@ class PerformanceAndroidEventProcessorTest { fun `do not add app start metric twice`() { val sut = fixture.getSut() - var tr1 = SentryTransaction(fixture.tracer) + var tr1 = getTransaction() setAppStart(false) tr1 = sut.process(tr1, null) - var tr2 = SentryTransaction(fixture.tracer) + var tr2 = getTransaction() tr2 = sut.process(tr2, null) assertTrue(tr1.measurements.containsKey("app_start_warm")) @@ -72,7 +78,7 @@ class PerformanceAndroidEventProcessorTest { fun `do not add app start metric if its not ready`() { val sut = fixture.getSut() - var tr = SentryTransaction(fixture.tracer) + var tr = getTransaction() tr = sut.process(tr, null) @@ -83,7 +89,7 @@ class PerformanceAndroidEventProcessorTest { fun `do not add app start metric if performance is disabled`() { val sut = fixture.getSut(tracesSampleRate = null) - var tr = SentryTransaction(fixture.tracer) + var tr = getTransaction() tr = sut.process(tr, null) @@ -95,4 +101,9 @@ class PerformanceAndroidEventProcessorTest { AppStartState.getInstance().setAppStartTime(0, Date()) AppStartState.getInstance().setAppStartEnd() } + + private fun getTransaction(): SentryTransaction { + fixture.tracer.startChild("app.start") + return SentryTransaction(fixture.tracer) + } } From 82f9d2fb602ffa3b197bd03748d7c18c287a0350 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 10 Jun 2021 12:45:36 +0200 Subject: [PATCH 15/21] format --- .../core/ActivityLifecycleIntegrationTest.kt | 17 ++++++++++++++++- .../PerformanceAndroidEventProcessorTest.kt | 15 +++++++++++++-- 2 files changed, 29 insertions(+), 3 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 bbd3003996..b04ab3ee72 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 @@ -109,6 +109,7 @@ class ActivityLifecycleIntegrationTest { fun `When ActivityBreadcrumbsIntegration is closed, it should unregister the callback`() { val sut = fixture.getSut() sut.register(fixture.hub, fixture.options) + sut.close() verify(fixture.application).unregisterActivityLifecycleCallbacks(any()) @@ -121,6 +122,7 @@ class ActivityLifecycleIntegrationTest { val activity = mock() sut.onActivityCreated(activity, fixture.bundle) + verify(fixture.hub).addBreadcrumb(check { assertEquals("ui.lifecycle", it.category) assertEquals("navigation", it.type) @@ -133,6 +135,7 @@ class ActivityLifecycleIntegrationTest { fun `When activity is created, it should add a breadcrumb`() { val sut = fixture.getSut() sut.register(fixture.hub, fixture.options) + val activity = mock() sut.onActivityCreated(activity, fixture.bundle) @@ -143,6 +146,7 @@ class ActivityLifecycleIntegrationTest { fun `When activity is started, it should add a breadcrumb`() { val sut = fixture.getSut() sut.register(fixture.hub, fixture.options) + val activity = mock() sut.onActivityStarted(activity) @@ -153,8 +157,10 @@ class ActivityLifecycleIntegrationTest { fun `When activity is resumed, it should add a breadcrumb`() { val sut = fixture.getSut() sut.register(fixture.hub, fixture.options) + val activity = mock() sut.onActivityResumed(activity) + verify(fixture.hub).addBreadcrumb(any()) } @@ -162,8 +168,10 @@ class ActivityLifecycleIntegrationTest { fun `When activity is paused, it should add a breadcrumb`() { val sut = fixture.getSut() sut.register(fixture.hub, fixture.options) + val activity = mock() sut.onActivityPaused(activity) + verify(fixture.hub).addBreadcrumb(any()) } @@ -171,8 +179,10 @@ class ActivityLifecycleIntegrationTest { fun `When activity is stopped, it should add a breadcrumb`() { val sut = fixture.getSut() sut.register(fixture.hub, fixture.options) + val activity = mock() sut.onActivityStopped(activity) + verify(fixture.hub).addBreadcrumb(any()) } @@ -180,8 +190,10 @@ class ActivityLifecycleIntegrationTest { fun `When activity is save instance, it should add a breadcrumb`() { val sut = fixture.getSut() sut.register(fixture.hub, fixture.options) + val activity = mock() sut.onActivitySaveInstanceState(activity, fixture.bundle) + verify(fixture.hub).addBreadcrumb(any()) } @@ -189,8 +201,10 @@ class ActivityLifecycleIntegrationTest { fun `When activity is destroyed, it should add a breadcrumb`() { val sut = fixture.getSut() sut.register(fixture.hub, fixture.options) + val activity = mock() sut.onActivityDestroyed(activity) + verify(fixture.hub).addBreadcrumb(any()) } @@ -198,6 +212,7 @@ class ActivityLifecycleIntegrationTest { fun `When tracing is disabled, do not start tracing`() { val sut = fixture.getSut() sut.register(fixture.hub, fixture.options) + val activity = mock() sut.onActivityPreCreated(activity, fixture.bundle) @@ -275,7 +290,7 @@ class ActivityLifecycleIntegrationTest { whenever(fixture.hub.configureScope(any())).thenAnswer { val scope = Scope(fixture.options) val previousTransaction = SentryTracer(TransactionContext("name", "op"), fixture.hub) - scope.setTransaction(previousTransaction) + scope.transaction = previousTransaction sut.applyScope(scope, fixture.transaction) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt index 15b83e565a..a7784e8faf 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt @@ -96,14 +96,25 @@ class PerformanceAndroidEventProcessorTest { assertTrue(tr.measurements.isEmpty()) } + @Test + fun `do not add app start metric if no app_start span`() { + val sut = fixture.getSut(tracesSampleRate = null) + + var tr = getTransaction("task") + + tr = sut.process(tr, null) + + assertTrue(tr.measurements.isEmpty()) + } + private fun setAppStart(coldStart: Boolean = true) { AppStartState.getInstance().isColdStart = coldStart AppStartState.getInstance().setAppStartTime(0, Date()) AppStartState.getInstance().setAppStartEnd() } - private fun getTransaction(): SentryTransaction { - fixture.tracer.startChild("app.start") + private fun getTransaction(op: String = "app.start"): SentryTransaction { + fixture.tracer.startChild(op) return SentryTransaction(fixture.tracer) } } From a94e30fec5a80498f92a93c4006784a2e38cb772 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 10 Jun 2021 12:52:20 +0200 Subject: [PATCH 16/21] fix --- .../test/java/io/sentry/SentryTracerTest.kt | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/sentry/src/test/java/io/sentry/SentryTracerTest.kt b/sentry/src/test/java/io/sentry/SentryTracerTest.kt index 56a6f70b2d..e01c4544b9 100644 --- a/sentry/src/test/java/io/sentry/SentryTracerTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTracerTest.kt @@ -6,12 +6,14 @@ import com.nhaarman.mockitokotlin2.spy import com.nhaarman.mockitokotlin2.verify import io.sentry.protocol.App import io.sentry.protocol.Request +import java.util.Date import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertNotEquals import kotlin.test.assertNotNull import kotlin.test.assertNull +import kotlin.test.assertSame import kotlin.test.assertTrue class SentryTracerTest { @@ -26,9 +28,12 @@ class SentryTracerTest { hub.bindClient(mock()) } - fun getSut(optionsConfiguration: Sentry.OptionsConfiguration = Sentry.OptionsConfiguration {}): SentryTracer { + fun getSut( + optionsConfiguration: Sentry.OptionsConfiguration = Sentry.OptionsConfiguration {}, + startTimestamp: Date? = null + ): SentryTracer { optionsConfiguration.configure(options) - return SentryTracer(TransactionContext("name", "op"), hub) + return SentryTracer(TransactionContext("name", "op"), hub, startTimestamp) } } @@ -307,4 +312,19 @@ class SentryTracerTest { assertTrue(transaction.isFinished) } + + @Test + fun `when startTimestamp is given, use it as startTimestamp`() { + val date = Date(0) + val transaction = fixture.getSut(startTimestamp = date) + + assertSame(date, transaction.startTimestamp) + } + + @Test + fun `when startTimestamp is nullable, set it automatically`() { + val transaction = fixture.getSut(startTimestamp = null) + + assertNotNull(transaction.startTimestamp) + } } From 137db3302f0b7e1ea2d486c8ff03fbc5f1cdeab3 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 10 Jun 2021 13:29:09 +0200 Subject: [PATCH 17/21] tests --- .../core/ActivityLifecycleIntegrationTest.kt | 84 +++++++++++++++++-- 1 file changed, 77 insertions(+), 7 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 b04ab3ee72..c52e720ea0 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 @@ -5,8 +5,10 @@ import android.app.Application import android.os.Bundle import com.nhaarman.mockitokotlin2.any import com.nhaarman.mockitokotlin2.check +import com.nhaarman.mockitokotlin2.eq import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.never +import com.nhaarman.mockitokotlin2.times import com.nhaarman.mockitokotlin2.verify import com.nhaarman.mockitokotlin2.whenever import io.sentry.Breadcrumb @@ -23,6 +25,7 @@ import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertNotNull +import kotlin.test.assertSame import kotlin.test.assertTrue class ActivityLifecycleIntegrationTest { @@ -217,6 +220,7 @@ class ActivityLifecycleIntegrationTest { sut.onActivityPreCreated(activity, fixture.bundle) verify(fixture.hub, never()).startTransaction(any(), any()) + verify(fixture.hub, never()).startTransaction(any(), any(), any()) } @Test @@ -231,6 +235,7 @@ class ActivityLifecycleIntegrationTest { // call only once verify(fixture.hub).startTransaction(any(), any()) + verify(fixture.hub, never()).startTransaction(any(), any(), any()) } @Test @@ -239,12 +244,14 @@ class ActivityLifecycleIntegrationTest { fixture.options.tracesSampleRate = 1.0 sut.register(fixture.hub, fixture.options) + setAppStartTime() + val activity = mock() sut.onActivityPreCreated(activity, fixture.bundle) - verify(fixture.hub).startTransaction(any(), check { + verify(fixture.hub).startTransaction(any(), check { assertEquals("navigation", it) - }) + }, any()) } @Test @@ -253,12 +260,14 @@ class ActivityLifecycleIntegrationTest { fixture.options.tracesSampleRate = 1.0 sut.register(fixture.hub, fixture.options) + setAppStartTime() + val activity = mock() sut.onActivityPreCreated(activity, fixture.bundle) - verify(fixture.hub).startTransaction(check { + verify(fixture.hub).startTransaction(check { assertEquals("Activity", it) - }, any()) + }, any(), any()) } @Test @@ -419,6 +428,7 @@ class ActivityLifecycleIntegrationTest { sut.onActivityCreated(activity, mock()) verify(fixture.hub, never()).startTransaction(any(), any()) + verify(fixture.hub, never()).startTransaction(any(), any(), any()) } @Test @@ -440,10 +450,12 @@ class ActivityLifecycleIntegrationTest { fixture.options.tracesSampleRate = 1.0 sut.register(fixture.hub, fixture.options) + setAppStartTime() + val activity = mock() sut.onActivityCreated(activity, mock()) - verify(fixture.hub).startTransaction(any(), any()) + verify(fixture.hub).startTransaction(any(), any(), any()) } @Test @@ -504,8 +516,7 @@ class ActivityLifecycleIntegrationTest { fixture.options.tracesSampleRate = 1.0 sut.register(fixture.hub, fixture.options) - // set by SentryPerformanceProvider so forcing it here - AppStartState.getInstance().setAppStartTime(0, Date()) + setAppStartTime() val activity = mock() sut.onActivityCreated(activity, null) @@ -514,4 +525,63 @@ class ActivityLifecycleIntegrationTest { // SystemClock.uptimeMillis() always returns 0, can't assert real values assertNotNull(AppStartState.getInstance().appStartInterval) } + + @Test + fun `When firstActivityCreated is true, start transaction with given appStartTime`() { + val sut = fixture.getSut() + fixture.options.tracesSampleRate = 1.0 + sut.register(fixture.hub, fixture.options) + + val date = Date(0) + setAppStartTime(date) + + val activity = mock() + sut.onActivityPreCreated(activity, fixture.bundle) + + // call only once + verify(fixture.hub).startTransaction(any(), any(), eq(date)) + } + + @Test + fun `When firstActivityCreated is true, start app start span with given appStartTime`() { + val sut = fixture.getSut() + fixture.options.tracesSampleRate = 1.0 + sut.register(fixture.hub, fixture.options) + + val date = Date(0) + setAppStartTime(date) + + val activity = mock() + sut.onActivityPreCreated(activity, fixture.bundle) + + val span = fixture.transaction.children.first() + assertEquals(span.operation, "app.start") + assertSame(span.startTimestamp, date) + } + + @Test + fun `When firstActivityCreated is false, start transaction but not with given appStartTime`() { + val sut = fixture.getSut() + fixture.options.tracesSampleRate = 1.0 + sut.register(fixture.hub, fixture.options) + + setAppStartTime() + + val activity = mock() + sut.onActivityPreCreated(activity, fixture.bundle) + + verify(fixture.hub).startTransaction(any(), any(), any()) + sut.onActivityCreated(activity, fixture.bundle) + sut.onActivityPostResumed(activity) + + val newActivity = mock() + sut.onActivityPreCreated(newActivity, fixture.bundle) + + verify(fixture.hub).startTransaction(any(), any()) + } + + private fun setAppStartTime(date: Date = Date(0)) { + // set by SentryPerformanceProvider so forcing it here + AppStartState.getInstance().setAppStartTime(0, date) + } } From 51297b5c09420c7b237cadf0b4c994a326e24731 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 10 Jun 2021 13:36:57 +0200 Subject: [PATCH 18/21] ui load op for ui transactions --- .../sentry/android/core/ActivityLifecycleIntegration.java | 6 +++--- .../sentry/android/core/ActivityLifecycleIntegrationTest.kt | 5 ++--- 2 files changed, 5 insertions(+), 6 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 c6fe13adff..93ccf12283 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 @@ -28,7 +28,7 @@ public final class ActivityLifecycleIntegration implements Integration, Closeable, Application.ActivityLifecycleCallbacks { - private static final String NAV_OP = "navigation"; + private static final String UI_LOAD_OP = "ui.load"; static final String APP_START_OP = "app.start"; private final @NotNull Application application; @@ -133,10 +133,10 @@ private void startTracing(final @NonNull Activity activity) { // in case appStartTime isn't available, we don't create a span for it. if (firstActivityCreated || appStartTime == null) { - transaction = hub.startTransaction(activityName, NAV_OP); + transaction = hub.startTransaction(activityName, UI_LOAD_OP); } else { // start transaction with app start timestamp - transaction = hub.startTransaction(activityName, NAV_OP, appStartTime); + transaction = hub.startTransaction(activityName, UI_LOAD_OP, appStartTime); // start specific span for app start // TODO: add description cold/warm or different operation? how do we break down // per app start cold/warm? 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 c52e720ea0..3752ee413a 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 @@ -8,7 +8,6 @@ import com.nhaarman.mockitokotlin2.check import com.nhaarman.mockitokotlin2.eq import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.never -import com.nhaarman.mockitokotlin2.times import com.nhaarman.mockitokotlin2.verify import com.nhaarman.mockitokotlin2.whenever import io.sentry.Breadcrumb @@ -239,7 +238,7 @@ class ActivityLifecycleIntegrationTest { } @Test - fun `Transaction op is navigation`() { + fun `Transaction op is ui_load`() { val sut = fixture.getSut() fixture.options.tracesSampleRate = 1.0 sut.register(fixture.hub, fixture.options) @@ -250,7 +249,7 @@ class ActivityLifecycleIntegrationTest { sut.onActivityPreCreated(activity, fixture.bundle) verify(fixture.hub).startTransaction(any(), check { - assertEquals("navigation", it) + assertEquals("ui.load", it) }, any()) } From 65f8a0d0f34cd1bad242a071a3f31cf6b7ce1f2e Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 10 Jun 2021 14:22:22 +0200 Subject: [PATCH 19/21] fix --- .../java/io/sentry/android/core/AppStartState.java | 2 +- .../core/PerformanceAndroidEventProcessor.java | 4 ++-- .../android/core/SentryPerformanceProvider.java | 11 +++++++++-- .../android/core/SentryPerformanceProviderTest.kt | 12 +++++++++--- 4 files changed, 21 insertions(+), 8 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java b/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java index 0d994d68a5..49d00e2ee3 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java @@ -42,7 +42,7 @@ void setAppStartEnd(final long appStartEndMillis) { } @Nullable - Long getAppStartInterval() { + synchronized Long getAppStartInterval() { if (appStartMillis == null || appStartEndMillis == null) { return null; } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java index cabdcbf9f5..a51aa04c06 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java @@ -24,8 +24,8 @@ final class PerformanceAndroidEventProcessor implements EventProcessor { @Override public synchronized @NotNull SentryTransaction process( @NotNull SentryTransaction transaction, @Nullable Object hint) { - // the app start metric is sent only once when the 1st transaction happens - // after the app start is collected. + // the app start measurement is only sent once and only if the transaction has + // the app.start span, which is automatically created by the SDK. if (!sentStartMeasurement && tracingEnabled && hasAppStartSpan(transaction.getSpans())) { final Long appStartUpInterval = AppStartState.getInstance().getAppStartInterval(); // if appStartUpInterval is null, metrics are not ready to be sent diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java index 96648f158b..4fb7b32c8e 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java @@ -13,6 +13,7 @@ import java.util.Date; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.TestOnly; /** * SentryPerformanceProvider is responsible for collecting data (eg appStart) as early as possible @@ -23,9 +24,9 @@ public final class SentryPerformanceProvider extends ContentProvider { // static to rely on Class load - private static final @NotNull Date appStartTime = DateUtils.getCurrentDateTime(); + private static @NotNull Date appStartTime = DateUtils.getCurrentDateTime(); // SystemClock.uptimeMillis() isn't affected by phone provider or clock changes. - private static final long appStartMillis = SystemClock.uptimeMillis(); + private static long appStartMillis = SystemClock.uptimeMillis(); public SentryPerformanceProvider() { AppStartState.getInstance().setAppStartTime(appStartMillis, appStartTime); @@ -83,4 +84,10 @@ public int update( @Nullable String[] selectionArgs) { return 0; } + + @TestOnly + static void setAppStartTime(final long appStartMillisLong, final @NotNull Date appStartTimeDate) { + appStartMillis = appStartMillisLong; + appStartTime = appStartTimeDate; + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SentryPerformanceProviderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SentryPerformanceProviderTest.kt index 6aa2ca6ad4..5e7916e4f7 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SentryPerformanceProviderTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SentryPerformanceProviderTest.kt @@ -2,9 +2,10 @@ package io.sentry.android.core import android.content.pm.ProviderInfo import androidx.test.ext.junit.runners.AndroidJUnit4 +import java.util.Date import kotlin.test.BeforeTest import kotlin.test.Test -import kotlin.test.assertNotNull +import kotlin.test.assertEquals import org.junit.runner.RunWith @RunWith(AndroidJUnit4::class) @@ -22,13 +23,18 @@ class SentryPerformanceProviderTest { val mockContext = ContextUtilsTest.createMockContext() providerInfo.authority = AUTHORITY + val providerAppStartMillis = 10L + val providerAppStartTime = Date(0) + SentryPerformanceProvider.setAppStartTime(providerAppStartMillis, providerAppStartTime) + val provider = SentryPerformanceProvider() provider.attachInfo(mockContext, providerInfo) // done by ActivityLifecycleIntegration so forcing it here - AppStartState.getInstance().setAppStartEnd() + val lifecycleAppEndMillis = 20L + AppStartState.getInstance().setAppStartEnd(lifecycleAppEndMillis) - assertNotNull(AppStartState.getInstance().appStartInterval) + assertEquals(10L, AppStartState.getInstance().appStartInterval) } companion object { From b1a88c35c7bab1ec94d7d0f2e79c24cbf7c5d712 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 10 Jun 2021 15:14:08 +0200 Subject: [PATCH 20/21] fix --- .../core/ActivityLifecycleIntegration.java | 29 ++++++++++++---- .../core/ActivityLifecycleIntegrationTest.kt | 34 +++++++++++++++++++ sentry/api/sentry.api | 10 +++--- sentry/src/main/java/io/sentry/Hub.java | 2 +- .../src/main/java/io/sentry/HubAdapter.java | 2 +- sentry/src/main/java/io/sentry/IHub.java | 4 +-- sentry/src/main/java/io/sentry/ISpan.java | 3 +- sentry/src/main/java/io/sentry/NoOpHub.java | 2 +- sentry/src/main/java/io/sentry/NoOpSpan.java | 3 +- .../main/java/io/sentry/NoOpTransaction.java | 3 +- sentry/src/main/java/io/sentry/Sentry.java | 2 +- .../src/main/java/io/sentry/SentryTracer.java | 21 ++++++------ sentry/src/main/java/io/sentry/Span.java | 7 ++-- 13 files changed, 88 insertions(+), 34 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 93ccf12283..7c6084d61b 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 @@ -138,9 +138,8 @@ private void startTracing(final @NonNull Activity activity) { // start transaction with app start timestamp transaction = hub.startTransaction(activityName, UI_LOAD_OP, appStartTime); // start specific span for app start - // TODO: add description cold/warm or different operation? how do we break down - // per app start cold/warm? - appStartSpan = transaction.startChild(APP_START_OP, appStartTime); + + appStartSpan = transaction.startChild(APP_START_OP, getColdStartDesc(), appStartTime); } // lets bind to the scope so other integrations can pick it up @@ -200,6 +199,8 @@ public synchronized void onActivityPreCreated( // only executed if API >= 29 otherwise it happens on onActivityCreated if (isAllActivityCallbacksAvailable) { + setColdStart(savedInstanceState); + // if activity has global fields being init. and // they are slow, this won't count the whole fields/ctor initialization time, but only // when onCreate is actually called. @@ -210,10 +211,8 @@ public synchronized void onActivityPreCreated( @Override public synchronized void onActivityCreated( final @NonNull Activity activity, final @Nullable Bundle savedInstanceState) { - if (!firstActivityCreated && performanceEnabled) { - // if Activity has savedInstanceState then its a warm start - // https://developer.android.com/topic/performance/vitals/launch-time#warm - AppStartState.getInstance().setColdStart(savedInstanceState == null); + if (!isAllActivityCallbacksAvailable) { + setColdStart(savedInstanceState); } addBreadcrumb(activity, "created"); @@ -298,4 +297,20 @@ public synchronized void onActivityDestroyed(final @NonNull Activity activity) { WeakHashMap getActivitiesWithOngoingTransactions() { return activitiesWithOngoingTransactions; } + + private void setColdStart(final @Nullable Bundle savedInstanceState) { + if (!firstActivityCreated && performanceEnabled) { + // if Activity has savedInstanceState then its a warm start + // https://developer.android.com/topic/performance/vitals/launch-time#warm + AppStartState.getInstance().setColdStart(savedInstanceState == null); + } + } + + private @NotNull String getColdStartDesc() { + if (AppStartState.getInstance().isColdStart()) { + return "Cold Start"; + } else { + return "Warm Start"; + } + } } 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 3752ee413a..e5bfeda6db 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 @@ -558,6 +558,40 @@ class ActivityLifecycleIntegrationTest { assertSame(span.startTimestamp, date) } + @Test + fun `When firstActivityCreated is true, start app start span with Warm description`() { + val sut = fixture.getSut() + fixture.options.tracesSampleRate = 1.0 + sut.register(fixture.hub, fixture.options) + + val date = Date(0) + setAppStartTime(date) + + val activity = mock() + sut.onActivityPreCreated(activity, fixture.bundle) + + val span = fixture.transaction.children.first() + assertEquals(span.description, "Warm Start") + assertSame(span.startTimestamp, date) + } + + @Test + fun `When firstActivityCreated is true, start app start span with Cold description`() { + val sut = fixture.getSut() + fixture.options.tracesSampleRate = 1.0 + sut.register(fixture.hub, fixture.options) + + val date = Date(0) + setAppStartTime(date) + + val activity = mock() + sut.onActivityPreCreated(activity, null) + + val span = fixture.transaction.children.first() + assertEquals(span.description, "Cold Start") + assertSame(span.startTimestamp, date) + } + @Test fun `When firstActivityCreated is false, start transaction but not with given appStartTime`() { val sut = fixture.getSut() diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index d6bb1d8325..5a168ec1b5 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -304,7 +304,7 @@ public abstract interface class io/sentry/ISpan { public abstract fun setThrowable (Ljava/lang/Throwable;)V 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/util/Date;)Lio/sentry/ISpan; + public abstract fun startChild (Ljava/lang/String;Ljava/lang/String;Ljava/util/Date;)Lio/sentry/ISpan; public abstract fun toSentryTrace ()Lio/sentry/SentryTraceHeader; } @@ -412,7 +412,7 @@ public final class io/sentry/NoOpSpan : io/sentry/ISpan { 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/util/Date;)Lio/sentry/ISpan; + public fun startChild (Ljava/lang/String;Ljava/lang/String;Ljava/util/Date;)Lio/sentry/ISpan; public fun toSentryTrace ()Lio/sentry/SentryTraceHeader; } @@ -443,7 +443,7 @@ public final class io/sentry/NoOpTransaction : io/sentry/ITransaction { 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/util/Date;)Lio/sentry/ISpan; + public fun startChild (Ljava/lang/String;Ljava/lang/String;Ljava/util/Date;)Lio/sentry/ISpan; public fun toSentryTrace ()Lio/sentry/SentryTraceHeader; } @@ -951,7 +951,7 @@ public final class io/sentry/SentryTracer : io/sentry/ITransaction { 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/util/Date;)Lio/sentry/ISpan; + public fun startChild (Ljava/lang/String;Ljava/lang/String;Ljava/util/Date;)Lio/sentry/ISpan; public fun toSentryTrace ()Lio/sentry/SentryTraceHeader; } @@ -1028,7 +1028,7 @@ public final class io/sentry/Span : io/sentry/ISpan { 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/util/Date;)Lio/sentry/ISpan; + public fun startChild (Ljava/lang/String;Ljava/lang/String;Ljava/util/Date;)Lio/sentry/ISpan; public fun toSentryTrace ()Lio/sentry/SentryTraceHeader; } diff --git a/sentry/src/main/java/io/sentry/Hub.java b/sentry/src/main/java/io/sentry/Hub.java index 1276e7cb65..18f3cd1dd0 100644 --- a/sentry/src/main/java/io/sentry/Hub.java +++ b/sentry/src/main/java/io/sentry/Hub.java @@ -590,7 +590,7 @@ public void flush(long timeoutMillis) { @NotNull TransactionContext transactionContext, @Nullable CustomSamplingContext customSamplingContext, boolean bindToScope, - @NotNull Date startTimestamp) { + @Nullable Date startTimestamp) { return createTransaction( transactionContext, customSamplingContext, bindToScope, startTimestamp); } diff --git a/sentry/src/main/java/io/sentry/HubAdapter.java b/sentry/src/main/java/io/sentry/HubAdapter.java index cb56bd0eb5..c29f6f31eb 100644 --- a/sentry/src/main/java/io/sentry/HubAdapter.java +++ b/sentry/src/main/java/io/sentry/HubAdapter.java @@ -181,7 +181,7 @@ public void flush(long timeoutMillis) { @NotNull TransactionContext transactionContexts, @Nullable CustomSamplingContext customSamplingContext, boolean bindToScope, - @NotNull Date startTimestamp) { + @Nullable Date startTimestamp) { return Sentry.startTransaction( transactionContexts, customSamplingContext, bindToScope, startTimestamp); } diff --git a/sentry/src/main/java/io/sentry/IHub.java b/sentry/src/main/java/io/sentry/IHub.java index 0245554818..1a5523c3df 100644 --- a/sentry/src/main/java/io/sentry/IHub.java +++ b/sentry/src/main/java/io/sentry/IHub.java @@ -380,7 +380,7 @@ ITransaction startTransaction( @NotNull TransactionContext transactionContexts, @Nullable CustomSamplingContext customSamplingContext, boolean bindToScope, - @NotNull Date startTimestamp); + @Nullable Date startTimestamp); /** * Creates a Transaction and returns the instance. Based on the {@link @@ -398,7 +398,7 @@ ITransaction startTransaction( @ApiStatus.Internal default @NotNull ITransaction startTransaction( - final @NotNull String name, final @NotNull String operation, @NotNull Date startTimestamp) { + final @NotNull String name, final @NotNull String operation, @Nullable Date startTimestamp) { return startTransaction(new TransactionContext(name, operation), null, false, startTimestamp); } diff --git a/sentry/src/main/java/io/sentry/ISpan.java b/sentry/src/main/java/io/sentry/ISpan.java index 2bd5bf233a..07b8a180fe 100644 --- a/sentry/src/main/java/io/sentry/ISpan.java +++ b/sentry/src/main/java/io/sentry/ISpan.java @@ -18,7 +18,8 @@ public interface ISpan { @ApiStatus.Internal @NotNull - ISpan startChild(@NotNull String operation, @NotNull Date timestamp); + ISpan startChild( + @NotNull String operation, @Nullable String description, @Nullable Date timestamp); /** * 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 152f6b8c6b..11139f82aa 100644 --- a/sentry/src/main/java/io/sentry/NoOpHub.java +++ b/sentry/src/main/java/io/sentry/NoOpHub.java @@ -140,7 +140,7 @@ public void flush(long timeoutMillis) {} @NotNull TransactionContext transactionContexts, @Nullable CustomSamplingContext customSamplingContext, boolean bindToScope, - @NotNull Date startTimestamp) { + @Nullable Date startTimestamp) { return NoOpTransaction.getInstance(); } diff --git a/sentry/src/main/java/io/sentry/NoOpSpan.java b/sentry/src/main/java/io/sentry/NoOpSpan.java index c9c9871cea..ea58bc8ca7 100644 --- a/sentry/src/main/java/io/sentry/NoOpSpan.java +++ b/sentry/src/main/java/io/sentry/NoOpSpan.java @@ -21,7 +21,8 @@ public static NoOpSpan getInstance() { } @Override - public @NotNull ISpan startChild(@NotNull String operation, @NotNull Date timestamp) { + public @NotNull ISpan startChild( + @NotNull String operation, @Nullable String description, @Nullable Date timestamp) { return NoOpSpan.getInstance(); } diff --git a/sentry/src/main/java/io/sentry/NoOpTransaction.java b/sentry/src/main/java/io/sentry/NoOpTransaction.java index c0d015295a..3fff45748d 100644 --- a/sentry/src/main/java/io/sentry/NoOpTransaction.java +++ b/sentry/src/main/java/io/sentry/NoOpTransaction.java @@ -34,7 +34,8 @@ public void setName(@NotNull String name) {} } @Override - public @NotNull ISpan startChild(@NotNull String operation, @NotNull Date timestamp) { + public @NotNull ISpan startChild( + @NotNull String operation, @Nullable String description, @Nullable Date timestamp) { return NoOpSpan.getInstance(); } diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index 6a6ea14ee9..773e292d75 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -658,7 +658,7 @@ public static void endSession() { final @NotNull TransactionContext transactionContexts, final @Nullable CustomSamplingContext customSamplingContext, final boolean bindToScope, - final @NotNull Date startTimestamp) { + final @Nullable Date startTimestamp) { return getCurrentHub() .startTransaction(transactionContexts, customSamplingContext, bindToScope, startTimestamp); } diff --git a/sentry/src/main/java/io/sentry/SentryTracer.java b/sentry/src/main/java/io/sentry/SentryTracer.java index 7ec519829f..ea7ba8e336 100644 --- a/sentry/src/main/java/io/sentry/SentryTracer.java +++ b/sentry/src/main/java/io/sentry/SentryTracer.java @@ -72,8 +72,9 @@ ISpan startChild( ISpan startChild( final @NotNull SpanId parentSpanId, final @NotNull String operation, - final @NotNull Date timestamp) { - return createChild(parentSpanId, operation, timestamp); + final @Nullable String description, + final @Nullable Date timestamp) { + return createChild(parentSpanId, operation, description, timestamp); } /** @@ -84,18 +85,20 @@ ISpan startChild( */ @NotNull private ISpan createChild(final @NotNull SpanId parentSpanId, final @NotNull String operation) { - return createChild(parentSpanId, operation, null); + return createChild(parentSpanId, operation, null, null); } @NotNull private ISpan createChild( final @NotNull SpanId parentSpanId, final @NotNull String operation, + final @Nullable String description, @Nullable Date timestamp) { Objects.requireNonNull(parentSpanId, "parentSpanId is required"); Objects.requireNonNull(operation, "operation is required"); final Span span = new Span(root.getTraceId(), parentSpanId, this, operation, this.hub, timestamp); + span.setDescription(description); this.children.add(span); return span; } @@ -106,8 +109,9 @@ private ISpan createChild( } @Override - public @NotNull ISpan startChild(final @NotNull String operation, @NotNull Date timestamp) { - return createChild(operation, null, timestamp); + public @NotNull ISpan startChild( + final @NotNull String operation, @Nullable String description, @Nullable Date timestamp) { + return createChild(operation, description, timestamp); } @Override @@ -121,12 +125,7 @@ private ISpan createChild( final @Nullable String description, @Nullable Date timestamp) { if (children.size() < hub.getOptions().getMaxSpans()) { - // TODO: overloads would be better - if (timestamp != null) { - return root.startChild(operation, timestamp); - } else { - return root.startChild(operation, description); - } + return root.startChild(operation, description, timestamp); } else { hub.getOptions() .getLogger() diff --git a/sentry/src/main/java/io/sentry/Span.java b/sentry/src/main/java/io/sentry/Span.java index 933de84d06..a917116823 100644 --- a/sentry/src/main/java/io/sentry/Span.java +++ b/sentry/src/main/java/io/sentry/Span.java @@ -82,8 +82,11 @@ public Span( } @Override - public @NotNull ISpan startChild(final @NotNull String operation, final @NotNull Date timestamp) { - return transaction.startChild(context.getSpanId(), operation, timestamp); + public @NotNull ISpan startChild( + final @NotNull String operation, + final @Nullable String description, + final @Nullable Date timestamp) { + return transaction.startChild(context.getSpanId(), operation, description, timestamp); } @Override From c43cb3e263232764fb639b2e354612f548a465b7 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Fri, 11 Jun 2021 08:55:17 +0200 Subject: [PATCH 21/21] rename span ops --- .../core/ActivityLifecycleIntegration.java | 15 ++++++++++--- .../PerformanceAndroidEventProcessor.java | 6 ++++-- .../core/ActivityLifecycleIntegrationTest.kt | 21 +++++++++++++++++-- .../PerformanceAndroidEventProcessorTest.kt | 4 ++-- 4 files changed, 37 insertions(+), 9 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 7c6084d61b..85ef0153dd 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 @@ -29,7 +29,8 @@ public final class ActivityLifecycleIntegration implements Integration, Closeable, Application.ActivityLifecycleCallbacks { private static final String UI_LOAD_OP = "ui.load"; - static final String APP_START_OP = "app.start"; + static final String APP_START_WARM = "app.start.warm"; + static final String APP_START_COLD = "app.start.cold"; private final @NotNull Application application; private @Nullable IHub hub; @@ -139,7 +140,7 @@ private void startTracing(final @NonNull Activity activity) { transaction = hub.startTransaction(activityName, UI_LOAD_OP, appStartTime); // start specific span for app start - appStartSpan = transaction.startChild(APP_START_OP, getColdStartDesc(), appStartTime); + appStartSpan = transaction.startChild(getAppStartOp(), getAppStartDesc(), appStartTime); } // lets bind to the scope so other integrations can pick it up @@ -306,11 +307,19 @@ private void setColdStart(final @Nullable Bundle savedInstanceState) { } } - private @NotNull String getColdStartDesc() { + private @NotNull String getAppStartDesc() { if (AppStartState.getInstance().isColdStart()) { return "Cold Start"; } else { return "Warm Start"; } } + + private @NotNull String getAppStartOp() { + if (AppStartState.getInstance().isColdStart()) { + return APP_START_COLD; + } else { + return APP_START_WARM; + } + } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java index a51aa04c06..861691d3af 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java @@ -1,6 +1,7 @@ package io.sentry.android.core; -import static io.sentry.android.core.ActivityLifecycleIntegration.APP_START_OP; +import static io.sentry.android.core.ActivityLifecycleIntegration.APP_START_COLD; +import static io.sentry.android.core.ActivityLifecycleIntegration.APP_START_WARM; import io.sentry.EventProcessor; import io.sentry.protocol.MeasurementValue; @@ -45,7 +46,8 @@ final class PerformanceAndroidEventProcessor implements EventProcessor { private boolean hasAppStartSpan(final @NotNull List spans) { for (final SentrySpan span : spans) { - if (span.getOp().contentEquals(APP_START_OP)) { + if (span.getOp().contentEquals(APP_START_COLD) + || span.getOp().contentEquals(APP_START_WARM)) { return true; } } 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 e5bfeda6db..013830bc72 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 @@ -542,7 +542,7 @@ class ActivityLifecycleIntegrationTest { } @Test - fun `When firstActivityCreated is true, start app start span with given appStartTime`() { + fun `When firstActivityCreated is true, start app start warm span with given appStartTime`() { val sut = fixture.getSut() fixture.options.tracesSampleRate = 1.0 sut.register(fixture.hub, fixture.options) @@ -554,7 +554,24 @@ class ActivityLifecycleIntegrationTest { sut.onActivityPreCreated(activity, fixture.bundle) val span = fixture.transaction.children.first() - assertEquals(span.operation, "app.start") + assertEquals(span.operation, "app.start.warm") + assertSame(span.startTimestamp, date) + } + + @Test + fun `When firstActivityCreated is true, start app start cold span with given appStartTime`() { + val sut = fixture.getSut() + fixture.options.tracesSampleRate = 1.0 + sut.register(fixture.hub, fixture.options) + + val date = Date(0) + setAppStartTime(date) + + val activity = mock() + sut.onActivityPreCreated(activity, null) + + val span = fixture.transaction.children.first() + assertEquals(span.operation, "app.start.cold") assertSame(span.startTimestamp, date) } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt index a7784e8faf..891d53d7b7 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt @@ -50,7 +50,7 @@ class PerformanceAndroidEventProcessorTest { fun `add warm start measurement`() { val sut = fixture.getSut() - var tr = getTransaction() + var tr = getTransaction("app.start.warm") setAppStart(false) tr = sut.process(tr, null) @@ -113,7 +113,7 @@ class PerformanceAndroidEventProcessorTest { AppStartState.getInstance().setAppStartEnd() } - private fun getTransaction(op: String = "app.start"): SentryTransaction { + private fun getTransaction(op: String = "app.start.cold"): SentryTransaction { fixture.tracer.startChild(op) return SentryTransaction(fixture.tracer) }