feat(apple): Add enableTracingWhenCrashing #11732
Conversation
Add docs for the new option enableTracingWhenCrashing.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
Bundle ReportChanges will decrease total bundle size by 15 bytes (-0.0%) ⬇️. This is within the configured threshold ✅ Detailed changes
|
| @@ -271,3 +271,15 @@ String entries do not have to be full matches, meaning the URL of a request is m | |||
| If <PlatformIdentifier name="trace-propagation-targets" /> is not provided, trace data is attached to every outgoing request from the instrumented client. | |||
|
|
|||
| </ConfigKey> | |||
|
|
|||
| <ConfigKey name="enable-tracing-when-crashes"> | |||
There was a problem hiding this comment.
This is a very odd name in light of us removing enableTracing from the SDKs.
I'm also curious how this is implemented.
There was a problem hiding this comment.
Thanks for pointing that out. Naming is hard 😅. It's basically finishing transactions bounded to the scope when an app crashes. The code is here getsentry/sentry-cocoa#4504.
I also considered enableCrashTracing and enableTracingCrashes but that's closer to tracing actual crashes, which the SDK already does. And I have a typo there. It should be enableTracingWhenCrashing. Is it still odd?
There was a problem hiding this comment.
@philipphofmann do we need a flag for this actually? That's the default behavior on android. Does it introduce any unwanted side-effects?
There was a problem hiding this comment.
Ok, so this will only work if tracing is actually enabled, and will then persist the transaction to disc?
There was a problem hiding this comment.
Does it introduce any unwanted side-effects?
Maybe yes. Other signal handlers might be effected, and the app can get stuck after writing the crash report to disk. So my plan is to make this the default behaviour, but for now it should be an experimental feature.
Ok, so this will only work if tracing is actually enabled, and will then persist the transaction to disc?
Yes, indeed.
There was a problem hiding this comment.
Ok, but then it does not enable tracing. So I would suggest we call it something less confusing, like persistTracesOnDiscIfCrashed.
There was a problem hiding this comment.
We prefix everything with enable in the Cocoa SDK, so I would like to keep that pattern. What about enablePersistingTracesWhenCrashing, @cleptric?
DESCRIBE YOUR PR
Add docs for the new option enableTracingWhenCrashing.
IS YOUR CHANGE URGENT?
Help us prioritize incoming PRs by letting us know when the change needs to go live.
SLA
Thanks in advance for your help!
PRE-MERGE CHECKLIST
Make sure you've checked the following before merging your changes: