-
-
Notifications
You must be signed in to change notification settings - Fork 171
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
feat: Add support for in-app classification #514
Conversation
This needed some changes to the way that options are passed through when applying the scope to events.
* *in-app*, which match this pattern. | ||
* Matching is done on both symbols names and object file names they belong to. | ||
*/ | ||
SENTRY_API void sentry_options_add_in_app_include( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also add in_app_exclude
. After merging, let's add native
to supported platforms here:
* When on-device symbolication is enabled (see | ||
* `sentry_options_set_symbolize_stacktraces`), this will flag stack frames as | ||
* *in-app*, which match this pattern. | ||
* Matching is done on both symbols names and object file names they belong to. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Pattern" here is underspecified. If you compare with existing SDKs, such as JavaScript documented here, this is only a prefix match on the module.
@untitaker @mitsuhiko - did we never support functions for this? Being able to do a prefix match on functions sounds pretty essential, tbh.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prefix match on the module was entirely sufficient for python. function names of a library don't share a common pattern one could match against
istm native just needs a different API, but that's fine IMO
|
||
sentry_string_list_t *in_app = symbolize_data->options->in_app_includes; | ||
while (in_app) { | ||
if (strstr(symbol_name, in_app->str) != NULL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depending on if we go with a prefix match, you can use strncmp
here.
This needed some changes to the way that options are passed through when applying the scope to events.
Fixes #472
Fixes #473 (we already have
max_breadcrumbs
, andclose
is being added in #518)