Startup Profiling 2 - Add options and sampling logic #3121

stefanosiano · 2023-12-29T10:08:01Z

📜 Description

followup of #3101
added SentryStartupProfilingOptions class with Json ser/deser
added startupProfilingConfigFile deletion and creation on init
added sampling decision on SDK init with isForNextStartup flag set to true
added SentryOptions.getCacheDirPathWithoutDsn for startupProfiling config file

Here is the decision tree.
The only missing part is actually starting the profiler and bind it to the first transaction, which will come in the next PR.
Note: the auto instrumentation flag is available on SentryAndroidOptions, so it will be evaluated (in the next PR) before starting the startup profiler

💡 Motivation and Context

For the startup profiling feature, we need to add an option to enable it and we also need to respect the sampling decisions of the user.
Then, we need to save this info to disk, as the startup profiler will not have access to the SentryOptions object, as it will start before the SDK is initialized.

💚 How did you test it?

Unit tests

📝 Checklist

I reviewed the submitted code.
I added tests to verify the changes.
No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
I updated the docs if needed.
Review from the native team if needed.
No breaking change or entry added to the changelog.
No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

added SentryOptions.enableStartupProfiling

added startupProfilingConfigFile deletion and creation on init added sampling decision on SDK init with isForNextStartup flag set to true added SentryOptions.getCacheDirPathWithoutDsn for startupProfiling config file

…to feat/startup-profiling2-add-options # Conflicts: # sentry/src/main/java/io/sentry/SentryOptions.java # sentry/src/test/java/io/sentry/SentryOptionsTest.kt # sentry/src/test/java/io/sentry/SentryTest.kt

github-actions · 2023-12-29T10:15:16Z

	Messages
📖	Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 4aa19d2

github-actions · 2023-12-29T10:26:58Z

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	379.72 ms	457.96 ms	78.24 ms
Size	1.70 MiB	2.27 MiB	584.06 KiB

Baseline results on branch: feat/early-profiling1-decouple-profiler-transaction

Startup times

Revision	Plain	With Sentry	Diff
`e42754c`	382.06 ms	465.49 ms	83.43 ms
`604628a`	389.84 ms	456.78 ms	66.94 ms
`1da0e4d`	375.46 ms	448.53 ms	73.07 ms

App size

Revision	Plain	With Sentry	Diff
`e42754c`	1.72 MiB	2.27 MiB	557.10 KiB
`604628a`	1.72 MiB	2.27 MiB	558.57 KiB
`1da0e4d`	1.72 MiB	2.27 MiB	558.36 KiB

Previous results on branch: feat/startup-profiling2-add-options

Startup times

Revision	Plain	With Sentry	Diff
`f19450a`	394.36 ms	473.10 ms	78.74 ms
`47df090`	386.30 ms	467.52 ms	81.22 ms

App size

Revision	Plain	With Sentry	Diff
`f19450a`	1.70 MiB	2.27 MiB	583.38 KiB
`47df090`	1.72 MiB	2.27 MiB	561.22 KiB

romtsn · 2024-01-11T11:07:30Z

sentry/src/main/java/io/sentry/SentryStartupProfilingOptions.java

