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

Allow to use custom UIViewController name when tracking breadcumbs #4642

Open
m1entus opened this issue Dec 17, 2024 · 3 comments · May be fixed by #4646
Open

Allow to use custom UIViewController name when tracking breadcumbs #4642

m1entus opened this issue Dec 17, 2024 · 3 comments · May be fixed by #4646

Comments

@m1entus
Copy link

m1entus commented Dec 17, 2024

Problem Statement

I would love to be able to report custom controller name when sending breadcumbs. Unfortunately we are using modules and each of our feature module UIViewControleller is ViewController as a class Feature.ViewController and seems it is always showing ViewController as a name in the panel. Ideally instead of NSString *name = [SwiftDescriptor getObjectClassName:controller]; we could use some like if ([controller conformsToProtocol:@protocol(ViewControllerBreadcrumbTracking)] then use custom name instead of getObjectClassName.

protocol ViewControllerBreadcrumbTracking {
    var screenName: String { get }
}
#if SENTRY_HAS_UIKIT
- (void)swizzleViewDidAppear
{

    // SentrySwizzleInstanceMethod declaration shadows a local variable. The swizzling is working
    // fine and we accept this warning.
#    pragma clang diagnostic push
#    pragma clang diagnostic ignored "-Wshadow"

    static const void *swizzleViewDidAppearKey = &swizzleViewDidAppearKey;
    SEL selector = NSSelectorFromString(@"viewDidAppear:");
    SentryBreadcrumbTracker *__weak weakSelf = self;

    SentrySwizzleMode mode = SentrySwizzleModeOncePerClassAndSuperclasses;

#    if defined(TEST) || defined(TESTCI)
    // some tests need to swizzle multiple times, once for each test case. but since they're in the
    // same process, if they set something other than "always", subsequent swizzles fail. override
    // it here for tests
    mode = SentrySwizzleModeAlways;
#    endif // defined(TEST) || defined(TESTCI)

    SentrySwizzleInstanceMethod(UIViewController.class, selector, SentrySWReturnType(void),
        SentrySWArguments(BOOL animated), SentrySWReplacement({
            SentryBreadcrumb *crumb = [[SentryBreadcrumb alloc] initWithLevel:kSentryLevelInfo
                                                                     category:@"ui.lifecycle"];
            crumb.type = @"navigation";
            crumb.data = [SentryBreadcrumbTracker fetchInfoAboutViewController:self];

            [weakSelf.delegate addBreadcrumb:crumb];

            SentrySWCallOriginal(animated);
        }),
        mode, swizzleViewDidAppearKey);
#    pragma clang diagnostic pop
}
+ (NSDictionary *)fetchInfoAboutViewController:(UIViewController *)controller
{
    NSMutableDictionary *info = @{}.mutableCopy;

    info[@"screen"] = [SwiftDescriptor getObjectClassName:controller];

    if ([controller.navigationItem.title length] != 0) {
        info[@"title"] = controller.navigationItem.title;
    } else if ([controller.title length] != 0) {
        info[@"title"] = controller.title;
    }

    info[@"beingPresented"] = controller.beingPresented ? @"true" : @"false";

    if (controller.presentingViewController != nil) {
        info[@"presentingViewController"] =
            [SwiftDescriptor getObjectClassName:controller.presentingViewController];
    }

    if (controller.parentViewController != nil) {
        info[@"parentViewController"] =
            [SwiftDescriptor getObjectClassName:controller.parentViewController];
    }

    if (controller.view.window != nil) {
        info[@"window"] = controller.view.window.description;
        info[@"window_isKeyWindow"] = controller.view.window.isKeyWindow ? @"true" : @"false";
        info[@"window_windowLevel"] =
            [NSString stringWithFormat:@"%f", controller.view.window.windowLevel];
        info[@"is_window_rootViewController"]
            = controller.view.window.rootViewController == controller ? @"true" : @"false";
    }

    return info;
}

Solution Brainstorm

Use

protocol ViewControllerBreadcrumbTracking {
    var screenName: String { get }
}

If viewController responds to ViewControllerBreadcrumbTracking then use screenName instead of [SwiftDescriptor getObjectClassName:controller]

Are you willing to submit a PR?

I could if we agree on solution.

@philipphofmann
Copy link
Member

philipphofmann commented Dec 17, 2024

@m1entus, your issue talks about breadcumbs but the code snipped refers to transactions. I guess that's a mistake?

For breadcrumbs, you could maybe use beforeBreadcrumb to set the proper UIViewController name: https://docs.sentry.io/platforms/apple/configuration/options/#before-breadcrumb. Does that work for you? If not, please elaborate.

@m1entus
Copy link
Author

m1entus commented Dec 17, 2024

@philipphofmann Sorry wrong code, but it is using the same methods which is [SwiftDescriptor getObjectClassName:controller] (i updated PR). Not sure how beforeBreadcrumb will help in that case if breadcumb data already will have String which is ViewController, i attached screenshot to show you a preview how it look like for us:

Image

Ideally for me to have: screen: MyFeature.ViewController.

FIY: marking MyFeature.ViewController as annotation for Objective-C @objc(MyFeatureViewController) doesn't help.

@brustolin
Copy link
Contributor

protocol ViewControllerBreadcrumbTracking {
var screenName: String { get }
}

I like this idea.
If you're willing to open a PR, I don’t see why not following this approach.

@m1entus m1entus linked a pull request Dec 18, 2024 that will close this issue
7 tasks
@kahest kahest linked a pull request Dec 18, 2024 that will close this issue
7 tasks
@kahest kahest moved this from Needs Discussion to In Progress in Mobile & Cross Platform SDK Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

3 participants