Skip to content

Conversation

denrase
Copy link
Collaborator

@denrase denrase commented Oct 6, 2025

📜 Description

Structured Logs: Move options out of experimental

💡 Motivation and Context

Closes #6013

💚 How did you test it?

Running CI

📝 Checklist

You have to check all boxes before merging:

  • 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.
  • I updated the wizard 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.

@denrase denrase marked this pull request as ready for review October 6, 2025 09:30
Copy link

codecov bot commented Oct 6, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@c1254d8). Learn more about missing BASE report.
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##             main     #6359   +/-   ##
========================================
  Coverage        ?   86.859%           
========================================
  Files           ?       441           
  Lines           ?     37267           
  Branches        ?     17349           
========================================
  Hits            ?     32370           
  Misses          ?      4856           
  Partials        ?        41           
Files with missing lines Coverage Δ
Sources/Sentry/SentryOptions.m 97.241% <100.000%> (ø)
Sources/Sentry/SentyOptionsInternal.m 99.675% <100.000%> (ø)
Sources/Swift/Helper/SentrySDK.swift 87.573% <100.000%> (ø)
Sources/Swift/SentryExperimentalOptions.swift 81.818% <ø> (ø)

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 c1254d8...0164ed6. Read the comment docs.

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.

I think before we do this we should make sure default stdout collection is turned on so that doesn’t need to done in a new major version

@philipphofmann
Copy link
Member

I think before we do this we should make sure default stdout collection is turned on so that doesn’t need to done in a new major version

Fair point. We can also create an issue to follow up with this, merge the PR and then ensure we don't release without ⬆️ The issue could be in the essential issues for V9 milestone in Linear.

cursor[bot]

This comment was marked as outdated.

Copy link
Contributor

github-actions bot commented Oct 8, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1227.71 ms 1260.82 ms 33.11 ms
Size 23.75 KiB 994.84 KiB 971.10 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
119ab1c 1226.79 ms 1254.55 ms 27.76 ms
5cbd333 1219.93 ms 1241.76 ms 21.83 ms
9add417 1224.33 ms 1243.06 ms 18.73 ms
a3dfd57 1230.78 ms 1244.91 ms 14.14 ms
f8029e2 1245.16 ms 1261.32 ms 16.16 ms
99104c9 1224.84 ms 1247.08 ms 22.24 ms
bc4bb46 1235.63 ms 1264.24 ms 28.61 ms
4e3915a 1230.02 ms 1258.90 ms 28.88 ms
2137530 1223.39 ms 1253.25 ms 29.86 ms
00d9740 1223.53 ms 1249.75 ms 26.22 ms

App size

Revision Plain With Sentry Diff
119ab1c 23.75 KiB 993.70 KiB 969.95 KiB
5cbd333 23.74 KiB 969.77 KiB 946.02 KiB
9add417 23.75 KiB 908.40 KiB 884.65 KiB
a3dfd57 23.75 KiB 913.63 KiB 889.87 KiB
f8029e2 23.75 KiB 893.72 KiB 869.97 KiB
99104c9 23.75 KiB 894.83 KiB 871.09 KiB
bc4bb46 23.75 KiB 980.81 KiB 957.07 KiB
4e3915a 23.75 KiB 858.69 KiB 834.94 KiB
2137530 23.75 KiB 933.33 KiB 909.58 KiB
00d9740 23.75 KiB 938.32 KiB 914.57 KiB

Previous results on branch: denrase/structured-logs-out-of-experimental

Startup times

Revision Plain With Sentry Diff
3705042 1217.00 ms 1240.06 ms 23.06 ms
d570a8e 1222.16 ms 1253.17 ms 31.00 ms
60ed5fd 1233.49 ms 1265.63 ms 32.14 ms

App size

Revision Plain With Sentry Diff
3705042 23.75 KiB 986.87 KiB 963.12 KiB
d570a8e 23.75 KiB 994.85 KiB 971.10 KiB
60ed5fd 23.75 KiB 994.85 KiB 971.10 KiB

@denrase
Copy link
Collaborator Author

denrase commented Oct 8, 2025

@philipphofmann Also added beforeSendLog to SentryOptionsTest

@denrase
Copy link
Collaborator Author

denrase commented Oct 8, 2025

@philipphofmann Do you have any idea why the public API job is failing? I did run & commit make generate-public-api

@denrase
Copy link
Collaborator Author

denrase commented Oct 8, 2025

Cross platform is failing because this is breaking API.

@philipphofmann
Copy link
Member

philipphofmann commented Oct 10, 2025

@philipphofmann Do you have any idea why the public API job is failing? I did run & commit make generate-public-api

You have to run it with Xcode 16. make generate-public-api. I also made the same mistake in another PR. I opened a PR to let the script fail if you're not running it with Xcode 16 #6396.

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.

Thank you @denrase. Please create a follow up issue for #6359 (review) and link it in Linear as mentioned here #6359 (comment). Then we can merge this.

@denrase
Copy link
Collaborator Author

denrase commented Oct 13, 2025

@philipphofmann Added #6411.

@denrase denrase enabled auto-merge (squash) October 14, 2025 12:02
@denrase denrase disabled auto-merge October 14, 2025 12:02
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.

Structured Logs: Move options out of experimental

3 participants