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.

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

Automatically detect package name for in-app-frame detection #818

Closed
ueman opened this issue Mar 31, 2022 · 7 comments
Closed

Automatically detect package name for in-app-frame detection #818

ueman opened this issue Mar 31, 2022 · 7 comments

Comments

@ueman
Copy link
Collaborator

ueman commented Mar 31, 2022

The Flutter initialization can be changed to the following, in order to automatically detect the application package name to correctly identify in-app-stack-frames.

/// Initializes the SDK
  static Future<void> init(
    FlutterOptionsConfiguration optionsConfiguration, {
    AppRunner? appRunner,
    @internal PackageLoader packageLoader = _loadPackageInfo,
    @internal MethodChannel channel = _channel,
    @internal PlatformChecker? platformChecker,
  }) async {
    final package = Trace.current(1).frames.first.package;

    final flutterOptions = SentryFlutterOptions();

    if (package != null) {
      flutterOptions.addInAppInclude(package);
      flutterOptions.considerInAppFramesByDefault = false;
    }

    if (platformChecker != null) {
      flutterOptions.platformChecker = platformChecker;
    }

    final nativeChannel = SentryNativeChannel(channel, flutterOptions);
    final native = SentryNative();
    native.setNativeChannel(nativeChannel);

    // first step is to install the native integration and set default values,
    // so we are able to capture future errors.
    final defaultIntegrations = _createDefaultIntegrations(
      packageLoader,
      channel,
      flutterOptions,
    );
    for (final defaultIntegration in defaultIntegrations) {
      flutterOptions.addIntegration(defaultIntegration);
    }

    await _initDefaultValues(flutterOptions, channel);

    await Sentry.init(
      (options) async {
        await optionsConfiguration(options as SentryFlutterOptions);
      },
      appRunner: appRunner,
      // ignore: invalid_use_of_internal_member
      options: flutterOptions,
    );
  }

This works at least on Flutter VM platforms. I haven't looked at pure Dart and Web use cases.

Relates to #729. If both in-app-includes and in-app-excludes work, the results should be fairly reliable.

@marandaneto
Copy link
Contributor

@ueman which package is this one? Trace.current(1).frames.first.package

The idea isn't bad, the only problem I see is if the SDK is being initialized in a library or 3rd party, that could lead to a wrong inApp, I guess?

@marandaneto
Copy link
Contributor

For a normal App, it'd return:

#0 Sentry.init (package:sentry/src/sentry.dart:44:30)

#1 main (file:///Users/user/Github/sentry-dart/dart/example/bin/example.dart:17:3)

So it'd only work for Flutter apps most likely.

@ueman
Copy link
Collaborator Author

ueman commented Apr 1, 2022

@ueman which package is this one? Trace.current(1).frames.first.package

It's from the stack_trace library which is already in use by the SDK.

The idea isn't bad, the only problem I see is if the SDK is being initialized in a library or 3rd party, that could lead to a wrong inApp, I guess?

Yep, that's true, but still better than the current default of in-app = true for all frames.

@marandaneto
Copy link
Contributor

trace.frames.first.library would return bin/example.dart from a Dart App.
So in the isInApp function, we could do: final packageOrLibrary = frame.package ?? frame.library; and verify the inAppIncludes and inAppExcludes.
Maybe that's doable.

@ueman
Copy link
Collaborator Author

ueman commented Apr 1, 2022

The idea isn't bad, the only problem I see is if the SDK is being initialized in a library or 3rd party, that could lead to a wrong inApp, I guess?

Maybe it's not a big problem if it gets logged with something like:
Automatically detected '<package_name>' as in app include. It it's not correct you can change it by doing <steps to change it>?

@marandaneto
Copy link
Contributor

Yeah, they can remove it from the inAppIncludes anyway, let's toy with it, it still needs testing on the Web due to minified JS and Flutter + split debug symbols.

@brustolin
Copy link
Collaborator

Since this is possible to do it manually, and also, doing It automatically can lead to unwanted side effects, for example, when initialising the SDK later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants