Skip to content

feat(profiling): Add Android ProfilingManager (Perfetto) support#5251

Open
43jay wants to merge 33 commits into
mainfrom
claude/dreamy-solomon
Open

feat(profiling): Add Android ProfilingManager (Perfetto) support#5251
43jay wants to merge 33 commits into
mainfrom
claude/dreamy-solomon

Conversation

@43jay

@43jay 43jay commented Mar 31, 2026

Copy link
Copy Markdown
Contributor

📜 Description

Adds opt-in useProfilingManager option that uses Android's ProfilingManager API (API 35+) for Perfetto-based stack sampling instead of the legacy Debug.startMethodTracingSampling engine.

PerfettoContinuousProfiler is mutually exclusive with AndroidContinuousProfiler — 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 standalone PerfettoContinuousProfiler because:

  1. Mutually exclusiveAndroidContinuousProfiler has a lot of state and the if (perfetto) { ... } else { legacy } branching makes paths hard to follow => the two codepaths will never be active at the same time.
  2. Threading — a large # different threads are involved and reasoning about locking is harder with two backends in one class
Thread Callers Creation site
Caller's thread (main/app) startProfiler, stopProfiler, close(true), reevaluateSampling Not created by Sentry
FrameMetrics HandlerThread Writes frame measurements to PerfettoProfiler's ConcurrentLinkedDeque (code) new HandlerThread("...SentryFrameMetricsCollector")
SentryExecutorServiceThreadFactory-N stopInternal(true) — scheduled chunk timer. Also sendChunk() submits work here. new Thread(r, "SentryExecutorServiceThreadFactory-" + cnt++)
SentryAsyncConnection-N onRateLimitChanged — inline callback (code) new Thread(r, "SentryAsyncConnection-" + cnt++)
Timer daemon onRateLimitChanged — rate limit expiry (code); close(false) — session timeout (code); not a direct caller but CompositePerformanceCollector runs setup() and collect() every 100ms (code) JDK internal — new Timer(true) in RateLimiter, LifecycleWatcher, CompositePerformanceCollector
OTel span processor startProfiler(TRACE) (code), stopProfiler(TRACE) (code) Created by OpenTelemetry SDK — not Sentry-controlled
  1. App-start profiling — the legacy profiler has special null-scopes handling for app-start. ProfilingManager doesn't support app-start, so this complexity doesn't apply
  2. API level annotations — confining all ProfilingManager call sites to PerfettoContinuousProfiler + PerfettoProfiler means fewer @SuppressLint("NewApi") scattered through AndroidContinuousProfiler

Key files

  • SentryOptions.useProfilingManager — opt-in flag, readable from manifest io.sentry.profiling.use-profiling-manager
  • PerfettoContinuousProfilerIContinuousProfiler impl, @RequiresApi(35), delegates to PerfettoProfiler
  • PerfettoProfiler — wraps ProfilingManager.requestProfiling(PROFILING_TYPE_STACK_SAMPLING, ...)
  • SentryEnvelopeItem.fromPerfettoProfileChunk() — binary envelope format with meta_length header
  • AndroidContinuousProfiler — legacy only, no Perfetto references

💡 Motivation and Context

Android's ProfilingManager (API 35+) provides OS-level Perfetto stack sampling. The legacy Debug.startMethodTracingSampling path is preserved unchanged. On API < 35 with useProfilingManager=true, profiling is disabled (no silent fallback).

💚 How did you test it?

  • Manual testing on Pixel Fold AVD (API 35) — verified Perfetto chunks captured with content_type: "perfetto"
  • Extracted .pftrace files and inspected in Perfetto UI
Unit Tests

Run all Tests:

export JAVA_HOME=/Library/Java/JavaVirtualMachines/zulu-17.jdk/Contents/Home
./gradlew \
  :sentry-android-core:testDebugUnitTest \
    --tests "io.sentry.android.core.AndroidContinuousProfilerTest" \
    --tests "io.sentry.android.core.AndroidOptionsInitializerTest" \
    --tests "io.sentry.android.core.ManifestMetadataReaderTest" \
    --tests "io.sentry.android.core.ChunkMeasurementCollectorTest" \
    --tests "io.sentry.android.core.PerfettoContinuousProfilerTest" \
  :sentry:test \
    --tests "io.sentry.SentryEnvelopeItemTest" \
    --tests "io.sentry.SentryOptionsTest"

PerfettoContinuousProfilerTest

> JAVA_HOME=$(/usr/libexec/java_home -v 17) ./gradlew :sentry-android-core:testDebugUnitTest --tests "io.sentry.android.core.PerfettoContinuousProfilerTest"

