Skip to content

Conversation

@krystofwoldrich
Copy link
Contributor

@krystofwoldrich krystofwoldrich commented Aug 8, 2024

📢 Type of change

  • Refactoring

📜 Description

This PR refactors ReactNativeNavigationInstrumentation and ReactNavigationInstrumentation properties of reactNativeTracingIntegration to standalone integrations.

This PR removes RoutingInstrumentation interface which is replaced by startIdleNavigationSpan, getDefaultIdleNavigationSpanOptions and getCurrentReactNativeTracingIntegration().setCurrentRoute('new_route').

React Navigation

React Navigation v4 support is removed. The integration supports v5 and newer.

import Sentry from '@sentry/react-native';
import { NavigationContainer } from '@react-navigation/native';

// Before
const reactNavigationIntegration = new Sentry.ReactNavigationInstrumentation();

Sentry.init({
  tracesSampleRate: 1.0,
  integrations: [
    new Sentry.ReactNativeTracing({ routingInstrumentation }),
  ],
});
// After
const reactNavigationIntegration = Sentry.reactNavigationIntegration();

Sentry.init({
  tracesSampleRate: 1.0,
  integrations: [reactNavigationIntegration],
});
// Unchanged
function RootComponent() {
  const navigation = React.useRef(null);

  return <NavigationContainer ref={navigation}
    onReady={() => {
      reactNavigationIntegration.registerNavigationContainer(navigation);
    }}>
  </NavigationContainer>;
}

React Native Navigation

import Sentry from '@sentry/react-native';
import { Navigation } from 'react-native-navigation';

// Before
Sentry.init({
  tracesSampleRate: 1.0,
  integrations: [
    new Sentry.ReactNativeTracing({
      routingInstrumentation: new Sentry.ReactNativeNavigationInstrumentation(
        Navigation,
      ),
    }),
  ],
});
// After
Sentry.init({
  tracesSampleRate: 1.0,
  integrations: [
    Sentry.reactNativeNavigationIntegration({ navigation: Navigation })
  ],
});

Custom Navigation

// Before
import Sentry from '@sentry/react-native';

class CustomInstrumentation extends Sentry.RoutingInstrumentation {
  constructor(navigator) {
    super();

    this.navigator.registerRouteChangeListener(
      this.routeListener.bind(this),
    );
  }

  routeListener(newRoute) {
    // Again, ensure this is called BEFORE the route changes and BEFORE the route is mounted.
    this.onRouteWillChange({
      name: newRoute.name,
      op: "navigation",
    });
  }
}

Sentry.init({
  tracesSampleRate: 1.0,
  integrations: [
    new Sentry.ReactNativeTracing({
      routingInstrumentation: new CustomInstrumentation(navigator),
    }),
  ],
});
// After
import Sentry from '@sentry/react-native';

const customNavigationIntegration = ({navigator}) => {
  navigator.registerRouteChangeListener((newRoute) => {
    Sentry.startIdleNavigationSpan({
      name: newRoute.name,
      op: 'navigation'
    })
  });
  
  return {
  	name: "CustomNavigation",
  };
};

Sentry.init({
  tracesSampleRate: 1.0,
  integrations: [customNavigationIntegration({navigator})],
});

💡 Motivation and Context

This change refactoring decouples React Native Tracing and the navigation integrations. Now each navigation integration is one standalone file which contains the navigation library specific code.

💚 How did you test it?

sample app, unit tests

📝 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

krystofwoldrich and others added 30 commits June 3, 2024 18:39
Co-authored-by: LucasZF <lucas-zimerman1@hotmail.com>
@krystofwoldrich krystofwoldrich changed the title wip: Refactor Navigation Integrations to use new function style (7) wip: Refactor Navigation Integrations to use new function style Aug 8, 2024
@github-actions
Copy link
Contributor

github-actions bot commented Aug 9, 2024

Android (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 438.54 ms 464.74 ms 26.20 ms
Size 17.73 MiB 20.04 MiB 2.31 MiB

@github-actions
Copy link
Contributor

github-actions bot commented Aug 9, 2024

iOS (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1238.41 ms 1236.06 ms -2.35 ms
Size 2.36 MiB 3.06 MiB 713.28 KiB

@github-actions
Copy link
Contributor

github-actions bot commented Aug 9, 2024

iOS (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1243.08 ms 1235.40 ms -7.69 ms
Size 2.92 MiB 3.62 MiB 719.33 KiB

@krystofwoldrich krystofwoldrich marked this pull request as ready for review August 12, 2024 09:41
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.

Thank you for all the comments on the code! LGTM!

Base automatically changed from kw/tracing-integrations-independent-order to v6 August 14, 2024 07:53
@krystofwoldrich krystofwoldrich merged commit 643c544 into v6 Aug 14, 2024
@krystofwoldrich krystofwoldrich deleted the kw/ref-navigation-integrations branch August 14, 2024 07:58
@lucas-zimerman
Copy link
Collaborator

@krystofwoldrich we should add this to the migration guide to avoid issues like #4099

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