Skip to content
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

feat: Add support for Sentry Spotlight #3642

Merged
merged 14 commits into from
Feb 19, 2024
Merged

Conversation

philipphofmann
Copy link
Member

@philipphofmann philipphofmann commented Feb 15, 2024

📜 Description

Add support for sending SDK events to Spotlight.

This PR is based on #3641.

💡 Motivation and Context

Fixes the Cocoa part of #3491.

💚 How did you test it?

Unit tests and manually sending events to a locally running Spotlight instance.

📝 Checklist

You have to check all boxes before merging:

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

Copy link

github-actions bot commented Feb 15, 2024

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

Generated by 🚫 dangerJS against c29978f

Base automatically changed from ref/multiple-transports to main February 15, 2024 13:29
Copy link

codecov bot commented Feb 15, 2024

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Comparison is base (1bdf1af) 89.213% compared to head (64f1a5a) 89.272%.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #3642       +/-   ##
=============================================
+ Coverage   89.213%   89.272%   +0.059%     
=============================================
  Files          532       534        +2     
  Lines        58722     58972      +250     
  Branches     21077     21163       +86     
=============================================
+ Hits         52388     52646      +258     
+ Misses        5297      5294        -3     
+ Partials      1037      1032        -5     
Files Coverage Δ
Sources/Sentry/SentryNSURLRequest.m 78.873% <100.000%> (+2.682%) ⬆️
Sources/Sentry/SentryNSURLRequestBuilder.m 100.000% <100.000%> (ø)
Sources/Sentry/SentryOptions.m 97.664% <100.000%> (+0.050%) ⬆️
Sources/Sentry/SentryTransportFactory.m 100.000% <100.000%> (ø)
...Tests/Networking/SentryTransportFactoryTests.swift 100.000% <100.000%> (ø)
...s/SentryTests/Networking/TestNSURLRequestBuilder.m 100.000% <100.000%> (ø)
Tests/SentryTests/Protocol/TestData.swift 98.936% <ø> (ø)
Tests/SentryTests/SentryOptionsTest.m 97.772% <100.000%> (+0.032%) ⬆️
...sts/Networking/SentrySpotlightTransportTests.swift 99.218% <99.218%> (ø)
Sources/Sentry/SentrySpotlightTransport.m 86.274% <86.274%> (ø)

... and 8 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 1bdf1af...64f1a5a. Read the comment docs.

Copy link

github-actions bot commented Feb 15, 2024

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1190.86 ms 1212.08 ms 21.22 ms
Size 21.58 KiB 424.30 KiB 402.72 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
11b2ffa 1204.86 ms 1218.16 ms 13.31 ms
8c50edb 1212.98 ms 1233.72 ms 20.74 ms
e778bd2 1224.66 ms 1252.16 ms 27.50 ms
4d3df92 1235.18 ms 1252.29 ms 17.10 ms
556c407 1256.56 ms 1274.60 ms 18.04 ms
0d32275 1215.31 ms 1240.19 ms 24.88 ms
af1f4dd 1207.33 ms 1230.04 ms 22.71 ms
034be1c 1222.67 ms 1236.22 ms 13.55 ms
0559a8f 1212.37 ms 1232.12 ms 19.76 ms
881a955 1222.16 ms 1237.22 ms 15.06 ms

App size

Revision Plain With Sentry Diff
11b2ffa 22.85 KiB 412.67 KiB 389.82 KiB
8c50edb 20.76 KiB 432.31 KiB 411.55 KiB
e778bd2 20.76 KiB 426.15 KiB 405.39 KiB
4d3df92 22.85 KiB 413.44 KiB 390.59 KiB
556c407 22.85 KiB 413.98 KiB 391.13 KiB
0d32275 22.84 KiB 403.14 KiB 380.29 KiB
af1f4dd 22.85 KiB 414.71 KiB 391.86 KiB
034be1c 20.76 KiB 436.66 KiB 415.90 KiB
0559a8f 21.58 KiB 419.81 KiB 398.22 KiB
881a955 22.85 KiB 407.63 KiB 384.78 KiB

Previous results on branch: feat/spotlight-support

Startup times

Revision Plain With Sentry Diff
1c08ded 1247.13 ms 1258.02 ms 10.89 ms
3fe5bdf 1233.47 ms 1248.85 ms 15.38 ms

App size

Revision Plain With Sentry Diff
1c08ded 21.58 KiB 421.65 KiB 400.06 KiB
3fe5bdf 21.58 KiB 421.64 KiB 400.06 KiB

Copy link
Contributor

@brustolin brustolin left a comment

Choose a reason for hiding this comment

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

Looks good.

A few non blocking suggestions.

Samples/iOS-Swift/iOS-Swift/AppDelegate.swift Show resolved Hide resolved
Sources/Sentry/Public/SentryOptions.h Show resolved Hide resolved
Sources/Sentry/SentryOptions.m Show resolved Hide resolved
Sources/Sentry/SentrySpotlightTransport.m Outdated Show resolved Hide resolved
@philipphofmann philipphofmann merged commit eff9311 into main Feb 19, 2024
61 of 64 checks passed
@philipphofmann philipphofmann deleted the feat/spotlight-support branch February 19, 2024 08:54
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