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(profiling): new start and stop API #4855

Open
wants to merge 26 commits into
base: armcknight/git-chain/profiling/new-continuous-apis/0-topic-branch
Choose a base branch
from

Conversation

armcknight
Copy link
Member

@armcknight armcknight commented Feb 15, 2025

stub the new start and stop methods we'll offer for continuous profiling. there are some TODOs for checks that come in later PR branches as those API are implemented and become available.

#skip-changelog; for #4853

Copy link

github-actions bot commented Feb 15, 2025

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

Generated by 🚫 dangerJS against 48bb10b

Copy link

codecov bot commented Feb 15, 2025

Codecov Report

Attention: Patch coverage is 96.72897% with 7 lines in your changes missing coverage. Please review.

Project coverage is 92.506%. Comparing base (c964075) to head (48bb10b).
Report is 12 commits behind head on armcknight/git-chain/profiling/new-continuous-apis/0-topic-branch.

Files with missing lines Patch % Lines
Tests/SentryTests/SentrySDKTests.swift 97.474% 5 Missing ⚠️
Sources/Sentry/SentrySDK.m 87.500% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                                           Coverage Diff                                           @@
##           armcknight/git-chain/profiling/new-continuous-apis/0-topic-branch     #4855       +/-   ##
=======================================================================================================
+ Coverage                                                             92.470%   92.506%   +0.036%     
=======================================================================================================
  Files                                                                    666       666               
  Lines                                                                  78926     78993       +67     
  Branches                                                               28572     28599       +27     
=======================================================================================================
+ Hits                                                                   72983     73074       +91     
+ Misses                                                                  5845      5820       -25     
- Partials                                                                  98        99        +1     
Files with missing lines Coverage Δ
Sources/Sentry/SentrySDK.m 90.625% <87.500%> (+1.339%) ⬆️
Tests/SentryTests/SentrySDKTests.swift 97.161% <97.474%> (+0.170%) ⬆️

... and 11 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 c964075...48bb10b. Read the comment docs.

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

Copy link

github-actions bot commented Feb 15, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1202.57 ms 1236.37 ms 33.79 ms
Size 22.30 KiB 823.67 KiB 801.37 KiB

Baseline results on branch: armcknight/git-chain/profiling/new-continuous-apis/0-topic-branch

Startup times

Revision Plain With Sentry Diff
b81c36b 1229.65 ms 1252.16 ms 22.51 ms

App size

Revision Plain With Sentry Diff
b81c36b 22.30 KiB 823.44 KiB 801.13 KiB

Previous results on branch: armcknight/profiling/new-continuous-apis/2-new-start-stop-api

Startup times

Revision Plain With Sentry Diff
c760c84 1221.76 ms 1243.46 ms 21.70 ms
6f57ac5 1230.02 ms 1244.73 ms 14.71 ms
6b4657c 1215.92 ms 1236.57 ms 20.65 ms
96e7192 1206.88 ms 1235.85 ms 28.97 ms
468d437 1228.67 ms 1250.08 ms 21.41 ms

App size

Revision Plain With Sentry Diff
c760c84 22.30 KiB 823.68 KiB 801.38 KiB
6f57ac5 22.32 KiB 819.64 KiB 797.32 KiB
6b4657c 22.32 KiB 819.64 KiB 797.32 KiB
96e7192 22.30 KiB 822.04 KiB 799.74 KiB
468d437 22.30 KiB 822.04 KiB 799.73 KiB

@armcknight armcknight force-pushed the armcknight/profiling/new-continuous-apis/1-deprecations branch from 2888371 to 3b04d2d Compare February 18, 2025 07:04
@armcknight armcknight force-pushed the armcknight/profiling/new-continuous-apis/2-new-start-stop-api branch from a09d3e7 to 537c49d Compare February 18, 2025 07:04
@armcknight armcknight force-pushed the armcknight/profiling/new-continuous-apis/1-deprecations branch from 3b04d2d to 29e5d36 Compare March 6, 2025 23:29
@armcknight armcknight force-pushed the armcknight/profiling/new-continuous-apis/1-deprecations branch from 29e5d36 to 5b002e0 Compare March 6, 2025 23:45
@armcknight armcknight force-pushed the armcknight/profiling/new-continuous-apis/2-new-start-stop-api branch from 537c49d to 52a2d68 Compare March 6, 2025 23:45
@armcknight armcknight marked this pull request as ready for review March 7, 2025 00:01
@armcknight armcknight force-pushed the armcknight/profiling/new-continuous-apis/2-new-start-stop-api branch from 2990aba to d49fbd0 Compare March 7, 2025 00:19
@armcknight armcknight force-pushed the armcknight/profiling/new-continuous-apis/2-new-start-stop-api branch from d49fbd0 to 11a6de0 Compare March 7, 2025 02:49
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.

As this doesn't get merged to main, the TODOs are OK, LGTM

Base automatically changed from armcknight/profiling/new-continuous-apis/1-deprecations to armcknight/git-chain/profiling/new-continuous-apis/0-topic-branch March 7, 2025 19:14
@armcknight armcknight force-pushed the armcknight/profiling/new-continuous-apis/2-new-start-stop-api branch from b7d9f5a to 48bb10b Compare March 8, 2025 02:06
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.

3 participants