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.

Sign up for GitHub

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

Jump to bottom

(6) fix(tracing): ReactNativeTracing and initial navigation spans have to be created after integrations setup #4000

Merged
merged 44 commits into from
Aug 14, 2024
Merged

(6) fix(tracing): ReactNativeTracing and initial navigation spans have to be created after integrations setup #4000

merged 44 commits into from
Aug 14, 2024

Conversation

krystofwoldrich
Copy link
Member

📢 Type of change

  • Bugfix

📜 Description

This PR removes the dependency of initializing tracing integrations like NativeFramesTracking and StallTracking before ReactNativeTracing integration.

This dependency was introduced in #3996 and #3997 when extracting the mentioned into standalone integration.

The cause of the dependency was that ReactNativeTracing start initiali navigation span during the integrations setup, but should do it after all integrations are setup.

💚 How did you test it?

integration tests, sample app

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled
  • All tests passing
  • No breaking changes

#skip-changelog

krystofwoldrich and others added 30 commits June 3, 2024 18:39
@krystofwoldrich
chore(tracing): extract app start to a standalone integration
c3dcfd3
@krystofwoldrich
Merge branch 'v6' into kw/ref-app-start-integration
88d6988
@krystofwoldrich
fix merge
fe53af5
@krystofwoldrich
fix spans, app start is now reported to Sentry
d4ac89f
@krystofwoldrich
fix uikit init and minimal instrumentation edge cases
7c5dcaa
@krystofwoldrich
update js docs
0b42472
@krystofwoldrich
Merge branch 'v6' into kw/ref-app-start-integration
9765eaf
@krystofwoldrich
Add App Start Integration tests
b9e9e9a
@krystofwoldrich
Remove app start test from react native tracing
b334931
@krystofwoldrich
clean up app start tests
4d16787
@krystofwoldrich
fix test affected by the app start extraction
4d84922
@krystofwoldrich
Add standalone app start
0ff2020
@krystofwoldrich
fix
b1eab51
@krystofwoldrich
ref(tracing): Extract NativeFrames as standalone integration
6d1cd70
@krystofwoldrich
Add integration handling test
5eaaad2
@krystofwoldrich
Merge branch 'kw/ref-app-start-integration' into kw/ref-native-frames…
eba6820 
…-integration
@krystofwoldrich
clean up integrations tests
91c1eb8
@krystofwoldrich
move native frames tests
db70c09
@krystofwoldrich
add changelog
e199244
@krystofwoldrich
Merge branch 'kw/ref-app-start-integration' into kw/ref-native-frames…
2956344 
…-integration
@krystofwoldrich
fix
adb53fc
@krystofwoldrich
move the app start test to tracing
699fda7
@krystofwoldrich
Merge branch 'kw/ref-app-start-integration' into kw/ref-native-frames…
7a386ab 
…-integration
@krystofwoldrich
fix tests
ad98ac0
@krystofwoldrich
add changelog
f2b9abe
@krystofwoldrich
ref(tracing): Extract Stall Tracking to a standalone integration
89b2354
@krystofwoldrich
misc(tracing): Remove ReactNativeTracing deprecated options
4f3ca7b
@krystofwoldrich
fix changelog
5c12e5c
@krystofwoldrich
ref(tracing): Extract UserInteractionTracing as standalone interaction
a111bdf
@krystofwoldrich @lucas-zimerman
Apply suggestions from code review
92e04ee 
Co-authored-by: LucasZF <[email protected]>
@krystofwoldrich krystofwoldrich changed the title fix(tracing): ReactNativeTracing and initial navigation spans have to be created after integrations setup (6) fix(tracing): ReactNativeTracing and initial navigation spans have to be created after integrations setup Aug 8, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Aug 8, 2024 •
edited
Loading

Android (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 443.21 ms 485.47 ms 42.26 ms
Size 17.73 MiB 20.04 MiB 2.31 MiB

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Aug 8, 2024 •
edited
Loading

iOS (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1219.07 ms 1224.96 ms 5.89 ms
Size 2.36 MiB 3.06 MiB 714.01 KiB

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Aug 8, 2024 •
edited
Loading

iOS (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1237.57 ms 1241.16 ms 3.59 ms
Size 2.92 MiB 3.62 MiB 719.85 KiB

lucas-zimerman
lucas-zimerman approved these changes Aug 9, 2024
View reviewed changes
Copy link
Collaborator

@lucas-zimerman lucas-zimerman left a comment

Choose a reason for hiding this comment

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

LGTM!

@krystofwoldrich
Copy link
Member Author

I've fixed the JS tests.

The e2e fails due to #4004 unrelated to this PR, it will be fixed when I merge main to v6.

@krystofwoldrich krystofwoldrich mentioned this pull request Aug 12, 2024
Closed
Base automatically changed from kw/rn-tracing-removed-deprecated-options to v6 August 13, 2024 08:38
lucas-zimerman
lucas-zimerman approved these changes Aug 13, 2024
View reviewed changes
Copy link
Collaborator

@lucas-zimerman lucas-zimerman left a comment

Choose a reason for hiding this comment

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

LGTM!

@krystofwoldrich krystofwoldrich merged commit eeb83da into v6 Aug 14, 2024
56 of 58 checks passed
@krystofwoldrich krystofwoldrich deleted the kw/tracing-integrations-independent-order branch August 14, 2024 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lucas-zimerman lucas-zimerman lucas-zimerman approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@krystofwoldrich @lucas-zimerman