feat(profiling): Add Android ProfilingManager (Perfetto) support#5251
Open
feat(profiling): Add Android ProfilingManager (Perfetto) support#5251
Conversation
Contributor
|
Contributor
Semver Impact of This PR🟡 Minor (new features) 📋 Changelog PreviewThis is how your changes will appear in the changelog. This PR will not appear in the changelog. 🤖 This preview updates automatically when you update the PR. |
📲 Install BuildsAndroid
|
Contributor
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| d15471f | 315.61 ms | 360.22 ms | 44.61 ms |
| 5b1a06b | 352.27 ms | 413.70 ms | 61.43 ms |
| b3d8889 | 371.69 ms | 432.96 ms | 61.26 ms |
| 5f14e5d | 325.76 ms | 368.32 ms | 42.56 ms |
| 85d7417 | 347.21 ms | 394.35 ms | 47.15 ms |
| b3d8889 | 420.46 ms | 453.71 ms | 33.26 ms |
| d15471f | 361.89 ms | 378.07 ms | 16.18 ms |
| 96449e8 | 361.30 ms | 423.39 ms | 62.09 ms |
| 694d587 | 379.62 ms | 400.80 ms | 21.18 ms |
| bb0ff41 | 321.00 ms | 378.28 ms | 57.28 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| d15471f | 1.58 MiB | 2.13 MiB | 559.54 KiB |
| 5b1a06b | 0 B | 0 B | 0 B |
| b3d8889 | 1.58 MiB | 2.10 MiB | 535.06 KiB |
| 5f14e5d | 1.58 MiB | 2.19 MiB | 620.00 KiB |
| 85d7417 | 1.58 MiB | 2.10 MiB | 533.44 KiB |
| b3d8889 | 1.58 MiB | 2.10 MiB | 535.07 KiB |
| d15471f | 1.58 MiB | 2.13 MiB | 559.54 KiB |
| 96449e8 | 1.58 MiB | 2.11 MiB | 539.35 KiB |
| 694d587 | 1.58 MiB | 2.19 MiB | 620.06 KiB |
| bb0ff41 | 0 B | 0 B | 0 B |
Previous results on branch: claude/dreamy-solomon
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 60b848d | 300.86 ms | 376.37 ms | 75.51 ms |
| c26f799 | 319.88 ms | 358.02 ms | 38.14 ms |
| 0db7260 | 329.68 ms | 375.67 ms | 45.99 ms |
| 7da193d | 319.39 ms | 375.24 ms | 55.85 ms |
| b3c0878 | 316.40 ms | 345.51 ms | 29.11 ms |
| fbe1e06 | 312.62 ms | 370.68 ms | 58.06 ms |
| cee9ee9 | 342.21 ms | 405.42 ms | 63.21 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 60b848d | 0 B | 0 B | 0 B |
| c26f799 | 0 B | 0 B | 0 B |
| 0db7260 | 0 B | 0 B | 0 B |
| 7da193d | 0 B | 0 B | 0 B |
| b3c0878 | 0 B | 0 B | 0 B |
| fbe1e06 | 0 B | 0 B | 0 B |
| cee9ee9 | 0 B | 0 B | 0 B |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Serialize uses field instead of getter for
meta_lengthSentryEnvelopeItemHeader.serialize()now usesgetMetaLength()(captured once in a local) so callable-backed Perfetto chunks correctly emitmeta_lengthin envelope headers.
Or push these changes by commenting:
@cursor push 56eb859503
Preview (56eb859503)
diff --git a/sentry/src/main/java/io/sentry/SentryEnvelopeItemHeader.java b/sentry/src/main/java/io/sentry/SentryEnvelopeItemHeader.java
--- a/sentry/src/main/java/io/sentry/SentryEnvelopeItemHeader.java
+++ b/sentry/src/main/java/io/sentry/SentryEnvelopeItemHeader.java
@@ -219,8 +219,9 @@
if (itemCount != null) {
writer.name(JsonKeys.ITEM_COUNT).value(itemCount);
}
- if (metaLength != null) {
- writer.name(JsonKeys.META_LENGTH).value(metaLength);
+ final @Nullable Integer metaLengthValue = getMetaLength();
+ if (metaLengthValue != null) {
+ writer.name(JsonKeys.META_LENGTH).value(metaLengthValue);
}
writer.name(JsonKeys.LENGTH).value(getLength());
if (unknown != null) {This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
43jay
commented
Apr 7, 2026
43jay
force-pushed
the
claude/dreamy-solomon
branch
from
4e173d3 to
b4b28c9
Compare
Adds a new boolean option `useProfilingManager` that gates whether the SDK uses Android's ProfilingManager API (API 35+) for Perfetto-based profiling. On devices below API 35 where ProfilingManager is not available, no profiling data is collected — the legacy Debug-based profiler is not used as a fallback. Wired through SentryOptions and ManifestMetadataReader (AndroidManifest meta-data). Defaults to false (opt-in). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds UI controls to the profiling sample activity for testing both legacy and Perfetto profiling paths. Enables useProfilingManager flag in the sample manifest for API 35+ testing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Show active profiler status line with (i) info button to show SDK config (sample rates, lifecycle mode, use-profiling-manager) - Conditionally show Start(Manual) or Start(Transaction) button based on profileLifecycle mode, since each is a no-op in the wrong mode - Hide duration seekbar in MANUAL mode (only affects transaction length) - Remove inline profiling result TextView; show results via Toast and in the (i) dialog instead - Apply AppTheme.Main to fix edge-to-edge clipping on API 35+ - Add indices to the bitmap list items so user can see the list view jumping around Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ager is set When useProfilingManager is true, SentryPerformanceProvider now skips creating the legacy Debug-based profiler at app start. This ensures AndroidOptionsInitializer creates a Perfetto profiler instead, without needing special handover logic between the two profiling engines. The useProfilingManager flag is persisted in SentryAppStartProfilingOptions (written at end of Sentry.init(), read on next app launch) so the decision is available before SDK initialization. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> squash into options commit
…Profiler Introduces PerfettoProfiler, which uses Android's ProfilingManager system service (API 35+) for Perfetto-based stack sampling. When useProfilingManager is enabled, AndroidContinuousProfiler selects PerfettoProfiler at init time via createWithProfilingManager(); on older devices no profiling data is collected and the legacy Debug-based profiler is not used as a fallback. Key changes: - PerfettoProfiler: calls requestProfiling(STACK_SAMPLING), waits for ProfilingResult via CountDownLatch, reads .pftrace via getResultFilePath() - AndroidContinuousProfiler: factory methods createLegacy() / createWithProfilingManager() replace the public constructor; init() split into initLegacy() / initProfilingManager() for clarity; stopFuture uses cancel(false) to avoid interrupting the Perfetto result wait - AndroidOptionsInitializer: branches on isUseProfilingManager() to select the correct factory method - SentryEnvelopeItem: fromPerfettoProfileChunk() builds a single envelope item with meta_length header separating JSON metadata from binary .pftrace - SentryEnvelopeItemHeader: adds metaLength field for the binary format - ProfileChunk: adds contentType and version fields; Builder.setContentType() - SentryClient: routes Perfetto chunks to fromPerfettoProfileChunk() Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
43jay
force-pushed
the
claude/dreamy-solomon
branch
from
b4b28c9 to
83b1f1a
Compare
| () -> { | ||
| try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { | ||
| stopInternal(true); | ||
| } |
There was a problem hiding this comment.
Lock held during blocking endAndCollect risks ANR
Medium Severity
The scheduled chunk timer acquires the reentrant lock and then calls stopInternal, which calls perfettoProfiler.endAndCollect(). That method calls resultLatch.await(5, SECONDS), blocking the thread while the lock is held. Any call to startProfiler, stopProfiler, isRunning, getChunkId, getProfilerId, or close from another thread (including the main/UI thread) will be blocked for up to 5 seconds. On Android, a 5-second main-thread block causes an ANR. The legacy AndroidContinuousProfiler doesn't have this problem because its endAndCollect is non-blocking.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 10c415f. Configure here.
8 tasks
43jay
force-pushed
the
claude/dreamy-solomon
branch
from
10c415f to
873bad3
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 3 total unresolved issues (including 1 from previous review).
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Unconditional
shouldStopreset causes unintended profiler continuationshouldStopis now reset only in TRACE starts and in MANUAL starts that actually proceed, so a skipped MANUAL start no longer clears a pending TRACE stop.
- ✅ Fixed: Missing API level guard for PerfettoContinuousProfiler creation
setupProfilernow guardsuseProfilingManagerbehind an API 35+ check and falls back toNoOpContinuousProfileron lower API levels to avoid loading Perfetto classes.
Or push these changes by commenting:
@cursor push ce6f706c8b
Preview (ce6f706c8b)
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
--- 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
@@ -337,6 +337,17 @@
performanceCollector.start(chunkId.toString());
}
} else {
+ if (options.isUseProfilingManager()
+ && buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.VANILLA_ICE_CREAM) {
+ options
+ .getLogger()
+ .log(
+ SentryLevel.INFO,
+ "useProfilingManager is enabled, but API level is below %d. Continuous profiling is disabled.",
+ Build.VERSION_CODES.VANILLA_ICE_CREAM);
+ options.setContinuousProfiler(NoOpContinuousProfiler.getInstance());
+ return;
+ }
final @NotNull SentryFrameMetricsCollector frameMetricsCollector =
Objects.requireNonNull(
options.getFrameMetricsCollector(), "options.getFrameMetricsCollector is required");
diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/PerfettoContinuousProfiler.java b/sentry-android-core/src/main/java/io/sentry/android/core/PerfettoContinuousProfiler.java
--- a/sentry-android-core/src/main/java/io/sentry/android/core/PerfettoContinuousProfiler.java
+++ b/sentry-android-core/src/main/java/io/sentry/android/core/PerfettoContinuousProfiler.java
@@ -107,7 +107,6 @@
final @NotNull ProfileLifecycle profileLifecycle,
final @NotNull TracesSampler tracesSampler) {
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
- shouldStop = false;
if (shouldSample) {
isSampled = tracesSampler.sampleSessionProfile(SentryRandom.current().nextDouble());
shouldSample = false;
@@ -118,6 +117,7 @@
}
switch (profileLifecycle) {
case TRACE:
+ shouldStop = false;
activeTraceCount = Math.max(0, activeTraceCount); // safety check.
activeTraceCount++;
break;
@@ -128,6 +128,7 @@
"Unexpected call to startProfiler(MANUAL) while profiler already running. Skipping.");
return;
}
+ shouldStop = false;
break;
}
if (!isRunning()) {
diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt
--- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt
+++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt
@@ -376,7 +376,16 @@
assertTrue(fixture.sentryOptions.continuousProfiler is AndroidContinuousProfiler)
}
+ @Config(sdk = [34])
@Test
+ fun `init with profiling manager below API 35 sets no-op continuous profiler`() {
+ fixture.initSut(configureOptions = { isUseProfilingManager = true }, useRealContext = true)
+
+ assertEquals(NoOpTransactionProfiler.getInstance(), fixture.sentryOptions.transactionProfiler)
+ assertEquals(NoOpContinuousProfiler.getInstance(), fixture.sentryOptions.continuousProfiler)
+ }
+
+ @Test
fun `init with profilesSampleRate should set Android transaction profiler`() {
fixture.initSut(configureOptions = { profilesSampleRate = 1.0 })
diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/PerfettoContinuousProfilerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/PerfettoContinuousProfilerTest.kt
--- a/sentry-android-core/src/test/java/io/sentry/android/core/PerfettoContinuousProfilerTest.kt
+++ b/sentry-android-core/src/test/java/io/sentry/android/core/PerfettoContinuousProfilerTest.kt
@@ -132,4 +132,24 @@
"Profiler should continue running after chunk restart — shouldStop must be reset on start",
)
}
+
+ @Test
+ fun `manual start while trace profiling is running does not cancel pending trace stop`() {
+ val profiler = fixture.getSut()
+
+ profiler.startProfiler(ProfileLifecycle.TRACE, fixture.mockTracesSampler)
+ assertTrue(profiler.isRunning)
+
+ profiler.stopProfiler(ProfileLifecycle.TRACE)
+ profiler.startProfiler(ProfileLifecycle.MANUAL, fixture.mockTracesSampler)
+
+ fixture.executor.runAll()
+
+ assertFalse(profiler.isRunning)
+ verify(fixture.mockLogger)
+ .log(
+ eq(SentryLevel.WARNING),
+ eq("Unexpected call to startProfiler(MANUAL) while profiler already running. Skipping."),
+ )
+ }
}This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
…eader SentryEnvelopeItemHeader.serialize() checked the raw metaLength field instead of calling getMetaLength(), so the callable path used by Perfetto profile chunks was never invoked and meta_length was never written to the envelope header JSON. Refactor SentryEnvelopeItemHeader to remove the metaLength field entirely — all constructors now store a single calculateMetaLength callable. Eager constructors (deserializer) wrap the Integer in a lambda. All constructors delegate to one private primary constructor. In fromPerfettoProfileChunk, replace the round-trip through ProfileChunk.setMetaLength/getMetaLength with a local AtomicReference shared between the CachedItem lambda and the header callable, keeping meta_length as an envelope transport concern rather than in ProfileChunk
…uousProfiler Separate the Perfetto/ProfilingManager profiling backend into its own IContinuousProfiler implementation to keep the two backends independent. - AndroidContinuousProfiler is restored to legacy-only (no Perfetto fields, no conditional branches, no @SuppressLint annotations) - PerfettoContinuousProfiler is a new @RequiresApi(35) class that delegates to PerfettoProfiler and always sets content_type="perfetto" - AndroidOptionsInitializer branches on useProfilingManager to pick the right implementation - Consistent locking: startInternal/stopInternal both require caller to hold the lock, with callers wrapped accordingly - Renamed rootSpanCounter to activeTraceCount in PerfettoContinuousProfiler - Extracted tryResolveScopes/onScopesAvailable from initScopes in both classes - Fixed duplicate listener bug in PerfettoProfiler (was using local lambda instead of class-scope profilingResultListener)
…ofilerTestCases.kt Shared test logic (sampling, lifecycle, rate limiting, chunk restart, close) is defined as extension functions on IContinuousProfiler in ContinuousProfilerTestCases.kt. AndroidContinuousProfilerTest delegates to these for common behavior while keeping legacy-specific tests inline. This enables PerfettoContinuousProfilerTest to reuse the same test cases with its own fixture. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
43jay
force-pushed
the
claude/dreamy-solomon
branch
from
6370485 to
fdd7287
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
…ases Reuse 17 existing tests by delegating to shared extension functions in ContinuousProfilerTestCases.kt Add 1 Perfetto-specific test for the manual mode duplicate start warning message.
…ousProfiler Currently PerfettoContinuousProfiler is not doing app-start profiling. Because of this, scopes are always available. Remove the legacy patterns that were carried over from AndroidContinuousProfiler: - Replace tryResolveScopes/onScopesAvailable with resolveScopes() that returns @NotNull IScopes and logs an error if scopes is unexpectedly unavailable - Remove payloadBuilders list, payloadLock, and sendChunks() buffering; replace with sendChunk() that sends a single chunk immediately - Remove scopes != null guards and SentryNanotimeDate fallback Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Lock in isRunning(), getProfilerId(), getChunkId() so all public getters are synchronized with writes in startInternal/stopInternal - Lock in reevaluateSampling() - Remove volatile from shouldSample; all accesses are now under the same lock - Replace ArrayDeque with ConcurrentLinkedDeque in PerfettoProfiler for frame measurement collections; these are written by the FrameMetrics HandlerThread and read by the executor thread in endAndCollect() Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…o PerfettoContinuousProfiler This separation keeps PerfettoProfiler focused on the ProfilingManager API and puts all measurement collection (frame metrics + performanceCollector) at the PerfettoContinuousProfiler layer, making both classes easier to reason about independently.
…entCollector Rename FrameMetricsProfiler to ChunkMeasurementCollector and expand it to own the measurement lifecycle for both types of profiling we do outside of perfetto: 1) frame metrics (slow/frozen frames, refresh rate) (previous) 2) performance data (CPU usage, memory footprint) from the CompositePerformanceCollector (new change). Also restores the performanceCollector impl that was accidentally removed in an earlier commit.
shouldStop = false was unconditionally set at the top of startProfiler, which could cancel a pending stop if startProfiler was called on an early-return path (e.g. MANUAL while already running). Move the reset to inside the !isRunning() block so it only triggers when the profiler is actually about to start. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
43jay
force-pushed
the
claude/dreamy-solomon
branch
from
96dae7f to
11331ea
Compare
markushi
requested changes
Apr 17, 2026
Member
markushi
left a comment
markushi
left a comment
There was a problem hiding this comment.
Looking good overall! I left a few comments.
| private final AutoClosableReentrantLock payloadLock = new AutoClosableReentrantLock(); | ||
|
|
||
| public AndroidContinuousProfiler( | ||
| public static AndroidContinuousProfiler createLegacy( |
Member
There was a problem hiding this comment.
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.
| frameMetricsCollector, | ||
| () -> options.getExecutorService(), | ||
| () -> | ||
| new PerfettoProfiler(context.getApplicationContext(), options.getLogger())) |
Member
There was a problem hiding this comment.
Yeah, this should be guarded by API level checks. Did you mean to do this in this PR? #5290
Please directly address it within this PR instead.
Collaborator
Author
There was a problem hiding this comment.
hmm, that's strange. I wasn't aware of that PR. It looks like cursor took matters into its own hands there.. i used the web tool but it was taking ages and the solution didn't make sense ultimately so i thought I canned it.
| private final @NotNull BuildInfoProvider buildInfoProvider; | ||
| private final @NotNull LazyEvaluator.Evaluator<PerfettoProfiler> perfettoProfilerSupplier; | ||
|
|
||
| private @Nullable PerfettoProfiler perfettoProfiler = null; |
Member
There was a problem hiding this comment.
Suggested change
| private @Nullable PerfettoProfiler perfettoProfiler = null; | |
| private @Nullable volatile PerfettoProfiler perfettoProfiler = null; |
| private @Nullable CompositePerformanceCollector performanceCollector = null; | ||
| private @Nullable String chunkId = null; | ||
|
|
||
| private final @NotNull ConcurrentLinkedDeque<ProfileMeasurementValue> |
Member
There was a problem hiding this comment.
That's quite some duplicate code similar to AndroidProfiler, ideally we can re-use the ChunkMeasurementCollector in both places.
Guard with buildInfoProvider.getSdkInfoVersion() >= VANILLA_ICE_CREAM. On API < 35, log a warning and leave the profiler as NoOp.
…ization ChunkMeasurementCollector#stop was passing its data maps by reference into ProfileMeasurement. The collected data is submitted async, so races with the next ChunkMeasurementCollector#start which reset the referenced maps Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| new ProfileMeasurement(ProfileMeasurement.UNIT_BYTES, nativeMemoryUsageMeasurements)); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Duplicate measurement collection code not shared with AndroidProfiler
Low Severity
ChunkMeasurementCollector duplicates the frame metrics and performance data collection logic already present in AndroidProfiler. The reviewer flagged this for reuse across both profiler implementations, and the author indicated it was fixed, but the collector remains a private nested class used only by PerfettoContinuousProfiler. Extracting it as a shared component would reduce the maintenance burden and risk of divergent behavior between the two profiling backends.
Reviewed by Cursor Bugbot for commit ffd5c6b. Configure here.
ACProfiler is legacy, no need to modify the code.
The .api file declared getMetaLength()/setMetaLength(int) on ProfileChunk, but those methods don't exist in the source. Related stale entries on SentryEnvelopeItemHeader (8-arg private ctor and package-private getMetaLength) were also removed. Regenerated via: ./gradlew :sentry:apiDump Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
In SentryEnvelopeItemHeader.serialize(), meta_length was being evaluated before length. For Perfetto profile chunks, the metaLength value is only populated as a side effect of materializing the payload, which the length computation triggers. Added a regression test that drives header.serialize() directly without first touching item.data. Also renamed the existing test to describe what it actually covers (payload layout), since the old name overpromised header coverage. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 4 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1a6742c. Configure here.
| assertTrue(isRunning) | ||
| mocks.executor.runAll() | ||
| assertFalse(isRunning) | ||
| } |
There was a problem hiding this comment.
Shared test weakens legacy profiler close() assertions
Low Severity
The shared testCloseWithoutTerminatingStopsAfterChunk dropped the assertEquals(0, profiler.rootSpanCounter) assertion that the original AndroidContinuousProfilerTest had. The original test verified that close(false) immediately resets rootSpanCounter to 0 even though the profiler is still running. This important behavioral contract is no longer tested for the legacy profiler after the refactoring to shared test cases.
Reviewed by Cursor Bugbot for commit 1a6742c. Configure here.
| <!-- Enable ProfilingManager (Perfetto) on API 35+ --> | ||
| <meta-data | ||
| android:name="io.sentry.profiling.use-profiling-manager" | ||
| android:value="true" /> |
There was a problem hiding this comment.
Sample app enables Perfetto by default via manifest
Low Severity
The sample app's AndroidManifest.xml hardcodes io.sentry.profiling.use-profiling-manager to true. Since the PR description states this is an opt-in feature and the SDK intentionally does not fall back to legacy profiling on API < 35, this means the sample app will have no profiling at all when running on devices below API 35. This looks like a development/testing artifact that was left in.
Reviewed by Cursor Bugbot for commit 1a6742c. Configure here.
Comment on lines
+292
to
+302
| stopInternal(true); | ||
| } | ||
| }, | ||
| MAX_CHUNK_DURATION_MILLIS); | ||
| } catch (RejectedExecutionException e) { | ||
| logger.log( | ||
| SentryLevel.ERROR, | ||
| "Failed to schedule profiling chunk finish. Did you call Sentry.close()?", | ||
| e); | ||
| shouldStop = true; | ||
| } |
There was a problem hiding this comment.
Bug: A RejectedExecutionException when scheduling a profiler stop task prevents the Perfetto session from being stopped, causing a resource leak of trace files when the app backgrounds.
Severity: MEDIUM
Suggested Fix
The catch block for RejectedExecutionException should not only set shouldStop = true but also directly call a method to stop the underlying Perfetto profiler session and clean up associated resources, ensuring cleanup happens immediately upon failure to schedule the stop task.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
sentry-android-core/src/main/java/io/sentry/android/core/PerfettoContinuousProfiler.java#L285-L302
Potential issue: When scheduling a profiler stop task fails with a
`RejectedExecutionException`, the `shouldStop` flag is set, but the underlying Perfetto
profiler session is not actually stopped. If the application is subsequently
backgrounded, the `close(false)` method is called, which does not trigger the cleanup
logic in `stopInternal()`. As a result, `perfettoProfiler.endAndCollect()` is never
invoked, leading to a resource leak where trace files are left on the device's
persistent storage indefinitely. This can cause disk space issues over time.
| }; | ||
| } | ||
|
|
||
| public boolean start(final long durationMs) { |
There was a problem hiding this comment.
Bug: A race condition exists because the resultLatch and profilingResult fields are not volatile, leading to potential silent loss of profiling data due to memory visibility issues between threads.
Severity: HIGH
Suggested Fix
Mark both the resultLatch and profilingResult fields as volatile to ensure that writes to these fields are immediately visible to other threads, preventing the race condition.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
sentry-android-core/src/main/java/io/sentry/android/core/PerfettoProfiler.java#L65
Potential issue: The `resultLatch` and `profilingResult` fields are written on a caller
thread and read by a system callback thread without being marked as `volatile`. The
`Runnable::run` executor provides no memory barrier, creating a race condition. The
callback thread may read a stale `null` value for `resultLatch`, skipping the
`countDown()` call. This causes the main thread to time out while waiting for the latch
in `endAndCollect()`, leading to the silent loss of profiling data.
Comment on lines
+307
to
+312
| if (stopFuture != null) { | ||
| stopFuture.cancel(false); | ||
| } | ||
|
|
||
| // Make sure perfetto was running | ||
| if (perfettoProfiler == null || !isRunning) { |
There was a problem hiding this comment.
Bug: When a profiler restart is blocked by rate limiting, the profilerId is incorrectly reset, breaking the continuity of the profiling session across multiple chunks.
Severity: MEDIUM
Suggested Fix
Refine the logic in stopInternal(false) to distinguish between a failed initial start and a failed restart. The profilerId should not be reset if isRunning is false as a result of a previous chunk completing successfully.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
sentry-android-core/src/main/java/io/sentry/android/core/PerfettoContinuousProfiler.java#L307-L312
Potential issue: When a profiling chunk completes and the profiler attempts to restart
for the next chunk, it may be blocked by a rate limit. This triggers a call to
`stopInternal(false)`. Inside this method, a check `if (perfettoProfiler == null ||
!isRunning)` incorrectly passes because `isRunning` was already set to `false` after the
previous chunk. This leads to the `profilerId` being reset to `SentryId.EMPTY_ID`. This
breaks the intended design where `profilerId` should persist across chunks to correlate
them as part of the same session.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
📜 Description
Adds opt-in
useProfilingManageroption that uses Android'sProfilingManagerAPI (API 35+) for Perfetto-based stack sampling instead of the legacyDebug.startMethodTracingSamplingengine.PerfettoContinuousProfileris mutually exclusive withAndroidContinuousProfiler— the option gates which implementation is created at init time. The legacy path is unchanged.Why a new ContinuousProfiler class
The first few commits wire the Perfetto backend into
AndroidContinuousProfiler(ported from an earlier branch). The later commits extract a standalonePerfettoContinuousProfilerbecause:AndroidContinuousProfilerhas a lot of state and theif (perfetto) { ... } else { legacy }branching makes paths hard to follow => the two codepaths will never be active at the same time.startProfiler,stopProfiler,close(true),reevaluateSamplingConcurrentLinkedDeque(code)new HandlerThread("...SentryFrameMetricsCollector")stopInternal(true)— scheduled chunk timer. AlsosendChunk()submits work here.new Thread(r, "SentryExecutorServiceThreadFactory-" + cnt++)onRateLimitChanged— inline callback (code)new Thread(r, "SentryAsyncConnection-" + cnt++)onRateLimitChanged— rate limit expiry (code);close(false)— session timeout (code); not a direct caller butCompositePerformanceCollectorrunssetup()andcollect()every 100ms (code)new Timer(true)in RateLimiter, LifecycleWatcher, CompositePerformanceCollectorstartProfiler(TRACE)(code),stopProfiler(TRACE)(code)PerfettoContinuousProfiler+PerfettoProfilermeans fewer@SuppressLint("NewApi")scattered throughAndroidContinuousProfilerKey files
SentryOptions.useProfilingManager— opt-in flag, readable from manifestio.sentry.profiling.use-profiling-managerPerfettoContinuousProfiler—IContinuousProfilerimpl,@RequiresApi(35), delegates toPerfettoProfilerPerfettoProfiler— wrapsProfilingManager.requestProfiling(PROFILING_TYPE_STACK_SAMPLING, ...)SentryEnvelopeItem.fromPerfettoProfileChunk()— binary envelope format withmeta_lengthheaderAndroidContinuousProfiler— legacy only, no Perfetto references💡 Motivation and Context
Android's
ProfilingManager(API 35+) provides OS-level Perfetto stack sampling. The legacyDebug.startMethodTracingSamplingpath is preserved unchanged. On API < 35 withuseProfilingManager=true, profiling is disabled (no silent fallback).💚 How did you test it?
content_type: "perfetto".pftracefiles and inspected in Perfetto UIUnit Tests
Run all Tests:
PerfettoContinuousProfilerTestSentryOptionsTest,ManifestMetadataReaderTest,SentryEnvelopeItemTest📝 Checklist
sendDefaultPIIis enabled.Testing locally
🔮 Next steps
PROFILING_TYPE_STACK_SAMPLINGtraces (ProfilingManager doesn't seem to includelinux.process_statsdata source)