diff --git a/src/sentry_scope.c b/src/sentry_scope.c index e48dd9b39..162edaada 100644 --- a/src/sentry_scope.c +++ b/src/sentry_scope.c @@ -312,116 +312,6 @@ sentry__scope_get_span_or_transaction(void) } #endif -static int -cmp_breadcrumb(sentry_value_t a, sentry_value_t b, bool *error) -{ - sentry_value_t timestamp_a = sentry_value_get_by_key(a, "timestamp"); - sentry_value_t timestamp_b = sentry_value_get_by_key(b, "timestamp"); - if (sentry_value_is_null(timestamp_a)) { - *error = true; - return -1; - } - if (sentry_value_is_null(timestamp_b)) { - *error = true; - return 1; - } - - return strcmp(sentry_value_as_string(timestamp_a), - sentry_value_as_string(timestamp_b)); -} - -static bool -append_breadcrumb(sentry_value_t target, sentry_value_t source, size_t index) -{ - int rv = sentry_value_append( - target, sentry_value_get_by_index_owned(source, index)); - if (rv != 0) { - SENTRY_ERROR("Failed to merge breadcrumbs"); - sentry_value_decref(target); - return false; - } - return true; -} - -static sentry_value_t -merge_breadcrumbs(sentry_value_t list_a, sentry_value_t list_b, size_t max) -{ - size_t len_a = sentry_value_get_type(list_a) == SENTRY_VALUE_TYPE_LIST - ? sentry_value_get_length(list_a) - : 0; - size_t len_b = sentry_value_get_type(list_b) == SENTRY_VALUE_TYPE_LIST - ? sentry_value_get_length(list_b) - : 0; - - if (len_a == 0 && len_b == 0) { - return sentry_value_new_null(); - } else if (len_a == 0) { - sentry_value_incref(list_b); - return list_b; - } else if (len_b == 0) { - sentry_value_incref(list_a); - return list_a; - } - - bool error = false; - size_t idx_a = 0; - size_t idx_b = 0; - size_t total = len_a + len_b; - size_t skip = total > max ? total - max : 0; - sentry_value_t result = sentry__value_new_list_with_size(total - skip); - - // skip oldest breadcrumbs to fit max - while (idx_a < len_a && idx_b < len_b && idx_a + idx_b < skip) { - sentry_value_t item_a = sentry_value_get_by_index(list_a, idx_a); - sentry_value_t item_b = sentry_value_get_by_index(list_b, idx_b); - - if (cmp_breadcrumb(item_a, item_b, &error) <= 0) { - idx_a++; - } else { - idx_b++; - } - } - while (idx_a < len_a && idx_a + idx_b < skip) { - idx_a++; - } - while (idx_b < len_b && idx_a + idx_b < skip) { - idx_b++; - } - - // merge the remaining breadcrumbs in timestamp order - while (idx_a < len_a && idx_b < len_b) { - sentry_value_t item_a = sentry_value_get_by_index(list_a, idx_a); - sentry_value_t item_b = sentry_value_get_by_index(list_b, idx_b); - - if (cmp_breadcrumb(item_a, item_b, &error) <= 0) { - if (!append_breadcrumb(result, list_a, idx_a++)) { - return sentry_value_new_null(); - } - } else { - if (!append_breadcrumb(result, list_b, idx_b++)) { - return sentry_value_new_null(); - } - } - } - while (idx_a < len_a) { - if (!append_breadcrumb(result, list_a, idx_a++)) { - return sentry_value_new_null(); - } - } - while (idx_b < len_b) { - if (!append_breadcrumb(result, list_b, idx_b++)) { - return sentry_value_new_null(); - } - } - - if (error) { - SENTRY_WARN("Detected missing timestamps while merging breadcrumbs. " - "This may lead to unexpected results."); - } - - return result; -} - void sentry__scope_apply_to_event(const sentry_scope_t *scope, const sentry_options_t *options, sentry_value_t event, @@ -522,8 +412,8 @@ sentry__scope_apply_to_event(const sentry_scope_t *scope, sentry_value_t scope_breadcrumbs = sentry__ringbuffer_to_list(scope->breadcrumbs); sentry_value_set_by_key(event, "breadcrumbs", - merge_breadcrumbs(event_breadcrumbs, scope_breadcrumbs, - options->max_breadcrumbs)); + sentry__value_merge_breadcrumbs(event_breadcrumbs, + scope_breadcrumbs, options->max_breadcrumbs)); sentry_value_decref(scope_breadcrumbs); } diff --git a/src/sentry_value.c b/src/sentry_value.c index 2f10e77eb..438187982 100644 --- a/src/sentry_value.c +++ b/src/sentry_value.c @@ -1741,3 +1741,114 @@ sentry__value_from_msgpack(const char *buf, size_t buf_len) return result; } + +static int +cmp_breadcrumb(sentry_value_t a, sentry_value_t b, bool *error) +{ + sentry_value_t timestamp_a = sentry_value_get_by_key(a, "timestamp"); + sentry_value_t timestamp_b = sentry_value_get_by_key(b, "timestamp"); + if (sentry_value_is_null(timestamp_a)) { + *error = true; + return -1; + } + if (sentry_value_is_null(timestamp_b)) { + *error = true; + return 1; + } + + return strcmp(sentry_value_as_string(timestamp_a), + sentry_value_as_string(timestamp_b)); +} + +static bool +append_breadcrumb(sentry_value_t target, sentry_value_t source, size_t index) +{ + int rv = sentry_value_append( + target, sentry_value_get_by_index_owned(source, index)); + if (rv != 0) { + SENTRY_ERROR("Failed to merge breadcrumbs"); + sentry_value_decref(target); + return false; + } + return true; +} + +sentry_value_t +sentry__value_merge_breadcrumbs( + sentry_value_t list_a, sentry_value_t list_b, size_t max) +{ + size_t len_a = sentry_value_get_type(list_a) == SENTRY_VALUE_TYPE_LIST + ? sentry_value_get_length(list_a) + : 0; + size_t len_b = sentry_value_get_type(list_b) == SENTRY_VALUE_TYPE_LIST + ? sentry_value_get_length(list_b) + : 0; + + if (len_a == 0 && len_b == 0) { + return sentry_value_new_null(); + } else if (len_a == 0) { + sentry_value_incref(list_b); + return list_b; + } else if (len_b == 0) { + sentry_value_incref(list_a); + return list_a; + } + + bool error = false; + size_t idx_a = 0; + size_t idx_b = 0; + size_t total = len_a + len_b; + size_t skip = total > max ? total - max : 0; + sentry_value_t result = sentry__value_new_list_with_size(total - skip); + + // skip oldest breadcrumbs to fit max + while (idx_a < len_a && idx_b < len_b && idx_a + idx_b < skip) { + sentry_value_t item_a = sentry_value_get_by_index(list_a, idx_a); + sentry_value_t item_b = sentry_value_get_by_index(list_b, idx_b); + + if (cmp_breadcrumb(item_a, item_b, &error) <= 0) { + idx_a++; + } else { + idx_b++; + } + } + while (idx_a < len_a && idx_a + idx_b < skip) { + idx_a++; + } + while (idx_b < len_b && idx_a + idx_b < skip) { + idx_b++; + } + + // merge the remaining breadcrumbs in timestamp order + while (idx_a < len_a && idx_b < len_b) { + sentry_value_t item_a = sentry_value_get_by_index(list_a, idx_a); + sentry_value_t item_b = sentry_value_get_by_index(list_b, idx_b); + + if (cmp_breadcrumb(item_a, item_b, &error) <= 0) { + if (!append_breadcrumb(result, list_a, idx_a++)) { + return sentry_value_new_null(); + } + } else { + if (!append_breadcrumb(result, list_b, idx_b++)) { + return sentry_value_new_null(); + } + } + } + while (idx_a < len_a) { + if (!append_breadcrumb(result, list_a, idx_a++)) { + return sentry_value_new_null(); + } + } + while (idx_b < len_b) { + if (!append_breadcrumb(result, list_b, idx_b++)) { + return sentry_value_new_null(); + } + } + + if (error) { + SENTRY_WARN("Detected missing timestamps while merging breadcrumbs. " + "This may lead to unexpected results."); + } + + return result; +} diff --git a/src/sentry_value.h b/src/sentry_value.h index b2928ce52..7048b0bef 100644 --- a/src/sentry_value.h +++ b/src/sentry_value.h @@ -125,4 +125,11 @@ void sentry__value_add_attribute(sentry_value_t attributes, */ sentry_value_t sentry__value_from_msgpack(const char *buf, size_t buf_len); +/** + * Merges two breadcrumb lists in timestamp order, keeping at most `max` items. + * Returns a new list with the merged breadcrumbs. + */ +sentry_value_t sentry__value_merge_breadcrumbs( + sentry_value_t list_a, sentry_value_t list_b, size_t max); + #endif diff --git a/tests/unit/test_value.c b/tests/unit/test_value.c index 8073105e3..21a5f18bd 100644 --- a/tests/unit/test_value.c +++ b/tests/unit/test_value.c @@ -6,6 +6,15 @@ #include #include +static sentry_value_t +breadcrumb_with_ts(const char *message, const char *timestamp) +{ + sentry_value_t breadcrumb = sentry_value_new_breadcrumb(NULL, message); + sentry_value_set_by_key( + breadcrumb, "timestamp", sentry_value_new_string(timestamp)); + return breadcrumb; +} + SENTRY_TEST(value_null) { sentry_value_t val = sentry_value_new_null(); @@ -1564,3 +1573,114 @@ SENTRY_TEST(value_from_msgpack_flat_buffer) sentry_value_decref(val3); sentry_value_decref(result); } + +#define TEST_CHECK_MESSAGE_EQUAL(breadcrumbs, index, message) \ + TEST_CHECK_STRING_EQUAL( \ + sentry_value_as_string(sentry_value_get_by_key( \ + sentry_value_get_by_index(breadcrumbs, index), "message")), \ + message) + +SENTRY_TEST(value_merge_breadcrumbs_both_empty) +{ + sentry_value_t list_a = sentry_value_new_list(); + sentry_value_t list_b = sentry_value_new_list(); + + sentry_value_t result = sentry__value_merge_breadcrumbs(list_a, list_b, 10); + TEST_CHECK(sentry_value_is_null(result)); + + sentry_value_decref(list_a); + sentry_value_decref(list_b); +} + +SENTRY_TEST(value_merge_breadcrumbs_one_empty) +{ + sentry_value_t list_a = sentry_value_new_list(); + sentry_value_append( + list_a, breadcrumb_with_ts("a1", "2024-01-01T00:00:01")); + sentry_value_append( + list_a, breadcrumb_with_ts("a2", "2024-01-01T00:00:02")); + sentry_value_t list_b = sentry_value_new_list(); + + // list_b is empty -> return list_a + sentry_value_t result = sentry__value_merge_breadcrumbs(list_a, list_b, 10); + TEST_CHECK(sentry_value_get_type(result) == SENTRY_VALUE_TYPE_LIST); + TEST_CHECK_INT_EQUAL(sentry_value_get_length(result), 2); + TEST_CHECK_MESSAGE_EQUAL(result, 0, "a1"); + TEST_CHECK_MESSAGE_EQUAL(result, 1, "a2"); + sentry_value_decref(result); + + // list_a is empty -> return list_b + sentry_value_t list_c = sentry_value_new_list(); + result = sentry__value_merge_breadcrumbs(list_c, list_a, 10); + TEST_CHECK(sentry_value_get_type(result) == SENTRY_VALUE_TYPE_LIST); + TEST_CHECK_INT_EQUAL(sentry_value_get_length(result), 2); + TEST_CHECK_MESSAGE_EQUAL(result, 0, "a1"); + TEST_CHECK_MESSAGE_EQUAL(result, 1, "a2"); + sentry_value_decref(result); + + sentry_value_decref(list_a); + sentry_value_decref(list_b); + sentry_value_decref(list_c); +} + +SENTRY_TEST(value_merge_breadcrumbs_interleaved) +{ + sentry_value_t list_a = sentry_value_new_list(); + sentry_value_append( + list_a, breadcrumb_with_ts("a1", "2024-01-01T00:00:01")); + sentry_value_append( + list_a, breadcrumb_with_ts("a4", "2024-01-01T00:00:04")); + + sentry_value_t list_b = sentry_value_new_list(); + sentry_value_append( + list_b, breadcrumb_with_ts("b2", "2024-01-01T00:00:02")); + sentry_value_append( + list_b, breadcrumb_with_ts("b3", "2024-01-01T00:00:03")); + + sentry_value_t result = sentry__value_merge_breadcrumbs(list_a, list_b, 10); + TEST_CHECK(sentry_value_get_type(result) == SENTRY_VALUE_TYPE_LIST); + TEST_CHECK_INT_EQUAL(sentry_value_get_length(result), 4); + TEST_CHECK_MESSAGE_EQUAL(result, 0, "a1"); + TEST_CHECK_MESSAGE_EQUAL(result, 1, "b2"); + TEST_CHECK_MESSAGE_EQUAL(result, 2, "b3"); + TEST_CHECK_MESSAGE_EQUAL(result, 3, "a4"); + + sentry_value_decref(result); + sentry_value_decref(list_a); + sentry_value_decref(list_b); +} + +SENTRY_TEST(value_merge_breadcrumbs_max_limit) +{ + sentry_value_t list_a = sentry_value_new_list(); + sentry_value_append( + list_a, breadcrumb_with_ts("a1", "2024-01-01T00:00:01")); + sentry_value_append( + list_a, breadcrumb_with_ts("a3", "2024-01-01T00:00:03")); + + sentry_value_t list_b = sentry_value_new_list(); + sentry_value_append( + list_b, breadcrumb_with_ts("b2", "2024-01-01T00:00:02")); + sentry_value_append( + list_b, breadcrumb_with_ts("b4", "2024-01-01T00:00:04")); + + // max=3 -> oldest (a1) should be dropped + sentry_value_t result = sentry__value_merge_breadcrumbs(list_a, list_b, 3); + TEST_CHECK(sentry_value_get_type(result) == SENTRY_VALUE_TYPE_LIST); + TEST_CHECK_INT_EQUAL(sentry_value_get_length(result), 3); + TEST_CHECK_MESSAGE_EQUAL(result, 0, "b2"); + TEST_CHECK_MESSAGE_EQUAL(result, 1, "a3"); + TEST_CHECK_MESSAGE_EQUAL(result, 2, "b4"); + sentry_value_decref(result); + + // max=2 -> oldest two (a1, b2) should be dropped + result = sentry__value_merge_breadcrumbs(list_a, list_b, 2); + TEST_CHECK(sentry_value_get_type(result) == SENTRY_VALUE_TYPE_LIST); + TEST_CHECK_INT_EQUAL(sentry_value_get_length(result), 2); + TEST_CHECK_MESSAGE_EQUAL(result, 0, "a3"); + TEST_CHECK_MESSAGE_EQUAL(result, 1, "b4"); + sentry_value_decref(result); + + sentry_value_decref(list_a); + sentry_value_decref(list_b); +} diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index 4941e29ab..d47cfe7f7 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -78,12 +78,12 @@ XX(embedded_info_sentry_version) XX(empty_transport) XX(event_with_id) XX(exception_without_type_or_value_still_valid) -XX(formatted_log_messages) -XX(feedback_without_hint) -XX(feedback_with_null_hint) -XX(feedback_with_file_attachment) XX(feedback_with_bytes_attachment) +XX(feedback_with_file_attachment) XX(feedback_with_multiple_attachments) +XX(feedback_with_null_hint) +XX(feedback_without_hint) +XX(formatted_log_messages) XX(fuzz_json) XX(init_failure) XX(internal_uuid_api) @@ -228,6 +228,10 @@ XX(value_json_locales) XX(value_json_parsing) XX(value_json_surrogates) XX(value_list) +XX(value_merge_breadcrumbs_both_empty) +XX(value_merge_breadcrumbs_interleaved) +XX(value_merge_breadcrumbs_max_limit) +XX(value_merge_breadcrumbs_one_empty) XX(value_null) XX(value_object) XX(value_object_merge)