Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Handle App Start Continuous Profiling v8 (p4) #3730

Merged
merged 5 commits into from
Nov 14, 2024
Merged

Handle App Start Continuous Profiling v8 (p4) #3730

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ffc470f
create app start continuous profiler instead of transaction profiler,…
stefanosiano Sep 30, 2024
391485f
cut profiler setup out in a specific function to improve readability …
stefanosiano Nov 13, 2024
089f636
Merge branch 'feat/continuous-profiling-part3' into feat/continuous-p…
stefanosiano Nov 13, 2024
b7e889c
merged p3
stefanosiano Nov 13, 2024
540e276
Add new APIs for Continuous Profiling v8 (p5) (#3844)
stefanosiano Nov 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions sentry-android-core/api/sentry-android-core.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,7 @@ public class io/sentry/android/core/performance/AppStartMetrics : io/sentry/andr
public fun addActivityLifecycleTimeSpans (Lio/sentry/android/core/performance/ActivityLifecycleTimeSpan;)V
public fun clear ()V
public fun getActivityLifecycleTimeSpans ()Ljava/util/List;
public fun getAppStartContinuousProfiler ()Lio/sentry/IContinuousProfiler;
public fun getAppStartProfiler ()Lio/sentry/ITransactionProfiler;
public fun getAppStartSamplingDecision ()Lio/sentry/TracesSamplingDecision;
public fun getAppStartTimeSpan ()Lio/sentry/android/core/performance/TimeSpan;
Expand All @@ -463,6 +464,7 @@ public class io/sentry/android/core/performance/AppStartMetrics : io/sentry/andr
public static fun onContentProviderPostCreate (Landroid/content/ContentProvider;)V
public fun registerApplicationForegroundCheck (Landroid/app/Application;)V
public fun setAppLaunchedInForeground (Z)V
public fun setAppStartContinuousProfiler (Lio/sentry/IContinuousProfiler;)V
public fun setAppStartProfiler (Lio/sentry/ITransactionProfiler;)V
public fun setAppStartSamplingDecision (Lio/sentry/TracesSamplingDecision;)V
public fun setAppStartType (Lio/sentry/android/core/performance/AppStartMetrics$AppStartType;)V
Expand Down
81 changes: 50 additions & 31 deletions sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import android.content.pm.PackageInfo;
import io.sentry.DeduplicateMultithreadedEventProcessor;
import io.sentry.DefaultTransactionPerformanceCollector;
import io.sentry.IContinuousProfiler;
import io.sentry.ILogger;
import io.sentry.ITransactionProfiler;
import io.sentry.NoOpConnectionStatusProvider;
Expand Down Expand Up @@ -160,41 +161,59 @@ static void initializeIntegrationsAndProcessors(
options.addEventProcessor(new AnrV2EventProcessor(context, options, buildInfoProvider));
options.setTransportGate(new AndroidTransportGate(options));

if (options.isProfilingEnabled()) {
// Check if the profiler was already instantiated in the app start.
// We use the Android profiler, that uses a global start/stop api, so we need to preserve the
// state of the profiler, and it's only possible retaining the instance.
final @NotNull AppStartMetrics appStartMetrics = AppStartMetrics.getInstance();
final @Nullable ITransactionProfiler appStartTransactionProfiler;
final @Nullable IContinuousProfiler appStartContinuousProfiler;
synchronized (appStartMetrics) {
appStartTransactionProfiler = appStartMetrics.getAppStartProfiler();
appStartContinuousProfiler = appStartMetrics.getAppStartContinuousProfiler();
appStartMetrics.setAppStartProfiler(null);
appStartMetrics.setAppStartContinuousProfiler(null);
}

if (options.isProfilingEnabled() || options.getProfilesSampleRate() != null) {
options.setContinuousProfiler(NoOpContinuousProfiler.getInstance());
stefanosiano marked this conversation as resolved.
Show resolved Hide resolved
// Check if the profiler was already instantiated in the app start.
// We use the Android profiler, that uses a global start/stop api, so we need to preserve the
// state of the profiler, and it's only possible retaining the instance.
synchronized (AppStartMetrics.getInstance()) {
final @Nullable ITransactionProfiler appStartProfiler =
AppStartMetrics.getInstance().getAppStartProfiler();
if (appStartProfiler != null) {
options.setTransactionProfiler(appStartProfiler);
AppStartMetrics.getInstance().setAppStartProfiler(null);
} else {
options.setTransactionProfiler(
new AndroidTransactionProfiler(
context,
options,
buildInfoProvider,
Objects.requireNonNull(
options.getFrameMetricsCollector(),
"options.getFrameMetricsCollector is required")));
}
// This is a safeguard, but it should never happen, as the app start profiler should be the
// continuous one.
if (appStartContinuousProfiler != null) {
appStartContinuousProfiler.close();
}
if (appStartTransactionProfiler != null) {
options.setTransactionProfiler(appStartTransactionProfiler);
} else {
options.setTransactionProfiler(
new AndroidTransactionProfiler(
context,
options,
buildInfoProvider,
Objects.requireNonNull(
options.getFrameMetricsCollector(),
"options.getFrameMetricsCollector is required")));
}
} else {
options.setTransactionProfiler(NoOpTransactionProfiler.getInstance());
// todo handle app start continuous profiler
options.setContinuousProfiler(
new AndroidContinuousProfiler(
buildInfoProvider,
Objects.requireNonNull(
options.getFrameMetricsCollector(),
"options.getFrameMetricsCollector is required"),
options.getLogger(),
options.getProfilingTracesDirPath(),
options.getProfilingTracesHz(),
options.getExecutorService()));
// This is a safeguard, but it should never happen, as the app start profiler should be the
// transaction one.
if (appStartTransactionProfiler != null) {
appStartTransactionProfiler.close();
}
if (appStartContinuousProfiler != null) {
options.setContinuousProfiler(appStartContinuousProfiler);
} else {
options.setContinuousProfiler(
new AndroidContinuousProfiler(
buildInfoProvider,
Objects.requireNonNull(
options.getFrameMetricsCollector(),
"options.getFrameMetricsCollector is required"),
options.getLogger(),
options.getProfilingTracesDirPath(),
options.getProfilingTracesHz(),
options.getExecutorService()));
}
}

options.setModulesLoader(new AssetsModulesLoader(context, options.getLogger()));
Expand Down
95 changes: 66 additions & 29 deletions sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import android.os.Process;
import android.os.SystemClock;
import androidx.annotation.NonNull;
import io.sentry.IContinuousProfiler;
import io.sentry.ILogger;
import io.sentry.ITransactionProfiler;
import io.sentry.JsonSerializer;
Expand Down Expand Up @@ -98,6 +99,11 @@ public void shutdown() {
if (appStartProfiler != null) {
appStartProfiler.close();
}
final @Nullable IContinuousProfiler appStartContinuousProfiler =
Copy link
Member

Choose a reason for hiding this comment

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

as I understood there could be only one profiler at a time (cont. or tx-based), right? Just got an idea we could have a general IProfiler interface (that would have all the methods that IContinuousProfiler currently has), and then it'd have a sub-interface ITransactionProfiler with the tx-specific methods.

This way you could only manage one instance of IProfiler everywhere (like here) and it would be more readable I guess.

Copy link
Member Author

Choose a reason for hiding this comment

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

you're probably right on this, but in the long term we will deprecate and optionally remove the transaction profiler.
I think the extraction of the AndroidProfiler is enough for now.
But we can refactor it if you have a strong preference

AppStartMetrics.getInstance().getAppStartContinuousProfiler();
if (appStartContinuousProfiler != null) {
appStartContinuousProfiler.close();
}
}
}

Expand Down Expand Up @@ -135,41 +141,18 @@ private void launchAppStartProfiler(final @NotNull AppStartMetrics appStartMetri
return;
}

if (profilingOptions.isContinuousProfilingEnabled()) {
createAndStartContinuousProfiler(context, profilingOptions, appStartMetrics);
return;
}

if (!profilingOptions.isProfilingEnabled()) {
logger.log(
SentryLevel.INFO, "Profiling is not enabled. App start profiling will not start.");
return;
}

final @NotNull TracesSamplingDecision appStartSamplingDecision =
new TracesSamplingDecision(
profilingOptions.isTraceSampled(),
profilingOptions.getTraceSampleRate(),
profilingOptions.isProfileSampled(),
profilingOptions.getProfileSampleRate());
// We store any sampling decision, so we can respect it when the first transaction starts
appStartMetrics.setAppStartSamplingDecision(appStartSamplingDecision);

if (!(appStartSamplingDecision.getProfileSampled()
&& appStartSamplingDecision.getSampled())) {
logger.log(SentryLevel.DEBUG, "App start profiling was not sampled. It will not start.");
return;
}
logger.log(SentryLevel.DEBUG, "App start profiling started.");

final @NotNull ITransactionProfiler appStartProfiler =
new AndroidTransactionProfiler(
context.getApplicationContext(),
buildInfoProvider,
new SentryFrameMetricsCollector(
context.getApplicationContext(), logger, buildInfoProvider),
logger,
profilingOptions.getProfilingTracesDirPath(),
profilingOptions.isProfilingEnabled(),
profilingOptions.getProfilingTracesHz(),
new SentryExecutorService());
appStartMetrics.setAppStartProfiler(appStartProfiler);
appStartProfiler.start();
createAndStartTransactionProfiler(context, profilingOptions, appStartMetrics);

} catch (FileNotFoundException e) {
logger.log(SentryLevel.ERROR, "App start profiling config file not found. ", e);
Expand All @@ -178,6 +161,60 @@ private void launchAppStartProfiler(final @NotNull AppStartMetrics appStartMetri
}
}

private void createAndStartContinuousProfiler(
final @NotNull Context context,
final @NotNull SentryAppStartProfilingOptions profilingOptions,
final @NotNull AppStartMetrics appStartMetrics) {
final @NotNull IContinuousProfiler appStartContinuousProfiler =
new AndroidContinuousProfiler(
buildInfoProvider,
new SentryFrameMetricsCollector(
context.getApplicationContext(), logger, buildInfoProvider),
logger,
profilingOptions.getProfilingTracesDirPath(),
profilingOptions.getProfilingTracesHz(),
new SentryExecutorService());
appStartMetrics.setAppStartProfiler(null);
appStartMetrics.setAppStartContinuousProfiler(appStartContinuousProfiler);
logger.log(SentryLevel.DEBUG, "App start continuous profiling started.");
appStartContinuousProfiler.start();
}

private void createAndStartTransactionProfiler(
final @NotNull Context context,
final @NotNull SentryAppStartProfilingOptions profilingOptions,
final @NotNull AppStartMetrics appStartMetrics) {
final @NotNull TracesSamplingDecision appStartSamplingDecision =
new TracesSamplingDecision(
profilingOptions.isTraceSampled(),
profilingOptions.getTraceSampleRate(),
profilingOptions.isProfileSampled(),
profilingOptions.getProfileSampleRate());
// We store any sampling decision, so we can respect it when the first transaction starts
appStartMetrics.setAppStartSamplingDecision(appStartSamplingDecision);

if (!(appStartSamplingDecision.getProfileSampled() && appStartSamplingDecision.getSampled())) {
logger.log(SentryLevel.DEBUG, "App start profiling was not sampled. It will not start.");
return;
}

final @NotNull ITransactionProfiler appStartProfiler =
new AndroidTransactionProfiler(
context.getApplicationContext(),
buildInfoProvider,
new SentryFrameMetricsCollector(
context.getApplicationContext(), logger, buildInfoProvider),
logger,
profilingOptions.getProfilingTracesDirPath(),
profilingOptions.isProfilingEnabled(),
profilingOptions.getProfilingTracesHz(),
new SentryExecutorService());
appStartMetrics.setAppStartContinuousProfiler(null);
appStartMetrics.setAppStartProfiler(appStartProfiler);
logger.log(SentryLevel.DEBUG, "App start profiling started.");
appStartProfiler.start();
}

@SuppressLint("NewApi")
private void onAppLaunched(
final @Nullable Context context, final @NotNull AppStartMetrics appStartMetrics) {
Expand Down
21 changes: 20 additions & 1 deletion sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting;
import io.sentry.IContinuousProfiler;
import io.sentry.ITransactionProfiler;
import io.sentry.SentryDate;
import io.sentry.SentryNanotimeDate;
Expand Down Expand Up @@ -53,6 +54,7 @@ public enum AppStartType {
private final @NotNull Map<ContentProvider, TimeSpan> contentProviderOnCreates;
private final @NotNull List<ActivityLifecycleTimeSpan> activityLifecycles;
private @Nullable ITransactionProfiler appStartProfiler = null;
private @Nullable IContinuousProfiler appStartContinuousProfiler = null;
private @Nullable TracesSamplingDecision appStartSamplingDecision = null;
private @Nullable SentryDate onCreateTime = null;
private boolean appLaunchTooLong = false;
Expand Down Expand Up @@ -183,6 +185,10 @@ public void clear() {
appStartProfiler.close();
}
appStartProfiler = null;
if (appStartContinuousProfiler != null) {
appStartContinuousProfiler.close();
}
appStartContinuousProfiler = null;
appStartSamplingDecision = null;
appLaunchTooLong = false;
appLaunchedInForeground = false;
Expand All @@ -198,6 +204,15 @@ public void setAppStartProfiler(final @Nullable ITransactionProfiler appStartPro
this.appStartProfiler = appStartProfiler;
}

public @Nullable IContinuousProfiler getAppStartContinuousProfiler() {
return appStartContinuousProfiler;
}

public void setAppStartContinuousProfiler(
final @Nullable IContinuousProfiler appStartContinuousProfiler) {
this.appStartContinuousProfiler = appStartContinuousProfiler;
}

public void setAppStartSamplingDecision(
final @Nullable TracesSamplingDecision appStartSamplingDecision) {
this.appStartSamplingDecision = appStartSamplingDecision;
Expand Down Expand Up @@ -256,11 +271,15 @@ private void checkCreateTimeOnMain(final @NotNull Application application) {
if (onCreateTime == null) {
appLaunchedInForeground = false;

// we stop the app start profiler, as it's useless and likely to timeout
// we stop the app start profilers, as they are useless and likely to timeout
if (appStartProfiler != null && appStartProfiler.isRunning()) {
appStartProfiler.close();
appStartProfiler = null;
}
if (appStartContinuousProfiler != null && appStartContinuousProfiler.isRunning()) {
appStartContinuousProfiler.close();
appStartContinuousProfiler = null;
}
}
application.unregisterActivityLifecycleCallbacks(instance);
});
Expand Down
Loading