+
+final class SentryStartupProfilingOptions implements JsonUnknown, JsonSerializable {
+
+  boolean profileSampled;


should these be final?

nope, as these are read and deserialized from json (L100)

sentry/src/main/java/io/sentry/Sentry.java

romtsn · 2024-01-11T12:44:04Z

sentry/src/main/java/io/sentry/Sentry.java

+                    sampleStartupProfiling(options);
+                final @NotNull SentryStartupProfilingOptions startupProfilingOptions =
+                    new SentryStartupProfilingOptions(options, startupSamplingDecision);
+                try (Writer fileWriter = new SentryFileWriter(startupProfilingConfigFile)) {


Suggested change

try (Writer fileWriter = new SentryFileWriter(startupProfilingConfigFile)) {

try (Writer fileWriter = new FileWriter(startupProfilingConfigFile)) {

SentryFileWriter is specific for performance instrumentation, I guess we don't want to create spans for our own file operations :)

Also, let's use FileOutputStream maybe, to align with all other places where we write to a file?

sentry/src/main/java/io/sentry/Sentry.java

romtsn · 2024-01-11T12:51:42Z

CHANGELOG.md

@@ -6,6 +6,7 @@

 - Handle `monitor`/`check_in` in client reports and rate limiter ([#3096](https://github.com/getsentry/sentry-java/pull/3096))
 - Startup profiling 1 - Decouple Profiler from Transaction ([#3101](https://github.com/getsentry/sentry-java/pull/3101))
+- Startup profiling 2 - Add options and sampling logic ([#3121](https://github.com/getsentry/sentry-java/pull/3121))


As mentioned in the other PR, maybe let's squash these changelogs into one?

romtsn

A few points, but LGTM otherwise

used FileOutputStream instead for FileWriter

* first activity transaction now inherits startup sampling decision, if available * if a startup profiler was instantiated, it will be reused in AndroidOptionsInitializer, instead of creating a new one * added ITransactionProfiler.isRunning * startup profiler and sampling decision is stored in AppStartMetrics * startup profile is bound to the startup transaction * added io.sentry.profiling.enable-startup manifest option * moved profilingTracesHz from SentryAndroidOptions to SentryOptions * added startup profiling launch in SentryPerformanceProvider * added isStartupTransaction to TransactionOptions

stefanosiano · 2024-01-15T10:27:50Z

@romtsn may you have a last look here and here, as well, before merging? 😄

romtsn · 2024-01-15T10:35:51Z

@romtsn may you have a last look here and here, as well, before merging? 😄

yeah looks great, thanks for the thorough changelog!

* Startup Profiling 1 - Decouple Profiler from Transaction (#3101) * removed the need of a transaction to start a profile in AndroidTransactionProfiler, allowing to bind one later * added a constructor to AndroidTransactionProfiler without options * added options as param to AndroidTransactionProfiler.onTransactionFinish * Startup Profiling 2 - Add options and sampling logic (#3121) * added TransactionContext.isForNextStartup * added SentryOptions.enableStartupProfiling * added SentryStartupProfilingOptions class with Json ser/deser * added startupProfilingConfigFile deletion and creation on init * added sampling decision on SDK init with isForNextStartup flag set to true * added SentryOptions.getCacheDirPathWithoutDsn for startupProfiling config file * Startup Profiling 3 - Add ContentProvider and start profile (#3128) * first activity transaction now inherits startup sampling decision, if available * if a startup profiler was instantiated, it will be reused in AndroidOptionsInitializer, instead of creating a new one * added ITransactionProfiler.isRunning * startup profiler and sampling decision is stored in AppStartMetrics * startup profile is bound to the startup transaction * added io.sentry.profiling.enable-startup manifest option * moved profilingTracesHz from SentryAndroidOptions to SentryOptions * added startup profiling launch in SentryPerformanceProvider * added isStartupTransaction to TransactionOptions

stefanosiano added 4 commits December 21, 2023 17:08

added TransactionContext.isForNextStartup

ed69adc

added SentryOptions.enableStartupProfiling

added SentryStartupProfilingOptions class with Json ser/deser

3f7d206

added startupProfilingConfigFile deletion and creation on init added sampling decision on SDK init with isForNextStartup flag set to true added SentryOptions.getCacheDirPathWithoutDsn for startupProfiling config file

Merge branch 'feat/early-profiling1-decouple-profiler-transaction' in…

1727f76

…to feat/startup-profiling2-add-options # Conflicts: # sentry/src/main/java/io/sentry/SentryOptions.java # sentry/src/test/java/io/sentry/SentryOptionsTest.kt # sentry/src/test/java/io/sentry/SentryTest.kt

updated changelog

a44b5e3

stefanosiano marked this pull request as ready for review December 29, 2023 10:14

stefanosiano requested review from adinauer, romtsn and markushi as code owners December 29, 2023 10:14

bruno-garcia mentioned this pull request Jan 4, 2024

Profiling for process startup. getsentry/sentry-dotnet#3003

Open

romtsn reviewed Jan 11, 2024

View reviewed changes

sentry/src/main/java/io/sentry/Sentry.java Outdated Show resolved Hide resolved

romtsn reviewed Jan 11, 2024

View reviewed changes

sentry/src/main/java/io/sentry/Sentry.java Outdated Show resolved Hide resolved

romtsn reviewed Jan 11, 2024

View reviewed changes

sentry/src/main/java/io/sentry/Sentry.java Outdated Show resolved Hide resolved

romtsn reviewed Jan 11, 2024

View reviewed changes

romtsn approved these changes Jan 11, 2024

View reviewed changes

stefanosiano added 3 commits January 11, 2024 15:36

put startup config deletion inside try catch

c4b79d2

used FileOutputStream instead for FileWriter

updated changelog

4aa19d2

stefanosiano merged commit 02f6aa7 into feat/early-profiling1-decouple-profiler-transaction Jan 15, 2024
15 checks passed

stefanosiano deleted the feat/startup-profiling2-add-options branch January 15, 2024 12:07

sdzhong mentioned this pull request Jan 30, 2024

Add Startup Profiling to EmpowerPlantActivity ui.load Transaction sentry-demos/android#126

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Startup Profiling 2 - Add options and sampling logic #3121

Startup Profiling 2 - Add options and sampling logic #3121

stefanosiano commented Dec 29, 2023 •

edited

Loading

github-actions bot commented Dec 29, 2023 •

edited

Loading

github-actions bot commented Dec 29, 2023 •

edited

Loading

Baseline results on branch: feat/early-profiling1-decouple-profiler-transaction

Startup times

App size

Previous results on branch: feat/startup-profiling2-add-options

Startup times

App size

romtsn Jan 11, 2024

stefanosiano Jan 11, 2024

romtsn Jan 11, 2024

romtsn Jan 11, 2024

romtsn Jan 11, 2024

romtsn left a comment

stefanosiano commented Jan 15, 2024

romtsn commented Jan 15, 2024


		final class SentryStartupProfilingOptions implements JsonUnknown, JsonSerializable {

		boolean profileSampled;

	try (Writer fileWriter = new SentryFileWriter(startupProfilingConfigFile)) {
	try (Writer fileWriter = new FileWriter(startupProfilingConfigFile)) {

Startup Profiling 2 - Add options and sampling logic #3121

Startup Profiling 2 - Add options and sampling logic #3121

Conversation

stefanosiano commented Dec 29, 2023 • edited Loading

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

🔮 Next steps

github-actions bot commented Dec 29, 2023 • edited Loading

github-actions bot commented Dec 29, 2023 • edited Loading

Performance metrics 🚀

Baseline results on branch: feat/early-profiling1-decouple-profiler-transaction

Startup times

App size

Previous results on branch: feat/startup-profiling2-add-options

Startup times

App size

romtsn Jan 11, 2024

Choose a reason for hiding this comment

stefanosiano Jan 11, 2024

Choose a reason for hiding this comment

romtsn Jan 11, 2024

Choose a reason for hiding this comment

romtsn Jan 11, 2024

Choose a reason for hiding this comment

romtsn Jan 11, 2024

Choose a reason for hiding this comment

romtsn left a comment

Choose a reason for hiding this comment

stefanosiano commented Jan 15, 2024

romtsn commented Jan 15, 2024

stefanosiano commented Dec 29, 2023 •

edited

Loading

github-actions bot commented Dec 29, 2023 •

edited

Loading

github-actions bot commented Dec 29, 2023 •

edited

Loading