From ec8f02c972a4c62c64779ec6d877e3cd74cedb8e Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Wed, 14 Apr 2021 15:17:52 +0200 Subject: [PATCH] feat: Add support for in-app classification This needed some changes to the way that options are passed through when applying the scope to events. --- include/sentry.h | 15 +++++- src/backends/sentry_backend_crashpad.cpp | 5 +- src/sentry_backend.h | 26 +++++----- src/sentry_core.c | 2 +- src/sentry_core.h | 5 -- src/sentry_options.c | 26 ++++++++++ src/sentry_options.h | 10 ++++ src/sentry_scope.c | 61 +++++++++++++++++------- src/sentry_scope.h | 3 +- tests/unit/test_mpack.c | 4 +- tests/unit/test_unwinder.c | 53 ++++++++++++++++++++ tests/unit/tests.inc | 1 + 12 files changed, 169 insertions(+), 42 deletions(-) diff --git a/include/sentry.h b/include/sentry.h index b7e4978c73..977af1e793 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -834,12 +834,23 @@ SENTRY_API void sentry_options_set_symbolize_stacktraces( SENTRY_API int sentry_options_get_symbolize_stacktraces( const sentry_options_t *opts); +/** + * Adds a new pattern for *in-app* classification. + * + * 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. + */ +SENTRY_API void sentry_options_add_in_app_include( + sentry_options_t *opts, const char *pattern); + /** * Adds a new attachment to be sent along. * * `path` is assumed to be in platform-specific filesystem path encoding. - * API Users on windows are encouraged to use `sentry_options_add_attachmentw` - * instead. + * API Users on windows are encouraged to use + * `sentry_options_add_attachmentw` instead. */ SENTRY_API void sentry_options_add_attachment( sentry_options_t *opts, const char *path); diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index 04f71b6bd5..d5f904d9ca 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -65,7 +65,8 @@ sentry__crashpad_backend_user_consent_changed(sentry_backend_t *backend) } static void -sentry__crashpad_backend_flush_scope(sentry_backend_t *backend) +sentry__crashpad_backend_flush_scope( + sentry_backend_t *backend, const sentry_options_t *options) { const crashpad_state_t *data = (crashpad_state_t *)backend->data; if (!data->event_path) { @@ -78,7 +79,7 @@ sentry__crashpad_backend_flush_scope(sentry_backend_t *backend) sentry_value_t event = sentry_value_new_object(); SENTRY_WITH_SCOPE (scope) { // we want the scope without any modules or breadcrumbs - sentry__scope_apply_to_event(scope, event, SENTRY_SCOPE_NONE); + sentry__scope_apply_to_event(scope, options, event, SENTRY_SCOPE_NONE); } size_t mpack_size; diff --git a/src/sentry_backend.h b/src/sentry_backend.h index 1eb025d382..02faf33039 100644 --- a/src/sentry_backend.h +++ b/src/sentry_backend.h @@ -11,24 +11,22 @@ * can ensure that any captured crash contains the sentry scope and other * information. */ -struct sentry_backend_s; -typedef struct sentry_backend_s { - int (*startup_func)( - struct sentry_backend_s *, const sentry_options_t *options); - void (*shutdown_func)(struct sentry_backend_s *); - void (*free_func)(struct sentry_backend_s *); - void (*except_func)( - struct sentry_backend_s *, const struct sentry_ucontext_s *); - void (*flush_scope_func)(struct sentry_backend_s *); +typedef struct sentry_backend_s sentry_backend_t; +struct sentry_backend_s { + int (*startup_func)(sentry_backend_t *, const sentry_options_t *options); + void (*shutdown_func)(sentry_backend_t *); + void (*free_func)(sentry_backend_t *); + void (*except_func)(sentry_backend_t *, const struct sentry_ucontext_s *); + void (*flush_scope_func)( + sentry_backend_t *, const sentry_options_t *options); // NOTE: The breadcrumb is not moved into the hook and does not need to be // `decref`-d internally. - void (*add_breadcrumb_func)( - struct sentry_backend_s *, sentry_value_t breadcrumb); - void (*user_consent_changed_func)(struct sentry_backend_s *); - uint64_t (*get_last_crash_func)(struct sentry_backend_s *); + void (*add_breadcrumb_func)(sentry_backend_t *, sentry_value_t breadcrumb); + void (*user_consent_changed_func)(sentry_backend_t *); + uint64_t (*get_last_crash_func)(sentry_backend_t *); void *data; bool can_capture_after_shutdown; -} sentry_backend_t; +}; /** * This will free a previously allocated backend. diff --git a/src/sentry_core.c b/src/sentry_core.c index 9e7aab29c4..15459f13d5 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -370,7 +370,7 @@ sentry__prepare_event(const sentry_options_t *options, sentry_value_t event, if (!options->symbolize_stacktraces) { mode &= ~SENTRY_SCOPE_STACKTRACES; } - sentry__scope_apply_to_event(scope, event, mode); + sentry__scope_apply_to_event(scope, options, event, mode); } if (options->before_send_func) { diff --git a/src/sentry_core.h b/src/sentry_core.h index 156801ff13..46e546630f 100644 --- a/src/sentry_core.h +++ b/src/sentry_core.h @@ -70,11 +70,6 @@ sentry_value_t sentry__ensure_event_id( */ const sentry_options_t *sentry__options_getref(void); -/** - * Release the lock on the global options. - */ -void sentry__options_unlock(void); - #define SENTRY_WITH_OPTIONS(Options) \ for (const sentry_options_t *Options = sentry__options_getref(); Options; \ sentry_options_free((sentry_options_t *)Options), Options = NULL) diff --git a/src/sentry_options.c b/src/sentry_options.c index 8468c2c58f..80f09b8fac 100644 --- a/src/sentry_options.c +++ b/src/sentry_options.c @@ -84,6 +84,15 @@ sentry_options_free(sentry_options_t *opts) sentry_transport_free(opts->transport); sentry__backend_free(opts->backend); + sentry_string_list_t *next_in_app = opts->in_app_includes; + while (next_in_app) { + sentry_string_list_t *in_app = next_in_app; + next_in_app = in_app->next; + + sentry_free(in_app->str); + sentry_free(in_app); + } + sentry_attachment_t *next_attachment = opts->attachments; while (next_attachment) { sentry_attachment_t *attachment = next_attachment; @@ -290,6 +299,23 @@ sentry_options_get_symbolize_stacktraces(const sentry_options_t *opts) return opts->symbolize_stacktraces; } +void +sentry_options_add_in_app_include(sentry_options_t *opts, const char *pattern) +{ + char *owned_pattern = sentry__string_clone(pattern); + if (!owned_pattern) { + return; + } + sentry_string_list_t *list = SENTRY_MAKE(sentry_string_list_t); + if (!list) { + sentry_free(owned_pattern); + return; + } + list->str = owned_pattern; + list->next = opts->in_app_includes; + opts->in_app_includes = list; +} + void sentry_options_set_system_crash_reporter_enabled( sentry_options_t *opts, int enabled) diff --git a/src/sentry_options.h b/src/sentry_options.h index 09b7275223..ac8d376c60 100644 --- a/src/sentry_options.h +++ b/src/sentry_options.h @@ -24,6 +24,15 @@ struct sentry_attachment_s { sentry_attachment_t *next; }; +/** + * A linked list of strings, used for *in-app* patterns. + */ +typedef struct sentry_string_list_s sentry_string_list_t; +struct sentry_string_list_s { + char *str; + sentry_string_list_t *next; +}; + /** * This is the main options struct, which is being accessed throughout all of * the sentry internals. @@ -47,6 +56,7 @@ typedef struct sentry_options_s { bool symbolize_stacktraces; bool system_crash_reporter_enabled; + sentry_string_list_t *in_app_includes; sentry_attachment_t *attachments; sentry_run_t *run; diff --git a/src/sentry_scope.c b/src/sentry_scope.c index 814b6f8156..98715589b1 100644 --- a/src/sentry_scope.c +++ b/src/sentry_scope.c @@ -127,7 +127,7 @@ sentry__scope_flush_unlock(const sentry_scope_t *scope) // we try to unlock the scope/session lock as soon as possible. The // backend will do its own `WITH_SCOPE` internally. if (options->backend && options->backend->flush_scope_func) { - options->backend->flush_scope_func(options->backend); + options->backend->flush_scope_func(options->backend, options); } } if (!did_unlock) { @@ -136,8 +136,9 @@ sentry__scope_flush_unlock(const sentry_scope_t *scope) } static void -sentry__foreach_stacktrace( - sentry_value_t event, void (*func)(sentry_value_t stacktrace)) +sentry__foreach_stacktrace(const sentry_options_t *options, + sentry_value_t event, + void (*func)(const sentry_options_t *options, sentry_value_t stacktrace)) { // We have stacktraces at the following locations: // * `exception[.values].X.stacktrace`: @@ -155,7 +156,7 @@ sentry__foreach_stacktrace( sentry_value_t stacktrace = sentry_value_get_by_key( sentry_value_get_by_index(exception, i), "stacktrace"); if (!sentry_value_is_null(stacktrace)) { - func(stacktrace); + func(options, stacktrace); } } } @@ -170,17 +171,24 @@ sentry__foreach_stacktrace( sentry_value_t stacktrace = sentry_value_get_by_key( sentry_value_get_by_index(threads, i), "stacktrace"); if (!sentry_value_is_null(stacktrace)) { - func(stacktrace); + func(options, stacktrace); } } } } +typedef struct { + sentry_value_t frame; + const sentry_options_t *options; +} sentry__symbolize_frame_data_t; + static void sentry__symbolize_frame(const sentry_frame_info_t *info, void *data) { // See https://develop.sentry.dev/sdk/event-payloads/stacktrace/ - sentry_value_t frame = *(sentry_value_t *)data; + sentry__symbolize_frame_data_t *symbolize_data + = (sentry__symbolize_frame_data_t *)data; + sentry_value_t frame = symbolize_data->frame; if (info->symbol && sentry_value_is_null(sentry_value_get_by_key(frame, "function"))) { @@ -206,10 +214,28 @@ sentry__symbolize_frame(const sentry_frame_info_t *info, void *data) sentry_value_set_by_key(frame, "image_addr", sentry__value_new_addr((uint64_t)(size_t)info->load_addr)); } + + const char *symbol_name + = sentry_value_as_string(sentry_value_get_by_key(frame, "function")); + const char *object_name + = sentry_value_as_string(sentry_value_get_by_key(frame, "package")); + + sentry_string_list_t *in_app = symbolize_data->options->in_app_includes; + while (in_app) { + if (strstr(symbol_name, in_app->str) != NULL + || strstr(object_name, in_app->str) != NULL) { + sentry_value_set_by_key( + frame, "in_app", sentry_value_new_bool(true)); + break; + } + + in_app = in_app->next; + } } static void -sentry__symbolize_stacktrace(sentry_value_t stacktrace) +sentry__symbolize_stacktrace( + const sentry_options_t *options, sentry_value_t stacktrace) { sentry_value_t frames = sentry_value_get_by_key(stacktrace, "frames"); if (sentry_value_get_type(frames) != SENTRY_VALUE_TYPE_LIST) { @@ -232,13 +258,17 @@ sentry__symbolize_stacktrace(sentry_value_t stacktrace) if (!addr) { continue; } - sentry__symbolize((void *)addr, sentry__symbolize_frame, &frame); + sentry__symbolize_frame_data_t data; + data.frame = frame; + data.options = options; + sentry__symbolize((void *)addr, sentry__symbolize_frame, &data); } } void -sentry__scope_apply_to_event( - const sentry_scope_t *scope, sentry_value_t event, sentry_scope_mode_t mode) +sentry__scope_apply_to_event(const sentry_scope_t *scope, + const sentry_options_t *options, sentry_value_t event, + sentry_scope_mode_t mode) { #define IS_NULL(Key) sentry_value_is_null(sentry_value_get_by_key(event, Key)) #define SET(Key, Value) sentry_value_set_by_key(event, Key, Value) @@ -264,11 +294,9 @@ sentry__scope_apply_to_event( PLACE_STRING("platform", "native"); - SENTRY_WITH_OPTIONS (options) { - PLACE_STRING("release", options->release); - PLACE_STRING("dist", options->dist); - PLACE_STRING("environment", options->environment); - } + PLACE_STRING("release", options->release); + PLACE_STRING("dist", options->dist); + PLACE_STRING("environment", options->environment); if (IS_NULL("level")) { SET("level", sentry__value_new_level(scope->level)); @@ -298,7 +326,8 @@ sentry__scope_apply_to_event( } if (mode & SENTRY_SCOPE_STACKTRACES) { - sentry__foreach_stacktrace(event, sentry__symbolize_stacktrace); + sentry__foreach_stacktrace( + options, event, sentry__symbolize_stacktrace); } #undef PLACE_STRING diff --git a/src/sentry_scope.h b/src/sentry_scope.h index e22cc945d6..a6bb6155e1 100644 --- a/src/sentry_scope.h +++ b/src/sentry_scope.h @@ -67,7 +67,8 @@ void sentry__scope_flush_unlock(const sentry_scope_t *scope); * attached. */ void sentry__scope_apply_to_event(const sentry_scope_t *scope, - sentry_value_t event, sentry_scope_mode_t mode); + const sentry_options_t *options, sentry_value_t event, + sentry_scope_mode_t mode); /** * This will update a sessions `distinct_id`, which is generated out of other diff --git a/tests/unit/test_mpack.c b/tests/unit/test_mpack.c index d805108eb8..391a3d892c 100644 --- a/tests/unit/test_mpack.c +++ b/tests/unit/test_mpack.c @@ -17,13 +17,15 @@ SENTRY_TEST(mpack_removed_tags) sentry_set_extra("int", sentry_value_new_int32(1234)); sentry_set_extra("double", sentry_value_new_double(12.34)); + sentry_options_t *options = sentry_options_new(); SENTRY_WITH_SCOPE (scope) { - sentry__scope_apply_to_event(scope, obj, SENTRY_SCOPE_NONE); + sentry__scope_apply_to_event(scope, options, obj, SENTRY_SCOPE_NONE); } size_t size; char *buf = sentry_value_to_msgpack(obj, &size); + sentry_options_free(options); sentry_value_decref(obj); sentry_free(buf); sentry__scope_cleanup(); diff --git a/tests/unit/test_unwinder.c b/tests/unit/test_unwinder.c index dd95477602..66fce36488 100644 --- a/tests/unit/test_unwinder.c +++ b/tests/unit/test_unwinder.c @@ -1,3 +1,4 @@ +#include "sentry_scope.h" #include "sentry_symbolizer.h" #include "sentry_testsupport.h" #include @@ -65,3 +66,55 @@ SENTRY_TEST(unwinder) } } } + +TEST_VISIBLE sentry_value_t +capture_inapp_event() +{ + sentry_value_t event = sentry_value_new_event(); + sentry_event_value_add_stacktrace(event, NULL, 0); + return event; +} + +SENTRY_TEST(inapp_stacktrace) +{ + sentry_options_t *options = sentry_options_new(); + sentry_options_add_in_app_include(options, "capture_inapp_event"); + + sentry_value_t event = capture_inapp_event(); + SENTRY_WITH_SCOPE (scope) { + // this will symbolize all the stacktraces, and flag `in_app`. + sentry__scope_apply_to_event(scope, options, event, SENTRY_SCOPE_ALL); + } + + size_t in_app = 0; + bool found = false; + + sentry_value_t threads = sentry_value_get_by_key(event, "threads"); + sentry_value_t values = sentry_value_get_by_key(threads, "values"); + sentry_value_t thread = sentry_value_get_by_index(values, 0); + sentry_value_t stacktrace = sentry_value_get_by_key(thread, "stacktrace"); + sentry_value_t frames = sentry_value_get_by_key(stacktrace, "frames"); + + size_t len = sentry_value_get_length(frames); + for (size_t i = 0; i < len; i++) { + sentry_value_t frame = sentry_value_get_by_index(frames, i); + + const char *symbol = sentry_value_as_string( + sentry_value_get_by_key(frame, "function")); + bool is_in_app + = sentry_value_is_true(sentry_value_get_by_key(frame, "in_app")); + if (strcmp(symbol, "capture_inapp_event") == 0) { + found = true; + TEST_CHECK(is_in_app); + } + + in_app += is_in_app; + } + + TEST_CHECK(found); + TEST_CHECK_INT_EQUAL(in_app, 1); + + sentry_options_free(options); + sentry_value_decref(event); + sentry__scope_cleanup(); +} diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index 08908530af..0a3e8b0579 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -13,6 +13,7 @@ XX(dsn_store_url_with_path) XX(dsn_store_url_without_path) XX(empty_transport) XX(fuzz_json) +XX(inapp_stacktrace) XX(init_failure) XX(invalid_dsn) XX(invalid_proxy)