Skip to content

Conversation

@krystofwoldrich
Copy link
Contributor

@krystofwoldrich krystofwoldrich commented Jun 3, 2024

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

Before this PR

  • AppStart Tracking is embedded in ReactNativeTracing implementation.
  • When automatic routing instrumentation is enabled appstart is attached to the first transaction.
  • When no routing instrumentation app start is standalone idle transaction.
import Sentry from '@sentry/react-native';

Sentry.init({
  tracesSampleRate: 1.0,
  integrations: [
    new Sentry.ReactNativeTracing({
      enableAppStartTracking: true, // default true
    }),
  ],
});

After this PR

  • AppStart Tracking is standalone integration.
  • When automatic routing instrumentation is enabled appstart is attached to the first transaction. No change, but this behaviour can be overwritten by user now.
  • When no routing instrumentation app start is standalone transaction. Linked by a traceId to the first transaction after start.
  • AppStart has standalone option which let user change the behaviour. By default we check presence of routing instrumentation like before.
  • Moving the tracking to a standalone integration will let us measure usage of this feature (enabled integrations telemetry).
import Sentry from '@sentry/react-native';

Sentry.init({
  tracesSampleRate: 1.0,
  enableAppStartTracking: true, // default true
  integrations: [
    Sentry.appStartIntegration({
      standalone: false, // default false
    }),
  ],
});

Examples After

Screenshot 2024-08-06 at 10 46 40
Screenshot 2024-08-06 at 10 44 24

💚 How did you test it?

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

@github-actions
Copy link
Contributor

github-actions bot commented Jun 3, 2024

Android (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 448.82 ms 519.66 ms 70.84 ms
Size 17.73 MiB 20.04 MiB 2.31 MiB

@github-actions
Copy link
Contributor

github-actions bot commented Jun 11, 2024

iOS (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1222.87 ms 1227.10 ms 4.23 ms
Size 2.36 MiB 3.05 MiB 706.54 KiB

@github-actions
Copy link
Contributor

github-actions bot commented Jun 11, 2024

iOS (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1237.00 ms 1231.53 ms -5.47 ms
Size 2.92 MiB 3.61 MiB 707.68 KiB

@github-actions
Copy link
Contributor

github-actions bot commented Jun 12, 2024

Android (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 344.73 ms 378.84 ms 34.11 ms
Size 7.15 MiB 8.19 MiB 1.04 MiB

@krystofwoldrich krystofwoldrich marked this pull request as ready for review August 6, 2024 09:40
@krystofwoldrich krystofwoldrich changed the title chore(tracing): extract app start to a standalone integration (1) chore(tracing): extract app start to a standalone integration Aug 8, 2024
@krystofwoldrich krystofwoldrich changed the title (1) chore(tracing): extract app start to a standalone integration (1) ref(tracing): extract app start to a standalone integration Aug 8, 2024
// The first root component mount is the app start finish.
&& tracingIntegration.onAppStartFinish(endTimestamp);
// eslint-disable-next-line @typescript-eslint/no-floating-promises
captureAppStart();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Q:Shouldn't we wait for it?

Copy link
Contributor Author

@krystofwoldrich krystofwoldrich Aug 9, 2024

Choose a reason for hiding this comment

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

I only trigger the capture of app start here, recording the timestamp and I don't await it as the fetch from native layer can happen async later

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.

Non-blocking question, LGTM!

@krystofwoldrich krystofwoldrich merged commit bfeaaf7 into v6 Aug 9, 2024
@krystofwoldrich krystofwoldrich deleted the kw/ref-app-start-integration branch August 9, 2024 08:40
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