Skip to content

Conversation

noahsmartin
Copy link
Contributor

@noahsmartin noahsmartin commented Sep 24, 2025

This removes SentryThreadInspector from the dependency container public interface, in favor of a protocol defined in Swift. This means we don't need to convert the entire thing to Swift before the dependency container

Resolves https://linear.app/getsentry/issue/COCOA-608/convert-sentrycrashstackentrymapper-to-swift and https://linear.app/getsentry/issue/COCOA-708/convert-sentrythreadinspector and https://linear.app/getsentry/issue/COCOA-707/convert-sentrystacktracebuilder

#skip-changelog


Note

Replaces public SentryThreadInspector exposure with Swift protocols and a SentryDefaultThreadInspector implementation, updating dependencies, integrations, and tests accordingly.

  • Thread Inspection:
    • Add Swift protocols SentryThreadInspector and SentryThreadInspecting for thread inspection.
    • Rename Obj-C class SentryThreadInspectorSentryDefaultThreadInspector and migrate call sites.
  • Core/DI:
    • SentryDependencyContainer: expose threadInspector as id<SentryThreadInspector> and lazily instantiate SentryDefaultThreadInspector.
    • SentryClient: ctor now takes/creates SentryDefaultThreadInspector and stores it; uses it for attaching threads/stacktraces.
  • Integrations:
    • Update ANR, Core Data, File I/O, Network tracking to depend on protocol or SentryDefaultThreadInspector and use getCurrentThreads*/stacktraceForCurrentThreadAsyncUnsafe.
  • Swift/Obj-C bridges & Tests:
    • Add new Swift files: SentryThreadInspector.swift, SentryThreadInspecting.swift.
    • Rename tests to SentryDefaultThreadInspectorTests; adapt helpers (TestThreadInspector) and bridging headers/imports.
  • Misc:
    • SentryProfileTimeseries.h: switch UIKit import to SentryProfilerDefines.

Written by Cursor Bugbot for commit da10f3b. This will update automatically on new commits. Configure here.

Copy link

linear bot commented Sep 24, 2025

Copy link

codecov bot commented Sep 24, 2025

Codecov Report

❌ Patch coverage is 28.57143% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 7.556%. Comparing base (5b469c0) to head (da10f3b).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
Sources/Sentry/SentryNetworkTracker.m 0.000% 2 Missing ⚠️
Sources/Sentry/SentryCoreDataTrackingIntegration.m 0.000% 1 Missing ⚠️
Sources/Sentry/SentryDependencyContainer.m 0.000% 1 Missing ⚠️
Sources/Sentry/SentryFileIOTracker.m 0.000% 1 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (5b469c0) and HEAD (da10f3b). Click for more details.

HEAD has 4 uploads less than BASE
Flag BASE (5b469c0) HEAD (da10f3b)
5 1
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main    #6273        +/-   ##
=============================================
- Coverage   86.824%   7.556%   -79.268%     
=============================================
  Files          438      404        -34     
  Lines        37328    35623      -1705     
  Branches     17438    14059      -3379     
=============================================
- Hits         32410     2692     -29718     
- Misses        4875    32920     +28045     
+ Partials        43       11        -32     
Files with missing lines Coverage Δ
Sources/Sentry/SentryANRTrackingIntegration.m 0.000% <ø> (-98.810%) ⬇️
Sources/Sentry/SentryClient.m 9.114% <100.000%> (-89.827%) ⬇️
Sources/Sentry/SentryCoreDataTracker.m 0.000% <ø> (-97.960%) ⬇️
Sources/Sentry/SentryDefaultThreadInspector.m 17.968% <ø> (ø)
Sources/Sentry/SentryFileIOTrackingIntegration.m 0.000% <ø> (-100.000%) ⬇️
Sources/Sentry/SentrySpan.m 35.249% <ø> (-62.453%) ⬇️
Sources/Sentry/SentryCoreDataTrackingIntegration.m 0.000% <0.000%> (-100.000%) ⬇️
Sources/Sentry/SentryDependencyContainer.m 37.549% <0.000%> (-52.174%) ⬇️
Sources/Sentry/SentryFileIOTracker.m 0.000% <0.000%> (-95.556%) ⬇️
Sources/Sentry/SentryNetworkTracker.m 0.000% <0.000%> (-96.729%) ⬇️

... and 412 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5b469c0...da10f3b. Read the comment docs.

@noahsmartin noahsmartin force-pushed the threadInspectorSwiftProtocol branch from bf419f3 to 0fd27d6 Compare September 24, 2025 18:40
Copy link

linear bot commented Sep 24, 2025

@noahsmartin noahsmartin force-pushed the threadInspectorSwiftProtocol branch 4 times, most recently from 1bb0934 to fe50cb1 Compare September 24, 2025 22:04
Copy link
Contributor

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1238.86 ms 1259.86 ms 21.00 ms
Size 23.75 KiB 980.96 KiB 957.21 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
5cfc768 1220.74 ms 1245.06 ms 24.32 ms
e3ebff3 1223.47 ms 1249.27 ms 25.80 ms
fccc4e5 1237.80 ms 1264.16 ms 26.37 ms
35f71f4 1208.96 ms 1225.61 ms 16.65 ms
fae97e5 1229.20 ms 1256.27 ms 27.06 ms
aff3b66 1229.53 ms 1263.08 ms 33.55 ms
ea2e54c 1207.78 ms 1218.13 ms 10.35 ms
07d7e83 1211.71 ms 1240.08 ms 28.37 ms
b045d0a 1227.48 ms 1252.22 ms 24.75 ms
f5666e7 1227.08 ms 1260.18 ms 33.10 ms

App size

Revision Plain With Sentry Diff
5cfc768 23.75 KiB 850.73 KiB 826.98 KiB
e3ebff3 23.75 KiB 878.48 KiB 854.73 KiB
fccc4e5 23.75 KiB 974.94 KiB 951.19 KiB
35f71f4 23.75 KiB 969.25 KiB 945.50 KiB
fae97e5 23.75 KiB 912.37 KiB 888.62 KiB
aff3b66 23.75 KiB 978.53 KiB 954.78 KiB
ea2e54c 23.75 KiB 919.70 KiB 895.95 KiB
07d7e83 23.75 KiB 913.27 KiB 889.52 KiB
b045d0a 23.75 KiB 880.21 KiB 856.46 KiB
f5666e7 23.75 KiB 963.18 KiB 939.43 KiB

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

I'm a bit confused. Can't the SentryThreadInspector implement the SentryThreadInspecting protocol? Then, the SentryDependencyContainer doesn't need to have a private property @property (nonatomic, strong) SentryThreadInspector *threadInspector;. We can have one public property for the new interface SentryThreadInspecting and it whould work.

@noahsmartin
Copy link
Contributor Author

Thanks @philipphofmann just made those changes

cursor[bot]

This comment was marked as outdated.

@noahsmartin noahsmartin force-pushed the threadInspectorSwiftProtocol branch from f19894e to da10f3b Compare September 30, 2025 13:41
Copy link
Contributor

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentryNetworkTracker.m

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.

2 participants