Skip to content

feat: Add support for tags and trace context fields for termination watch #5561

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 12 commits into from
Jul 8, 2025
Merged

feat: Add support for tags and trace context fields for termination watch #5561

merged 12 commits into from
Jul 8, 2025

Conversation

itaybre
Copy link
Contributor

@itaybre itaybre commented Jul 2, 2025

feat: Add support for tags and context fields for termination watch

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 2, 2025 •
edited
Loading

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 31469ba

@codecov Codecov
Copy link

codecov bot commented Jul 2, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 97.84946% with 2 lines in your changes missing coverage. Please review.

Project coverage is 86.344%. Comparing base (d05d866) to head (31469ba).
Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
...ersistence/SentryScopePersistentStore+Helper.swift 96.000% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #5561       +/-   ##
=============================================
- Coverage   86.367%   86.344%   -0.023%     
=============================================
  Files          408       413        +5     
  Lines        35209     35210        +1     
  Branches     15304     15311        +7     
=============================================
- Hits         30409     30402        -7     
- Misses        4754      4765       +11     
+ Partials        46        43        -3
Files with missing lines Coverage Δ
Sources/Sentry/SentryScope.m 96.990% <ø> (ø)
...es/Sentry/SentryWatchdogTerminationScopeObserver.m 85.000% <100.000%> (+10.641%) ⬆️
Sources/Sentry/SentryWatchdogTerminationTracker.m 100.000% <100.000%> (ø)
...try/SentryWatchdogTerminationTrackingIntegration.m 85.135% <100.000%> (+0.203%) ⬆️
...SentryWatchdogTerminationAttributesProcessor.swift 100.000% <100.000%> (ø)
...rsistence/SentryScopePersistentStore+Context.swift 100.000% <100.000%> (ø)
.../Persistence/SentryScopePersistentStore+Tags.swift 100.000% <100.000%> (ø)
.../Persistence/SentryScopePersistentStore+User.swift 100.000% <100.000%> (ø)
...Swift/Persistence/SentryScopePersistentStore.swift 100.000% <100.000%> (+1.234%) ⬆️
...ersistence/SentryScopePersistentStore+Helper.swift 96.000% <96.000%> (ø)

... and 19 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 d05d866...31469ba. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 2, 2025 •
edited
Loading

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1234.80 ms 1251.88 ms 17.08 ms
Size 23.75 KiB 880.00 KiB 856.25 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
8fd192f 1202.10 ms 1220.19 ms 18.09 ms
4d264fa 1223.48 ms 1246.91 ms 23.44 ms
32e7197 1226.91 ms 1245.48 ms 18.56 ms
2b02431 1229.63 ms 1248.98 ms 19.35 ms
f92cfa9 1228.45 ms 1251.33 ms 22.88 ms
c3fece5 1240.65 ms 1260.75 ms 20.10 ms
409a607 1229.57 ms 1251.45 ms 21.88 ms
2a9a505 1221.49 ms 1238.49 ms 17.00 ms
9389467 1218.62 ms 1244.86 ms 26.24 ms
4e3915a 1230.02 ms 1258.90 ms 28.88 ms

App size

Revision Plain With Sentry Diff
8fd192f 23.74 KiB 872.75 KiB 849.01 KiB
4d264fa 23.74 KiB 874.07 KiB 850.33 KiB
32e7197 23.75 KiB 866.69 KiB 842.94 KiB
2b02431 23.75 KiB 850.73 KiB 826.98 KiB
f92cfa9 23.75 KiB 855.38 KiB 831.62 KiB
c3fece5 23.75 KiB 874.17 KiB 850.42 KiB
409a607 23.74 KiB 874.08 KiB 850.33 KiB
2a9a505 23.75 KiB 874.46 KiB 850.71 KiB
9389467 23.75 KiB 866.51 KiB 842.76 KiB
4e3915a 23.75 KiB 858.69 KiB 834.94 KiB

