Skip to content

Conversation

@itaybre
Copy link
Contributor

@itaybre itaybre commented Oct 23, 2025

This PR adds a new method in PrivateSentrySDKOnly for SDKs which use our SDK can add a custom view hierarchy.

Fixes: #6491

@github-actions
Copy link
Contributor

github-actions bot commented Oct 23, 2025

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

Generated by 🚫 dangerJS against 6d3defc

@codecov
Copy link

codecov bot commented Oct 23, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.666%. Comparing base (5ff1b04) to head (6d3defc).
⚠️ Report is 41 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #6521       +/-   ##
=============================================
+ Coverage   85.443%   85.666%   +0.222%     
=============================================
  Files          451       452        +1     
  Lines        27424     27655      +231     
  Branches     11933     12122      +189     
=============================================
+ Hits         23432     23691      +259     
+ Misses        3948      3917       -31     
- Partials        44        47        +3     

see 145 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 5ff1b04...6d3defc. Read the comment docs.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 23, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1194.48 ms 1221.77 ms 27.29 ms
Size 23.75 KiB 1023.82 KiB 1000.08 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
fccc4e5 1237.80 ms 1264.16 ms 26.37 ms
d157d83 1228.02 ms 1252.47 ms 24.45 ms
570f725 1206.00 ms 1238.96 ms 32.96 ms
649265b 1235.16 ms 1264.59 ms 29.43 ms
3319d58 1226.46 ms 1256.56 ms 30.10 ms
c8dd5e4 1217.67 ms 1242.90 ms 25.23 ms
4267263 1233.13 ms 1266.13 ms 33.00 ms
d637379 1226.43 ms 1250.77 ms 24.34 ms
d3e7aa6 1226.06 ms 1248.87 ms 22.81 ms
7aaa0b6 1230.49 ms 1263.78 ms 33.29 ms

App size

Revision Plain With Sentry Diff
fccc4e5 23.75 KiB 974.94 KiB 951.19 KiB
d157d83 23.75 KiB 928.85 KiB 905.10 KiB
570f725 23.74 KiB 913.38 KiB 889.63 KiB
649265b 23.75 KiB 980.81 KiB 957.06 KiB
3319d58 23.75 KiB 1021.48 KiB 997.73 KiB
c8dd5e4 23.75 KiB 913.48 KiB 889.72 KiB
4267263 23.75 KiB 988.03 KiB 964.28 KiB
d637379 23.75 KiB 855.38 KiB 831.63 KiB
d3e7aa6 23.75 KiB 913.16 KiB 889.41 KiB
7aaa0b6 23.75 KiB 989.97 KiB 966.23 KiB

Previous results on branch: itay/view_hierarchy_downstream_2

Startup times

Revision Plain With Sentry Diff
331866b 1226.74 ms 1261.31 ms 34.57 ms
ada6c87 1191.10 ms 1216.51 ms 25.41 ms
658951f 1222.57 ms 1261.89 ms 39.32 ms

App size

Revision Plain With Sentry Diff
331866b 23.75 KiB 1.00 MiB 1005.94 KiB
ada6c87 23.75 KiB 1.01 MiB 1016.25 KiB
658951f 23.75 KiB 1.02 MiB 1016.47 KiB

Base automatically changed from itay/enable_private_only_tests to main October 23, 2025 20:53
Copy link
Contributor

@noahsmartin noahsmartin left a comment

Choose a reason for hiding this comment

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

Is it not entirely possible to achieve the same behavior today without adding to this private API? You can call SentrySDK.configureScope { ... } Wouldn't that allow the Godot SDK to do the same thing?

IMO we should try to have fewer hybrid SDK specific APIs, and move towards a version where anything that needs to be visible to an SDK should be part of the public API. So my suggestion is just to double check, do we really need to add an API for this?

@itaybre
Copy link
Contributor Author

itaybre commented Oct 23, 2025

Is it not entirely possible to achieve the same behavior today without adding to this private API? You can call SentrySDK.configureScope { ... } Wouldn't that allow the Godot SDK to do the same thing?

IMO we should try to have fewer hybrid SDK specific APIs, and move towards a version where anything that needs to be visible to an SDK should be part of the public API. So my suggestion is just to double check, do we really need to add an API for this?

The current blocker is SentryAttachment's attachmentType being private only

@noahsmartin
Copy link
Contributor

Makes sense thanks for explaining @itaybre Maybe we should make the attachment type property visible to hybrid SDKs, or visible to everyone? That way the extra logic that would never get called from regular users of the cocoa SDK doesn't have to be included in every native app and can instead be in the Godot repo, seems more scalable/better abstracted that way

@philipphofmann
Copy link
Member

Maybe we should make the attachment type property visible to hybrid SDKs, or visible to everyone?

We didn't decide on this yet, but I think we should come up with proper APIs also for hybrid. This visible only to hybrid only causes problems. We quite often break them. We should treat hybrid as any other SDK user. So IMO we should make the things they need public with a proper API and only change these APIs when doing a major.

@itaybre itaybre changed the title feat: Add view hierarchy attachment support for downstream SDKs feat: Expose attachment type on SentryAttachment Oct 29, 2025
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.

Thanks @itaybre.

@itaybre itaybre merged commit 5e5d875 into main Nov 4, 2025
196 of 200 checks passed
@itaybre itaybre deleted the itay/view_hierarchy_downstream_2 branch November 4, 2025 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expose SentryAttachment's attachmentType for Internal SDKs

4 participants