From c9c50a57a22138fde711b10e6530f68977ad5081 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Mon, 7 Nov 2022 16:39:55 +0100 Subject: [PATCH 01/20] added ProfileMeasurements to AndroidTransactionProfiler and ProfilingTraceData added frameMetrics (slow and frozen frames) and screen refresh rate added a SentryFrameMetricsCollector to collect frameMetrics changed a benchmark device on Saucelabs as it's not available anymore --- .sauce/sentry-uitest-android-benchmark.yml | 2 +- .../api/sentry-android-core.api | 23 ++ .../core/AndroidOptionsInitializer.java | 4 +- .../core/AndroidTransactionProfiler.java | 122 +++++++-- .../core/SentryFrameMetricsCollector.java | 206 ++++++++++++++++ .../core/AndroidTransactionProfilerTest.kt | 62 ++++- .../SentryFrameMetricsCollectorTest.kt | 233 ++++++++++++++++++ .../io/sentry/uitest/android/BaseUiTest.kt | 4 + .../io/sentry/uitest/android/EnvelopeTests.kt | 26 +- sentry/api/sentry.api | 61 ++++- .../main/java/io/sentry/JsonSerializer.java | 7 + .../java/io/sentry/ProfilingTraceData.java | 39 ++- .../ProfileMeasurement.java | 150 +++++++++++ .../ProfileMeasurementValue.java | 120 +++++++++ .../test/java/io/sentry/JsonSerializerTest.kt | 137 +++++++++- 15 files changed, 1154 insertions(+), 42 deletions(-) create mode 100644 sentry-android-core/src/main/java/io/sentry/android/core/SentryFrameMetricsCollector.java create mode 100644 sentry-android-core/src/test/java/io/sentry/android/core/internal/SentryFrameMetricsCollectorTest.kt create mode 100644 sentry/src/main/java/io/sentry/profilemeasurements/ProfileMeasurement.java create mode 100644 sentry/src/main/java/io/sentry/profilemeasurements/ProfileMeasurementValue.java diff --git a/.sauce/sentry-uitest-android-benchmark.yml b/.sauce/sentry-uitest-android-benchmark.yml index dfa0174b80..d1e8ac4db2 100644 --- a/.sauce/sentry-uitest-android-benchmark.yml +++ b/.sauce/sentry-uitest-android-benchmark.yml @@ -32,7 +32,7 @@ suites: - name: "Android 10 (api 29)" devices: - - id: OnePlus_7_Pro_real # OnePlus 7 Pro - api 29 (10) + - id: OnePlus_7T_real_us # OnePlus 7T - api 29 (10) - id: Nokia_7_1_real_us # Nokia 7.1 - api 29 (10) # At the time of writing (July, 4, 2022), the market share per android version is: diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 4bdcfecd27..cf12bcf0c1 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -164,6 +164,29 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr public fun setProfilingTracesIntervalMillis (I)V } +public final class io/sentry/android/core/SentryFrameMetricsCollector : android/app/Application$ActivityLifecycleCallbacks { + public fun (Landroid/content/Context;Lio/sentry/SentryOptions;Lio/sentry/android/core/BuildInfoProvider;)V + public fun (Landroid/content/Context;Lio/sentry/SentryOptions;Lio/sentry/android/core/BuildInfoProvider;Lio/sentry/android/core/SentryFrameMetricsCollector$WindowFrameMetricsManager;)V + public fun onActivityCreated (Landroid/app/Activity;Landroid/os/Bundle;)V + public fun onActivityDestroyed (Landroid/app/Activity;)V + public fun onActivityPaused (Landroid/app/Activity;)V + public fun onActivityResumed (Landroid/app/Activity;)V + public fun onActivitySaveInstanceState (Landroid/app/Activity;Landroid/os/Bundle;)V + public fun onActivityStarted (Landroid/app/Activity;)V + public fun onActivityStopped (Landroid/app/Activity;)V + public fun startCollection (Lio/sentry/android/core/SentryFrameMetricsCollector$FrameMetricsCollectorListener;)Ljava/lang/String; + public fun stopCollection (Ljava/lang/String;)V +} + +public abstract interface class io/sentry/android/core/SentryFrameMetricsCollector$FrameMetricsCollectorListener { + public abstract fun onFrameMetricCollected (Landroid/view/FrameMetrics;F)V +} + +public abstract interface class io/sentry/android/core/SentryFrameMetricsCollector$WindowFrameMetricsManager { + public fun addOnFrameMetricsAvailableListener (Landroid/view/Window;Landroid/view/Window$OnFrameMetricsAvailableListener;Landroid/os/Handler;)V + public fun removeOnFrameMetricsAvailableListener (Landroid/view/Window;Landroid/view/Window$OnFrameMetricsAvailableListener;)V +} + public final class io/sentry/android/core/SentryInitProvider : android/content/ContentProvider { public fun ()V public fun attachInfo (Landroid/content/Context;Landroid/content/pm/ProviderInfo;)V 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 0ecbbb9067..5e441d19cd 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 @@ -154,8 +154,10 @@ static void init( options.addEventProcessor(new PerformanceAndroidEventProcessor(options, activityFramesTracker)); options.setTransportGate(new AndroidTransportGate(context, options.getLogger())); + SentryFrameMetricsCollector frameMetricsCollector = + new SentryFrameMetricsCollector(context, options, buildInfoProvider); options.setTransactionProfiler( - new AndroidTransactionProfiler(context, options, buildInfoProvider)); + new AndroidTransactionProfiler(context, options, buildInfoProvider, frameMetricsCollector)); options.setModulesLoader(new AssetsModulesLoader(context, options.getLogger())); } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java index 6b3dc74baa..7f3ee8152a 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java @@ -11,6 +11,7 @@ import android.os.Debug; import android.os.Process; import android.os.SystemClock; +import android.view.FrameMetrics; import io.sentry.HubAdapter; import io.sentry.IHub; import io.sentry.ITransaction; @@ -21,14 +22,18 @@ import io.sentry.SentryLevel; import io.sentry.android.core.internal.util.CpuInfoUtils; import io.sentry.exception.SentryEnvelopeException; +import io.sentry.profilemeasurements.ProfileMeasurement; +import io.sentry.profilemeasurements.ProfileMeasurementValue; import io.sentry.util.Objects; import java.io.File; +import java.util.ArrayDeque; import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.UUID; import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -60,23 +65,41 @@ final class AndroidTransactionProfiler implements ITransactionProfiler { private long profileStartCpuMillis = 0; private boolean isInitialized = false; private int transactionsCounter = 0; + private @Nullable String frameMetricsCollectorId; + private final @NotNull SentryFrameMetricsCollector frameMetricsCollector; private final @NotNull Map transactionMap = new HashMap<>(); + private final @NotNull ArrayDeque screenFrameRateMeasurements = + new ArrayDeque<>(); + private final @NotNull ArrayDeque + adverseFrameRenderTimestampMeasurements = new ArrayDeque<>(); + private final @NotNull ArrayDeque + adverseFrozenFrameTimestampMeasurements = new ArrayDeque<>(); + private final @NotNull Map measurementsMap = new HashMap<>(); public AndroidTransactionProfiler( final @NotNull Context context, final @NotNull SentryAndroidOptions sentryAndroidOptions, - final @NotNull BuildInfoProvider buildInfoProvider) { - this(context, sentryAndroidOptions, buildInfoProvider, HubAdapter.getInstance()); + final @NotNull BuildInfoProvider buildInfoProvider, + final @NotNull SentryFrameMetricsCollector frameMetricsCollector) { + this( + context, + sentryAndroidOptions, + buildInfoProvider, + frameMetricsCollector, + HubAdapter.getInstance()); } public AndroidTransactionProfiler( final @NotNull Context context, final @NotNull SentryAndroidOptions sentryAndroidOptions, final @NotNull BuildInfoProvider buildInfoProvider, + final @NotNull SentryFrameMetricsCollector frameMetricsCollector, final @NotNull IHub hub) { this.context = Objects.requireNonNull(context, "The application context is required"); this.options = Objects.requireNonNull(sentryAndroidOptions, "SentryAndroidOptions is required"); this.hub = Objects.requireNonNull(hub, "Hub is required"); + this.frameMetricsCollector = + Objects.requireNonNull(frameMetricsCollector, "SentryFrameMetricsCollector is required"); this.buildInfoProvider = Objects.requireNonNull(buildInfoProvider, "The BuildInfoProvider is required."); this.packageInfo = ContextUtils.getPackageInfo(context, options.getLogger(), buildInfoProvider); @@ -144,21 +167,7 @@ public synchronized void onTransactionStart(@NotNull ITransaction transaction) { transactionsCounter--; return; } - - // We stop profiling after a timeout to avoid huge profiles to be sent - scheduledFinish = - options - .getExecutorService() - .schedule(() -> onTransactionFinish(transaction, true), PROFILING_TIMEOUT_MILLIS); - - transactionStartNanos = SystemClock.elapsedRealtimeNanos(); - profileStartCpuMillis = Process.getElapsedCpuTime(); - - ProfilingTransactionData transactionData = - new ProfilingTransactionData(transaction, transactionStartNanos, profileStartCpuMillis); - transactionMap.put(transaction.getEventId().toString(), transactionData); - - Debug.startMethodTracingSampling(traceFile.getPath(), BUFFER_SIZE_BYTES, intervalUs); + onFirstTransactionStarted(transaction, traceFile); } else { ProfilingTransactionData transactionData = new ProfilingTransactionData( @@ -175,6 +184,64 @@ public synchronized void onTransactionStart(@NotNull ITransaction transaction) { transactionsCounter); } + @SuppressLint("NewApi") + private void onFirstTransactionStarted( + @NotNull ITransaction transaction, @NotNull File traceFile) { + + measurementsMap.clear(); + screenFrameRateMeasurements.clear(); + adverseFrameRenderTimestampMeasurements.clear(); + adverseFrozenFrameTimestampMeasurements.clear(); + + frameMetricsCollectorId = + frameMetricsCollector.startCollection( + new SentryFrameMetricsCollector.FrameMetricsCollectorListener() { + final long nanosInSecond = TimeUnit.SECONDS.toNanos(1); + final long frozenFrameThresholdNanos = TimeUnit.MILLISECONDS.toNanos(700); + float lastRefreshRate = 0; + + @Override + public void onFrameMetricCollected( + @NotNull FrameMetrics frameMetrics, float refreshRate) { + long frameTimestampRelativeNanos = + SystemClock.elapsedRealtimeNanos() - transactionStartNanos; + long durationNanos = frameMetrics.getMetric(FrameMetrics.TOTAL_DURATION); + // Most frames take just a few nanoseconds longer than the optimal calculated + // duration. + // Therefore we subtract one, because otherwise almost all frames would be slow. + boolean isSlow = durationNanos > nanosInSecond / (refreshRate - 1); + float newRefreshRate = (int) (refreshRate * 100) / 100F; + if (durationNanos > frozenFrameThresholdNanos) { + adverseFrozenFrameTimestampMeasurements.addLast( + new ProfileMeasurementValue(frameTimestampRelativeNanos, durationNanos)); + } else if (isSlow) { + adverseFrameRenderTimestampMeasurements.addLast( + new ProfileMeasurementValue(frameTimestampRelativeNanos, durationNanos)); + } + if (newRefreshRate != lastRefreshRate) { + lastRefreshRate = newRefreshRate; + screenFrameRateMeasurements.addLast( + new ProfileMeasurementValue(frameTimestampRelativeNanos, newRefreshRate)); + } + } + }); + + // We stop profiling after a timeout to avoid huge profiles to be sent + scheduledFinish = + options + .getExecutorService() + .schedule(() -> onTransactionFinish(transaction, true), PROFILING_TIMEOUT_MILLIS); + + transactionStartNanos = SystemClock.elapsedRealtimeNanos(); + profileStartCpuMillis = Process.getElapsedCpuTime(); + + ProfilingTransactionData transactionData = + new ProfilingTransactionData(transaction, transactionStartNanos, profileStartCpuMillis); + transactionMap.put(transaction.getEventId().toString(), transactionData); + + Debug.startMethodTracingSampling(traceFile.getPath(), BUFFER_SIZE_BYTES, intervalUs); + } + @Override public synchronized void onTransactionFinish(@NotNull ITransaction transaction) { onTransactionFinish(transaction, false); @@ -226,8 +293,14 @@ private synchronized void onTransactionFinish( } return; } + onLastTransactionFinished(transaction, isTimeout); + } + @SuppressLint("NewApi") + private void onLastTransactionFinished(ITransaction transaction, boolean isTimeout) { Debug.stopMethodTracing(); + frameMetricsCollector.stopCollection(frameMetricsCollectorId); + long transactionEndNanos = SystemClock.elapsedRealtimeNanos(); long transactionEndCpuMillis = Process.getElapsedCpuTime(); long transactionDurationNanos = transactionEndNanos - transactionStartNanos; @@ -270,6 +343,18 @@ private synchronized void onTransactionFinish( profileStartCpuMillis); } + measurementsMap.put( + ProfileMeasurement.ID_SLOW_FRAME_RENDERS, + new ProfileMeasurement( + ProfileMeasurement.UNIT_NANOSECONDS, adverseFrameRenderTimestampMeasurements)); + measurementsMap.put( + ProfileMeasurement.ID_FROZEN_FRAME_RENDERS, + new ProfileMeasurement( + ProfileMeasurement.UNIT_NANOSECONDS, adverseFrozenFrameTimestampMeasurements)); + measurementsMap.put( + ProfileMeasurement.ID_SCREEN_FRAME_RATES, + new ProfileMeasurement(ProfileMeasurement.UNIT_HZ, screenFrameRateMeasurements)); + // cpu max frequencies are read with a lambda because reading files is involved, so it will be // done in the background when the trace file is read ProfilingTraceData profilingTraceData = @@ -292,7 +377,8 @@ private synchronized void onTransactionFinish( options.getEnvironment(), isTimeout ? ProfilingTraceData.TRUNCATION_REASON_TIMEOUT - : ProfilingTraceData.TRUNCATION_REASON_NORMAL); + : ProfilingTraceData.TRUNCATION_REASON_NORMAL, + measurementsMap); SentryEnvelope envelope; try { diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryFrameMetricsCollector.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryFrameMetricsCollector.java new file mode 100644 index 0000000000..736bbc4768 --- /dev/null +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryFrameMetricsCollector.java @@ -0,0 +1,206 @@ +package io.sentry.android.core; + +import android.annotation.SuppressLint; +import android.app.Activity; +import android.app.Application; +import android.content.Context; +import android.os.Build; +import android.os.Bundle; +import android.os.Handler; +import android.os.HandlerThread; +import android.view.FrameMetrics; +import android.view.Window; +import androidx.annotation.NonNull; +import androidx.annotation.RequiresApi; +import io.sentry.SentryLevel; +import io.sentry.SentryOptions; +import io.sentry.util.Objects; +import java.lang.ref.WeakReference; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Set; +import java.util.UUID; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +@ApiStatus.Internal +public final class SentryFrameMetricsCollector implements Application.ActivityLifecycleCallbacks { + private final @NotNull BuildInfoProvider buildInfoProvider; + private final @NotNull Set trackedWindows = new HashSet<>(); + private @Nullable Handler handler; + private @Nullable WeakReference currentWindow; + private final @NotNull HashMap listenerMap = + new HashMap<>(); + private boolean isAvailable = false; + private final WindowFrameMetricsManager windowFrameMetricsManager; + + private @Nullable Window.OnFrameMetricsAvailableListener frameMetricsAvailableListener; + + @SuppressWarnings("deprecation") + @SuppressLint("NewApi") + public SentryFrameMetricsCollector( + @NotNull Context context, + @NotNull SentryOptions options, + @NotNull BuildInfoProvider buildInfoProvider) { + this(context, options, buildInfoProvider, new WindowFrameMetricsManager() {}); + } + + @SuppressWarnings("deprecation") + @SuppressLint("NewApi") + public SentryFrameMetricsCollector( + @NotNull Context context, + @NotNull SentryOptions options, + @NotNull BuildInfoProvider buildInfoProvider, + @NotNull WindowFrameMetricsManager windowFrameMetricsManager) { + Objects.requireNonNull(context, "The context is required"); + Objects.requireNonNull(options, "SentryOptions is required"); + this.buildInfoProvider = + Objects.requireNonNull(buildInfoProvider, "BuildInfoProvider is required"); + this.windowFrameMetricsManager = + Objects.requireNonNull(windowFrameMetricsManager, "WindowFrameMetricsManager is required"); + + // registerActivityLifecycleCallbacks is only available if Context is an AppContext + if (!(context instanceof Application)) { + return; + } + // FrameMetrics api is only available since sdk version N + if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.N) { + return; + } + isAvailable = true; + + HandlerThread handlerThread = + new HandlerThread("io.sentry.android.core.SentryFrameMetricsCollector"); + handlerThread.setUncaughtExceptionHandler( + (thread, e) -> + options.getLogger().log(SentryLevel.ERROR, "Error during frames measurements.", e)); + handlerThread.start(); + handler = new Handler(handlerThread.getLooper()); + + // We have to register the lifecycle callback, even if no profile is started, otherwise when we + // start a profile, we wouldn't have the current activity and couldn't get the frameMetrics. + ((Application) context).registerActivityLifecycleCallbacks(this); + + frameMetricsAvailableListener = + (window, frameMetrics, dropCountSinceLastInvocation) -> { + float refreshRate = + buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.R + ? window.getContext().getDisplay().getRefreshRate() + : window.getWindowManager().getDefaultDisplay().getRefreshRate(); + for (FrameMetricsCollectorListener l : listenerMap.values()) { + l.onFrameMetricCollected(frameMetrics, refreshRate); + } + }; + } + + @Override + public void onActivityCreated(@NonNull Activity activity, @Nullable Bundle savedInstanceState) {} + + @Override + public void onActivityStarted(@NonNull Activity activity) { + setCurrentWindow(activity.getWindow()); + } + + @Override + public void onActivityResumed(@NonNull Activity activity) {} + + @Override + public void onActivityPaused(@NonNull Activity activity) {} + + @Override + public void onActivityStopped(@NonNull Activity activity) { + cleanCurrentWindow(activity.getWindow()); + } + + @Override + public void onActivitySaveInstanceState(@NonNull Activity activity, @NonNull Bundle outState) {} + + @Override + public void onActivityDestroyed(@NonNull Activity activity) {} + + public @Nullable String startCollection(FrameMetricsCollectorListener listener) { + if (!isAvailable) { + return null; + } + final String uid = UUID.randomUUID().toString(); + listenerMap.put(uid, listener); + trackCurrentWindow(); + return uid; + } + + public void stopCollection(@Nullable String listenerId) { + if (!isAvailable) { + return; + } + if (listenerId != null) { + listenerMap.remove(listenerId); + } + Window window = currentWindow != null ? currentWindow.get() : null; + if (window != null && listenerMap.isEmpty()) { + cleanCurrentWindow(window); + } + } + + @SuppressLint("NewApi") + private void cleanCurrentWindow(@NonNull Window window) { + if (currentWindow != null && currentWindow.get() == window) { + currentWindow = null; + } + if (trackedWindows.contains(window)) { + if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.N) { + windowFrameMetricsManager.removeOnFrameMetricsAvailableListener( + window, frameMetricsAvailableListener); + } + trackedWindows.remove(window); + } + } + + private void setCurrentWindow(@NotNull Window window) { + if (currentWindow != null && currentWindow.get() == window) { + return; + } + currentWindow = new WeakReference<>(window); + trackCurrentWindow(); + } + + @SuppressLint("NewApi") + private void trackCurrentWindow() { + Window window = currentWindow != null ? currentWindow.get() : null; + if (window == null || !isAvailable) { + return; + } + + if (!trackedWindows.contains(window) && !listenerMap.isEmpty()) { + + if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.N && handler != null) { + trackedWindows.add(window); + windowFrameMetricsManager.addOnFrameMetricsAvailableListener( + window, frameMetricsAvailableListener, handler); + } + } + } + + @ApiStatus.Internal + public interface FrameMetricsCollectorListener { + void onFrameMetricCollected(@NotNull FrameMetrics frameMetrics, float refreshRate); + } + + @ApiStatus.Internal + public interface WindowFrameMetricsManager { + @RequiresApi(api = Build.VERSION_CODES.N) + default void addOnFrameMetricsAvailableListener( + @NotNull Window window, + @Nullable Window.OnFrameMetricsAvailableListener frameMetricsAvailableListener, + @Nullable Handler handler) { + window.addOnFrameMetricsAvailableListener(frameMetricsAvailableListener, handler); + } + + @RequiresApi(api = Build.VERSION_CODES.N) + default void removeOnFrameMetricsAvailableListener( + @NotNull Window window, + @Nullable Window.OnFrameMetricsAvailableListener frameMetricsAvailableListener) { + window.removeOnFrameMetricsAvailableListener(frameMetricsAvailableListener); + } + } +} diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt index bbdc8dc525..309e06035a 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt @@ -12,6 +12,7 @@ import io.sentry.SentryLevel import io.sentry.SentryTracer import io.sentry.TransactionContext import io.sentry.assertEnvelopeItem +import io.sentry.profilemeasurements.ProfileMeasurement import io.sentry.test.getCtor import org.junit.runner.RunWith import org.mockito.kotlin.any @@ -37,7 +38,7 @@ class AndroidTransactionProfilerTest { private lateinit var context: Context private val className = "io.sentry.android.core.AndroidTransactionProfiler" - private val ctorTypes = arrayOf(Context::class.java, SentryAndroidOptions::class.java, BuildInfoProvider::class.java) + private val ctorTypes = arrayOf(Context::class.java, SentryAndroidOptions::class.java, BuildInfoProvider::class.java, SentryFrameMetricsCollector::class.java) private val fixture = Fixture() private class Fixture { @@ -54,6 +55,7 @@ class AndroidTransactionProfilerTest { } val hub: IHub = mock() + val frameMetricsCollector: SentryFrameMetricsCollector = mock() lateinit var transaction1: SentryTracer lateinit var transaction2: SentryTracer @@ -64,7 +66,7 @@ class AndroidTransactionProfilerTest { transaction1 = SentryTracer(TransactionContext("", ""), hub) transaction2 = SentryTracer(TransactionContext("", ""), hub) transaction3 = SentryTracer(TransactionContext("", ""), hub) - return AndroidTransactionProfiler(context, options, buildInfoProvider, hub) + return AndroidTransactionProfiler(context, options, buildInfoProvider, frameMetricsCollector, hub) } } @@ -90,10 +92,13 @@ class AndroidTransactionProfilerTest { ctor.newInstance(arrayOf(null, mock(), mock())) } assertFailsWith { - ctor.newInstance(arrayOf(mock(), null, mock())) + ctor.newInstance(arrayOf(mock(), null, mock(), mock())) } assertFailsWith { - ctor.newInstance(arrayOf(mock(), mock(), null)) + ctor.newInstance(arrayOf(mock(), mock(), null, mock())) + } + assertFailsWith { + ctor.newInstance(arrayOf(mock(), mock(), mock(), null)) } } @@ -328,4 +333,53 @@ class AndroidTransactionProfilerTest { } ) } + + @Test + fun `profiler starts collecting frame metrics when the first transaction starts`() { + val profiler = fixture.getSut(context) + profiler.onTransactionStart(fixture.transaction1) + verify(fixture.frameMetricsCollector, times(1)).startCollection(any()) + profiler.onTransactionStart(fixture.transaction2) + verify(fixture.frameMetricsCollector, times(1)).startCollection(any()) + } + + @Test + fun `profiler stops collecting frame metrics when the last transaction finishes`() { + val profiler = fixture.getSut(context) + val frameMetricsCollectorId = "id" + whenever(fixture.frameMetricsCollector.startCollection(any())).thenReturn(frameMetricsCollectorId) + profiler.onTransactionStart(fixture.transaction1) + profiler.onTransactionStart(fixture.transaction2) + profiler.onTransactionFinish(fixture.transaction1) + verify(fixture.frameMetricsCollector, never()).stopCollection(frameMetricsCollectorId) + profiler.onTransactionFinish(fixture.transaction2) + verify(fixture.frameMetricsCollector).stopCollection(frameMetricsCollectorId) + } + + @Test + fun `profiler includes measurements in envelope sent`() { + val profiler = fixture.getSut(context) + profiler.onTransactionStart(fixture.transaction1) + profiler.onTransactionFinish(fixture.transaction1) + verify(fixture.hub).captureEnvelope( + check { + assertEquals(1, it.items.count()) + assertEnvelopeItem(it.items.toList()) { _, item -> + assertEquals(fixture.transaction1.eventId.toString(), item.transactionId) + val expectedMeasurements = setOf( + ProfileMeasurement.ID_SLOW_FRAME_RENDERS, + ProfileMeasurement.ID_FROZEN_FRAME_RENDERS, + ProfileMeasurement.ID_SCREEN_FRAME_RATES + ) + assertEquals(expectedMeasurements, item.measurementsMap.keys) + val slowFrames = item.measurementsMap[ProfileMeasurement.ID_SLOW_FRAME_RENDERS]!! + val frozenFrames = item.measurementsMap[ProfileMeasurement.ID_FROZEN_FRAME_RENDERS]!! + val frameRates = item.measurementsMap[ProfileMeasurement.ID_SCREEN_FRAME_RATES]!! + assertEquals(ProfileMeasurement.UNIT_NANOSECONDS, slowFrames.unit) + assertEquals(ProfileMeasurement.UNIT_NANOSECONDS, frozenFrames.unit) + assertEquals(ProfileMeasurement.UNIT_HZ, frameRates.unit) + } + } + ) + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/internal/SentryFrameMetricsCollectorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/SentryFrameMetricsCollectorTest.kt new file mode 100644 index 0000000000..61bc416f05 --- /dev/null +++ b/sentry-android-core/src/test/java/io/sentry/android/core/internal/SentryFrameMetricsCollectorTest.kt @@ -0,0 +1,233 @@ +package io.sentry.android.core.internal + +import android.app.Activity +import android.content.Context +import android.os.Build +import android.os.Handler +import android.view.Window +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import io.sentry.ILogger +import io.sentry.SentryOptions +import io.sentry.android.core.BuildInfoProvider +import io.sentry.android.core.SentryFrameMetricsCollector +import io.sentry.test.getCtor +import org.junit.runner.RunWith +import org.mockito.Mockito.spy +import org.mockito.kotlin.mock +import org.mockito.kotlin.whenever +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertNotNull +import kotlin.test.assertNull + +@RunWith(AndroidJUnit4::class) +class SentryFrameMetricsCollectorTest { + private lateinit var context: Context + + private val className = "io.sentry.android.core.SentryFrameMetricsCollector" + private val ctorTypes = arrayOf(Context::class.java, SentryOptions::class.java, BuildInfoProvider::class.java) + private val fixture = Fixture() + + private class Fixture { + private val mockDsn = "http://key@localhost/proj" + val buildInfo = mock { + whenever(it.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) + } + val mockLogger = mock() + val options = spy(SentryOptions()).apply { + dsn = mockDsn + isDebug = true + setLogger(mockLogger) + } + + val activity = mock() + val window = mock() + val activity2 = mock() + val window2 = mock() + + var addOnFrameMetricsAvailableListenerCounter = 0 + var removeOnFrameMetricsAvailableListenerCounter = 0 + val windowFrameMetricsManager = object : SentryFrameMetricsCollector.WindowFrameMetricsManager { + override fun addOnFrameMetricsAvailableListener( + window: Window, + frameMetricsAvailableListener: Window.OnFrameMetricsAvailableListener?, + handler: Handler? + ) { + addOnFrameMetricsAvailableListenerCounter++ + } + + override fun removeOnFrameMetricsAvailableListener( + window: Window, + frameMetricsAvailableListener: Window.OnFrameMetricsAvailableListener? + ) { + removeOnFrameMetricsAvailableListenerCounter++ + } + } + + fun getSut(context: Context, buildInfoProvider: BuildInfoProvider = buildInfo): SentryFrameMetricsCollector { + whenever(activity.window).thenReturn(window) + whenever(activity2.window).thenReturn(window2) + addOnFrameMetricsAvailableListenerCounter = 0 + removeOnFrameMetricsAvailableListenerCounter = 0 + return SentryFrameMetricsCollector( + context, + options, + buildInfoProvider, + windowFrameMetricsManager + ) + } + } + + @BeforeTest + fun `set up`() { + context = ApplicationProvider.getApplicationContext() + } + + @Test + fun `when null param is provided, invalid argument is thrown`() { + val ctor = className.getCtor(ctorTypes) + + assertFailsWith { + ctor.newInstance(arrayOf(null, mock(), mock())) + } + assertFailsWith { + ctor.newInstance(arrayOf(mock(), null, mock())) + } + assertFailsWith { + ctor.newInstance(arrayOf(mock(), mock(), null)) + } + } + + @Test + fun `collector works only on api 24+`() { + val buildInfo = mock { + whenever(it.sdkInfoVersion).thenReturn(Build.VERSION_CODES.M) + } + val collector = fixture.getSut(context, buildInfo) + val id = collector.startCollection(mock()) + assertNull(id) + } + + @Test + fun `collector works only if context is instance of Application`() { + val collector = fixture.getSut(mock()) + val id = collector.startCollection(mock()) + assertNull(id) + } + + @Test + fun `startCollection returns an id`() { + val collector = fixture.getSut(context) + val id = collector.startCollection(mock()) + assertNotNull(id) + } + + @Test + fun `collector calls addOnFrameMetricsAvailableListener when an activity starts`() { + val collector = fixture.getSut(context) + + collector.startCollection(mock()) + assertEquals(0, fixture.addOnFrameMetricsAvailableListenerCounter) + collector.onActivityStarted(fixture.activity) + assertEquals(1, fixture.addOnFrameMetricsAvailableListenerCounter) + } + + @Test + fun `collector calls removeOnFrameMetricsAvailableListener when an activity stops`() { + val collector = fixture.getSut(context) + + collector.startCollection(mock()) + collector.onActivityStarted(fixture.activity) + assertEquals(0, fixture.removeOnFrameMetricsAvailableListenerCounter) + collector.onActivityStopped(fixture.activity) + assertEquals(1, fixture.removeOnFrameMetricsAvailableListenerCounter) + } + + @Test + fun `collector ignores activities if not started`() { + val collector = fixture.getSut(context) + + assertEquals(0, fixture.removeOnFrameMetricsAvailableListenerCounter) + assertEquals(0, fixture.addOnFrameMetricsAvailableListenerCounter) + collector.onActivityStarted(fixture.activity) + collector.onActivityStopped(fixture.activity) + assertEquals(0, fixture.removeOnFrameMetricsAvailableListenerCounter) + assertEquals(0, fixture.addOnFrameMetricsAvailableListenerCounter) + } + + @Test + fun `startCollection calls addOnFrameMetricsAvailableListener if an activity is already started`() { + val collector = fixture.getSut(context) + + collector.onActivityStarted(fixture.activity) + assertEquals(0, fixture.addOnFrameMetricsAvailableListenerCounter) + collector.startCollection(mock()) + assertEquals(1, fixture.addOnFrameMetricsAvailableListenerCounter) + } + + @Test + fun `stopCollection calls removeOnFrameMetricsAvailableListener even if an activity is still started`() { + val collector = fixture.getSut(context) + val id = collector.startCollection(mock()) + collector.onActivityStarted(fixture.activity) + + assertEquals(0, fixture.removeOnFrameMetricsAvailableListenerCounter) + collector.stopCollection(id) + assertEquals(1, fixture.removeOnFrameMetricsAvailableListenerCounter) + } + + @Test + fun `OnFrameMetricsAvailableListener is called once per activity`() { + val collector = fixture.getSut(context) + collector.startCollection(mock()) + + assertEquals(0, fixture.addOnFrameMetricsAvailableListenerCounter) + assertEquals(0, fixture.removeOnFrameMetricsAvailableListenerCounter) + + collector.onActivityStarted(fixture.activity) + collector.onActivityStarted(fixture.activity) + + collector.onActivityStopped(fixture.activity) + collector.onActivityStopped(fixture.activity) + + assertEquals(1, fixture.addOnFrameMetricsAvailableListenerCounter) + assertEquals(1, fixture.removeOnFrameMetricsAvailableListenerCounter) + } + + @Test + fun `stopCollection works only after startCollection`() { + val collector = fixture.getSut(context) + collector.startCollection(mock()) + collector.onActivityStarted(fixture.activity) + collector.stopCollection("testId") + assertEquals(0, fixture.removeOnFrameMetricsAvailableListenerCounter) + } + + @Test + fun `collector tracks multiple activities`() { + val collector = fixture.getSut(context) + collector.startCollection(mock()) + collector.onActivityStarted(fixture.activity) + collector.onActivityStarted(fixture.activity2) + assertEquals(2, fixture.addOnFrameMetricsAvailableListenerCounter) + collector.onActivityStopped(fixture.activity) + collector.onActivityStopped(fixture.activity2) + assertEquals(2, fixture.removeOnFrameMetricsAvailableListenerCounter) + } + + @Test + fun `collector tracks multiple collections`() { + val collector = fixture.getSut(context) + val id1 = collector.startCollection(mock()) + val id2 = collector.startCollection(mock()) + collector.onActivityStarted(fixture.activity) + assertEquals(1, fixture.addOnFrameMetricsAvailableListenerCounter) + collector.stopCollection(id1) + assertEquals(0, fixture.removeOnFrameMetricsAvailableListenerCounter) + collector.stopCollection(id2) + assertEquals(1, fixture.removeOnFrameMetricsAvailableListenerCounter) + } +} diff --git a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/BaseUiTest.kt b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/BaseUiTest.kt index 0fd166c770..2ac15bebc0 100644 --- a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/BaseUiTest.kt +++ b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/BaseUiTest.kt @@ -72,6 +72,10 @@ abstract class BaseUiTest { } SentryAndroid.init(context) { it.dsn = mockDsn + // We don't use test orchestrator, due to problems with Saucelabs. + // So the app data is not deleted between tests. Thus, We don't know when sessions will actually be sent. + // To avoid any interference between tests we can just disable them by default. + it.isEnableAutoSessionTracking = false optionsConfiguration?.invoke(it) } } diff --git a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt index f42a336cab..469337749a 100644 --- a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt +++ b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt @@ -12,6 +12,7 @@ import io.sentry.ProfilingTraceData import io.sentry.Sentry import io.sentry.SentryEvent import io.sentry.android.core.SentryAndroidOptions +import io.sentry.profilemeasurements.ProfileMeasurement import io.sentry.protocol.SentryTransaction import org.junit.runner.RunWith import java.io.File @@ -48,10 +49,17 @@ class EnvelopeTests : BaseUiTest() { initSentry(true) { options: SentryAndroidOptions -> options.tracesSampleRate = 1.0 options.profilesSampleRate = 1.0 + options.isEnableAutoActivityLifecycleTracing = false } + + IdlingRegistry.getInstance().register(ProfilingSampleActivity.scrollingIdlingResource) + val sampleScenario = launchActivity() + val transaction = Sentry.startTransaction("e2etests", "test1") + swipeList(1) + sampleScenario.moveToState(Lifecycle.State.DESTROYED) + IdlingRegistry.getInstance().unregister(ProfilingSampleActivity.scrollingIdlingResource) relayIdlingResource.increment() relayIdlingResource.increment() - val transaction = Sentry.startTransaction("e2etests", "test1") transaction.finish() relay.assert { @@ -63,6 +71,19 @@ class EnvelopeTests : BaseUiTest() { assertTrue(profilingTraceData.environment.isNotEmpty()) assertTrue(profilingTraceData.cpuArchitecture.isNotEmpty()) assertTrue(profilingTraceData.transactions.isNotEmpty()) + assertTrue(profilingTraceData.measurementsMap.isNotEmpty()) + + // We check the measurements have been collected with expected units + val slowFrames = profilingTraceData.measurementsMap[ProfileMeasurement.ID_SLOW_FRAME_RENDERS]!! + val frozenFrames = profilingTraceData.measurementsMap[ProfileMeasurement.ID_FROZEN_FRAME_RENDERS]!! + val frameRates = profilingTraceData.measurementsMap[ProfileMeasurement.ID_SCREEN_FRAME_RATES]!! + assertEquals(ProfileMeasurement.UNIT_NANOSECONDS, slowFrames.unit) + assertEquals(ProfileMeasurement.UNIT_NANOSECONDS, frozenFrames.unit) + assertEquals(ProfileMeasurement.UNIT_HZ, frameRates.unit) + + // There could be no slow/frozen frames, but we expect at least one frame rate + assertTrue(frameRates.values.isNotEmpty()) + // We should find the transaction id that started the profiling in the list of transactions val transactionData = profilingTraceData.transactions .firstOrNull { t -> t.id == transaction.eventId.toString() } @@ -187,7 +208,7 @@ class EnvelopeTests : BaseUiTest() { } } -// @Test + @Test fun checkTimedOutProfile() { // We increase the IdlingResources timeout to exceed the profiling timeout IdlingPolicies.setIdlingResourceTimeout(1, TimeUnit.MINUTES) @@ -219,6 +240,7 @@ class EnvelopeTests : BaseUiTest() { options.dsn = "https://640fae2f19ac4ba78ad740175f50195f@o1137848.ingest.sentry.io/6191083" options.tracesSampleRate = 1.0 options.profilesSampleRate = 1.0 + options.isEnableAutoActivityLifecycleTracing = false } val transaction = Sentry.startTransaction("e2etests", "testProfile") diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 4d5c9ee87f..4e0eb07b90 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -812,7 +812,7 @@ public final class io/sentry/ProfilingTraceData : io/sentry/JsonSerializable, io public static final field TRUNCATION_REASON_NORMAL Ljava/lang/String; public static final field TRUNCATION_REASON_TIMEOUT Ljava/lang/String; public fun (Ljava/io/File;Lio/sentry/ITransaction;)V - public fun (Ljava/io/File;Ljava/util/List;Lio/sentry/ITransaction;Ljava/lang/String;ILjava/lang/String;Ljava/util/concurrent/Callable;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Boolean;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)V + public fun (Ljava/io/File;Ljava/util/List;Lio/sentry/ITransaction;Ljava/lang/String;ILjava/lang/String;Ljava/util/concurrent/Callable;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Boolean;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/util/Map;)V public fun getAndroidApiLevel ()I public fun getBuildId ()Ljava/lang/String; public fun getCpuArchitecture ()Ljava/lang/String; @@ -826,6 +826,7 @@ public final class io/sentry/ProfilingTraceData : io/sentry/JsonSerializable, io public fun getDevicePhysicalMemoryBytes ()Ljava/lang/String; public fun getDurationNs ()Ljava/lang/String; public fun getEnvironment ()Ljava/lang/String; + public fun getMeasurementsMap ()Ljava/util/Map; public fun getPlatform ()Ljava/lang/String; public fun getProfileId ()Ljava/lang/String; public fun getSampledProfile ()Ljava/lang/String; @@ -859,6 +860,8 @@ public final class io/sentry/ProfilingTraceData : io/sentry/JsonSerializable, io public fun setTraceId (Ljava/lang/String;)V public fun setTransactionId (Ljava/lang/String;)V public fun setTransactionName (Ljava/lang/String;)V + public fun setTransactions (Ljava/util/List;)V + public fun setTruncationReason (Ljava/lang/String;)V public fun setUnknown (Ljava/util/Map;)V public fun setVersionCode (Ljava/lang/String;)V public fun setVersionName (Ljava/lang/String;)V @@ -885,6 +888,7 @@ public final class io/sentry/ProfilingTraceData$JsonKeys { public static final field DEVICE_PHYSICAL_MEMORY_BYTES Ljava/lang/String; public static final field DURATION_NS Ljava/lang/String; public static final field ENVIRONMENT Ljava/lang/String; + public static final field MEASUREMENTS Ljava/lang/String; public static final field PLATFORM Ljava/lang/String; public static final field PROFILE_ID Ljava/lang/String; public static final field SAMPLED_PROFILE Ljava/lang/String; @@ -2214,6 +2218,61 @@ public final class io/sentry/internal/modules/ResourcesModulesLoader : io/sentry public fun (Lio/sentry/ILogger;)V } +public final class io/sentry/profilemeasurements/ProfileMeasurement : io/sentry/JsonSerializable, io/sentry/JsonUnknown { + public static final field ID_FROZEN_FRAME_RENDERS Ljava/lang/String; + public static final field ID_SCREEN_FRAME_RATES Ljava/lang/String; + public static final field ID_SLOW_FRAME_RENDERS Ljava/lang/String; + public static final field ID_UNKNOWN Ljava/lang/String; + public static final field UNIT_HZ Ljava/lang/String; + public static final field UNIT_NANOSECONDS Ljava/lang/String; + public static final field UNIT_UNKNOWN Ljava/lang/String; + public fun ()V + public fun (Ljava/lang/String;Ljava/util/Collection;)V + public fun equals (Ljava/lang/Object;)Z + public fun getUnit ()Ljava/lang/String; + public fun getUnknown ()Ljava/util/Map; + public fun getValues ()Ljava/util/Collection; + public fun hashCode ()I + public fun serialize (Lio/sentry/JsonObjectWriter;Lio/sentry/ILogger;)V + public fun setUnit (Ljava/lang/String;)V + public fun setUnknown (Ljava/util/Map;)V + public fun setValues (Ljava/util/Collection;)V +} + +public final class io/sentry/profilemeasurements/ProfileMeasurement$Deserializer : io/sentry/JsonDeserializer { + public fun ()V + public fun deserialize (Lio/sentry/JsonObjectReader;Lio/sentry/ILogger;)Lio/sentry/profilemeasurements/ProfileMeasurement; + public synthetic fun deserialize (Lio/sentry/JsonObjectReader;Lio/sentry/ILogger;)Ljava/lang/Object; +} + +public final class io/sentry/profilemeasurements/ProfileMeasurement$JsonKeys { + public static final field UNIT Ljava/lang/String; + public static final field VALUES Ljava/lang/String; + public fun ()V +} + +public final class io/sentry/profilemeasurements/ProfileMeasurementValue : io/sentry/JsonSerializable, io/sentry/JsonUnknown { + public fun ()V + public fun (Ljava/lang/Long;Ljava/lang/Number;)V + public fun equals (Ljava/lang/Object;)Z + public fun getUnknown ()Ljava/util/Map; + public fun hashCode ()I + public fun serialize (Lio/sentry/JsonObjectWriter;Lio/sentry/ILogger;)V + public fun setUnknown (Ljava/util/Map;)V +} + +public final class io/sentry/profilemeasurements/ProfileMeasurementValue$Deserializer : io/sentry/JsonDeserializer { + public fun ()V + public fun deserialize (Lio/sentry/JsonObjectReader;Lio/sentry/ILogger;)Lio/sentry/profilemeasurements/ProfileMeasurementValue; + public synthetic fun deserialize (Lio/sentry/JsonObjectReader;Lio/sentry/ILogger;)Ljava/lang/Object; +} + +public final class io/sentry/profilemeasurements/ProfileMeasurementValue$JsonKeys { + public static final field START_NS Ljava/lang/String; + public static final field VALUE Ljava/lang/String; + public fun ()V +} + public final class io/sentry/protocol/App : io/sentry/JsonSerializable, io/sentry/JsonUnknown { public static final field TYPE Ljava/lang/String; public fun ()V diff --git a/sentry/src/main/java/io/sentry/JsonSerializer.java b/sentry/src/main/java/io/sentry/JsonSerializer.java index 52c5d2e2da..b409110573 100644 --- a/sentry/src/main/java/io/sentry/JsonSerializer.java +++ b/sentry/src/main/java/io/sentry/JsonSerializer.java @@ -1,6 +1,8 @@ package io.sentry; import io.sentry.clientreport.ClientReport; +import io.sentry.profilemeasurements.ProfileMeasurement; +import io.sentry.profilemeasurements.ProfileMeasurementValue; import io.sentry.protocol.App; import io.sentry.protocol.Browser; import io.sentry.protocol.Contexts; @@ -77,6 +79,11 @@ public JsonSerializer(@NotNull SentryOptions options) { deserializersByClass.put(Message.class, new Message.Deserializer()); deserializersByClass.put(OperatingSystem.class, new OperatingSystem.Deserializer()); deserializersByClass.put(ProfilingTraceData.class, new ProfilingTraceData.Deserializer()); + deserializersByClass.put( + ProfilingTransactionData.class, new ProfilingTransactionData.Deserializer()); + deserializersByClass.put(ProfileMeasurement.class, new ProfileMeasurement.Deserializer()); + deserializersByClass.put( + ProfileMeasurementValue.class, new ProfileMeasurementValue.Deserializer()); deserializersByClass.put(Request.class, new Request.Deserializer()); deserializersByClass.put(SdkInfo.class, new SdkInfo.Deserializer()); deserializersByClass.put(SdkVersion.class, new SdkVersion.Deserializer()); diff --git a/sentry/src/main/java/io/sentry/ProfilingTraceData.java b/sentry/src/main/java/io/sentry/ProfilingTraceData.java index 764bb005bc..7d67c502c9 100644 --- a/sentry/src/main/java/io/sentry/ProfilingTraceData.java +++ b/sentry/src/main/java/io/sentry/ProfilingTraceData.java @@ -1,9 +1,11 @@ package io.sentry; +import io.sentry.profilemeasurements.ProfileMeasurement; import io.sentry.vendor.gson.stream.JsonToken; import java.io.File; import java.io.IOException; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; @@ -28,8 +30,8 @@ public final class ProfilingTraceData implements JsonUnknown, JsonSerializable { // Backgrounded reason is not used, yet, but it's one of the possible values @ApiStatus.Internal public static final String TRUNCATION_REASON_BACKGROUNDED = "backgrounded"; - private @NotNull File traceFile; - private @Nullable Callable> deviceCpuFrequenciesReader; + private final @NotNull File traceFile; + private final @NotNull Callable> deviceCpuFrequenciesReader; // Device metadata private int androidApiLevel; @@ -62,6 +64,7 @@ public final class ProfilingTraceData implements JsonUnknown, JsonSerializable { private @NotNull String profileId; private @NotNull String environment; private @NotNull String truncationReason; + private final @NotNull Map measurementsMap; // Stacktrace (file) /** Profile trace encoded with Base64 */ @@ -90,7 +93,8 @@ public ProfilingTraceData(@NotNull File traceFile, @NotNull ITransaction transac null, null, null, - TRUNCATION_REASON_NORMAL); + TRUNCATION_REASON_NORMAL, + new HashMap<>()); } public ProfilingTraceData( @@ -110,7 +114,8 @@ public ProfilingTraceData( @Nullable String versionName, @Nullable String versionCode, @Nullable String environment, - @NotNull String truncationReason) { + @NotNull String truncationReason, + @NotNull Map measurementsMap) { this.traceFile = traceFile; this.cpuArchitecture = cpuArchitecture; this.deviceCpuFrequenciesReader = deviceCpuFrequenciesReader; @@ -147,6 +152,7 @@ public ProfilingTraceData( if (!isTruncationReasonValid()) { this.truncationReason = TRUNCATION_REASON_NORMAL; } + this.measurementsMap = measurementsMap; } private boolean isTruncationReasonValid() { @@ -257,6 +263,10 @@ public boolean isDeviceIsEmulator() { return truncationReason; } + public @NotNull Map getMeasurementsMap() { + return measurementsMap; + } + public void setAndroidApiLevel(int androidApiLevel) { this.androidApiLevel = androidApiLevel; } @@ -297,6 +307,14 @@ public void setDevicePhysicalMemoryBytes(@NotNull String devicePhysicalMemoryByt this.devicePhysicalMemoryBytes = devicePhysicalMemoryBytes; } + public void setTruncationReason(@NotNull String truncationReason) { + this.truncationReason = truncationReason; + } + + public void setTransactions(@NotNull List transactions) { + this.transactions = transactions; + } + public void setBuildId(@NotNull String buildId) { this.buildId = buildId; } @@ -339,9 +357,7 @@ public void setSampledProfile(@Nullable String sampledProfile) { public void readDeviceCpuFrequencies() { try { - if (deviceCpuFrequenciesReader != null) { - this.deviceCpuFrequencies = deviceCpuFrequenciesReader.call(); - } + this.deviceCpuFrequencies = deviceCpuFrequenciesReader.call(); } catch (Throwable ignored) { // should never happen } @@ -374,6 +390,7 @@ public static final class JsonKeys { public static final String ENVIRONMENT = "environment"; public static final String SAMPLED_PROFILE = "sampled_profile"; public static final String TRUNCATION_REASON = "truncation_reason"; + public static final String MEASUREMENTS = "measurements"; } @Override @@ -409,6 +426,7 @@ public void serialize(@NotNull JsonObjectWriter writer, @NotNull ILogger logger) if (sampledProfile != null) { writer.name(JsonKeys.SAMPLED_PROFILE).value(sampledProfile); } + writer.name(JsonKeys.MEASUREMENTS).value(logger, measurementsMap); if (unknown != null) { for (String key : unknown.keySet()) { Object value = unknown.get(key); @@ -582,6 +600,13 @@ public static final class Deserializer implements JsonDeserializer measurements = + reader.nextMapOrNull(logger, new ProfileMeasurement.Deserializer()); + if (measurements != null) { + data.measurementsMap.putAll(measurements); + } + break; case JsonKeys.SAMPLED_PROFILE: String sampledProfile = reader.nextStringOrNull(); if (sampledProfile != null) { diff --git a/sentry/src/main/java/io/sentry/profilemeasurements/ProfileMeasurement.java b/sentry/src/main/java/io/sentry/profilemeasurements/ProfileMeasurement.java new file mode 100644 index 0000000000..0dd8ab95f6 --- /dev/null +++ b/sentry/src/main/java/io/sentry/profilemeasurements/ProfileMeasurement.java @@ -0,0 +1,150 @@ +package io.sentry.profilemeasurements; + +import io.sentry.ILogger; +import io.sentry.JsonDeserializer; +import io.sentry.JsonObjectReader; +import io.sentry.JsonObjectWriter; +import io.sentry.JsonSerializable; +import io.sentry.JsonUnknown; +import io.sentry.vendor.gson.stream.JsonToken; +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.concurrent.ConcurrentHashMap; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +@ApiStatus.Internal +public final class ProfileMeasurement implements JsonUnknown, JsonSerializable { + + public static final String ID_FROZEN_FRAME_RENDERS = "frozen_frame_renders"; + public static final String ID_SLOW_FRAME_RENDERS = "slow_frame_renders"; + public static final String ID_SCREEN_FRAME_RATES = "screen_frame_rates"; + public static final String ID_UNKNOWN = "unknown"; + + public static final String UNIT_HZ = "hz"; + public static final String UNIT_NANOSECONDS = "nanoseconds"; + public static final String UNIT_UNKNOWN = "unknown"; + + private @Nullable Map unknown; + private @NotNull String unit; // Unit of the value + private @NotNull Collection values; + + public ProfileMeasurement() { + this(UNIT_UNKNOWN, new ArrayList<>()); + } + + public ProfileMeasurement( + @NotNull String unit, @NotNull Collection values) { + this.unit = unit; + this.values = values; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + ProfileMeasurement that = (ProfileMeasurement) o; + return Objects.equals(unknown, that.unknown) + && unit.equals(that.unit) + && new ArrayList<>(values).equals(new ArrayList<>(that.values)); + } + + @Override + public int hashCode() { + return Objects.hash(unknown, unit, values); + } + + // JsonSerializable + + public static final class JsonKeys { + public static final String UNIT = "unit"; + public static final String VALUES = "values"; + } + + @Override + public void serialize(@NotNull JsonObjectWriter writer, @NotNull ILogger logger) + throws IOException { + writer.beginObject(); + writer.name(JsonKeys.UNIT).value(logger, unit); + writer.name(JsonKeys.VALUES).value(logger, values); + if (unknown != null) { + for (String key : unknown.keySet()) { + Object value = unknown.get(key); + writer.name(key); + writer.value(logger, value); + } + } + writer.endObject(); + } + + @Nullable + @Override + public Map getUnknown() { + return unknown; + } + + public @NotNull String getUnit() { + return unit; + } + + @Override + public void setUnknown(@Nullable Map unknown) { + this.unknown = unknown; + } + + public void setUnit(@NotNull String unit) { + this.unit = unit; + } + + public @NotNull Collection getValues() { + return values; + } + + public void setValues(@NotNull Collection values) { + this.values = values; + } + + public static final class Deserializer implements JsonDeserializer { + + @Override + public @NotNull ProfileMeasurement deserialize( + @NotNull JsonObjectReader reader, @NotNull ILogger logger) throws Exception { + reader.beginObject(); + ProfileMeasurement data = new ProfileMeasurement(); + Map unknown = null; + + while (reader.peek() == JsonToken.NAME) { + final String nextName = reader.nextName(); + switch (nextName) { + case JsonKeys.UNIT: + String unit = reader.nextStringOrNull(); + if (unit != null) { + data.unit = unit; + } + break; + case JsonKeys.VALUES: + List values = + reader.nextList(logger, new ProfileMeasurementValue.Deserializer()); + if (values != null) { + data.values = values; + } + break; + default: + if (unknown == null) { + unknown = new ConcurrentHashMap<>(); + } + reader.nextUnknown(logger, unknown, nextName); + break; + } + } + data.setUnknown(unknown); + reader.endObject(); + return data; + } + } +} diff --git a/sentry/src/main/java/io/sentry/profilemeasurements/ProfileMeasurementValue.java b/sentry/src/main/java/io/sentry/profilemeasurements/ProfileMeasurementValue.java new file mode 100644 index 0000000000..f4b863a2f5 --- /dev/null +++ b/sentry/src/main/java/io/sentry/profilemeasurements/ProfileMeasurementValue.java @@ -0,0 +1,120 @@ +package io.sentry.profilemeasurements; + +import io.sentry.ILogger; +import io.sentry.JsonDeserializer; +import io.sentry.JsonObjectReader; +import io.sentry.JsonObjectWriter; +import io.sentry.JsonSerializable; +import io.sentry.JsonUnknown; +import io.sentry.vendor.gson.stream.JsonToken; +import java.io.IOException; +import java.util.Map; +import java.util.Objects; +import java.util.concurrent.ConcurrentHashMap; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +@ApiStatus.Internal +public final class ProfileMeasurementValue implements JsonUnknown, JsonSerializable { + + private @Nullable Map unknown; + private @NotNull Long relativeStartNs; // timestamp in nanoseconds this frame was started + private @NotNull String value; // frame duration in nanoseconds + + public ProfileMeasurementValue() { + this(0L, 0); + } + + public ProfileMeasurementValue(@NotNull Long relativeStartNs, @NotNull Number value) { + this.relativeStartNs = relativeStartNs; + this.value = value.toString(); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + ProfileMeasurementValue that = (ProfileMeasurementValue) o; + return Objects.equals(unknown, that.unknown) + && relativeStartNs.equals(that.relativeStartNs) + && value.equals(that.value); + } + + @Override + public int hashCode() { + return Objects.hash(unknown, relativeStartNs, value); + } + + // JsonSerializable + + public static final class JsonKeys { + public static final String VALUE = "value"; + public static final String START_NS = "elapsed_since_start_ns"; + } + + @Override + public void serialize(@NotNull JsonObjectWriter writer, @NotNull ILogger logger) + throws IOException { + writer.beginObject(); + writer.name(JsonKeys.VALUE).value(logger, value); + writer.name(JsonKeys.START_NS).value(logger, relativeStartNs); + if (unknown != null) { + for (String key : unknown.keySet()) { + Object value = unknown.get(key); + writer.name(key); + writer.value(logger, value); + } + } + writer.endObject(); + } + + @Nullable + @Override + public Map getUnknown() { + return unknown; + } + + @Override + public void setUnknown(@Nullable Map unknown) { + this.unknown = unknown; + } + + public static final class Deserializer implements JsonDeserializer { + + @Override + public @NotNull ProfileMeasurementValue deserialize( + @NotNull JsonObjectReader reader, @NotNull ILogger logger) throws Exception { + reader.beginObject(); + ProfileMeasurementValue data = new ProfileMeasurementValue(); + Map unknown = null; + + while (reader.peek() == JsonToken.NAME) { + final String nextName = reader.nextName(); + switch (nextName) { + case JsonKeys.VALUE: + String value = reader.nextStringOrNull(); + if (value != null) { + data.value = value; + } + break; + case JsonKeys.START_NS: + Long startNs = reader.nextLongOrNull(); + if (startNs != null) { + data.relativeStartNs = startNs; + } + break; + default: + if (unknown == null) { + unknown = new ConcurrentHashMap<>(); + } + reader.nextUnknown(logger, unknown, nextName); + break; + } + } + data.setUnknown(unknown); + reader.endObject(); + return data; + } + } +} diff --git a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt index 57ef90e4b3..10799d3a3e 100644 --- a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt +++ b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt @@ -1,6 +1,8 @@ package io.sentry import io.sentry.exception.SentryEnvelopeException +import io.sentry.profilemeasurements.ProfileMeasurement +import io.sentry.profilemeasurements.ProfileMeasurementValue import io.sentry.protocol.Device import io.sentry.protocol.Request import io.sentry.protocol.SdkVersion @@ -384,8 +386,7 @@ class JsonSerializerTest { assertTrue( sdkInfo.packages!!.any { - it.name == "maven:io.sentry:sentry-android-core" - it.version == "4.5.6" + it.name == "io.sentry:maven:sentry-android-core" && it.version == "4.5.6" } ) } @@ -425,8 +426,7 @@ class JsonSerializerTest { assertNotNull(sdkVersion.packages) assertTrue( sdkVersion.packages!!.any { - it.name == "abc" - it.version == "4.5.6" + it.name == "abc" && it.version == "4.5.6" } ) } @@ -499,9 +499,14 @@ class JsonSerializerTest { profilingTraceData.deviceOsBuildNumber = "deviceOsBuildNumber" profilingTraceData.deviceOsVersion = "11" profilingTraceData.isDeviceIsEmulator = true + profilingTraceData.cpuArchitecture = "cpuArchitecture" profilingTraceData.deviceCpuFrequencies = listOf(1, 2, 3, 4) profilingTraceData.devicePhysicalMemoryBytes = "2000000" profilingTraceData.buildId = "buildId" + profilingTraceData.transactions = listOf( + ProfilingTransactionData(NoOpTransaction.getInstance(), 1, 2), + ProfilingTransactionData(NoOpTransaction.getInstance(), 2, 3) + ) profilingTraceData.transactionName = "transactionName" profilingTraceData.durationNs = "100" profilingTraceData.versionName = "versionName" @@ -510,12 +515,20 @@ class JsonSerializerTest { profilingTraceData.traceId = "traceId" profilingTraceData.profileId = "profileId" profilingTraceData.environment = "environment" + profilingTraceData.truncationReason = "truncationReason" + profilingTraceData.measurementsMap.putAll( + hashMapOf( + ProfileMeasurement.ID_SCREEN_FRAME_RATES to + ProfileMeasurement( + ProfileMeasurement.UNIT_HZ, + listOf(ProfileMeasurementValue(1, 60.1F)) + ) + ) + ) profilingTraceData.sampledProfile = "sampled profile in base 64" - val stringWriter = StringWriter() - fixture.serializer.serialize(profilingTraceData, stringWriter) - - val reader = StringReader(stringWriter.toString()) + val actual = serializeToString(profilingTraceData) + val reader = StringReader(actual) val objectReader = JsonObjectReader(reader) val element = JsonObjectDeserializer().deserialize(objectReader) as Map<*, *> @@ -527,10 +540,49 @@ class JsonSerializerTest { assertEquals("android", element["device_os_name"] as String) assertEquals("11", element["device_os_version"] as String) assertEquals(true, element["device_is_emulator"] as Boolean) + assertEquals("cpuArchitecture", element["architecture"] as String) assertEquals(listOf(1, 2, 3, 4), element["device_cpu_frequencies"] as List) assertEquals("2000000", element["device_physical_memory_bytes"] as String) assertEquals("android", element["platform"] as String) assertEquals("buildId", element["build_id"] as String) + assertEquals( + listOf( + mapOf( + "trace_id" to "00000000000000000000000000000000", + "relative_cpu_end_ms" to null, + "name" to "", + "relative_start_ns" to 1, + "relative_end_ns" to null, + "id" to "00000000000000000000000000000000", + "relative_cpu_start_ms" to 2 + ), + mapOf( + "trace_id" to "00000000000000000000000000000000", + "relative_cpu_end_ms" to null, + "name" to "", + "relative_start_ns" to 2, + "relative_end_ns" to null, + "id" to "00000000000000000000000000000000", + "relative_cpu_start_ms" to 3 + ) + ), + element["transactions"] + ) + assertEquals( + mapOf( + ProfileMeasurement.ID_SCREEN_FRAME_RATES to + mapOf( + "unit" to ProfileMeasurement.UNIT_HZ, + "values" to listOf( + mapOf( + "value" to "60.1", + "elapsed_since_start_ns" to 1 + ) + ) + ) + ), + element["measurements"] + ) assertEquals("transactionName", element["transaction_name"] as String) assertEquals("100", element["duration_ns"] as String) assertEquals("versionName", element["version_name"] as String) @@ -539,6 +591,7 @@ class JsonSerializerTest { assertEquals("traceId", element["trace_id"] as String) assertEquals("profileId", element["profile_id"] as String) assertEquals("environment", element["environment"] as String) + assertEquals("truncationReason", element["truncation_reason"] as String) assertEquals("sampled profile in base 64", element["sampled_profile"] as String) } @@ -574,6 +627,20 @@ class JsonSerializerTest { "relative_end_ns":21 } ], + "measurements":{ + "screen_frame_rates": { + "unit":"hz", + "values":[ + {"value":"60.1","elapsed_since_start_ns":"1"} + ] + }, + "frozen_frame_renders": { + "unit":"nanoseconds", + "values":[ + {"value":"100","elapsed_since_start_ns":"2"} + ] + } + }, "transaction_name":"transactionName", "duration_ns":"100", "version_name":"versionName", @@ -582,6 +649,7 @@ class JsonSerializerTest { "trace_id":"traceId", "profile_id":"profileId", "environment":"environment", + "truncation_reason":"truncationReason", "sampled_profile":"sampled profile in base 64" }""" val profilingTraceData = fixture.serializer.deserialize(StringReader(json), ProfilingTraceData::class.java) @@ -616,6 +684,17 @@ class JsonSerializerTest { } ) assertEquals(expectedTransactions, profilingTraceData.transactions) + val expectedMeasurements = mapOf( + ProfileMeasurement.ID_SCREEN_FRAME_RATES to ProfileMeasurement( + ProfileMeasurement.UNIT_HZ, + listOf(ProfileMeasurementValue(1, 60.1)) + ), + ProfileMeasurement.ID_FROZEN_FRAME_RENDERS to ProfileMeasurement( + ProfileMeasurement.UNIT_NANOSECONDS, + listOf(ProfileMeasurementValue(2, 100)) + ) + ) + assertEquals(expectedMeasurements, profilingTraceData.measurementsMap) assertEquals("transactionName", profilingTraceData.transactionName) assertEquals("100", profilingTraceData.durationNs) assertEquals("versionName", profilingTraceData.versionName) @@ -624,9 +703,51 @@ class JsonSerializerTest { assertEquals("traceId", profilingTraceData.traceId) assertEquals("profileId", profilingTraceData.profileId) assertEquals("environment", profilingTraceData.environment) + assertEquals("truncationReason", profilingTraceData.truncationReason) assertEquals("sampled profile in base 64", profilingTraceData.sampledProfile) } + @Test + fun `serializes profileMeasurement`() { + val measurementValues = listOf(ProfileMeasurementValue(1, 2), ProfileMeasurementValue(3, 4)) + val profileMeasurement = ProfileMeasurement(ProfileMeasurement.UNIT_NANOSECONDS, measurementValues) + val actual = serializeToString(profileMeasurement) + val expected = "{\"unit\":\"nanoseconds\",\"values\":[{\"value\":\"2\",\"elapsed_since_start_ns\":1},{\"value\":\"4\",\"elapsed_since_start_ns\":3}]}" + assertEquals(expected, actual) + } + + @Test + fun `deserializes profileMeasurement`() { + val json = """{ + "unit":"hz", + "values":[ + {"value":"60.1","elapsed_since_start_ns":"1"},{"value":"100","elapsed_since_start_ns":"2"} + ] + }""" + val profileMeasurement = fixture.serializer.deserialize(StringReader(json), ProfileMeasurement::class.java) + val expected = ProfileMeasurement( + ProfileMeasurement.UNIT_HZ, + listOf(ProfileMeasurementValue(1, 60.1), ProfileMeasurementValue(2, 100)) + ) + assertEquals(expected, profileMeasurement) + } + + @Test + fun `serializes profileMeasurementValue`() { + val profileMeasurementValue = ProfileMeasurementValue(1, 2) + val actual = serializeToString(profileMeasurementValue) + val expected = "{\"value\":\"2\",\"elapsed_since_start_ns\":1}" + assertEquals(expected, actual) + } + + @Test + fun `deserializes profileMeasurementValue`() { + val json = """{"value":"60.1","elapsed_since_start_ns":"1"}""" + val profileMeasurementValue = fixture.serializer.deserialize(StringReader(json), ProfileMeasurementValue::class.java) + val expected = ProfileMeasurementValue(1, 60.1) + assertEquals(expected, profileMeasurementValue) + } + @Test fun `serializes transaction`() { val trace = TransactionContext("transaction-name", "http") From 1585c22ea792a7af4c582e579a0d049083ddddbf Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Mon, 7 Nov 2022 17:38:59 +0100 Subject: [PATCH 02/20] added ProfileMeasurements to AndroidTransactionProfiler and ProfilingTraceData added frameMetrics (slow and frozen frames) and screen refresh rate added a SentryFrameMetricsCollector to collect frameMetrics changed a benchmark device on Saucelabs as it's not available anymore --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 381ae518c6..f3bb776c95 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ - Provide hook for Jetpack Compose navigation instrumentation ([#2320](https://github.com/getsentry/sentry-java/pull/2320)) - Populate `event.modules` with dependencies metadata ([#2324](https://github.com/getsentry/sentry-java/pull/2324)) - Support Spring 6 and Spring Boot 3 ([#2289](https://github.com/getsentry/sentry-java/pull/2289)) +- added FrameMetrics to Android profiling data ([#2342](https://github.com/getsentry/sentry-java/pull/2342)) ### Dependencies From fed5b219e02e0a0e2a2bac708787d2304f02ef02 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Mon, 7 Nov 2022 19:49:36 +0100 Subject: [PATCH 03/20] added ProfileMeasurements to AndroidTransactionProfiler and ProfilingTraceData added frameMetrics (slow and frozen frames) and screen refresh rate added a SentryFrameMetricsCollector to collect frameMetrics changed a benchmark device on Saucelabs as it's not available anymore --- .sauce/sentry-uitest-android-ui.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.sauce/sentry-uitest-android-ui.yml b/.sauce/sentry-uitest-android-ui.yml index dc647faf86..17c7aaf689 100644 --- a/.sauce/sentry-uitest-android-ui.yml +++ b/.sauce/sentry-uitest-android-ui.yml @@ -32,7 +32,7 @@ suites: - name: "Android 10 Ui test (api 29)" devices: - - id: OnePlus_7_Pro_real # OnePlus 7 Pro - api 29 (10) + - id: OnePlus_7T_real_us # OnePlus 7 Pro - api 29 (10) # Controls what artifacts to fetch when the suite on Sauce Cloud has finished. artifacts: From c5db5300ae1c40a2c496105863b2cfec601095bd Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Mon, 7 Nov 2022 20:07:30 +0100 Subject: [PATCH 04/20] added ProfileMeasurements to AndroidTransactionProfiler and ProfilingTraceData added frameMetrics (slow and frozen frames) and screen refresh rate added a SentryFrameMetricsCollector to collect frameMetrics changed a benchmark device on Saucelabs as it's not available anymore --- .sauce/sentry-uitest-android-ui.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.sauce/sentry-uitest-android-ui.yml b/.sauce/sentry-uitest-android-ui.yml index 17c7aaf689..cc6023af85 100644 --- a/.sauce/sentry-uitest-android-ui.yml +++ b/.sauce/sentry-uitest-android-ui.yml @@ -32,7 +32,7 @@ suites: - name: "Android 10 Ui test (api 29)" devices: - - id: OnePlus_7T_real_us # OnePlus 7 Pro - api 29 (10) + - id: OnePlus_7T_real_us # OnePlus 7T - api 29 (10) # Controls what artifacts to fetch when the suite on Sauce Cloud has finished. artifacts: From 0da7bc208a3d92ad7c7e42e8fc0d17edf23b8a19 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Tue, 8 Nov 2022 09:35:07 +0100 Subject: [PATCH 05/20] updated espresso test dependency to run on Android 13 devices --- buildSrc/src/main/java/Config.kt | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/buildSrc/src/main/java/Config.kt b/buildSrc/src/main/java/Config.kt index 34ad1fbff3..6238a710da 100644 --- a/buildSrc/src/main/java/Config.kt +++ b/buildSrc/src/main/java/Config.kt @@ -139,18 +139,16 @@ object Config { } object TestLibs { - private val androidxTestVersion = "1.4.0" - - // todo This beta version is needed to run ui tests on Android 13. - // It will be replaced by androidxTestVersion when 1.5.0 will be out. - private val androidxTestVersionBeta = "1.5.0-beta01" - private val espressoVersion = "3.4.0" + // todo These rc versions are needed to run ui tests on Android 13. + // They will be replaced by stable version when 1.5.0/3.5.0 will be out. + private val androidxTestVersion = "1.5.0-rc01" + private val espressoVersion = "3.5.0-rc01" val androidJUnitRunner = "androidx.test.runner.AndroidJUnitRunner" val kotlinTestJunit = "org.jetbrains.kotlin:kotlin-test-junit:$kotlinVersion" - val androidxCore = "androidx.test:core:$androidxTestVersionBeta" + val androidxCore = "androidx.test:core:$androidxTestVersion" val androidxRunner = "androidx.test:runner:$androidxTestVersion" - val androidxTestCoreKtx = "androidx.test:core-ktx:$androidxTestVersionBeta" + val androidxTestCoreKtx = "androidx.test:core-ktx:$androidxTestVersion" val androidxTestRules = "androidx.test:rules:$androidxTestVersion" val espressoCore = "androidx.test.espresso:espresso-core:$espressoVersion" val espressoIdlingResource = "androidx.test.espresso:espresso-idling-resource:$espressoVersion" From d6fe562b87813114e01d493d15eb1752b5fbfaaa Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Thu, 10 Nov 2022 09:41:38 +0100 Subject: [PATCH 06/20] frameMetrics collection anticipated to onActivityCreated instead of onActivityStarted --- .../sentry/android/core/SentryFrameMetricsCollector.java | 8 ++++---- .../java/io/sentry/uitest/android/EnvelopeTests.kt | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryFrameMetricsCollector.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryFrameMetricsCollector.java index 736bbc4768..3dea59f9e1 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryFrameMetricsCollector.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryFrameMetricsCollector.java @@ -95,13 +95,13 @@ public SentryFrameMetricsCollector( } @Override - public void onActivityCreated(@NonNull Activity activity, @Nullable Bundle savedInstanceState) {} - - @Override - public void onActivityStarted(@NonNull Activity activity) { + public void onActivityCreated(@NonNull Activity activity, @Nullable Bundle savedInstanceState) { setCurrentWindow(activity.getWindow()); } + @Override + public void onActivityStarted(@NonNull Activity activity) {} + @Override public void onActivityResumed(@NonNull Activity activity) {} diff --git a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt index 469337749a..921c3066ff 100644 --- a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt +++ b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt @@ -53,8 +53,8 @@ class EnvelopeTests : BaseUiTest() { } IdlingRegistry.getInstance().register(ProfilingSampleActivity.scrollingIdlingResource) - val sampleScenario = launchActivity() val transaction = Sentry.startTransaction("e2etests", "test1") + val sampleScenario = launchActivity() swipeList(1) sampleScenario.moveToState(Lifecycle.State.DESTROYED) IdlingRegistry.getInstance().unregister(ProfilingSampleActivity.scrollingIdlingResource) From 1db06284cd89ae13cbeb97d646e7a846dba36ff6 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Thu, 10 Nov 2022 11:27:40 +0100 Subject: [PATCH 07/20] updated tests --- .../SentryFrameMetricsCollectorTest.kt | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/internal/SentryFrameMetricsCollectorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/SentryFrameMetricsCollectorTest.kt index 61bc416f05..cff19e2973 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/internal/SentryFrameMetricsCollectorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/internal/SentryFrameMetricsCollectorTest.kt @@ -131,7 +131,7 @@ class SentryFrameMetricsCollectorTest { collector.startCollection(mock()) assertEquals(0, fixture.addOnFrameMetricsAvailableListenerCounter) - collector.onActivityStarted(fixture.activity) + collector.onActivityCreated(fixture.activity, mock()) assertEquals(1, fixture.addOnFrameMetricsAvailableListenerCounter) } @@ -140,7 +140,7 @@ class SentryFrameMetricsCollectorTest { val collector = fixture.getSut(context) collector.startCollection(mock()) - collector.onActivityStarted(fixture.activity) + collector.onActivityCreated(fixture.activity, mock()) assertEquals(0, fixture.removeOnFrameMetricsAvailableListenerCounter) collector.onActivityStopped(fixture.activity) assertEquals(1, fixture.removeOnFrameMetricsAvailableListenerCounter) @@ -152,7 +152,7 @@ class SentryFrameMetricsCollectorTest { assertEquals(0, fixture.removeOnFrameMetricsAvailableListenerCounter) assertEquals(0, fixture.addOnFrameMetricsAvailableListenerCounter) - collector.onActivityStarted(fixture.activity) + collector.onActivityCreated(fixture.activity, mock()) collector.onActivityStopped(fixture.activity) assertEquals(0, fixture.removeOnFrameMetricsAvailableListenerCounter) assertEquals(0, fixture.addOnFrameMetricsAvailableListenerCounter) @@ -162,7 +162,7 @@ class SentryFrameMetricsCollectorTest { fun `startCollection calls addOnFrameMetricsAvailableListener if an activity is already started`() { val collector = fixture.getSut(context) - collector.onActivityStarted(fixture.activity) + collector.onActivityCreated(fixture.activity, mock()) assertEquals(0, fixture.addOnFrameMetricsAvailableListenerCounter) collector.startCollection(mock()) assertEquals(1, fixture.addOnFrameMetricsAvailableListenerCounter) @@ -172,7 +172,7 @@ class SentryFrameMetricsCollectorTest { fun `stopCollection calls removeOnFrameMetricsAvailableListener even if an activity is still started`() { val collector = fixture.getSut(context) val id = collector.startCollection(mock()) - collector.onActivityStarted(fixture.activity) + collector.onActivityCreated(fixture.activity, mock()) assertEquals(0, fixture.removeOnFrameMetricsAvailableListenerCounter) collector.stopCollection(id) @@ -187,8 +187,8 @@ class SentryFrameMetricsCollectorTest { assertEquals(0, fixture.addOnFrameMetricsAvailableListenerCounter) assertEquals(0, fixture.removeOnFrameMetricsAvailableListenerCounter) - collector.onActivityStarted(fixture.activity) - collector.onActivityStarted(fixture.activity) + collector.onActivityCreated(fixture.activity, mock()) + collector.onActivityCreated(fixture.activity, mock()) collector.onActivityStopped(fixture.activity) collector.onActivityStopped(fixture.activity) @@ -201,7 +201,7 @@ class SentryFrameMetricsCollectorTest { fun `stopCollection works only after startCollection`() { val collector = fixture.getSut(context) collector.startCollection(mock()) - collector.onActivityStarted(fixture.activity) + collector.onActivityCreated(fixture.activity, mock()) collector.stopCollection("testId") assertEquals(0, fixture.removeOnFrameMetricsAvailableListenerCounter) } @@ -210,8 +210,8 @@ class SentryFrameMetricsCollectorTest { fun `collector tracks multiple activities`() { val collector = fixture.getSut(context) collector.startCollection(mock()) - collector.onActivityStarted(fixture.activity) - collector.onActivityStarted(fixture.activity2) + collector.onActivityCreated(fixture.activity, mock()) + collector.onActivityCreated(fixture.activity2, mock()) assertEquals(2, fixture.addOnFrameMetricsAvailableListenerCounter) collector.onActivityStopped(fixture.activity) collector.onActivityStopped(fixture.activity2) @@ -223,7 +223,7 @@ class SentryFrameMetricsCollectorTest { val collector = fixture.getSut(context) val id1 = collector.startCollection(mock()) val id2 = collector.startCollection(mock()) - collector.onActivityStarted(fixture.activity) + collector.onActivityCreated(fixture.activity, mock()) assertEquals(1, fixture.addOnFrameMetricsAvailableListenerCounter) collector.stopCollection(id1) assertEquals(0, fixture.removeOnFrameMetricsAvailableListenerCounter) From aad144f7e3b807793f6b0d07946b58109668d206 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Thu, 10 Nov 2022 11:55:40 +0100 Subject: [PATCH 08/20] We disable "Don't keep activities" developer option after receiving an error on Saucelabs --- .../io/sentry/uitest/android/benchmark/BaseBenchmarkTest.kt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sentry-android-integration-tests/sentry-uitest-android-benchmark/src/androidTest/java/io/sentry/uitest/android/benchmark/BaseBenchmarkTest.kt b/sentry-android-integration-tests/sentry-uitest-android-benchmark/src/androidTest/java/io/sentry/uitest/android/benchmark/BaseBenchmarkTest.kt index 4417d0f72d..2d39464ae5 100644 --- a/sentry-android-integration-tests/sentry-uitest-android-benchmark/src/androidTest/java/io/sentry/uitest/android/benchmark/BaseBenchmarkTest.kt +++ b/sentry-android-integration-tests/sentry-uitest-android-benchmark/src/androidTest/java/io/sentry/uitest/android/benchmark/BaseBenchmarkTest.kt @@ -1,6 +1,7 @@ package io.sentry.uitest.android.benchmark import android.content.Context +import android.provider.Settings import android.view.Choreographer import androidx.lifecycle.Lifecycle import androidx.test.core.app.ApplicationProvider @@ -24,6 +25,10 @@ abstract class BaseBenchmarkTest { runner.runOnMainSync { choreographer = Choreographer.getInstance() } + // We disable "Don't keep activities" developer option after receiving an error on Saucelabs: + // "Don't keep activities" developer options must be disabled for ActivityScenario + Settings.System.putInt(context.contentResolver, Settings.Global.ALWAYS_FINISH_ACTIVITIES, 0); + // We need the refresh rate, but we can get it only from the activity, so we start and destroy one val benchmarkScenario = launchActivity() benchmarkScenario.moveToState(Lifecycle.State.DESTROYED) From f7c07ca2d3a675000c5e4c5d6d5d3177de01cb18 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Thu, 10 Nov 2022 11:56:47 +0100 Subject: [PATCH 09/20] We disable "Don't keep activities" developer option after receiving an error on Saucelabs --- .../io/sentry/uitest/android/benchmark/BaseBenchmarkTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-android-integration-tests/sentry-uitest-android-benchmark/src/androidTest/java/io/sentry/uitest/android/benchmark/BaseBenchmarkTest.kt b/sentry-android-integration-tests/sentry-uitest-android-benchmark/src/androidTest/java/io/sentry/uitest/android/benchmark/BaseBenchmarkTest.kt index 2d39464ae5..8fbf36c807 100644 --- a/sentry-android-integration-tests/sentry-uitest-android-benchmark/src/androidTest/java/io/sentry/uitest/android/benchmark/BaseBenchmarkTest.kt +++ b/sentry-android-integration-tests/sentry-uitest-android-benchmark/src/androidTest/java/io/sentry/uitest/android/benchmark/BaseBenchmarkTest.kt @@ -27,7 +27,7 @@ abstract class BaseBenchmarkTest { } // We disable "Don't keep activities" developer option after receiving an error on Saucelabs: // "Don't keep activities" developer options must be disabled for ActivityScenario - Settings.System.putInt(context.contentResolver, Settings.Global.ALWAYS_FINISH_ACTIVITIES, 0); + Settings.System.putInt(context.contentResolver, Settings.Global.ALWAYS_FINISH_ACTIVITIES, 0) // We need the refresh rate, but we can get it only from the activity, so we start and destroy one val benchmarkScenario = launchActivity() From a8d132026a3ae4749ce78c96d7951f236e5b42a8 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Thu, 10 Nov 2022 13:16:12 +0100 Subject: [PATCH 10/20] disabling "Don't keep activities" developer option programmatically doesn't work --- .../io/sentry/uitest/android/benchmark/BaseBenchmarkTest.kt | 3 --- 1 file changed, 3 deletions(-) diff --git a/sentry-android-integration-tests/sentry-uitest-android-benchmark/src/androidTest/java/io/sentry/uitest/android/benchmark/BaseBenchmarkTest.kt b/sentry-android-integration-tests/sentry-uitest-android-benchmark/src/androidTest/java/io/sentry/uitest/android/benchmark/BaseBenchmarkTest.kt index 8fbf36c807..0ed3a6e854 100644 --- a/sentry-android-integration-tests/sentry-uitest-android-benchmark/src/androidTest/java/io/sentry/uitest/android/benchmark/BaseBenchmarkTest.kt +++ b/sentry-android-integration-tests/sentry-uitest-android-benchmark/src/androidTest/java/io/sentry/uitest/android/benchmark/BaseBenchmarkTest.kt @@ -25,9 +25,6 @@ abstract class BaseBenchmarkTest { runner.runOnMainSync { choreographer = Choreographer.getInstance() } - // We disable "Don't keep activities" developer option after receiving an error on Saucelabs: - // "Don't keep activities" developer options must be disabled for ActivityScenario - Settings.System.putInt(context.contentResolver, Settings.Global.ALWAYS_FINISH_ACTIVITIES, 0) // We need the refresh rate, but we can get it only from the activity, so we start and destroy one val benchmarkScenario = launchActivity() From 3d70b80788a0b10030c646d75c861b70aabec685 Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Thu, 10 Nov 2022 12:19:27 +0000 Subject: [PATCH 11/20] Format code --- .../java/io/sentry/uitest/android/benchmark/BaseBenchmarkTest.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/sentry-android-integration-tests/sentry-uitest-android-benchmark/src/androidTest/java/io/sentry/uitest/android/benchmark/BaseBenchmarkTest.kt b/sentry-android-integration-tests/sentry-uitest-android-benchmark/src/androidTest/java/io/sentry/uitest/android/benchmark/BaseBenchmarkTest.kt index 0ed3a6e854..d124e41f63 100644 --- a/sentry-android-integration-tests/sentry-uitest-android-benchmark/src/androidTest/java/io/sentry/uitest/android/benchmark/BaseBenchmarkTest.kt +++ b/sentry-android-integration-tests/sentry-uitest-android-benchmark/src/androidTest/java/io/sentry/uitest/android/benchmark/BaseBenchmarkTest.kt @@ -1,7 +1,6 @@ package io.sentry.uitest.android.benchmark import android.content.Context -import android.provider.Settings import android.view.Choreographer import androidx.lifecycle.Lifecycle import androidx.test.core.app.ApplicationProvider From 5b906fa4db0473d171157305e9a11ecea0c6f7c4 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Thu, 10 Nov 2022 17:01:03 +0100 Subject: [PATCH 12/20] updated changelog --- CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e4407ac703..ba1da36ab3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ - Fix `Gpu.vendorId` should be a String ([#2343](https://github.com/getsentry/sentry-java/pull/2343)) +### Features + +- added FrameMetrics to Android profiling data ([#2342](https://github.com/getsentry/sentry-java/pull/2342)) + ## 6.7.0 ### Fixes @@ -19,7 +23,6 @@ - Provide hook for Jetpack Compose navigation instrumentation ([#2320](https://github.com/getsentry/sentry-java/pull/2320)) - Populate `event.modules` with dependencies metadata ([#2324](https://github.com/getsentry/sentry-java/pull/2324)) - Support Spring 6 and Spring Boot 3 ([#2289](https://github.com/getsentry/sentry-java/pull/2289)) -- added FrameMetrics to Android profiling data ([#2342](https://github.com/getsentry/sentry-java/pull/2342)) ### Dependencies From 5bd9d16930a26ff4a1fcdb82f7862740fad615ba Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Fri, 11 Nov 2022 15:43:02 +0100 Subject: [PATCH 13/20] measurements are not sent if empty changed nanoseconds unit to nanosecond --- .../core/AndroidTransactionProfiler.java | 43 +++++++++++-------- .../core/AndroidTransactionProfilerTest.kt | 28 ------------ .../io/sentry/uitest/android/EnvelopeTests.kt | 16 ++++--- .../ProfileMeasurement.java | 2 +- .../test/java/io/sentry/JsonSerializerTest.kt | 4 +- 5 files changed, 37 insertions(+), 56 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java index 7f3ee8152a..56fe2deb61 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java @@ -70,10 +70,10 @@ final class AndroidTransactionProfiler implements ITransactionProfiler { private final @NotNull Map transactionMap = new HashMap<>(); private final @NotNull ArrayDeque screenFrameRateMeasurements = new ArrayDeque<>(); - private final @NotNull ArrayDeque - adverseFrameRenderTimestampMeasurements = new ArrayDeque<>(); - private final @NotNull ArrayDeque - adverseFrozenFrameTimestampMeasurements = new ArrayDeque<>(); + private final @NotNull ArrayDeque slowFrameRenderMeasurements = + new ArrayDeque<>(); + private final @NotNull ArrayDeque frozenFrameRenderMeasurements = + new ArrayDeque<>(); private final @NotNull Map measurementsMap = new HashMap<>(); public AndroidTransactionProfiler( @@ -190,8 +190,8 @@ private void onFirstTransactionStarted( measurementsMap.clear(); screenFrameRateMeasurements.clear(); - adverseFrameRenderTimestampMeasurements.clear(); - adverseFrozenFrameTimestampMeasurements.clear(); + slowFrameRenderMeasurements.clear(); + frozenFrameRenderMeasurements.clear(); frameMetricsCollectorId = frameMetricsCollector.startCollection( @@ -212,10 +212,10 @@ public void onFrameMetricCollected( boolean isSlow = durationNanos > nanosInSecond / (refreshRate - 1); float newRefreshRate = (int) (refreshRate * 100) / 100F; if (durationNanos > frozenFrameThresholdNanos) { - adverseFrozenFrameTimestampMeasurements.addLast( + frozenFrameRenderMeasurements.addLast( new ProfileMeasurementValue(frameTimestampRelativeNanos, durationNanos)); } else if (isSlow) { - adverseFrameRenderTimestampMeasurements.addLast( + slowFrameRenderMeasurements.addLast( new ProfileMeasurementValue(frameTimestampRelativeNanos, durationNanos)); } if (newRefreshRate != lastRefreshRate) { @@ -343,17 +343,22 @@ private void onLastTransactionFinished(ITransaction transaction, boolean isTimeo profileStartCpuMillis); } - measurementsMap.put( - ProfileMeasurement.ID_SLOW_FRAME_RENDERS, - new ProfileMeasurement( - ProfileMeasurement.UNIT_NANOSECONDS, adverseFrameRenderTimestampMeasurements)); - measurementsMap.put( - ProfileMeasurement.ID_FROZEN_FRAME_RENDERS, - new ProfileMeasurement( - ProfileMeasurement.UNIT_NANOSECONDS, adverseFrozenFrameTimestampMeasurements)); - measurementsMap.put( - ProfileMeasurement.ID_SCREEN_FRAME_RATES, - new ProfileMeasurement(ProfileMeasurement.UNIT_HZ, screenFrameRateMeasurements)); + if (!slowFrameRenderMeasurements.isEmpty()) { + measurementsMap.put( + ProfileMeasurement.ID_SLOW_FRAME_RENDERS, + new ProfileMeasurement(ProfileMeasurement.UNIT_NANOSECONDS, slowFrameRenderMeasurements)); + } + if (!frozenFrameRenderMeasurements.isEmpty()) { + measurementsMap.put( + ProfileMeasurement.ID_FROZEN_FRAME_RENDERS, + new ProfileMeasurement( + ProfileMeasurement.UNIT_NANOSECONDS, frozenFrameRenderMeasurements)); + } + if (!screenFrameRateMeasurements.isEmpty()) { + measurementsMap.put( + ProfileMeasurement.ID_SCREEN_FRAME_RATES, + new ProfileMeasurement(ProfileMeasurement.UNIT_HZ, screenFrameRateMeasurements)); + } // cpu max frequencies are read with a lambda because reading files is involved, so it will be // done in the background when the trace file is read diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt index 309e06035a..50d7fbb95e 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt @@ -12,7 +12,6 @@ import io.sentry.SentryLevel import io.sentry.SentryTracer import io.sentry.TransactionContext import io.sentry.assertEnvelopeItem -import io.sentry.profilemeasurements.ProfileMeasurement import io.sentry.test.getCtor import org.junit.runner.RunWith import org.mockito.kotlin.any @@ -355,31 +354,4 @@ class AndroidTransactionProfilerTest { profiler.onTransactionFinish(fixture.transaction2) verify(fixture.frameMetricsCollector).stopCollection(frameMetricsCollectorId) } - - @Test - fun `profiler includes measurements in envelope sent`() { - val profiler = fixture.getSut(context) - profiler.onTransactionStart(fixture.transaction1) - profiler.onTransactionFinish(fixture.transaction1) - verify(fixture.hub).captureEnvelope( - check { - assertEquals(1, it.items.count()) - assertEnvelopeItem(it.items.toList()) { _, item -> - assertEquals(fixture.transaction1.eventId.toString(), item.transactionId) - val expectedMeasurements = setOf( - ProfileMeasurement.ID_SLOW_FRAME_RENDERS, - ProfileMeasurement.ID_FROZEN_FRAME_RENDERS, - ProfileMeasurement.ID_SCREEN_FRAME_RATES - ) - assertEquals(expectedMeasurements, item.measurementsMap.keys) - val slowFrames = item.measurementsMap[ProfileMeasurement.ID_SLOW_FRAME_RENDERS]!! - val frozenFrames = item.measurementsMap[ProfileMeasurement.ID_FROZEN_FRAME_RENDERS]!! - val frameRates = item.measurementsMap[ProfileMeasurement.ID_SCREEN_FRAME_RATES]!! - assertEquals(ProfileMeasurement.UNIT_NANOSECONDS, slowFrames.unit) - assertEquals(ProfileMeasurement.UNIT_NANOSECONDS, frozenFrames.unit) - assertEquals(ProfileMeasurement.UNIT_HZ, frameRates.unit) - } - } - ) - } } diff --git a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt index 921c3066ff..72e195c2e9 100644 --- a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt +++ b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt @@ -74,14 +74,18 @@ class EnvelopeTests : BaseUiTest() { assertTrue(profilingTraceData.measurementsMap.isNotEmpty()) // We check the measurements have been collected with expected units - val slowFrames = profilingTraceData.measurementsMap[ProfileMeasurement.ID_SLOW_FRAME_RENDERS]!! - val frozenFrames = profilingTraceData.measurementsMap[ProfileMeasurement.ID_FROZEN_FRAME_RENDERS]!! + val slowFrames = profilingTraceData.measurementsMap[ProfileMeasurement.ID_SLOW_FRAME_RENDERS] + val frozenFrames = profilingTraceData.measurementsMap[ProfileMeasurement.ID_FROZEN_FRAME_RENDERS] val frameRates = profilingTraceData.measurementsMap[ProfileMeasurement.ID_SCREEN_FRAME_RATES]!! - assertEquals(ProfileMeasurement.UNIT_NANOSECONDS, slowFrames.unit) - assertEquals(ProfileMeasurement.UNIT_NANOSECONDS, frozenFrames.unit) - assertEquals(ProfileMeasurement.UNIT_HZ, frameRates.unit) - + // Slow and frozen frames can be null (in case there were none) + if (slowFrames != null) { + assertEquals(ProfileMeasurement.UNIT_NANOSECONDS, slowFrames.unit) + } + if (frozenFrames != null) { + assertEquals(ProfileMeasurement.UNIT_NANOSECONDS, frozenFrames.unit) + } // There could be no slow/frozen frames, but we expect at least one frame rate + assertEquals(ProfileMeasurement.UNIT_HZ, frameRates.unit) assertTrue(frameRates.values.isNotEmpty()) // We should find the transaction id that started the profiling in the list of transactions diff --git a/sentry/src/main/java/io/sentry/profilemeasurements/ProfileMeasurement.java b/sentry/src/main/java/io/sentry/profilemeasurements/ProfileMeasurement.java index 0dd8ab95f6..855acb9792 100644 --- a/sentry/src/main/java/io/sentry/profilemeasurements/ProfileMeasurement.java +++ b/sentry/src/main/java/io/sentry/profilemeasurements/ProfileMeasurement.java @@ -27,7 +27,7 @@ public final class ProfileMeasurement implements JsonUnknown, JsonSerializable { public static final String ID_UNKNOWN = "unknown"; public static final String UNIT_HZ = "hz"; - public static final String UNIT_NANOSECONDS = "nanoseconds"; + public static final String UNIT_NANOSECONDS = "nanosecond"; public static final String UNIT_UNKNOWN = "unknown"; private @Nullable Map unknown; diff --git a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt index 10799d3a3e..53a4af094e 100644 --- a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt +++ b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt @@ -635,7 +635,7 @@ class JsonSerializerTest { ] }, "frozen_frame_renders": { - "unit":"nanoseconds", + "unit":"nanosecond", "values":[ {"value":"100","elapsed_since_start_ns":"2"} ] @@ -712,7 +712,7 @@ class JsonSerializerTest { val measurementValues = listOf(ProfileMeasurementValue(1, 2), ProfileMeasurementValue(3, 4)) val profileMeasurement = ProfileMeasurement(ProfileMeasurement.UNIT_NANOSECONDS, measurementValues) val actual = serializeToString(profileMeasurement) - val expected = "{\"unit\":\"nanoseconds\",\"values\":[{\"value\":\"2\",\"elapsed_since_start_ns\":1},{\"value\":\"4\",\"elapsed_since_start_ns\":3}]}" + val expected = "{\"unit\":\"nanosecond\",\"values\":[{\"value\":\"2\",\"elapsed_since_start_ns\":1},{\"value\":\"4\",\"elapsed_since_start_ns\":3}]}" assertEquals(expected, actual) } From 0fa21e0c4bf7ceac558b55ca253ff89763314366 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Fri, 11 Nov 2022 16:45:52 +0100 Subject: [PATCH 14/20] updated benchmark device from OnePlus Nord N200 to Google Pixel 3a --- .sauce/sentry-uitest-android-benchmark-lite.yml | 2 +- .sauce/sentry-uitest-android-benchmark.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.sauce/sentry-uitest-android-benchmark-lite.yml b/.sauce/sentry-uitest-android-benchmark-lite.yml index 9ba72dec00..12dcb4cec2 100644 --- a/.sauce/sentry-uitest-android-benchmark-lite.yml +++ b/.sauce/sentry-uitest-android-benchmark-lite.yml @@ -20,7 +20,7 @@ suites: - name: "Android 11 (api 30)" devices: - - id: OnePlus_Nord_N200_5G_real_us # OnePlus Nord N200 5G - api 30 (11) + - id: Google_Pixel_3a_real # Google Pixel 3a - api 30 (11) artifacts: download: diff --git a/.sauce/sentry-uitest-android-benchmark.yml b/.sauce/sentry-uitest-android-benchmark.yml index d7d376b8e4..ca17d3659b 100644 --- a/.sauce/sentry-uitest-android-benchmark.yml +++ b/.sauce/sentry-uitest-android-benchmark.yml @@ -28,7 +28,7 @@ suites: devices: - id: OnePlus_9_Pro_real_us # OnePlus 9 Pro - api 30 (11) - high end - id: Google_Pixel_4_real_us # Google Pixel 4 - api 30 (11) - mid end - - id: OnePlus_Nord_N200_5G_real_us # OnePlus Nord N200 5G - api 30 (11) - low end + - id: Google_Pixel_3a_real # Google Pixel 3a - api 30 (11) - low end - name: "Android 10 (api 29)" devices: From cafe0f398ce67a166e7b164a669f875e00fd13ba Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Tue, 15 Nov 2022 13:10:28 +0100 Subject: [PATCH 15/20] added finals everywhere added automatic activity transaction in ui test --- CHANGELOG.md | 2 +- .../api/sentry-android-core.api | 23 ----- .../core/AndroidOptionsInitializer.java | 3 +- .../core/AndroidTransactionProfiler.java | 11 +-- .../util}/SentryFrameMetricsCollector.java | 58 ++++++------ .../core/AndroidTransactionProfilerTest.kt | 1 + .../SentryFrameMetricsCollectorTest.kt | 5 +- .../io/sentry/uitest/android/EnvelopeTests.kt | 8 +- .../java/io/sentry/ProfilingTraceData.java | 89 ++++++++++--------- .../ProfileMeasurement.java | 12 +-- .../ProfileMeasurementValue.java | 8 +- 11 files changed, 102 insertions(+), 118 deletions(-) rename sentry-android-core/src/main/java/io/sentry/android/core/{ => internal/util}/SentryFrameMetricsCollector.java (76%) rename sentry-android-core/src/test/java/io/sentry/android/core/internal/{ => util}/SentryFrameMetricsCollectorTest.kt (98%) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba1da36ab3..7487b7ac6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ ### Features -- added FrameMetrics to Android profiling data ([#2342](https://github.com/getsentry/sentry-java/pull/2342)) +- Add FrameMetrics to Android profiling data ([#2342](https://github.com/getsentry/sentry-java/pull/2342)) ## 6.7.0 diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index cf12bcf0c1..4bdcfecd27 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -164,29 +164,6 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr public fun setProfilingTracesIntervalMillis (I)V } -public final class io/sentry/android/core/SentryFrameMetricsCollector : android/app/Application$ActivityLifecycleCallbacks { - public fun (Landroid/content/Context;Lio/sentry/SentryOptions;Lio/sentry/android/core/BuildInfoProvider;)V - public fun (Landroid/content/Context;Lio/sentry/SentryOptions;Lio/sentry/android/core/BuildInfoProvider;Lio/sentry/android/core/SentryFrameMetricsCollector$WindowFrameMetricsManager;)V - public fun onActivityCreated (Landroid/app/Activity;Landroid/os/Bundle;)V - public fun onActivityDestroyed (Landroid/app/Activity;)V - public fun onActivityPaused (Landroid/app/Activity;)V - public fun onActivityResumed (Landroid/app/Activity;)V - public fun onActivitySaveInstanceState (Landroid/app/Activity;Landroid/os/Bundle;)V - public fun onActivityStarted (Landroid/app/Activity;)V - public fun onActivityStopped (Landroid/app/Activity;)V - public fun startCollection (Lio/sentry/android/core/SentryFrameMetricsCollector$FrameMetricsCollectorListener;)Ljava/lang/String; - public fun stopCollection (Ljava/lang/String;)V -} - -public abstract interface class io/sentry/android/core/SentryFrameMetricsCollector$FrameMetricsCollectorListener { - public abstract fun onFrameMetricCollected (Landroid/view/FrameMetrics;F)V -} - -public abstract interface class io/sentry/android/core/SentryFrameMetricsCollector$WindowFrameMetricsManager { - public fun addOnFrameMetricsAvailableListener (Landroid/view/Window;Landroid/view/Window$OnFrameMetricsAvailableListener;Landroid/os/Handler;)V - public fun removeOnFrameMetricsAvailableListener (Landroid/view/Window;Landroid/view/Window$OnFrameMetricsAvailableListener;)V -} - public final class io/sentry/android/core/SentryInitProvider : android/content/ContentProvider { public fun ()V public fun attachInfo (Landroid/content/Context;Landroid/content/pm/ProviderInfo;)V 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 5e441d19cd..8f3543825b 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 @@ -13,6 +13,7 @@ import io.sentry.SentryLevel; import io.sentry.android.core.cache.AndroidEnvelopeCache; import io.sentry.android.core.internal.modules.AssetsModulesLoader; +import io.sentry.android.core.internal.util.SentryFrameMetricsCollector; import io.sentry.android.fragment.FragmentLifecycleIntegration; import io.sentry.android.timber.SentryTimberIntegration; import io.sentry.util.Objects; @@ -154,7 +155,7 @@ static void init( options.addEventProcessor(new PerformanceAndroidEventProcessor(options, activityFramesTracker)); options.setTransportGate(new AndroidTransportGate(context, options.getLogger())); - SentryFrameMetricsCollector frameMetricsCollector = + final SentryFrameMetricsCollector frameMetricsCollector = new SentryFrameMetricsCollector(context, options, buildInfoProvider); options.setTransactionProfiler( new AndroidTransactionProfiler(context, options, buildInfoProvider, frameMetricsCollector)); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java index 56fe2deb61..0e588fa122 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java @@ -21,6 +21,7 @@ import io.sentry.SentryEnvelope; import io.sentry.SentryLevel; import io.sentry.android.core.internal.util.CpuInfoUtils; +import io.sentry.android.core.internal.util.SentryFrameMetricsCollector; import io.sentry.exception.SentryEnvelopeException; import io.sentry.profilemeasurements.ProfileMeasurement; import io.sentry.profilemeasurements.ProfileMeasurementValue; @@ -140,7 +141,7 @@ private void init() { @SuppressLint("NewApi") @Override - public synchronized void onTransactionStart(@NotNull ITransaction transaction) { + public synchronized void onTransactionStart(final @NotNull ITransaction transaction) { // Debug.startMethodTracingSampling() is only available since Lollipop if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.LOLLIPOP) return; @@ -186,7 +187,7 @@ public synchronized void onTransactionStart(@NotNull ITransaction transaction) { @SuppressLint("NewApi") private void onFirstTransactionStarted( - @NotNull ITransaction transaction, @NotNull File traceFile) { + final @NotNull ITransaction transaction, final @NotNull File traceFile) { measurementsMap.clear(); screenFrameRateMeasurements.clear(); @@ -243,13 +244,13 @@ public void onFrameMetricCollected( } @Override - public synchronized void onTransactionFinish(@NotNull ITransaction transaction) { + public synchronized void onTransactionFinish(final @NotNull ITransaction transaction) { onTransactionFinish(transaction, false); } @SuppressLint("NewApi") private synchronized void onTransactionFinish( - @NotNull ITransaction transaction, boolean isTimeout) { + final @NotNull ITransaction transaction, final boolean isTimeout) { // onTransactionStart() is only available since Lollipop // and SystemClock.elapsedRealtimeNanos() since Jelly Bean @@ -297,7 +298,7 @@ private synchronized void onTransactionFinish( } @SuppressLint("NewApi") - private void onLastTransactionFinished(ITransaction transaction, boolean isTimeout) { + private void onLastTransactionFinished(final ITransaction transaction, final boolean isTimeout) { Debug.stopMethodTracing(); frameMetricsCollector.stopCollection(frameMetricsCollectorId); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryFrameMetricsCollector.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollector.java similarity index 76% rename from sentry-android-core/src/main/java/io/sentry/android/core/SentryFrameMetricsCollector.java rename to sentry-android-core/src/main/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollector.java index 3dea59f9e1..a8dbeca586 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryFrameMetricsCollector.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollector.java @@ -1,4 +1,4 @@ -package io.sentry.android.core; +package io.sentry.android.core.internal.util; import android.annotation.SuppressLint; import android.app.Activity; @@ -10,10 +10,10 @@ import android.os.HandlerThread; import android.view.FrameMetrics; import android.view.Window; -import androidx.annotation.NonNull; import androidx.annotation.RequiresApi; import io.sentry.SentryLevel; import io.sentry.SentryOptions; +import io.sentry.android.core.BuildInfoProvider; import io.sentry.util.Objects; import java.lang.ref.WeakReference; import java.util.HashMap; @@ -40,19 +40,19 @@ public final class SentryFrameMetricsCollector implements Application.ActivityLi @SuppressWarnings("deprecation") @SuppressLint("NewApi") public SentryFrameMetricsCollector( - @NotNull Context context, - @NotNull SentryOptions options, - @NotNull BuildInfoProvider buildInfoProvider) { + final @NotNull Context context, + final @NotNull SentryOptions options, + final @NotNull BuildInfoProvider buildInfoProvider) { this(context, options, buildInfoProvider, new WindowFrameMetricsManager() {}); } @SuppressWarnings("deprecation") @SuppressLint("NewApi") public SentryFrameMetricsCollector( - @NotNull Context context, - @NotNull SentryOptions options, - @NotNull BuildInfoProvider buildInfoProvider, - @NotNull WindowFrameMetricsManager windowFrameMetricsManager) { + final @NotNull Context context, + final @NotNull SentryOptions options, + final @NotNull BuildInfoProvider buildInfoProvider, + final @NotNull WindowFrameMetricsManager windowFrameMetricsManager) { Objects.requireNonNull(context, "The context is required"); Objects.requireNonNull(options, "SentryOptions is required"); this.buildInfoProvider = @@ -71,7 +71,7 @@ public SentryFrameMetricsCollector( isAvailable = true; HandlerThread handlerThread = - new HandlerThread("io.sentry.android.core.SentryFrameMetricsCollector"); + new HandlerThread("io.sentry.android.core.internal.util.SentryFrameMetricsCollector"); handlerThread.setUncaughtExceptionHandler( (thread, e) -> options.getLogger().log(SentryLevel.ERROR, "Error during frames measurements.", e)); @@ -95,31 +95,31 @@ public SentryFrameMetricsCollector( } @Override - public void onActivityCreated(@NonNull Activity activity, @Nullable Bundle savedInstanceState) { + public void onActivityCreated(@NotNull Activity activity, @Nullable Bundle savedInstanceState) { setCurrentWindow(activity.getWindow()); } @Override - public void onActivityStarted(@NonNull Activity activity) {} + public void onActivityStarted(@NotNull Activity activity) {} @Override - public void onActivityResumed(@NonNull Activity activity) {} + public void onActivityResumed(@NotNull Activity activity) {} @Override - public void onActivityPaused(@NonNull Activity activity) {} + public void onActivityPaused(@NotNull Activity activity) {} @Override - public void onActivityStopped(@NonNull Activity activity) { - cleanCurrentWindow(activity.getWindow()); + public void onActivityStopped(@NotNull Activity activity) { + clearCurrentWindow(activity.getWindow()); } @Override - public void onActivitySaveInstanceState(@NonNull Activity activity, @NonNull Bundle outState) {} + public void onActivitySaveInstanceState(@NotNull Activity activity, @NotNull Bundle outState) {} @Override - public void onActivityDestroyed(@NonNull Activity activity) {} + public void onActivityDestroyed(@NotNull Activity activity) {} - public @Nullable String startCollection(FrameMetricsCollectorListener listener) { + public @Nullable String startCollection(final @NotNull FrameMetricsCollectorListener listener) { if (!isAvailable) { return null; } @@ -129,7 +129,7 @@ public void onActivityDestroyed(@NonNull Activity activity) {} return uid; } - public void stopCollection(@Nullable String listenerId) { + public void stopCollection(final @Nullable String listenerId) { if (!isAvailable) { return; } @@ -138,12 +138,12 @@ public void stopCollection(@Nullable String listenerId) { } Window window = currentWindow != null ? currentWindow.get() : null; if (window != null && listenerMap.isEmpty()) { - cleanCurrentWindow(window); + clearCurrentWindow(window); } } @SuppressLint("NewApi") - private void cleanCurrentWindow(@NonNull Window window) { + private void clearCurrentWindow(final @NotNull Window window) { if (currentWindow != null && currentWindow.get() == window) { currentWindow = null; } @@ -156,7 +156,7 @@ private void cleanCurrentWindow(@NonNull Window window) { } } - private void setCurrentWindow(@NotNull Window window) { + private void setCurrentWindow(final @NotNull Window window) { if (currentWindow != null && currentWindow.get() == window) { return; } @@ -183,23 +183,23 @@ private void trackCurrentWindow() { @ApiStatus.Internal public interface FrameMetricsCollectorListener { - void onFrameMetricCollected(@NotNull FrameMetrics frameMetrics, float refreshRate); + void onFrameMetricCollected(final @NotNull FrameMetrics frameMetrics, final float refreshRate); } @ApiStatus.Internal public interface WindowFrameMetricsManager { @RequiresApi(api = Build.VERSION_CODES.N) default void addOnFrameMetricsAvailableListener( - @NotNull Window window, - @Nullable Window.OnFrameMetricsAvailableListener frameMetricsAvailableListener, - @Nullable Handler handler) { + final @NotNull Window window, + final @Nullable Window.OnFrameMetricsAvailableListener frameMetricsAvailableListener, + final @Nullable Handler handler) { window.addOnFrameMetricsAvailableListener(frameMetricsAvailableListener, handler); } @RequiresApi(api = Build.VERSION_CODES.N) default void removeOnFrameMetricsAvailableListener( - @NotNull Window window, - @Nullable Window.OnFrameMetricsAvailableListener frameMetricsAvailableListener) { + final @NotNull Window window, + final @Nullable Window.OnFrameMetricsAvailableListener frameMetricsAvailableListener) { window.removeOnFrameMetricsAvailableListener(frameMetricsAvailableListener); } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt index 50d7fbb95e..fd0af55838 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt @@ -11,6 +11,7 @@ import io.sentry.ProfilingTraceData import io.sentry.SentryLevel import io.sentry.SentryTracer import io.sentry.TransactionContext +import io.sentry.android.core.internal.util.SentryFrameMetricsCollector import io.sentry.assertEnvelopeItem import io.sentry.test.getCtor import org.junit.runner.RunWith diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/internal/SentryFrameMetricsCollectorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollectorTest.kt similarity index 98% rename from sentry-android-core/src/test/java/io/sentry/android/core/internal/SentryFrameMetricsCollectorTest.kt rename to sentry-android-core/src/test/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollectorTest.kt index cff19e2973..8d95724b28 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/internal/SentryFrameMetricsCollectorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollectorTest.kt @@ -1,4 +1,4 @@ -package io.sentry.android.core.internal +package io.sentry.android.core.internal.util import android.app.Activity import android.content.Context @@ -10,7 +10,6 @@ import androidx.test.ext.junit.runners.AndroidJUnit4 import io.sentry.ILogger import io.sentry.SentryOptions import io.sentry.android.core.BuildInfoProvider -import io.sentry.android.core.SentryFrameMetricsCollector import io.sentry.test.getCtor import org.junit.runner.RunWith import org.mockito.Mockito.spy @@ -27,7 +26,7 @@ import kotlin.test.assertNull class SentryFrameMetricsCollectorTest { private lateinit var context: Context - private val className = "io.sentry.android.core.SentryFrameMetricsCollector" + private val className = "io.sentry.android.core.internal.util.SentryFrameMetricsCollector" private val ctorTypes = arrayOf(Context::class.java, SentryOptions::class.java, BuildInfoProvider::class.java) private val fixture = Fixture() diff --git a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt index 72e195c2e9..491205ec45 100644 --- a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt +++ b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt @@ -49,9 +49,9 @@ class EnvelopeTests : BaseUiTest() { initSentry(true) { options: SentryAndroidOptions -> options.tracesSampleRate = 1.0 options.profilesSampleRate = 1.0 - options.isEnableAutoActivityLifecycleTracing = false } + relayIdlingResource.increment() IdlingRegistry.getInstance().register(ProfilingSampleActivity.scrollingIdlingResource) val transaction = Sentry.startTransaction("e2etests", "test1") val sampleScenario = launchActivity() @@ -63,6 +63,11 @@ class EnvelopeTests : BaseUiTest() { transaction.finish() relay.assert { + assertEnvelope { + val transactionItem: SentryTransaction = it.assertItem() + it.assertNoOtherItems() + assertEquals("ProfilingSampleActivity", transactionItem.transaction) + } assertEnvelope { val profilingTraceData: ProfilingTraceData = it.assertItem() it.assertNoOtherItems() @@ -244,7 +249,6 @@ class EnvelopeTests : BaseUiTest() { options.dsn = "https://640fae2f19ac4ba78ad740175f50195f@o1137848.ingest.sentry.io/6191083" options.tracesSampleRate = 1.0 options.profilesSampleRate = 1.0 - options.isEnableAutoActivityLifecycleTracing = false } val transaction = Sentry.startTransaction("e2etests", "testProfile") diff --git a/sentry/src/main/java/io/sentry/ProfilingTraceData.java b/sentry/src/main/java/io/sentry/ProfilingTraceData.java index 7d67c502c9..c26fc32719 100644 --- a/sentry/src/main/java/io/sentry/ProfilingTraceData.java +++ b/sentry/src/main/java/io/sentry/ProfilingTraceData.java @@ -74,7 +74,8 @@ private ProfilingTraceData() { this(new File("dummy"), NoOpTransaction.getInstance()); } - public ProfilingTraceData(@NotNull File traceFile, @NotNull ITransaction transaction) { + public ProfilingTraceData( + final @NotNull File traceFile, final @NotNull ITransaction transaction) { this( traceFile, new ArrayList<>(), @@ -98,24 +99,24 @@ public ProfilingTraceData(@NotNull File traceFile, @NotNull ITransaction transac } public ProfilingTraceData( - @NotNull File traceFile, - @NotNull List transactions, - @NotNull ITransaction transaction, - @NotNull String durationNanos, - int sdkInt, - @NotNull String cpuArchitecture, - @NotNull Callable> deviceCpuFrequenciesReader, - @Nullable String deviceManufacturer, - @Nullable String deviceModel, - @Nullable String deviceOsVersion, - @Nullable Boolean deviceIsEmulator, - @Nullable String devicePhysicalMemoryBytes, - @Nullable String buildId, - @Nullable String versionName, - @Nullable String versionCode, - @Nullable String environment, - @NotNull String truncationReason, - @NotNull Map measurementsMap) { + final @NotNull File traceFile, + final @NotNull List transactions, + final @NotNull ITransaction transaction, + final @NotNull String durationNanos, + final int sdkInt, + final @NotNull String cpuArchitecture, + final @NotNull Callable> deviceCpuFrequenciesReader, + final @Nullable String deviceManufacturer, + final @Nullable String deviceModel, + final @Nullable String deviceOsVersion, + final @Nullable Boolean deviceIsEmulator, + final @Nullable String devicePhysicalMemoryBytes, + final @Nullable String buildId, + final @Nullable String versionName, + final @Nullable String versionCode, + final @Nullable String environment, + final @NotNull String truncationReason, + final @NotNull Map measurementsMap) { this.traceFile = traceFile; this.cpuArchitecture = cpuArchitecture; this.deviceCpuFrequenciesReader = deviceCpuFrequenciesReader; @@ -267,91 +268,91 @@ public boolean isDeviceIsEmulator() { return measurementsMap; } - public void setAndroidApiLevel(int androidApiLevel) { + public void setAndroidApiLevel(final int androidApiLevel) { this.androidApiLevel = androidApiLevel; } - public void setCpuArchitecture(@NotNull String cpuArchitecture) { + public void setCpuArchitecture(final @NotNull String cpuArchitecture) { this.cpuArchitecture = cpuArchitecture; } - public void setDeviceLocale(@NotNull String deviceLocale) { + public void setDeviceLocale(final @NotNull String deviceLocale) { this.deviceLocale = deviceLocale; } - public void setDeviceManufacturer(@NotNull String deviceManufacturer) { + public void setDeviceManufacturer(final @NotNull String deviceManufacturer) { this.deviceManufacturer = deviceManufacturer; } - public void setDeviceModel(@NotNull String deviceModel) { + public void setDeviceModel(final @NotNull String deviceModel) { this.deviceModel = deviceModel; } - public void setDeviceOsBuildNumber(@NotNull String deviceOsBuildNumber) { + public void setDeviceOsBuildNumber(final @NotNull String deviceOsBuildNumber) { this.deviceOsBuildNumber = deviceOsBuildNumber; } - public void setDeviceOsVersion(@NotNull String deviceOsVersion) { + public void setDeviceOsVersion(final @NotNull String deviceOsVersion) { this.deviceOsVersion = deviceOsVersion; } - public void setDeviceIsEmulator(boolean deviceIsEmulator) { + public void setDeviceIsEmulator(final boolean deviceIsEmulator) { this.deviceIsEmulator = deviceIsEmulator; } - public void setDeviceCpuFrequencies(@NotNull List deviceCpuFrequencies) { + public void setDeviceCpuFrequencies(final @NotNull List deviceCpuFrequencies) { this.deviceCpuFrequencies = deviceCpuFrequencies; } - public void setDevicePhysicalMemoryBytes(@NotNull String devicePhysicalMemoryBytes) { + public void setDevicePhysicalMemoryBytes(final @NotNull String devicePhysicalMemoryBytes) { this.devicePhysicalMemoryBytes = devicePhysicalMemoryBytes; } - public void setTruncationReason(@NotNull String truncationReason) { + public void setTruncationReason(final @NotNull String truncationReason) { this.truncationReason = truncationReason; } - public void setTransactions(@NotNull List transactions) { + public void setTransactions(final @NotNull List transactions) { this.transactions = transactions; } - public void setBuildId(@NotNull String buildId) { + public void setBuildId(final @NotNull String buildId) { this.buildId = buildId; } - public void setTransactionName(@NotNull String transactionName) { + public void setTransactionName(final @NotNull String transactionName) { this.transactionName = transactionName; } - public void setDurationNs(@NotNull String durationNs) { + public void setDurationNs(final @NotNull String durationNs) { this.durationNs = durationNs; } - public void setVersionName(@NotNull String versionName) { + public void setVersionName(final @NotNull String versionName) { this.versionName = versionName; } - public void setVersionCode(@NotNull String versionCode) { + public void setVersionCode(final @NotNull String versionCode) { this.versionCode = versionCode; } - public void setTransactionId(@NotNull String transactionId) { + public void setTransactionId(final @NotNull String transactionId) { this.transactionId = transactionId; } - public void setTraceId(@NotNull String traceId) { + public void setTraceId(final @NotNull String traceId) { this.traceId = traceId; } - public void setProfileId(@NotNull String profileId) { + public void setProfileId(final @NotNull String profileId) { this.profileId = profileId; } - public void setEnvironment(@NotNull String environment) { + public void setEnvironment(final @NotNull String environment) { this.environment = environment; } - public void setSampledProfile(@Nullable String sampledProfile) { + public void setSampledProfile(final @Nullable String sampledProfile) { this.sampledProfile = sampledProfile; } @@ -394,7 +395,7 @@ public static final class JsonKeys { } @Override - public void serialize(@NotNull JsonObjectWriter writer, @NotNull ILogger logger) + public void serialize(final @NotNull JsonObjectWriter writer, final @NotNull ILogger logger) throws IOException { writer.beginObject(); writer.name(JsonKeys.ANDROID_API_LEVEL).value(logger, androidApiLevel); @@ -444,7 +445,7 @@ public Map getUnknown() { } @Override - public void setUnknown(@Nullable Map unknown) { + public void setUnknown(final @Nullable Map unknown) { this.unknown = unknown; } @@ -453,7 +454,7 @@ public static final class Deserializer implements JsonDeserializer unknown = null; diff --git a/sentry/src/main/java/io/sentry/profilemeasurements/ProfileMeasurement.java b/sentry/src/main/java/io/sentry/profilemeasurements/ProfileMeasurement.java index 855acb9792..0e80cbb4a3 100644 --- a/sentry/src/main/java/io/sentry/profilemeasurements/ProfileMeasurement.java +++ b/sentry/src/main/java/io/sentry/profilemeasurements/ProfileMeasurement.java @@ -39,7 +39,7 @@ public ProfileMeasurement() { } public ProfileMeasurement( - @NotNull String unit, @NotNull Collection values) { + final @NotNull String unit, final @NotNull Collection values) { this.unit = unit; this.values = values; } @@ -67,7 +67,7 @@ public static final class JsonKeys { } @Override - public void serialize(@NotNull JsonObjectWriter writer, @NotNull ILogger logger) + public void serialize(final @NotNull JsonObjectWriter writer, final @NotNull ILogger logger) throws IOException { writer.beginObject(); writer.name(JsonKeys.UNIT).value(logger, unit); @@ -93,11 +93,11 @@ public Map getUnknown() { } @Override - public void setUnknown(@Nullable Map unknown) { + public void setUnknown(final @Nullable Map unknown) { this.unknown = unknown; } - public void setUnit(@NotNull String unit) { + public void setUnit(final @NotNull String unit) { this.unit = unit; } @@ -105,7 +105,7 @@ public void setUnit(@NotNull String unit) { return values; } - public void setValues(@NotNull Collection values) { + public void setValues(final @NotNull Collection values) { this.values = values; } @@ -113,7 +113,7 @@ public static final class Deserializer implements JsonDeserializer unknown = null; diff --git a/sentry/src/main/java/io/sentry/profilemeasurements/ProfileMeasurementValue.java b/sentry/src/main/java/io/sentry/profilemeasurements/ProfileMeasurementValue.java index f4b863a2f5..22d48b4a38 100644 --- a/sentry/src/main/java/io/sentry/profilemeasurements/ProfileMeasurementValue.java +++ b/sentry/src/main/java/io/sentry/profilemeasurements/ProfileMeasurementValue.java @@ -26,7 +26,7 @@ public ProfileMeasurementValue() { this(0L, 0); } - public ProfileMeasurementValue(@NotNull Long relativeStartNs, @NotNull Number value) { + public ProfileMeasurementValue(final @NotNull Long relativeStartNs, final @NotNull Number value) { this.relativeStartNs = relativeStartNs; this.value = value.toString(); } @@ -54,7 +54,7 @@ public static final class JsonKeys { } @Override - public void serialize(@NotNull JsonObjectWriter writer, @NotNull ILogger logger) + public void serialize(final @NotNull JsonObjectWriter writer, final @NotNull ILogger logger) throws IOException { writer.beginObject(); writer.name(JsonKeys.VALUE).value(logger, value); @@ -76,7 +76,7 @@ public Map getUnknown() { } @Override - public void setUnknown(@Nullable Map unknown) { + public void setUnknown(final @Nullable Map unknown) { this.unknown = unknown; } @@ -84,7 +84,7 @@ public static final class Deserializer implements JsonDeserializer unknown = null; From ace3c67e298dcadddd632b804722650bfc4e3e94 Mon Sep 17 00:00:00 2001 From: Stefano Date: Tue, 15 Nov 2022 18:59:50 +0100 Subject: [PATCH 16/20] Remove profiler main thread io (#2348) * removed useless file exist check in AndroidTransactionProfiler * wrapped AndroidTransactionProfiler methods in executor service * fixed crash in sample app's ProfilingActivity * updated ui tests --- CHANGELOG.md | 1 + .../core/AndroidTransactionProfiler.java | 29 +++++----- .../core/AndroidTransactionProfilerTest.kt | 33 +++++++++--- .../io/sentry/uitest/android/EnvelopeTests.kt | 34 ++++++------ .../samples/android/ProfilingActivity.kt | 54 +++++++++++-------- 5 files changed, 91 insertions(+), 60 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7487b7ac6b..313a2ffb75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixes - Fix `Gpu.vendorId` should be a String ([#2343](https://github.com/getsentry/sentry-java/pull/2343)) +- Remove profiler main thread io ([#2348](https://github.com/getsentry/sentry-java/pull/2348)) ### Features diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java index 0e588fa122..ced0d976e9 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java @@ -139,9 +139,13 @@ private void init() { traceFilesDir = new File(tracesFilesDirPath); } - @SuppressLint("NewApi") @Override public synchronized void onTransactionStart(final @NotNull ITransaction transaction) { + options.getExecutorService().submit(() -> onTransactionStartSafe(transaction)); + } + + @SuppressLint("NewApi") + private void onTransactionStartSafe(final @NotNull ITransaction transaction) { // Debug.startMethodTracingSampling() is only available since Lollipop if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.LOLLIPOP) return; @@ -151,24 +155,14 @@ public synchronized void onTransactionStart(final @NotNull ITransaction transact // traceFilesDir is null or intervalUs is 0 only if there was a problem in the init, but // we already logged that - if (traceFilesDir == null || intervalUs == 0 || !traceFilesDir.exists()) { + if (traceFilesDir == null || intervalUs == 0 || !traceFilesDir.canWrite()) { return; } transactionsCounter++; // When the first transaction is starting, we can start profiling if (transactionsCounter == 1) { - - traceFile = new File(traceFilesDir, UUID.randomUUID() + ".trace"); - - if (traceFile.exists()) { - options - .getLogger() - .log(SentryLevel.DEBUG, "Trace file already exists: %s", traceFile.getPath()); - transactionsCounter--; - return; - } - onFirstTransactionStarted(transaction, traceFile); + onFirstTransactionStarted(transaction); } else { ProfilingTransactionData transactionData = new ProfilingTransactionData( @@ -186,8 +180,9 @@ public synchronized void onTransactionStart(final @NotNull ITransaction transact } @SuppressLint("NewApi") - private void onFirstTransactionStarted( - final @NotNull ITransaction transaction, final @NotNull File traceFile) { + private void onFirstTransactionStarted(final @NotNull ITransaction transaction) { + // We create a file with a uuid name, so no need to check if it already exists + traceFile = new File(traceFilesDir, UUID.randomUUID() + ".trace"); measurementsMap.clear(); screenFrameRateMeasurements.clear(); @@ -245,11 +240,11 @@ public void onFrameMetricCollected( @Override public synchronized void onTransactionFinish(final @NotNull ITransaction transaction) { - onTransactionFinish(transaction, false); + options.getExecutorService().submit(() -> onTransactionFinish(transaction, false)); } @SuppressLint("NewApi") - private synchronized void onTransactionFinish( + private void onTransactionFinish( final @NotNull ITransaction transaction, final boolean isTimeout) { // onTransactionStart() is only available since Lollipop diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt index fd0af55838..57dcb5448c 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt @@ -16,7 +16,6 @@ import io.sentry.assertEnvelopeItem import io.sentry.test.getCtor import org.junit.runner.RunWith import org.mockito.kotlin.any -import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.check import org.mockito.kotlin.mock import org.mockito.kotlin.never @@ -25,6 +24,8 @@ import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import java.io.File +import java.util.concurrent.Future +import java.util.concurrent.FutureTask import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test @@ -47,11 +48,25 @@ class AndroidTransactionProfilerTest { whenever(it.sdkInfoVersion).thenReturn(Build.VERSION_CODES.LOLLIPOP) } val mockLogger = mock() + var lastScheduledRunnable: Runnable? = null + val mockExecutorService = object : ISentryExecutorService { + override fun submit(runnable: Runnable): Future<*> { + runnable.run() + return FutureTask {} + } + override fun schedule(runnable: Runnable, delayMillis: Long): Future<*> { + lastScheduledRunnable = runnable + return FutureTask {} + } + override fun close(timeoutMillis: Long) {} + } + val options = spy(SentryAndroidOptions()).apply { dsn = mockDsn profilesSampleRate = 1.0 isDebug = true setLogger(mockLogger) + executorService = mockExecutorService } val hub: IHub = mock() @@ -246,6 +261,15 @@ class AndroidTransactionProfilerTest { assertNotNull(traceData) } + @Test + fun `profiler uses background threads`() { + val profiler = fixture.getSut(context) + fixture.options.executorService = mock() + profiler.onTransactionStart(fixture.transaction1) + profiler.onTransactionFinish(fixture.transaction1) + verify(fixture.hub, never()).captureEnvelope(any()) + } + @Test fun `onTransactionFinish works only if previously started`() { val profiler = fixture.getSut(context) @@ -257,16 +281,11 @@ class AndroidTransactionProfilerTest { fun `timedOutData has timeout truncation reason`() { val profiler = fixture.getSut(context) - val executorService = mock() - val captor = argumentCaptor() - whenever(executorService.schedule(captor.capture(), any())).thenReturn(null) - whenever(fixture.options.executorService).thenReturn(executorService) - // Start and finish first transaction profiling profiler.onTransactionStart(fixture.transaction1) // Set timed out data by calling the timeout scheduled job - captor.firstValue.run() + fixture.lastScheduledRunnable?.run() // First transaction finishes: timed out data is returned profiler.onTransactionFinish(fixture.transaction1) diff --git a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt index 491205ec45..e41336fe48 100644 --- a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt +++ b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt @@ -19,7 +19,7 @@ import java.io.File import java.util.concurrent.TimeUnit import kotlin.test.Test import kotlin.test.assertEquals -import kotlin.test.assertFails +import kotlin.test.assertFailsWith import kotlin.test.assertFalse import kotlin.test.assertNotNull import kotlin.test.assertTrue @@ -68,6 +68,11 @@ class EnvelopeTests : BaseUiTest() { it.assertNoOtherItems() assertEquals("ProfilingSampleActivity", transactionItem.transaction) } + assertEnvelope { + val transactionItem: SentryTransaction = it.assertItem() + it.assertNoOtherItems() + assertEquals("e2etests", transactionItem.transaction) + } assertEnvelope { val profilingTraceData: ProfilingTraceData = it.assertItem() it.assertNoOtherItems() @@ -98,11 +103,6 @@ class EnvelopeTests : BaseUiTest() { .firstOrNull { t -> t.id == transaction.eventId.toString() } assertNotNull(transactionData) } - assertEnvelope { - val transactionItem: SentryTransaction = it.assertItem() - it.assertNoOtherItems() - assertEquals("e2etests", transactionItem.transaction) - } assertNoOtherEnvelopes() assertNoOtherRequests() } @@ -138,6 +138,12 @@ class EnvelopeTests : BaseUiTest() { assertEquals(transaction2.eventId.toString(), transactionItem.eventId.toString()) } // The profile is sent only in the last transaction envelope + assertEnvelope { + val transactionItem: SentryTransaction = it.assertItem() + it.assertNoOtherItems() + assertEquals(transaction3.eventId.toString(), transactionItem.eventId.toString()) + } + // The profile is sent only in the last transaction envelope assertEnvelope { val profilingTraceData: ProfilingTraceData = it.assertItem() it.assertNoOtherItems() @@ -171,12 +177,6 @@ class EnvelopeTests : BaseUiTest() { // The first and last transactions should be aligned to the start/stop of profile assertEquals(endTimes.last() - startTimes.first(), profilingTraceData.durationNs.toLong()) } - // The profile is sent only in the last transaction envelope - assertEnvelope { - val transactionItem: SentryTransaction = it.assertItem() - it.assertNoOtherItems() - assertEquals(transaction3.eventId.toString(), transactionItem.eventId.toString()) - } assertNoOtherEnvelopes() assertNoOtherRequests() } @@ -202,16 +202,20 @@ class EnvelopeTests : BaseUiTest() { } }.start() transaction.finish() - finished = true + // The profiler is stopped in background on the executor service, so we can stop deleting the trace file + // only after the profiler is stopped. This means we have to stop the deletion in the executorService + Sentry.getCurrentHub().options.executorService.submit { + finished = true + } relay.assert { - // The profile failed to be sent. Trying to read the envelope from the data transmitted throws an exception - assertFails { assertEnvelope {} } assertEnvelope { val transactionItem: SentryTransaction = it.assertItem() it.assertNoOtherItems() assertEquals("e2etests", transactionItem.transaction) } + // The profile failed to be sent. Trying to read the envelope from the data transmitted throws an exception + assertFailsWith { assertEnvelope {} } assertNoOtherEnvelopes() assertNoOtherRequests() } diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/ProfilingActivity.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/ProfilingActivity.kt index 95ed3b0bd5..60def396b9 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/ProfilingActivity.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/ProfilingActivity.kt @@ -59,14 +59,20 @@ class ProfilingActivity : AppCompatActivity() { executors.submit { runMathOperations() } } executors.submit { swipeList() } - binding.profilingStart.postDelayed({ finishTransactionAndPrintResults(t) }, (seconds * 1000).toLong()) + + Thread { + Thread.sleep((seconds * 1000).toLong()) + finishTransactionAndPrintResults(t) + binding.root.post { + binding.profilingProgressBar.visibility = View.GONE + } + }.start() } setContentView(binding.root) } private fun finishTransactionAndPrintResults(t: ITransaction) { t.finish() - binding.profilingProgressBar.visibility = View.GONE profileFinished = true val profilesDirPath = Sentry.getCurrentHub().options.profilingTracesDirPath if (profilesDirPath == null) { @@ -82,25 +88,31 @@ class ProfilingActivity : AppCompatActivity() { Thread.sleep((timeout - duration).coerceAtLeast(0)) } - // Get the last trace file, which is the current profile - val origProfileFile = File(profilesDirPath).listFiles()?.maxByOrNull { f -> f.lastModified() } - // Create a new profile file and copy the content of the original file into it - val profile = File(cacheDir, UUID.randomUUID().toString()) - origProfileFile?.copyTo(profile) - - val profileLength = profile.length() - val traceData = ProfilingTraceData(profile, t) - // Create envelope item from copied profile - val item = - SentryEnvelopeItem.fromProfilingTrace(traceData, Long.MAX_VALUE, Sentry.getCurrentHub().options.serializer) - val itemData = item.data - - // Compress the envelope item using Gzip - val bos = ByteArrayOutputStream() - GZIPOutputStream(bos).bufferedWriter().use { it.write(String(itemData)) } - - binding.profilingResult.text = - getString(R.string.profiling_result, profileLength, itemData.size, bos.toByteArray().size) + try { + // Get the last trace file, which is the current profile + val origProfileFile = File(profilesDirPath).listFiles()?.maxByOrNull { f -> f.lastModified() } + // Create a new profile file and copy the content of the original file into it + val profile = File(cacheDir, UUID.randomUUID().toString()) + origProfileFile?.copyTo(profile) + + val profileLength = profile.length() + val traceData = ProfilingTraceData(profile, t) + // Create envelope item from copied profile + val item = + SentryEnvelopeItem.fromProfilingTrace(traceData, Long.MAX_VALUE, Sentry.getCurrentHub().options.serializer) + val itemData = item.data + + // Compress the envelope item using Gzip + val bos = ByteArrayOutputStream() + GZIPOutputStream(bos).bufferedWriter().use { it.write(String(itemData)) } + + binding.root.post { + binding.profilingResult.text = + getString(R.string.profiling_result, profileLength, itemData.size, bos.toByteArray().size) + } + } catch (e: Exception) { + e.printStackTrace() + } } private fun swipeList() { From 80a206d1b434cc906ac89d23927a996e640ed2d0 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Wed, 16 Nov 2022 11:22:10 +0100 Subject: [PATCH 17/20] updated test dependencies to stable versions --- buildSrc/src/main/java/Config.kt | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/buildSrc/src/main/java/Config.kt b/buildSrc/src/main/java/Config.kt index 6e8718b81c..20aee6be05 100644 --- a/buildSrc/src/main/java/Config.kt +++ b/buildSrc/src/main/java/Config.kt @@ -139,10 +139,8 @@ object Config { } object TestLibs { - // todo These rc versions are needed to run ui tests on Android 13. - // They will be replaced by stable version when 1.5.0/3.5.0 will be out. - private val androidxTestVersion = "1.5.0-rc01" - private val espressoVersion = "3.5.0-rc01" + private val androidxTestVersion = "1.5.0" + private val espressoVersion = "3.5.0" val androidJUnitRunner = "androidx.test.runner.AndroidJUnitRunner" val kotlinTestJunit = "org.jetbrains.kotlin:kotlin-test-junit:$kotlinVersion" From c2599c162207264c98d812ed23891d6df22496ac Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Wed, 16 Nov 2022 13:20:44 +0100 Subject: [PATCH 18/20] SentryFrameMetricsCollector start collecting frameMetrics in onActivityStarted(), not onActivityCreated() anymore --- .../util/SentryFrameMetricsCollector.java | 10 +++++---- .../util/SentryFrameMetricsCollectorTest.kt | 22 +++++++++---------- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollector.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollector.java index a8dbeca586..4c70f86c23 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollector.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollector.java @@ -94,13 +94,15 @@ public SentryFrameMetricsCollector( }; } + // addOnFrameMetricsAvailableListener internally calls Activity.getWindow().getDecorView(), + // which cannot be called before setContentView. That's why we call it in onActivityStarted() @Override - public void onActivityCreated(@NotNull Activity activity, @Nullable Bundle savedInstanceState) { - setCurrentWindow(activity.getWindow()); - } + public void onActivityCreated(@NotNull Activity activity, @Nullable Bundle savedInstanceState) {} @Override - public void onActivityStarted(@NotNull Activity activity) {} + public void onActivityStarted(@NotNull Activity activity) { + setCurrentWindow(activity.getWindow()); + } @Override public void onActivityResumed(@NotNull Activity activity) {} diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollectorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollectorTest.kt index 8d95724b28..3fa3e77184 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollectorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollectorTest.kt @@ -130,7 +130,7 @@ class SentryFrameMetricsCollectorTest { collector.startCollection(mock()) assertEquals(0, fixture.addOnFrameMetricsAvailableListenerCounter) - collector.onActivityCreated(fixture.activity, mock()) + collector.onActivityStarted(fixture.activity) assertEquals(1, fixture.addOnFrameMetricsAvailableListenerCounter) } @@ -139,7 +139,7 @@ class SentryFrameMetricsCollectorTest { val collector = fixture.getSut(context) collector.startCollection(mock()) - collector.onActivityCreated(fixture.activity, mock()) + collector.onActivityStarted(fixture.activity) assertEquals(0, fixture.removeOnFrameMetricsAvailableListenerCounter) collector.onActivityStopped(fixture.activity) assertEquals(1, fixture.removeOnFrameMetricsAvailableListenerCounter) @@ -151,7 +151,7 @@ class SentryFrameMetricsCollectorTest { assertEquals(0, fixture.removeOnFrameMetricsAvailableListenerCounter) assertEquals(0, fixture.addOnFrameMetricsAvailableListenerCounter) - collector.onActivityCreated(fixture.activity, mock()) + collector.onActivityStarted(fixture.activity) collector.onActivityStopped(fixture.activity) assertEquals(0, fixture.removeOnFrameMetricsAvailableListenerCounter) assertEquals(0, fixture.addOnFrameMetricsAvailableListenerCounter) @@ -161,7 +161,7 @@ class SentryFrameMetricsCollectorTest { fun `startCollection calls addOnFrameMetricsAvailableListener if an activity is already started`() { val collector = fixture.getSut(context) - collector.onActivityCreated(fixture.activity, mock()) + collector.onActivityStarted(fixture.activity) assertEquals(0, fixture.addOnFrameMetricsAvailableListenerCounter) collector.startCollection(mock()) assertEquals(1, fixture.addOnFrameMetricsAvailableListenerCounter) @@ -171,7 +171,7 @@ class SentryFrameMetricsCollectorTest { fun `stopCollection calls removeOnFrameMetricsAvailableListener even if an activity is still started`() { val collector = fixture.getSut(context) val id = collector.startCollection(mock()) - collector.onActivityCreated(fixture.activity, mock()) + collector.onActivityStarted(fixture.activity) assertEquals(0, fixture.removeOnFrameMetricsAvailableListenerCounter) collector.stopCollection(id) @@ -186,8 +186,8 @@ class SentryFrameMetricsCollectorTest { assertEquals(0, fixture.addOnFrameMetricsAvailableListenerCounter) assertEquals(0, fixture.removeOnFrameMetricsAvailableListenerCounter) - collector.onActivityCreated(fixture.activity, mock()) - collector.onActivityCreated(fixture.activity, mock()) + collector.onActivityStarted(fixture.activity) + collector.onActivityStarted(fixture.activity) collector.onActivityStopped(fixture.activity) collector.onActivityStopped(fixture.activity) @@ -200,7 +200,7 @@ class SentryFrameMetricsCollectorTest { fun `stopCollection works only after startCollection`() { val collector = fixture.getSut(context) collector.startCollection(mock()) - collector.onActivityCreated(fixture.activity, mock()) + collector.onActivityStarted(fixture.activity) collector.stopCollection("testId") assertEquals(0, fixture.removeOnFrameMetricsAvailableListenerCounter) } @@ -209,8 +209,8 @@ class SentryFrameMetricsCollectorTest { fun `collector tracks multiple activities`() { val collector = fixture.getSut(context) collector.startCollection(mock()) - collector.onActivityCreated(fixture.activity, mock()) - collector.onActivityCreated(fixture.activity2, mock()) + collector.onActivityStarted(fixture.activity) + collector.onActivityStarted(fixture.activity2) assertEquals(2, fixture.addOnFrameMetricsAvailableListenerCounter) collector.onActivityStopped(fixture.activity) collector.onActivityStopped(fixture.activity2) @@ -222,7 +222,7 @@ class SentryFrameMetricsCollectorTest { val collector = fixture.getSut(context) val id1 = collector.startCollection(mock()) val id2 = collector.startCollection(mock()) - collector.onActivityCreated(fixture.activity, mock()) + collector.onActivityStarted(fixture.activity) assertEquals(1, fixture.addOnFrameMetricsAvailableListenerCounter) collector.stopCollection(id1) assertEquals(0, fixture.removeOnFrameMetricsAvailableListenerCounter) From e6065156e44dc73b0032ed692667469268c9da4c Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Thu, 17 Nov 2022 19:11:19 +0100 Subject: [PATCH 19/20] increased benchmark cpu overhead threshold from 5% to 5.5% frameMetricsListener removal wrapped in a try catch block --- .sauce/sentry-uitest-android-benchmark.yml | 2 +- .../internal/util/SentryFrameMetricsCollector.java | 13 ++++++++++--- .../uitest/android/benchmark/SentryBenchmarkTest.kt | 2 +- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/.sauce/sentry-uitest-android-benchmark.yml b/.sauce/sentry-uitest-android-benchmark.yml index ca17d3659b..f136a967f9 100644 --- a/.sauce/sentry-uitest-android-benchmark.yml +++ b/.sauce/sentry-uitest-android-benchmark.yml @@ -32,7 +32,7 @@ suites: - name: "Android 10 (api 29)" devices: - - id: OnePlus_7T_real_us # OnePlus 7T - api 29 (10) + - id: Google_Pixel_4_XL_real_us1 # Google Pixel 4 XL - api 29 (10) - id: Nokia_7_1_real_us # Nokia 7.1 - api 29 (10) # At the time of writing (July, 4, 2022), the market share per android version is: diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollector.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollector.java index 4c70f86c23..76ad621771 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollector.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/SentryFrameMetricsCollector.java @@ -28,6 +28,7 @@ public final class SentryFrameMetricsCollector implements Application.ActivityLifecycleCallbacks { private final @NotNull BuildInfoProvider buildInfoProvider; private final @NotNull Set trackedWindows = new HashSet<>(); + private final @NotNull SentryOptions options; private @Nullable Handler handler; private @Nullable WeakReference currentWindow; private final @NotNull HashMap listenerMap = @@ -54,7 +55,7 @@ public SentryFrameMetricsCollector( final @NotNull BuildInfoProvider buildInfoProvider, final @NotNull WindowFrameMetricsManager windowFrameMetricsManager) { Objects.requireNonNull(context, "The context is required"); - Objects.requireNonNull(options, "SentryOptions is required"); + this.options = Objects.requireNonNull(options, "SentryOptions is required"); this.buildInfoProvider = Objects.requireNonNull(buildInfoProvider, "BuildInfoProvider is required"); this.windowFrameMetricsManager = @@ -151,8 +152,14 @@ private void clearCurrentWindow(final @NotNull Window window) { } if (trackedWindows.contains(window)) { if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.N) { - windowFrameMetricsManager.removeOnFrameMetricsAvailableListener( - window, frameMetricsAvailableListener); + try { + windowFrameMetricsManager.removeOnFrameMetricsAvailableListener( + window, frameMetricsAvailableListener); + } catch (Exception e) { + options + .getLogger() + .log(SentryLevel.ERROR, "Failed to remove frameMetricsAvailableListener", e); + } } trackedWindows.remove(window); } diff --git a/sentry-android-integration-tests/sentry-uitest-android-benchmark/src/androidTest/java/io/sentry/uitest/android/benchmark/SentryBenchmarkTest.kt b/sentry-android-integration-tests/sentry-uitest-android-benchmark/src/androidTest/java/io/sentry/uitest/android/benchmark/SentryBenchmarkTest.kt index 9c607be133..90fcca6d89 100644 --- a/sentry-android-integration-tests/sentry-uitest-android-benchmark/src/androidTest/java/io/sentry/uitest/android/benchmark/SentryBenchmarkTest.kt +++ b/sentry-android-integration-tests/sentry-uitest-android-benchmark/src/androidTest/java/io/sentry/uitest/android/benchmark/SentryBenchmarkTest.kt @@ -89,7 +89,7 @@ class SentryBenchmarkTest : BaseBenchmarkTest() { comparisonResult.printResults() // Currently we just want to assert the cpu overhead - assertTrue(comparisonResult.cpuTimeIncreasePercentage in 0F..5F) + assertTrue(comparisonResult.cpuTimeIncreasePercentage in 0F..5.5F) } /** From bf05cd0e3aff822f2fb740f9d1c4cec7b4f96692 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Thu, 17 Nov 2022 19:13:47 +0100 Subject: [PATCH 20/20] updated changelog --- CHANGELOG.md | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 16e869fa26..3ee6a1d584 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,18 +1,26 @@ # Changelog +## Unreleased + +### Fixes + +- Remove profiler main thread io ([#2348](https://github.com/getsentry/sentry-java/pull/2348)) + +### Features + +- Add FrameMetrics to Android profiling data ([#2342](https://github.com/getsentry/sentry-java/pull/2342)) + ## 6.7.1 ### Fixes - Fix `Gpu.vendorId` should be a String ([#2343](https://github.com/getsentry/sentry-java/pull/2343)) -- Remove profiler main thread io ([#2348](https://github.com/getsentry/sentry-java/pull/2348)) - Don't set device name on Android if `sendDefaultPii` is disabled ([#2354](https://github.com/getsentry/sentry-java/pull/2354)) - Fix corrupted UUID on Motorola devices ([#2363](https://github.com/getsentry/sentry-java/pull/2363)) - Fix ANR on dropped uncaught exception events ([#2368](https://github.com/getsentry/sentry-java/pull/2368)) ### Features -- Add FrameMetrics to Android profiling data ([#2342](https://github.com/getsentry/sentry-java/pull/2342)) - Don't set device name on Android if `sendDefaultPii` is disabled ([#2354](https://github.com/getsentry/sentry-java/pull/2354)) - Fix corrupted UUID on Motorola devices ([#2363](https://github.com/getsentry/sentry-java/pull/2363)) - Update Spring Boot Jakarta to Spring Boot 3.0.0-RC2 ([#2347](https://github.com/getsentry/sentry-java/pull/2347))