Skip to content

test(profiling): reproduce root span tracking issue #5071

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

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

armcknight
Copy link
Member

#skip-changelog

trying to repro a failure mode that was fixed in #5063 so we can have a unit test for it. it needs to integrate SentryTimeToDisplayTracker and SentryUIViewControllerSwizzling and related subsystems together with launch profiling.

@armcknight armcknight changed the title Armcknight/test/reproduce root span tracking issue test(profiling): reproduce root span tracking issue Apr 8, 2025
Copy link
Contributor

github-actions bot commented Apr 8, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1229.65 ms 1240.80 ms 11.14 ms
Size 22.30 KiB 844.41 KiB 822.11 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
9854a57 1230.02 ms 1252.78 ms 22.75 ms
eaa1002 1205.50 ms 1235.08 ms 29.58 ms
596ccc1 1214.98 ms 1239.36 ms 24.38 ms
326b7eb 1223.41 ms 1235.66 ms 12.25 ms
51307b7 1223.08 ms 1240.76 ms 17.68 ms
84972e4 1220.10 ms 1243.25 ms 23.15 ms
cc0f809 1231.63 ms 1254.26 ms 22.63 ms
96af87c 1228.76 ms 1242.27 ms 13.51 ms
6230686 1219.06 ms 1244.47 ms 25.41 ms
bf809be 1221.58 ms 1239.85 ms 18.27 ms

App size

Revision Plain With Sentry Diff
9854a57 21.90 KiB 727.72 KiB 705.82 KiB
eaa1002 20.76 KiB 423.19 KiB 402.43 KiB
596ccc1 22.84 KiB 401.44 KiB 378.60 KiB
326b7eb 20.76 KiB 432.31 KiB 411.55 KiB
51307b7 22.85 KiB 407.63 KiB 384.78 KiB
84972e4 21.58 KiB 637.65 KiB 616.06 KiB
cc0f809 22.31 KiB 768.14 KiB 745.83 KiB
96af87c 21.58 KiB 654.58 KiB 633.00 KiB
6230686 21.90 KiB 708.34 KiB 686.44 KiB
bf809be 22.30 KiB 821.87 KiB 799.56 KiB

Copy link

codecov bot commented Apr 8, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 8.892%. Comparing base (53e4168) to head (4f08abe).

❗ There is a different number of reports uploaded between BASE (53e4168) and HEAD (4f08abe). Click for more details.

HEAD has 2 uploads less than BASE
Flag BASE (53e4168) HEAD (4f08abe)
3 1
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main    #5071        +/-   ##
=============================================
- Coverage   92.711%   8.892%   -83.820%     
=============================================
  Files          674      358       -316     
  Lines        83514    25730     -57784     
  Branches     30409       94     -30315     
=============================================
- Hits         77427     2288     -75139     
- Misses        5989    23442     +17453     
+ Partials        98        0        -98     
Files with missing lines Coverage Δ
...entry/Profiling/SentryProfiledTracerConcurrency.mm 9.677% <ø> (-82.062%) ⬇️

... and 665 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 53e4168...4f08abe. Read the comment docs.

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

Comment on lines +123 to +133
// Act
_sentry_nondeduplicated_startLaunchProfile()

// Assert
XCTAssertEqual(_gInFlightRootSpans, 1)
XCTAssertTrue(SentryContinuousProfiler.isCurrentlyProfiling())

let uivc = ProfiledViewController(nibName: nil, bundle: nil)
uivc.loadView()
uivc.viewWillAppear(false)
fixture.displayLinkWrapper.call()
Copy link
Member

Choose a reason for hiding this comment

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

m: As you pointed out on Slack, you want to have input on that test. I think we could make our lives easier here by limiting the setup and dependencies we require for testing. Ideally, yes we test with the started SDK, an UIViewController to start and finish a trace, but if this causes trouble, we can also emulate what that code is doing by directly interacting with the code we want to test.
If the code we want to test requires all the setup, we could try to refactor the code so it is easier to test. I'm sorry it's a bit hard to explain in GH comment what exactly would be required here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I tried initially starting with just the components I had in mind, things kept failing in weird ways due to the tightly coupled nature of many parts of the SDK.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can refactor parts of the code we want to test so it depends on less code.

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