-
-
Notifications
You must be signed in to change notification settings - Fork 468
feat(profiling): Add Android ProfilingManager (Perfetto) support #5251
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.
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
Open
43jay
wants to merge
19
commits into
main
Choose a base branch
from
claude/dreamy-solomon
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
9a60ad1
feat(profiling): Add useProfilingManager option
43jay 1250aa1
chore(samples): Update sample app to support Perfetto profiling testing
43jay 05d73b1
chore(samples): Improve ProfilingActivity UI for Perfetto testing
43jay 5f894d0
feat(profiling): Skip legacy app-start profiling when useProfilingManโฆ
43jay c2a4442
feat(profiling): Add PerfettoProfiler and wire into AndroidContinuousโฆ
43jay bc2e59d
fix(profiling): Fix meta_length not serialized in Perfetto envelope hโฆ
43jay a53c383
ref(profiling): Extract PerfettoContinuousProfiler from AndroidContinโฆ
43jay dd2c5a4
test(profiling): Extract shared profiler test cases into ContinuousPrโฆ
43jay 6ebea19
test(profiling): Wire PerfettoContinuousProfilerTest to shared test cโฆ
43jay 5e4bead
ref(profiling): Remove app-start profiling logic from PerfettoContinuโฆ
43jay 443cc45
ref(profiling): Improve thread safety in PerfettoContinuousProfiler
43jay dcee449
chore: Run spotlessApply and apiDump
43jay 331622b
ref(profiling): Move frame metrics collection from PerfettoProfiler tโฆ
43jay 85e54fc
ref(profiling): Consolidate measurement collection into ChunkMeasuremโฆ
43jay 11331ea
fix(profiling): Move shouldStop reset inside !isRunning() guard
43jay 4c48101
Format code
getsentry-bot 35b2bcb
fix(profiling): Add API level guard for PerfettoContinuousProfiler
43jay 268a1e0
fix(profiling): Snapshot frame measurement deques before async serialโฆ
43jay ffd5c6b
Format code
getsentry-bot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -68,13 +68,29 @@ public class AndroidContinuousProfiler | |
| private final AutoClosableReentrantLock lock = new AutoClosableReentrantLock(); | ||
| private final AutoClosableReentrantLock payloadLock = new AutoClosableReentrantLock(); | ||
|
|
||
| public AndroidContinuousProfiler( | ||
| public static AndroidContinuousProfiler createLegacy( | ||
| final @NotNull BuildInfoProvider buildInfoProvider, | ||
| final @NotNull SentryFrameMetricsCollector frameMetricsCollector, | ||
| final @NotNull ILogger logger, | ||
| final @Nullable String profilingTracesDirPath, | ||
| final int profilingTracesHz, | ||
| final @NotNull LazyEvaluator.Evaluator<ISentryExecutorService> executorServiceSupplier) { | ||
| return new AndroidContinuousProfiler( | ||
| buildInfoProvider, | ||
| frameMetricsCollector, | ||
| executorServiceSupplier, | ||
| logger, | ||
| profilingTracesHz, | ||
| profilingTracesDirPath); | ||
| } | ||
|
|
||
| private AndroidContinuousProfiler( | ||
| final @NotNull BuildInfoProvider buildInfoProvider, | ||
| final @NotNull SentryFrameMetricsCollector frameMetricsCollector, | ||
| final @NotNull LazyEvaluator.Evaluator<ISentryExecutorService> executorServiceSupplier, | ||
| final @NotNull ILogger logger, | ||
| final int profilingTracesHz, | ||
| final @Nullable String profilingTracesDirPath) { | ||
| this.logger = logger; | ||
| this.frameMetricsCollector = frameMetricsCollector; | ||
| this.buildInfoProvider = buildInfoProvider; | ||
|
|
@@ -89,6 +105,7 @@ private void init() { | |
| return; | ||
| } | ||
| isInitialized = true; | ||
|
|
||
| if (profilingTracesDirPath == null) { | ||
| logger.log( | ||
| SentryLevel.WARNING, | ||
|
|
@@ -152,21 +169,24 @@ public void startProfiler( | |
| } | ||
| } | ||
|
|
||
| private void initScopes() { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also don't think we need this change at all. Let's keep the legacy code as-is. |
||
| private void tryResolveScopes() { | ||
| if ((scopes == null || scopes == NoOpScopes.getInstance()) | ||
| && Sentry.getCurrentScopes() != NoOpScopes.getInstance()) { | ||
| this.scopes = Sentry.getCurrentScopes(); | ||
| this.performanceCollector = | ||
| Sentry.getCurrentScopes().getOptions().getCompositePerformanceCollector(); | ||
| final @Nullable RateLimiter rateLimiter = scopes.getRateLimiter(); | ||
| if (rateLimiter != null) { | ||
| rateLimiter.addRateLimitObserver(this); | ||
| } | ||
| onScopesAvailable(Sentry.getCurrentScopes()); | ||
| } | ||
| } | ||
|
|
||
| private void onScopesAvailable(final @NotNull IScopes resolvedScopes) { | ||
| this.scopes = resolvedScopes; | ||
| this.performanceCollector = resolvedScopes.getOptions().getCompositePerformanceCollector(); | ||
| final @Nullable RateLimiter rateLimiter = resolvedScopes.getRateLimiter(); | ||
| if (rateLimiter != null) { | ||
| rateLimiter.addRateLimitObserver(this); | ||
| } | ||
| } | ||
|
|
||
| private void start() { | ||
| initScopes(); | ||
| tryResolveScopes(); | ||
|
|
||
| // Debug.startMethodTracingSampling() is only available since Lollipop, but Android Profiler | ||
| // causes crashes on api 21 -> https://github.com/getsentry/sentry-java/issues/3392 | ||
|
|
@@ -259,7 +279,7 @@ public void stopProfiler(final @NotNull ProfileLifecycle profileLifecycle) { | |
| } | ||
|
|
||
| private void stop(final boolean restartProfiler) { | ||
| initScopes(); | ||
| tryResolveScopes(); | ||
| try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { | ||
| if (stopFuture != null) { | ||
| stopFuture.cancel(true); | ||
|
|
@@ -297,14 +317,15 @@ private void stop(final boolean restartProfiler) { | |
| // start profiling), meaning there's no scopes to send the chunks. In that case, we store | ||
| // the data in a list and send it when the next chunk is finished. | ||
| try (final @NotNull ISentryLifecycleToken ignored2 = payloadLock.acquire()) { | ||
| payloadBuilders.add( | ||
| final ProfileChunk.Builder builder = | ||
| new ProfileChunk.Builder( | ||
| profilerId, | ||
| chunkId, | ||
| endData.measurementsMap, | ||
| endData.traceFile, | ||
| startProfileChunkTimestamp, | ||
| ProfileChunk.PLATFORM_ANDROID)); | ||
| ProfileChunk.PLATFORM_ANDROID); | ||
| payloadBuilders.add(builder); | ||
| } | ||
| } | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need that, I'd keep the public constructor as-is instead. Instead let's add a comment to outline that this profiler is considered legacy.