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: Enable File I/O APM by default #2497

Merged
merged 4 commits into from
Dec 6, 2022

Conversation

kevinrenskers
Copy link
Contributor

@kevinrenskers kevinrenskers commented Dec 6, 2022

📜 Description

See title.

💡 Motivation and Context

Closes #2264.

💚 How did you test it?

Unit test.

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the docs if needed
  • Review from the native team if needed
  • No breaking changes

🔮 Next steps

@kevinrenskers
Copy link
Contributor Author

Are there any docs that need to be updated?

@github-actions
Copy link

github-actions bot commented Dec 6, 2022

Fails
🚫 Please consider adding a changelog entry for the next release.
Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

- Enable File I/O APM by default ([#2497](https://github.com/getsentry/sentry-cocoa/pull/2497))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description.

Generated by 🚫 dangerJS against 87d90c7

CHANGELOG.md Outdated
@@ -7,6 +7,7 @@ This version adds a dependency on Swift.
### Features

- Properly demangle Swift class name (#2162)
- Enable File I/O APM per default (#2497)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The changelog entry seems to be part of an already released section ## 8.0.0

Uhm what?

@github-actions
Copy link

github-actions bot commented Dec 6, 2022

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1202.67 ms 1230.84 ms 28.17 ms
Size 20.75 KiB 404.83 KiB 384.08 KiB

Baseline results on branch: 8.0.0

Startup times

Revision Plain With Sentry Diff
a9e77dc 1231.94 ms 1254.85 ms 22.91 ms
010583c 1198.23 ms 1225.52 ms 27.29 ms
88ac2c2 1223.04 ms 1243.12 ms 20.08 ms
bbe81cb 1257.25 ms 1272.24 ms 14.99 ms
57e140e 1223.94 ms 1252.50 ms 28.56 ms
6ef7dc2 1204.14 ms 1253.76 ms 49.62 ms
323cdfe 1263.20 ms 1272.14 ms 8.94 ms
5ed4bee 1226.02 ms 1247.50 ms 21.48 ms
3936dd2 1214.98 ms 1242.24 ms 27.26 ms
d446105 1237.06 ms 1261.34 ms 24.28 ms

App size

Revision Plain With Sentry Diff
a9e77dc 20.75 KiB 379.12 KiB 358.36 KiB
010583c 20.75 KiB 383.61 KiB 362.85 KiB
88ac2c2 20.75 KiB 373.61 KiB 352.86 KiB
bbe81cb 20.75 KiB 381.81 KiB 361.06 KiB
57e140e 20.75 KiB 383.61 KiB 362.85 KiB
6ef7dc2 20.75 KiB 383.39 KiB 362.64 KiB
323cdfe 20.75 KiB 383.50 KiB 362.75 KiB
5ed4bee 20.75 KiB 404.63 KiB 383.88 KiB
3936dd2 20.75 KiB 383.29 KiB 362.53 KiB
d446105 20.75 KiB 383.37 KiB 362.62 KiB

Previous results on branch: feat/2264-enableFileIOTracing

Startup times

Revision Plain With Sentry Diff
d7a9d1b 1258.29 ms 1268.88 ms 10.59 ms

App size

Revision Plain With Sentry Diff
d7a9d1b 20.75 KiB 404.83 KiB 384.08 KiB

@codecov
Copy link

codecov bot commented Dec 6, 2022

Codecov Report

Merging #2497 (6fa81f2) into 8.0.0 (5ed4bee) will not change coverage.
The diff coverage is 0.00%.

❗ Current head 6fa81f2 differs from pull request most recent head 87d90c7. Consider uploading reports for the commit 87d90c7 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##           8.0.0   #2497   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files        242     242           
  Lines      22416   22431   +15     
  Branches    9187    9189    +2     
=====================================
- Misses     22411   22426   +15     
  Partials       5       5           
Impacted Files Coverage Δ
Sources/Sentry/SentryOptions.m 0.00% <0.00%> (ø)
Sources/Sentry/SentrySystemEventBreadcrumbs.m 0.00% <0.00%> (ø)
Sources/Sentry/SentryNSNotificationCenterWrapper.m 0.00% <0.00%> (ø)
...s/Sentry/SentryAutoBreadcrumbTrackingIntegration.m 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5ed4bee...87d90c7. Read the comment docs.

@philipphofmann
Copy link
Member

Are there any docs that need to be updated?

Yes, but we do that once 8.0.0 is GA. We will batch that together in one bigger PR.

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.

m: Please also update the code docs

/**
* When enabled, the SDK tracks performance for file IO reads and writes with NSData if auto
* performance tracking and enableSwizzling are enabled. The default is <code>NO</code>.
*/
@property (nonatomic, assign) BOOL enableFileIOTracing;

LGTM

@kevinrenskers kevinrenskers changed the title feat: Enable File I/O APM per default feat: Enable File I/O APM by default Dec 6, 2022
@kevinrenskers kevinrenskers merged commit 4180ade into 8.0.0 Dec 6, 2022
@kevinrenskers kevinrenskers deleted the feat/2264-enableFileIOTracing branch December 6, 2022 13:48
kevinrenskers added a commit that referenced this pull request Dec 6, 2022
* 8.0.0:
  chore: Update homebrew bundle (#2495)
  ci: Fix Slather for Codecov (#2494)
  feat: Enable File I/O APM by default (#2497)
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.

Enable File I/O APM per default
2 participants