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

feat: App start measurement, initial ui.load transaction, and auto performance enable flag. #1704

Merged
merged 31 commits into from
Aug 6, 2021
Merged

feat: App start measurement, initial ui.load transaction, and auto performance enable flag. #1704

merged 31 commits into from
Aug 6, 2021

Conversation

jennmueng
Copy link
Member

@jennmueng jennmueng commented Jul 30, 2021 •
edited

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

Screen Shot 2021-07-30 at 2 56 50 PM

  • Auto performance monitoring (ReactNativeTracing integration) will now be enabled by default when tracing is enabled with either tracesSampleRate or tracesSampler.
    • Disable with enableAutoPerformanceTracking: false.
    • Configure by passing ReactNativeTracing to integrations.
  • enableStallTracking now depends on enableAutoPerformanceTracking.
  • App start span and measurements will be added to the first route transaction along with changing the operation to ui.load. If no routing instrumentation is set then a transaction is started (default).
  • Bumps Sentry Cocoa SDK to 7.2.0-beta.8
  • Bumps Sentry Android SDK to 5.1.0-beta.9

💡 Motivation and Context

Working towards #1701

💚 How did you test it?

Added unit tests for the app start instrumentation and the option flags.

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • All tests passing
  • No breaking changes

🔮 Next steps

@jennmueng jennmueng changed the base branch from master to jenn/native-bridge-refactor July 30, 2021 07:57
@jennmueng jennmueng changed the title feat: App start measurement, initial ui.load transaction, and auto performance by default. feat: App start measurement, initial ui.load transaction, and auto performance enable flag. Aug 3, 2021
@jennmueng jennmueng marked this pull request as ready for review August 3, 2021 04:08
@jennmueng jennmueng marked this pull request as draft August 3, 2021 04:08
@jennmueng jennmueng marked this pull request as ready for review August 3, 2021 04:19
@jennmueng jennmueng requested review from a team and HazAT August 3, 2021 04:19
@jennmueng jennmueng self-assigned this Aug 3, 2021
@jennmueng jennmueng moved this from In Progress to Waiting for Review in Mobile Platform Team Archived Aug 3, 2021
Base automatically changed from jenn/native-bridge-refactor to master August 3, 2021 14:06
bruno-garcia
bruno-garcia reviewed Aug 3, 2021
View reviewed changes
Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

There are some merge conflicts now

android/src/main/java/io/sentry/react/RNSentryModule.java Outdated Show resolved Hide resolved
@jennmueng
Copy link
Member Author

Weird, iOS sample app builds fine locally.. will look into this in the morning

bruno-garcia
bruno-garcia approved these changes Aug 3, 2021
View reviewed changes
Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

:shipit:

@jennmueng jennmueng marked this pull request as draft August 4, 2021 08:09
@jennmueng
Copy link
Member Author

@bruno-garcia We need to wait for getsentry/sentry-cocoa#1251 to be in a release, I misunderstood and thought it was in beta.8 of cocoa.

@jennmueng jennmueng marked this pull request as ready for review August 5, 2021 17:08
@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2021 •
edited

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

Generated by 🚫 dangerJS against 7056d75

@jennmueng jennmueng merged commit 957813a into master Aug 6, 2021
Mobile Platform Team Archived automation moved this from Waiting for Review to Done Aug 6, 2021
@jennmueng jennmueng deleted the jenn/app-start-actual branch August 6, 2021 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bruno-garcia bruno-garcia bruno-garcia approved these changes

@HazAT HazAT Awaiting requested review from HazAT

Assignees

@jennmueng jennmueng

Labels
None yet
Projects
No open projects
Mobile Platform Team Archived
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@jennmueng @bruno-garcia