-
Notifications
You must be signed in to change notification settings - Fork 27
Hang Capture Service #233
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
Hang Capture Service #233
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Generated by 🚫 Danger Swift against 2225322 |
aa6bb0a to
86dc2a7
Compare
ac29d23 to
a107311
Compare
f501b9d to
fa93535
Compare
5c3fcc5 to
febebe2
Compare
d852558 to
6af2a1b
Compare
1a30db7 to
2225322
Compare
6667e4c to
3ebda71
Compare
822f1e7 to
1e9ae37
Compare
ee8093d to
21a23d6
Compare
21a23d6 to
c3d260d
Compare
c3d260d to
a36ccaa
Compare
3333080 to
4e78004
Compare
026ee39 to
2274265
Compare
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.
Pull Request Overview
This PR implements a hang capture service to detect and report app hangs on iOS. The service monitors the main RunLoop and generates OpenTelemetry spans when hangs exceed Apple's default 0.25 second threshold.
Key changes include:
- Creates a HangWatchdog that monitors the main RunLoop using CFRunLoopObserver to detect when the thread is blocked
- Implements HangCaptureService that converts hang events into OpenTelemetry spans with "emb-thread-blockage" naming
- Adds configuration support with HangLimits to control the number of hangs per session (200) and samples per hang (0)
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/TestSupport/Mocks/MockEmbraceConfigurable.swift | Adds hangLimits property to mock configuration |
| Tests/TestSupport/EditableConfig.swift | Adds hangLimits property to test configuration |
| Tests/EmbraceIOTests/CaptureServiceBuilderTests.swift | Updates tests to include HangCaptureService in service count validation |
| Tests/EmbraceCoreTests/Capture/Hang/HangCaptureServiceTests.swift | Comprehensive test suite for hang detection functionality |
| Sources/EmbraceIO/Capture/CaptureServiceBuilder.swift | Integrates hang watchdog into default capture services |
| Sources/EmbraceIO/Capture/CaptureService+Helpers.swift | Adds helper method to create HangCaptureService |
| Sources/EmbraceCore/Capture/Hang/Watchdog/NanosecondClock.swift | High-precision timing utility for hang measurement |
| Sources/EmbraceCore/Capture/Hang/Watchdog/HangWatchdog.swift | Core hang detection implementation using RunLoop observers |
| Sources/EmbraceCore/Capture/Hang/HangCaptureService.swift | Service that converts hang events to OpenTelemetry spans |
| Sources/EmbraceCore/Capture/CaptureServices.swift | Integrates hang limits configuration and session lifecycle |
| Sources/EmbraceConfiguration/HangLimits.swift | Configuration class for hang capture limits |
| Sources/EmbraceConfiguration/EmbraceConfigurable/DefaultConfig.swift | Adds hangLimits to default configuration |
| Sources/EmbraceConfiguration/EmbraceConfigurable.swift | Adds hangLimits property to configuration protocol |
| Sources/EmbraceConfigInternal/EmbraceConfigurable/RemoteConfig/RemoteConfigPayload.swift | Remote configuration support for hang limits |
| Sources/EmbraceConfigInternal/EmbraceConfigurable/RemoteConfig.swift | Implements hangLimits property for remote config |
| Sources/EmbraceCommonInternal/Locks/UnfairLock.swift | Minor extension reorganization |
| Sources/EmbraceCaptureService/CaptureService.swift | Adds session lifecycle methods to base CaptureService |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
ad0f5d4 to
1231b39
Compare
| const prAvg = '${{ steps.pr-tests.outputs.pr_avg }}'; | ||
| const mainAvg = '${{ steps.main-tests.outputs.main_avg }}'; | ||
| const percentDiff = '${{ steps.comparison.outputs.percent_diff }}'; | ||
| const absDiff = '${{ steps.comparison.outputs.abs_diff }}'; |
Check notice
Code scanning / zizmor
code injection via template expansion
| const mainAvg = '${{ steps.main-tests.outputs.main_avg }}'; | ||
| const percentDiff = '${{ steps.comparison.outputs.percent_diff }}'; | ||
| const absDiff = '${{ steps.comparison.outputs.abs_diff }}'; | ||
| const result = '${{ steps.comparison.outputs.result }}'; |
Check notice
Code scanning / zizmor
code injection via template expansion
| - name: Checkout main branch | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| ref: main |
Check warning
Code scanning / zizmor
credential persistence through GitHub Actions artifacts
| - name: Calculate performance comparison | ||
| id: comparison | ||
| run: | | ||
| pr_avg=${{ needs.performance-test-pr.outputs.pr_avg }} |
Check notice
Code scanning / zizmor
code injection via template expansion
| id: comparison | ||
| run: | | ||
| pr_avg=${{ needs.performance-test-pr.outputs.pr_avg }} | ||
| main_avg=${{ needs.performance-test-main.outputs.main_avg }} |
Check notice
Code scanning / zizmor
code injection via template expansion
| run: | | ||
| pr_avg=${{ needs.performance-test-pr.outputs.pr_avg }} | ||
| main_avg=${{ needs.performance-test-main.outputs.main_avg }} | ||
| pr_std=${{ needs.performance-test-pr.outputs.pr_std }} |
Check notice
Code scanning / zizmor
code injection via template expansion
| pr_avg=${{ needs.performance-test-pr.outputs.pr_avg }} | ||
| main_avg=${{ needs.performance-test-main.outputs.main_avg }} | ||
| pr_std=${{ needs.performance-test-pr.outputs.pr_std }} | ||
| main_std=${{ needs.performance-test-main.outputs.main_std }} |
Check notice
Code scanning / zizmor
code injection via template expansion
| const percentDiff = '${{ steps.comparison.outputs.percent_diff }}'; | ||
| const absDiff = '${{ steps.comparison.outputs.abs_diff }}'; | ||
| const result = '${{ steps.comparison.outputs.result }}'; | ||
| const emoji = '${{ steps.comparison.outputs.emoji }}'; |
Check notice
Code scanning / zizmor
code injection via template expansion
| const absDiff = '${{ steps.comparison.outputs.abs_diff }}'; | ||
| const result = '${{ steps.comparison.outputs.result }}'; | ||
| const emoji = '${{ steps.comparison.outputs.emoji }}'; | ||
| const prStd = '${{ needs.performance-test-pr.outputs.pr_std }}'; |
Check notice
Code scanning / zizmor
code injection via template expansion
| const result = '${{ steps.comparison.outputs.result }}'; | ||
| const emoji = '${{ steps.comparison.outputs.emoji }}'; | ||
| const prStd = '${{ needs.performance-test-pr.outputs.pr_std }}'; | ||
| const mainStd = '${{ needs.performance-test-main.outputs.main_std }}'; |
Check notice
Code scanning / zizmor
code injection via template expansion
| const emoji = '${{ steps.comparison.outputs.emoji }}'; | ||
| const prStd = '${{ needs.performance-test-pr.outputs.pr_std }}'; | ||
| const mainStd = '${{ needs.performance-test-main.outputs.main_std }}'; | ||
| const tStat = '${{ steps.comparison.outputs.t_stat }}'; |
Check notice
Code scanning / zizmor
code injection via template expansion
| const prStd = '${{ needs.performance-test-pr.outputs.pr_std }}'; | ||
| const mainStd = '${{ needs.performance-test-main.outputs.main_std }}'; | ||
| const tStat = '${{ steps.comparison.outputs.t_stat }}'; | ||
| const significant = '${{ steps.comparison.outputs.significant }}'; |
Check notice
Code scanning / zizmor
code injection via template expansion
ac9ac6c to
99639aa
Compare
🚀 Swift Test Performance Report✅ Performance Improvement
Performance Impact
📊 Difference not statistically significant Analysis🎉 Significant improvement detected - Great optimization work! 📊 View detailed statistics
Note: This test uses a reduced sample size (20 runs) for faster CI execution. This comment will be updated on subsequent commits. |
Co-Authored-By: Copilot <175728472+Copilot@users.noreply.github.com>
ac277f7 to
5521530
Compare
This PR creates a service to capture hangs. Hangs are sent in the same manner as they are on Android as described here. A similar span is also sent using iOS terminology as an interim in order for it to appear in the timeline.
emb-thread-blockagewith the count of hangs in the session.Currently usinguxfor the type, I think it needs to beperf.thread_blockage.Client Work
Backend work
perf.thread_blockagespans in timeline.Gate
Set hang_per_session = 0 to ensure the watchdog doesn't run.
Visuals