PerfettoContinuousProfilerTest > close without terminating stops all profiles after chunk is finished PASSED
PerfettoContinuousProfilerTest > profiler does not send chunks after close PASSED
PerfettoContinuousProfilerTest > profiler logs a warning on start if not sampled PASSED
PerfettoContinuousProfilerTest > profiler does not start when offline PASSED
PerfettoContinuousProfilerTest > manual profiler can be started again after a full start-stop cycle PASSED
PerfettoContinuousProfilerTest > stopProfiler stops the profiler after chunk is finished PASSED
PerfettoContinuousProfilerTest > profiler ignores profilesSampleRate PASSED
PerfettoContinuousProfilerTest > isRunning reflects profiler status PASSED
PerfettoContinuousProfilerTest > profiler multiple starts are accepted in trace mode PASSED
PerfettoContinuousProfilerTest > profiler sends chunk on each restart PASSED
PerfettoContinuousProfilerTest > profiler sends another chunk on stop PASSED
PerfettoContinuousProfilerTest > profiler stops when rate limited PASSED
PerfettoContinuousProfilerTest > profiler stops and restart for each chunk PASSED
PerfettoContinuousProfilerTest > profiler multiple starts are ignored in manual mode PASSED
PerfettoContinuousProfilerTest > profiler does not start when rate limited PASSED
PerfettoContinuousProfilerTest > when reevaluateSampling, profiler evaluates sessionSampleRate on next start PASSED
PerfettoContinuousProfilerTest > profiler evaluates sessionSampleRate only the first time PASSED

SentryOptionsTest, ManifestMetadataReaderTest, SentryEnvelopeItemTest

JAVA_HOME=$(/usr/libexec/java_home -v 17) ./gradlew :sentry-android-core:testDebugUnitTest --tests "io.sentry.android.core.SentryOptionsTest" --tests "io.sentry.android.core.ManifestMetadataReaderTest" --tests "io.sentry.android.core.SentryEnvelopeItemTest"

📝 Checklist

  • I added GH Issue ID & Linear ID
  • 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.
  • I updated the wizard 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.

Testing locally

# Disable ProfilingManager rate limiting (required for repeated testing)
adb shell device_config put profiling_testing rate_limiter.disabled true

# Watch logcat for the file path
adb logcat -s Sentry | grep "ProfilingResult"

# Pull the .pftrace file (can't adb pull from app-private dir, use run-as + cat)
PKG="io.sentry.samples.android"
REMOTE_DIR="/data/user/0/$PKG/files/profiling"
adb shell "run-as $PKG cat '$REMOTE_DIR/<filename>'" > ~/Desktop/profile.pftrace

# Open in https://ui.perfetto.dev/

🔮 Next steps

  • Remove CountdownLatch from PerfettoProfiler
  • Refactor AndroidContinuousProfilerTest to extend existing test scenarios to PerfettoContinuousProfilerTest
  • Verify backend ingest WAE
  • Investigate missing thread names in PROFILING_TYPE_STACK_SAMPLING traces (ProfilingManager doesn't seem to include linux.process_stats data source)
  • Docs and CHANGELOG update once PR is stable #skip-changelog

@github-actions

github-actions Bot commented Mar 31, 2026

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

Generated by 🚫 dangerJS against 24bcfed

@github-actions

github-actions Bot commented Mar 31, 2026

Copy link
Copy Markdown
Contributor

Semver Impact of This PR

🟡 Minor (new features)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).


This PR will not appear in the changelog.


🤖 This preview updates automatically when you update the PR.

@sentry

sentry Bot commented Mar 31, 2026

Copy link
Copy Markdown

📲 Install Builds

Android

🔗 App Name App ID Version Configuration
SDK Size io.sentry.tests.size 8.44.0 (1) release

⚙️ sentry-android Build Distribution Settings

@github-actions

github-actions Bot commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 314.92 ms 358.61 ms 43.69 ms
Size 0 B 0 B 0 B

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
a416a65 333.78 ms 410.37 ms 76.59 ms
d15471f 304.55 ms 408.43 ms 103.87 ms
ca6b6d8 380.45 ms 460.38 ms 79.93 ms
c8125f3 397.65 ms 485.14 ms 87.49 ms
b936425 302.69 ms 372.86 ms 70.17 ms
ed33deb 334.19 ms 362.30 ms 28.11 ms
cf708bd 408.35 ms 458.98 ms 50.63 ms
eb95ded 317.51 ms 369.08 ms 51.57 ms
a5ab36f 320.47 ms 389.77 ms 69.30 ms
80fd6ad 321.06 ms 375.79 ms 54.73 ms

App size

Revision Plain With Sentry Diff
a416a65 1.58 MiB 2.12 MiB 555.26 KiB
d15471f 1.58 MiB 2.13 MiB 559.54 KiB
ca6b6d8 0 B 0 B 0 B
c8125f3 1.58 MiB 2.10 MiB 532.32 KiB
b936425 0 B 0 B 0 B
ed33deb 1.58 MiB 2.13 MiB 559.52 KiB
cf708bd 1.58 MiB 2.11 MiB 539.71 KiB
eb95ded 0 B 0 B 0 B
a5ab36f 1.58 MiB 2.12 MiB 555.26 KiB
80fd6ad 0 B 0 B 0 B