Previous results on branch: itay/cocoa-412-add-missing-information-to-watchdog-termination-events-unified_tags_and_context

Startup times

Revision Plain With Sentry Diff
39d1d15 1234.02 ms 1249.79 ms 15.77 ms
0a4ff6c 1221.47 ms 1248.59 ms 27.12 ms
79fad59 1216.94 ms 1245.86 ms 28.92 ms
d23e3a9 1227.20 ms 1249.34 ms 22.14 ms
dcfc971 1225.63 ms 1248.15 ms 22.51 ms

App size

Revision Plain With Sentry Diff
39d1d15 23.75 KiB 881.94 KiB 858.20 KiB
0a4ff6c 23.75 KiB 882.50 KiB 858.75 KiB
79fad59 23.75 KiB 882.11 KiB 858.36 KiB
d23e3a9 23.75 KiB 882.14 KiB 858.39 KiB
dcfc971 23.75 KiB 882.10 KiB 858.35 KiB

@itaybre itaybre force-pushed the itay/cocoa-412-add-missing-information-to-watchdog-termination-events-unified_dist_and_environment branch from d19797d to 8670d28 Compare July 3, 2025 18:45
@itaybre itaybre force-pushed the itay/cocoa-412-add-missing-information-to-watchdog-termination-events-unified_tags_and_context branch from e11d84e to 571c50d Compare July 3, 2025 18:57
@itaybre itaybre marked this pull request as ready for review July 3, 2025 18:58
@philprime philprime changed the title feat: Add support for tags and context fields for termination watch feat: Add support for tags and trace context fields for termination watch Jul 4, 2025
philprime
philprime reviewed Jul 4, 2025
View reviewed changes
Copy link
Member

@philprime philprime left a comment

Choose a reason for hiding this comment

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

Left a couple of comments to tackle first

@itaybre itaybre force-pushed the itay/cocoa-412-add-missing-information-to-watchdog-termination-events-unified_dist_and_environment branch 2 times, most recently from 4262ae3 to 5ab9258 Compare July 7, 2025 16:47
Base automatically changed from itay/cocoa-412-add-missing-information-to-watchdog-termination-events-unified_dist_and_environment to main July 7, 2025 17:35
@itaybre itaybre force-pushed the itay/cocoa-412-add-missing-information-to-watchdog-termination-events-unified_tags_and_context branch from 9f9fe66 to d81fe76 Compare July 7, 2025 18:26
@itaybre
Copy link
Contributor Author

itaybre commented Jul 7, 2025

Confirmed working
image

philprime
philprime reviewed Jul 8, 2025
View reviewed changes
Copy link
Member

@philprime philprime left a comment

Choose a reason for hiding this comment

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

Good progress, almost LGTM.
Please take a look at the code coverage reported by Codecov.
You are missing test cases for the sanitization. Furthermore, please double-check that the expectation+wait in the tests actually make sense. Left comments for the remainders of the trace context.

@itaybre itaybre enabled auto-merge (squash) July 8, 2025 22:09
@itaybre itaybre disabled auto-merge July 8, 2025 22:10
@itaybre itaybre merged commit 186f06f into main Jul 8, 2025
123 of 126 checks passed
@itaybre itaybre deleted the itay/cocoa-412-add-missing-information-to-watchdog-termination-events-unified_tags_and_context branch July 8, 2025 22:10
@philipphofmann philipphofmann mentioned this pull request Jul 9, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philprime philprime philprime left review comments

@noahsmartin noahsmartin noahsmartin approved these changes

@philipphofmann philipphofmann Awaiting requested review from philipphofmann philipphofmann is a code owner

@armcknight armcknight Awaiting requested review from armcknight

@trevor-e trevor-e Awaiting requested review from trevor-e

@NicoHinderling NicoHinderling Awaiting requested review from NicoHinderling

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@itaybre @philipphofmann @noahsmartin @philprime