- 
          
 - 
                Notifications
    
You must be signed in to change notification settings  - Fork 371
 
refactor: Change SentryFrame.function default from <redacted> to nil #6608
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This aligns with how other Sentry SDKs (like Java) handle missing function names. When the SDK can't detect the function name, it now keeps it as nil and lets the backend apply redaction if needed. Fixes #4686
          Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@              Coverage Diff              @@
##              main     #6608       +/-   ##
=============================================
+ Coverage   85.453%   85.499%   +0.046%     
=============================================
  Files          451       451               
  Lines        27566     27565        -1     
  Branches     12069     12070        +1     
=============================================
+ Hits         23556     23568       +12     
+ Misses        3963      3950       -13     
  Partials        47        47               
 ... and 5 files with indirect coverage changes Continue to review full report in Codecov by Sentry. 
  | 
    
          Performance metrics 🚀
 
 | 
    
| Revision | Plain | With Sentry | Diff | 
|---|---|---|---|
| e16fd46 | 1228.64 ms | 1251.57 ms | 22.93 ms | 
| b6f8eb3 | 1217.94 ms | 1246.57 ms | 28.63 ms | 
| be8375a | 1212.65 ms | 1239.72 ms | 27.08 ms | 
| e5773c1 | 1235.10 ms | 1264.15 ms | 29.04 ms | 
| 119ab1c | 1226.79 ms | 1254.55 ms | 27.76 ms | 
| 00d9740 | 1223.53 ms | 1249.75 ms | 26.22 ms | 
| daf8077 | 1202.78 ms | 1235.52 ms | 32.74 ms | 
| f92cfa9 | 1217.94 ms | 1240.06 ms | 22.12 ms | 
| 09471ff | 1239.98 ms | 1256.65 ms | 16.67 ms | 
| 2609f7a | 1218.17 ms | 1241.34 ms | 23.17 ms | 
App size
| Revision | Plain | With Sentry | Diff | 
|---|---|---|---|
| e16fd46 | 23.74 KiB | 1.01 MiB | 1008.95 KiB | 
| b6f8eb3 | 23.75 KiB | 988.70 KiB | 964.95 KiB | 
| be8375a | 23.75 KiB | 933.03 KiB | 909.28 KiB | 
| e5773c1 | 23.75 KiB | 1.00 MiB | 1005.08 KiB | 
| 119ab1c | 23.75 KiB | 993.70 KiB | 969.95 KiB | 
| 00d9740 | 23.75 KiB | 938.32 KiB | 914.57 KiB | 
| daf8077 | 23.75 KiB | 971.82 KiB | 948.07 KiB | 
| f92cfa9 | 23.75 KiB | 855.38 KiB | 831.63 KiB | 
| 09471ff | 23.75 KiB | 990.16 KiB | 966.41 KiB | 
| 2609f7a | 23.75 KiB | 867.04 KiB | 843.29 KiB | 
Previous results on branch: philprime/sentry-frame-function
Startup times
| Revision | Plain | With Sentry | Diff | 
|---|---|---|---|
| 911a661 | 1219.12 ms | 1249.21 ms | 30.09 ms | 
| 022314d | 1228.50 ms | 1255.27 ms | 26.77 ms | 
App size
| Revision | Plain | With Sentry | Diff | 
|---|---|---|---|
| 911a661 | 23.75 KiB | 1.02 MiB | 1016.53 KiB | 
| 022314d | 23.75 KiB | 1.02 MiB | 1016.49 KiB | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, LGTM.
Co-authored-by: Philipp Hofmann <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Reenabled Public `new` Constructor in Frame Class
Removing + (instancetype)new NS_UNAVAILABLE; from SentryFrame.h accidentally re-enables the new class method for SentryFrame. This is an unannounced public API change, unrelated to the PR's main goal, and new was previously explicitly disabled to discourage its use.
Sources/Sentry/Public/SentryFrame.h#L100-L102
sentry-cocoa/Sources/Sentry/Public/SentryFrame.h
Lines 100 to 102 in 8090235
| - (instancetype)init; | |
📜 Description
Changed
SentryFrame.functiondefault value from@"<redacted>"tonil. This aligns the Cocoa SDK with other Sentry SDKs (like Java) and lets the backend handle redaction when function names can't be detected.💡 Motivation and Context
Fixes #4686
The Cocoa SDK doesn't symbolicate function names locally (it's slow and doesn't work for App Store builds). When the function name is unknown, the SDK should keep it as
niland let the backend apply<redacted>if needed, rather than doing it client-side.💚 How did you test it?
nilinstead of<redacted>:SentryFrameTestsSentryInterfacesTestsSentryCrashReportConverterTestsSentryCrashStackEntryMapperTestsSentryDefaultThreadInspectorTestsSentryMetricKitIntegrationTests📝 Checklist
sendDefaultPIIis enabled.