@43jay 43jay marked this pull request as ready for review April 7, 2026 20:47
Comment thread sentry/src/main/java/io/sentry/SentryEnvelopeItem.java

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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_length
    • SentryEnvelopeItemHeader.serialize() now uses getMetaLength() (captured once in a local) so callable-backed Perfetto chunks correctly emit meta_length in envelope headers.

Create PR

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.

Comment thread sentry/src/main/java/io/sentry/SentryEnvelopeItemHeader.java
@43jay 43jay force-pushed the claude/dreamy-solomon branch from 4e173d3 to b4b28c9 Compare April 7, 2026 21:23
@43jay 43jay marked this pull request as draft April 9, 2026 17:30
43jay and others added 5 commits April 9, 2026 14:06
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 43jay force-pushed the claude/dreamy-solomon branch from b4b28c9 to 83b1f1a Compare April 9, 2026 20:36
@43jay 43jay marked this pull request as ready for review April 9, 2026 20:36
Comment thread sentry/src/main/java/io/sentry/SentryEnvelopeItem.java Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 shouldStop reset causes unintended profiler continuation
    • shouldStop is 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
    • setupProfiler now guards useProfilingManager behind an API 35+ check and falls back to NoOpContinuousProfiler on lower API levels to avoid loading Perfetto classes.

Create PR

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.

Comment thread sentry/src/test/java/io/sentry/SentryEnvelopeItemTest.kt Outdated
43jay added 2 commits April 13, 2026 16:23
…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)
Comment thread sentry/src/main/java/io/sentry/SentryEnvelopeItem.java
markushi and others added 2 commits May 15, 2026 20:06
… guard against orphaned profiler restart

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment thread sentry-android-core/api/sentry-android-core.api
* API 35+ devices. On older devices where ProfilingManager is not available, no profiling data is
* collected — the legacy {@code Debug}-based profiler is not used as a fallback.
*/
private boolean useProfilingManager = false;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

hm, I'm wondering if we should just kill this flag altogether and always use Perfetto on API 35+ and fallback to the old profiler on lower API levels? I guess the current status quo is bad enough to consider this a "fix" rather than making it an opt-in?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmmm if we would do that people would still see crashes - I guess we'd then have to add another "enableLegacyProfiling" bool. I guess it should be best to default this to "false" then.

Comment thread sentry/src/main/java/io/sentry/SentryEnvelopeItem.java
Comment thread sentry/src/main/java/io/sentry/SentryEnvelopeItem.java Outdated
Comment thread sentry-android-core/src/main/java/io/sentry/android/core/PerfettoProfiler.java Outdated

@romtsn romtsn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A couple of low-to-medium comments, but nothing blocking really (except the executor service submit on the same executor service thread, which can result in an endless loop I think).

The biggest question to me if we should get rid of the flag altogether and just use ProfilingManager by default on API 35+? Or we could keep it but make it true by default, just to have a backdoor to disable it, if something goes terribly wrong.

- Re-check shouldStop in async chunk callback to avoid an extra chunk when
  stop is requested while the callback is pending
- Use a lambda instead of a method ref for meta_length (Android desugaring)
- Remove unused getStopFuture()/getActiveTraceCount() test hooks
- Send chunks via the profiling executor, running inline when already on its
  thread to avoid self-submitting into the single-threaded executor
- Use 101Hz sampling to avoid lockstep with the display refresh rate

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 24bcfed. Configure here.

"Failed to schedule profiling chunk finish. Did you call Sentry.close()?",
e);
shouldStop = true;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Schedule failure leaves profiling stuck

Medium Severity

If startInternal successfully starts OS profiling and sets isRunning, but scheduling the chunk timer throws RejectedExecutionException, the code only sets shouldStop and never calls stopInternal or endAndCollect. The SDK keeps reporting a running profiler, blocks another manual start, and may never finalize or upload the active Perfetto session.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 24bcfed. Configure here.

Comment on lines +269 to +272
logger.log(
SentryLevel.ERROR,
"Failed to start Perfetto profiling. PerfettoProfiler.start() returned false.");
return;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: If PerfettoProfiler.start() returns false, the instance is not cleaned up via endAndCollect(), creating an orphaned object and a potential resource leak.
Severity: MEDIUM

Suggested Fix

Ensure endAndCollect() is called on the perfettoProfiler instance even if its start() method fails. In stopInternal(), modify the condition to call endAndCollect() if currentProfiler is not null, regardless of the isRunning flag's value. This guarantees that any created profiler instance is properly cleaned up.

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#L269-L272

Potential issue: In `PerfettoContinuousProfiler.startInternal()`, if the call to
`perfettoProfiler.start()` returns `false`, the method returns without setting the
`isRunning` flag to `true`. Later, when `stopInternal()` is called, it checks
`!isRunning` and exits early, failing to call `currentProfiler.endAndCollect()`. This
orphans the `PerfettoProfiler` instance. Since the instance's internal `started` flag
was set to `true` upon the `start()` call, it is left in an inconsistent state,
violating its single-use contract and creating a resource leak.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants