Inconsistent classification of in-app frames for iOS profiles

[noahsmartin]

Sometimes iOS profiles will label frames in the Sentry SDK as "application" and other times they will be "system". I'd expect the same code is always attributed the same way.

labeled as system:

labeled as application:

The problem seems to stem from an override that gets applied to frame classification when the symbol is in Sentry.framework, which only applies when using certain package managers that add Sentry as a dynamic framework instead of static. When it's added as a static framework this override does not kick in. More details in: #78

One fix for this could be to remove this override and always classify it as application, then we would be consistent, but in general with profiling the sentry SDK tends to get classified as system not application so that would introduce a new bug.

Example profile: https://sentry-sdks.sentry.io/insights/summary/profiles/?project=5428557&query=Noah3&statsPeriod=30d&transaction=Noah3%20Transaction

[viglia]

Considering what we discussed in the PR, and the comment from @cleptric in the SDKs channel (quote below), I'd say to make it consisten we want to always make it a system frame, not an application one.

we do not consider the sdk to be part of the application code.
This applies to the SDK not tracing its own http requests, removing frames from stacktraces when capturing exceptions that run through the SDK etc

---

@armcknight the issue here is that none of the checks (here) we make on the package work, because in this case, Sentry does not appear in the package and actually, this check on "/private/var/containers will mark is as application frame.

In this cases, what would be the best (and robust) way to detect them as system frames?
Not sure about some partial check Sentry* on the function name because with that we might end up wrongly classifying things that are not necessarily from the Sentry SDK itself.

Here below an example of such frame.

                {
                    "data": {},
                    "function": "-[SentryFramesTracker reportNewFrame]",
                    "in_app": true,
                    "instruction_addr": "0x103644af8",
                    "package": "/private/var/containers/Bundle/Application/B5677532-0EB3-4A29-9037-3994871C9A7E/iOS-Swift.app/iOS-Swift.debug.dylib",
                    "status": "missing",
                    "symbol": "-[SentryFramesTracker reportNewFrame]",
                    "platform": "cocoa"
                },

[armcknight]

The other issue with a case-insensitive *Sentry* match is that there may be frames from the Sentry SDK that don't actually contain the string in any form, this is true of some functions written in both Swift and C.

The static case I really don't think there's much we can do, and I agree that if we have to choose between one of the existing classes, that we should pick "application" vs "system". I disagree with the SDK team's analysis that Sentry frames are "not part of the application" since it is the customer's discretion to integrate and ship our SDK in their app.

It would be a bigger lift but it may make more sense to create a new classification especially for the Sentry SDK, if not for all 3rd party SDKs, but we still have the same problem of classifying statically linked symbols.

Maybe there is another kind of file we could upload with the CLI, like a load map that lets us see what symbols were actually linked from a static Sentry library?

[noahsmartin]

I'd say to make it consisten we want to always make it a system frame, not an application one.

I think it's the other way around, I don't think there is a way to make it consistent without considering all code in the app bundle part of the app code. If you start trying to separate individual source files into different categories I believe it will always run into edge cases/bugs, for example the linker could outline some code to dedup it between two files. If one of those files were in Sentry code and one wasn't how would you classify the new outlined code? You could maybe walk the stack back and see where the caller was, but that becomes even trickier with situations like a threads entry point. IMO the most bulletproof solution would be to simplify the logic to just "any code in the app bundle that the developer is in control of changing/updating is application code, anything that the end user is in control of updating (the operating system itself) is system code"

This would also be consistent with what developers are used to already, this screenshot shows the equivalent options in Xcode's flamegraph which behaves this way when you toggle "Hide System Libraries"