feat(profiling): drop profiler context from transactions when profiler gets rate-limited - #5993
Draft
markushi wants to merge 3 commits into
Draft
feat(profiling): drop profiler context from transactions when profiler gets rate-limited#5993markushi wants to merge 3 commits into
markushi wants to merge 3 commits into
Conversation
…r gets rate-limited
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
|
📲 Install BuildsAndroid
|
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
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
Transactions are tagged with the continuous profiler's
profiler_idas soon as they start, but the OS only tells us whether a Perfetto profile actually exists later. WhenProfilingManagerrejects the request — usually rate limiting, reported within a few milliseconds — no chunk is ever produced, and the transaction still ships aprofiler_idthat resolves to nothing.This adds a notification path so anything already tagged with a dead id can drop it before being sent:
IProfilingCanceledCallback(internal) withonProfilingCanceled(SentryId), plus register/unregister onIContinuousProfiler. OnlyPerfettoContinuousProfilerimplements them; the legacy Android and JVM profilers have no equivalent signal and no-op.PerfettoProfilergainedsetOnCanceledCallback, fired once per instance on every path that ends without a trace file: an OS error code, a null result path, a missing or empty trace file, the result timeout, andendAndCollectwithoutstart. The error path fires as soon as the OS reports, rather than waiting up to 60s for the chunk to end.PerfettoContinuousProfilertears the live chunk down on cancellation, so transactions started afterwards cannot pick up an id that is already known dead, and notifies its listeners.SentryTracerimplements the callback: it removes itsProfileContext, clearsSpanDataConvention.PROFILER_IDfrom the root and every child span, and stops tagging spans started after that point.💡 Motivation and Context
Rate limiting is the common case on API 35+, and it is reported roughly 1 ms after the request. Without this, a rate-limited session produces transactions that link to profiles the backend never receives, which surfaces in the UI as dangling profile references.
💚 How did you test it?
Unit tests, 24 added across three suites:
PerfettoProfilerTest— the callback fires on each of the five no-trace-file paths, fires only once when a failure is followed byendAndCollect, and does not fire when a trace file is produced.PerfettoContinuousProfilerTest— listeners get the profiler id of the running chunk, the id exists before the callback is installed, cancellation tears the chunk down and resets the id,start()returning false resets and notifies,close(true)notifies then drops listeners whileclose(false)keeps them, and a throwing listener neither escapes nor costs the others their notification.SentryTracerTest/DefaultSpanFactoryTest— register and unregister lifecycle, thewaitForChildrencase wherefinish()must not unregister yet, the profile context and span data being dropped on a matching id, and left alone on a non-matching one.📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps
Known gaps left deliberately, happy to fold any of them in:
finish()unregisters unconditionally, and tracers that are never finished stay registered untilclose(true).