Skip to content

Commit 659b164

Browse files
committed
fix(profiling): Cancel a pending stop when the profiler is started again
A stop request only sets shouldStop, and the profiler stops when the current chunk ends. A start call before that was ignored because the profiler was still running, and shouldStop stayed set. So the profiler stopped at the end of the chunk, even in TRACE mode while sampled spans were active. The legacy profiler also never reset shouldStop, so after the first stop every later profile ended after one chunk. Reset shouldStop on every sampled start call in AndroidContinuousProfiler and PerfettoContinuousProfiler. A manual start while the profiler runs is now an expected call, so the Perfetto profiler logs it at DEBUG level like the legacy profiler.
1 parent 258098e commit 659b164

6 files changed

Lines changed: 48 additions & 9 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
### Fixes
66

7+
- Keep UI profiling running when `startProfiler` is called before a requested stop takes effect at the end of the current chunk ([#XXXX](https://github.com/getsentry/sentry-java/pull/XXXX))
8+
- Before, the start call was ignored and the profiler still stopped at the end of the chunk, even in `TRACE` mode while sampled spans were active. On API 34 and lower, every later profile also stopped after one chunk.
79
- Keep sending events from Android 7.1.0 and older devices after Sentry changes its TLS certificate authority ([#6227](https://github.com/getsentry/sentry-java/pull/6227))
810
- In February 2027, Sentry moves its TLS certificates from DigiCert to Let's Encrypt and Google Trust Services ([announcement](https://sentry.io/changelog/were-changing-our-tls-certificate-authority-in-february-2027)).
911
- Android 7.1.0 (API 25) and older don't trust the Let's Encrypt root certificate (ISRG Root X1), so without this fix, the SDK on those devices can no longer send events to Sentry after the switch.

‎sentry-android-core/src/main/java/io/sentry/android/core/AndroidContinuousProfiler.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,8 @@ public void startProfiler(
131131
logger.log(SentryLevel.DEBUG, "Profiler was not started due to sampling decision.");
132132
return;
133133
}
134+
// A stop only takes effect when the current chunk ends, so a start before then cancels it.
135+
shouldStop = false;
134136
switch (profileLifecycle) {
135137
case TRACE:
136138
// rootSpanCounter should never be negative, unless the user changed profile lifecycle

‎sentry-android-core/src/main/java/io/sentry/android/core/PerfettoContinuousProfiler.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -141,23 +141,22 @@ public void startProfiler(
141141
logger.log(SentryLevel.DEBUG, "Profiler was not started due to sampling decision.");
142142
return;
143143
}
144+
// A stop only takes effect when the current chunk ends, so a start before then cancels it.
145+
shouldStop = false;
144146
switch (profileLifecycle) {
145147
case TRACE:
146148
activeTraceCount = Math.max(0, activeTraceCount); // safety check.
147149
activeTraceCount++;
148150
break;
149151
case MANUAL:
150152
if (isRunning()) {
151-
logger.log(
152-
SentryLevel.WARNING,
153-
"Unexpected call to startProfiler(MANUAL) while profiler already running. Skipping.");
153+
logger.log(SentryLevel.DEBUG, "Profiler is already running.");
154154
return;
155155
}
156156
break;
157157
}
158158
if (!isRunning()) {
159159
logger.log(SentryLevel.DEBUG, "Started Profiler.");
160-
shouldStop = false;
161160
startInternal();
162161
}
163162
}

‎sentry-android-core/src/test/java/io/sentry/android/core/AndroidContinuousProfilerTest.kt‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,18 @@ class AndroidContinuousProfilerTest {
278278
}
279279
.testDoesNotStartWhenOffline(mocks)
280280

281+
@Test
282+
fun `manual start cancels a pending stop`() =
283+
fixture.getSut().testManualStartCancelsPendingStop(mocks)
284+
285+
@Test
286+
fun `trace start cancels a pending stop`() =
287+
fixture.getSut().testTraceStartCancelsPendingStop(mocks)
288+
289+
@Test
290+
fun `manual profiler can be started again after a full start-stop cycle`() =
291+
fixture.getSut().testCanBeStartedAgainAfterStopCycle(mocks)
292+
281293
// -- Legacy-specific tests (AndroidContinuousProfiler only) --
282294

283295
@Test

‎sentry-android-core/src/test/java/io/sentry/android/core/ContinuousProfilerTestCases.kt‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,3 +192,23 @@ fun IContinuousProfiler.testCanBeStartedAgainAfterStopCycle(mocks: ProfilerMocks
192192
mocks.executor.runAll()
193193
assertTrue(isRunning, "shouldStop must be reset on start")
194194
}
195+
196+
fun IContinuousProfiler.testManualStartCancelsPendingStop(mocks: ProfilerMocks) {
197+
startProfiler(ProfileLifecycle.MANUAL, mocks.tracesSampler)
198+
val profilerIdBeforeStop = profilerId
199+
stopProfiler(ProfileLifecycle.MANUAL)
200+
startProfiler(ProfileLifecycle.MANUAL, mocks.tracesSampler)
201+
mocks.executor.runAll()
202+
assertTrue(isRunning, "a start before the chunk ends must cancel the pending stop")
203+
assertEquals(profilerIdBeforeStop, profilerId)
204+
}
205+
206+
fun IContinuousProfiler.testTraceStartCancelsPendingStop(mocks: ProfilerMocks) {
207+
startProfiler(ProfileLifecycle.TRACE, mocks.tracesSampler)
208+
val profilerIdBeforeStop = profilerId
209+
stopProfiler(ProfileLifecycle.TRACE)
210+
startProfiler(ProfileLifecycle.TRACE, mocks.tracesSampler)
211+
mocks.executor.runAll()
212+
assertTrue(isRunning, "a start before the chunk ends must cancel the pending stop")
213+
assertEquals(profilerIdBeforeStop, profilerId)
214+
}

‎sentry-android-core/src/test/java/io/sentry/android/core/PerfettoContinuousProfilerTest.kt‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,14 @@ class PerfettoContinuousProfilerTest {
178178
fun `manual profiler can be started again after a full start-stop cycle`() =
179179
fixture.getSut().testCanBeStartedAgainAfterStopCycle(mocks)
180180

181+
@Test
182+
fun `manual start cancels a pending stop`() =
183+
fixture.getSut().testManualStartCancelsPendingStop(mocks)
184+
185+
@Test
186+
fun `trace start cancels a pending stop`() =
187+
fixture.getSut().testTraceStartCancelsPendingStop(mocks)
188+
181189
// -- Perfetto-specific tests --
182190

183191
@Test
@@ -503,10 +511,6 @@ class PerfettoContinuousProfilerTest {
503511
assertTrue(profiler.isRunning)
504512
profiler.startProfiler(ProfileLifecycle.MANUAL, fixture.mockTracesSampler)
505513
assertTrue(profiler.isRunning)
506-
verify(fixture.mockLogger)
507-
.log(
508-
eq(SentryLevel.WARNING),
509-
eq("Unexpected call to startProfiler(MANUAL) while profiler already running. Skipping."),
510-
)
514+
verify(fixture.mockLogger).log(eq(SentryLevel.DEBUG), eq("Profiler is already running."))
511515
}
512516
}

0 commit comments

Comments
 (0)