diff --git a/CHANGELOG.md b/CHANGELOG.md index ec0a4a8de..d5f1123e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ **Features**: - Add reusable, user-owned scopes. `sentry_scope_new` creates a scope that `sentry_capture_event_with_scope` applies without consuming, so you can configure it once and reuse it across many captures instead of building a new local scope each time. `sentry_scope_clone` copies a scope, and `sentry_scope_free` releases it. ([#1855](https://github.com/getsentry/sentry-native/pull/1855)) +- Embed the crash event's breadcrumbs into session replay recordings, so breadcrumbs from the replay window show up on the replay timeline. ([#1875](https://github.com/getsentry/sentry-native/pull/1875)) - Add `sentry_transaction_discard` and `sentry_span_discard` for releasing unfinished transactions and spans without sending them. ([#1858](https://github.com/getsentry/sentry-native/pull/1858)) **Fixes**: diff --git a/src/backends/native/sentry_crash_daemon.c b/src/backends/native/sentry_crash_daemon.c index 2503956e9..9f4680c05 100644 --- a/src/backends/native/sentry_crash_daemon.c +++ b/src/backends/native/sentry_crash_daemon.c @@ -3042,16 +3042,8 @@ read_breadcrumb_ring_file(const sentry_path_t *run_folder, const char *name) sentry_free(buf); return sentry_value_new_null(); } - sentry_value_t list = sentry__value_from_msgpack(buf, size); + sentry_value_t list = sentry__value_from_msgpack_stream(buf, size); sentry_free(buf); - // `sentry__value_from_msgpack` only builds a list when the file holds 2+ - // concatenated values; a file with a single breadcrumb decodes to a bare - // object. Wrap it so the merge step (which ignores non-lists) keeps it. - if (sentry_value_get_type(list) == SENTRY_VALUE_TYPE_OBJECT) { - sentry_value_t wrapper = sentry_value_new_list(); - sentry_value_append(wrapper, list); - return wrapper; - } return list; } @@ -4322,10 +4314,11 @@ sentry__process_crash(const sentry_options_t *options, sentry_crash_ipc_t *ipc) cleanup: // Send the staged session-replay envelope same-session, enriched from the // crash event (`/__sentry-event`) so it shares the crash's - // tags/contexts/trace. Only flush when the crash itself was delivered: - // `cleanup` is also reached via `goto` on error paths where the crash was - // never captured, and flushing there would consume (and delete) the staged - // replay for a crash that never arrived. + // tags/contexts/trace and embeds its breadcrumbs. Only flush when the + // crash itself was delivered: `cleanup` is also reached via `goto` on + // error paths where the crash was never captured, and flushing there + // would consume (and delete) the staged replay for a crash that never + // arrived. if (crash_captured && options && options->transport && sentry__session_replay_has_pending(options)) { sentry_value_t crash_event = sentry_value_new_null(); @@ -4337,6 +4330,11 @@ sentry__process_crash(const sentry_options_t *options, sentry_crash_ipc_t *ipc) sentry_free(ev_json); } } + if (!sentry_value_is_null(crash_event)) { + // `__sentry-event` is scope-applied without breadcrumbs; merge + // the ring files so the replay recording can embed them + apply_breadcrumbs_from_ring_files(crash_event, run_folder, ctx); + } sentry__session_replay_flush_pending( options, options->transport, crash_event); sentry_value_decref(crash_event); diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index 8775b3cd8..e6c98ddaa 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -342,6 +342,24 @@ crashpad_backend_flush_scope( #endif } +// Decodes a breadcrumb ring file (an append-only stream of msgpack values) +// into a list. +static sentry_value_t +read_msgpack_stream_file(const sentry_path_t *path) +{ + if (!path) { + return sentry_value_new_null(); + } + size_t size; + char *data = sentry__path_read_to_buffer(path, &size); + if (!data) { + return sentry_value_new_null(); + } + sentry_value_t value = sentry__value_from_msgpack_stream(data, size); + sentry_free(data); + return value; +} + #if defined(SENTRY_PLATFORM_LINUX) || defined(SENTRY_PLATFORM_WINDOWS) static void flush_scope_from_handler( @@ -446,6 +464,15 @@ crashpad_handler(int signum, siginfo_t *info, ucontext_t *user_context) } if (sentry__session_replay_has_pending(options)) { + // the crash event was scope-applied without breadcrumbs, so + // set them on the in-memory copy to let the replay recording + // embed them; the on-disk `__sentry-event` was already + // written above and stays breadcrumb-free + SENTRY_WITH_SCOPE (scope) { + sentry_value_set_by_key(crash_event, "breadcrumbs", + sentry__ringbuffer_to_list(scope->breadcrumbs)); + } + sentry_transport_t *replay_transport = sentry_new_disk_transport(options->run); if (replay_transport) { @@ -572,9 +599,9 @@ report_to_envelope(const crashpad::CrashReportDatabase::Report &report, if (strcmp(filename, "__sentry-event") == 0) { event = read_msgpack_file(path); } else if (strcmp(filename, "__sentry-breadcrumb1") == 0) { - breadcrumbs1 = read_msgpack_file(path); + breadcrumbs1 = read_msgpack_stream_file(path); } else if (strcmp(filename, "__sentry-breadcrumb2") == 0) { - breadcrumbs2 = read_msgpack_file(path); + breadcrumbs2 = read_msgpack_stream_file(path); } else { sentry__attachments_add_path( &attachments, sentry__path_clone(path), nullptr, nullptr); diff --git a/src/sentry_session_replay.h b/src/sentry_session_replay.h index 666e58a60..65ff65cb6 100644 --- a/src/sentry_session_replay.h +++ b/src/sentry_session_replay.h @@ -46,8 +46,10 @@ bool sentry__session_replay_has_pending(const sentry_options_t *options); * * `scope_source` is the crash event (`/__sentry-event`); its scope fields * and trace id are copied onto the replay, and its timestamp ends the replay - * window. If it is NULL or carries no `contexts.replay.replay_id`, nothing is - * flushed. + * window. If it carries `breadcrumbs`, those falling inside the replay window + * are embedded in the recording as rrweb `breadcrumb` events so they show up + * on the replay timeline. If it is NULL or carries no + * `contexts.replay.replay_id`, nothing is flushed. */ void sentry__session_replay_flush_pending(const sentry_options_t *options, sentry_transport_t *transport, sentry_value_t scope_source); diff --git a/src/sentry_value.c b/src/sentry_value.c index 6a1ea6d06..464e410c6 100644 --- a/src/sentry_value.c +++ b/src/sentry_value.c @@ -1748,6 +1748,38 @@ sentry__value_from_msgpack(const char *buf, size_t buf_len) return sentry_value_new_null(); } + mpack_tree_t tree; + mpack_tree_init_data(&tree, buf, buf_len); + mpack_tree_parse(&tree); + + if (mpack_tree_error(&tree) != mpack_ok) { + mpack_tree_destroy(&tree); + return sentry_value_new_null(); + } + + size_t size = mpack_tree_size(&tree); + bool ok = true; + sentry_value_t value = value_from_mpack(mpack_tree_root(&tree), 0, &ok); + mpack_tree_destroy(&tree); + + // reject buffers with trailing data after the first value; buffers + // holding concatenated values must be decoded with + // `sentry__value_from_msgpack_stream` + if (!ok || size != buf_len) { + sentry_value_decref(value); + return sentry_value_new_null(); + } + + return value; +} + +sentry_value_t +sentry__value_from_msgpack_stream(const char *buf, size_t buf_len) +{ + if (!buf || buf_len == 0) { + return sentry_value_new_null(); + } + size_t offset = 0; sentry_value_t result = sentry_value_new_null(); @@ -1772,16 +1804,10 @@ sentry__value_from_msgpack(const char *buf, size_t buf_len) } mpack_tree_destroy(&tree); - if (offset == 0 && sentry_value_is_null(result)) { - if (offset + size < buf_len) { - result = sentry_value_new_list(); - sentry_value_append(result, value); - } else { - result = value; - } - } else { - sentry_value_append(result, value); + if (sentry_value_is_null(result)) { + result = sentry_value_new_list(); } + sentry_value_append(result, value); offset += size; } diff --git a/src/sentry_value.h b/src/sentry_value.h index 2a3a6df3b..280db3854 100644 --- a/src/sentry_value.h +++ b/src/sentry_value.h @@ -129,15 +129,30 @@ void sentry__value_add_attribute(sentry_value_t attributes, sentry_value_t value, const char *type, const char *name); /** - * Deserialize a sentry value from msgpack. + * Deserialize a single sentry value from msgpack. * - * If the buffer contains multiple sequential msgpack values (as in flat buffers - * like breadcrumb files), they are automatically wrapped in a list. + * The value must span the whole buffer; buffers containing multiple + * sequential msgpack values (as in append-only streams like breadcrumb ring + * files) are rejected with null and must be decoded with + * `sentry__value_from_msgpack_stream`. * * The returned value must be released with `sentry_value_decref`. */ sentry_value_t sentry__value_from_msgpack(const char *buf, size_t buf_len); +/** + * Deserialize a buffer of sequential msgpack values into a list. + * + * Unlike `sentry__value_from_msgpack`, the result is a list even when the + * buffer holds a single value, so files written as append-only streams (e.g. + * breadcrumb ring files) decode to a consistent shape. Returns null for an + * empty buffer or when the first value fails to parse. + * + * The returned value must be released with `sentry_value_decref`. + */ +sentry_value_t sentry__value_from_msgpack_stream( + 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. diff --git a/src/session_replay/sentry_session_replay.c b/src/session_replay/sentry_session_replay.c index 9c4f566a2..62be295d0 100644 --- a/src/session_replay/sentry_session_replay.c +++ b/src/session_replay/sentry_session_replay.c @@ -112,11 +112,80 @@ build_replay_event(sentry_value_t meta, const char *replay_id, double start_sec, return event; } -// Build the rrweb recording list (meta event + video event) describing the -// clip. +// Convert the breadcrumbs that fall inside the replay window into rrweb +// `breadcrumb` events (custom event type 5) and append them to `recording`, +// so they show up on the replay timeline. `breadcrumbs` is timestamp-ordered +// (it comes from the scope ring buffer or the merged ring files), and every +// in-window crumb lies at or after the meta/video events' `start_sec`, so +// appending keeps the recording chronologically sorted. +static void +append_breadcrumb_events(sentry_value_t recording, sentry_value_t breadcrumbs, + double start_sec, double end_sec) +{ + // events may carry breadcrumbs as either a bare list or `{"values": [...]}` + if (sentry_value_get_type(breadcrumbs) == SENTRY_VALUE_TYPE_OBJECT) { + breadcrumbs = sentry_value_get_by_key(breadcrumbs, "values"); + } + if (sentry_value_get_type(breadcrumbs) != SENTRY_VALUE_TYPE_LIST) { + return; + } + + const size_t len = sentry_value_get_length(breadcrumbs); + for (size_t i = 0; i < len; i++) { + sentry_value_t crumb = sentry_value_get_by_index(breadcrumbs, i); + const char *ts = sentry_value_as_string( + sentry_value_get_by_key(crumb, "timestamp")); + if (!ts || !ts[0]) { + continue; + } + const uint64_t usec = sentry__iso8601_to_usec(ts); + if (!usec) { + continue; + } + const double ts_sec = (double)usec / 1000000.0; + if (ts_sec < start_sec || ts_sec > end_sec) { + continue; + } + + sentry_value_t payload = sentry_value_new_object(); + const char *crumb_type + = sentry_value_as_string(sentry_value_get_by_key(crumb, "type")); + sentry_value_set_by_key(payload, "type", + sentry_value_new_string( + crumb_type && crumb_type[0] ? crumb_type : "default")); + // the rrweb payload timestamp is in seconds, the outer one in ms + sentry_value_set_by_key( + payload, "timestamp", sentry_value_new_double(ts_sec)); + static const char *const copy_keys[] + = { "category", "message", "level", "data" }; + for (size_t k = 0; k < sizeof(copy_keys) / sizeof(copy_keys[0]); k++) { + sentry_value_t v = sentry_value_get_by_key(crumb, copy_keys[k]); + if (!sentry_value_is_null(v)) { + sentry_value_incref(v); + sentry_value_set_by_key(payload, copy_keys[k], v); + } + } + + sentry_value_t crumb_data = sentry_value_new_object(); + sentry_value_set_by_key( + crumb_data, "tag", sentry_value_new_string("breadcrumb")); + sentry_value_set_by_key(crumb_data, "payload", payload); + + sentry_value_t crumb_event = sentry_value_new_object(); + sentry_value_set_by_key(crumb_event, "type", sentry_value_new_int32(5)); + sentry_value_set_by_key( + crumb_event, "timestamp", sentry_value_new_double(ts_sec * 1000.0)); + sentry_value_set_by_key(crumb_event, "data", crumb_data); + sentry_value_append(recording, crumb_event); + } +} + +// Build the rrweb recording list (meta event + video event + breadcrumb +// events) describing the clip. static sentry_value_t -build_replay_recording(sentry_value_t meta, double start_sec, - int32_t segment_id, double size_bytes, double duration_ms) +build_replay_recording(sentry_value_t meta, double start_sec, double end_sec, + int32_t segment_id, double size_bytes, double duration_ms, + sentry_value_t breadcrumbs) { const int32_t width = sentry_value_as_int32(sentry_value_get_by_key(meta, "width")); @@ -171,6 +240,7 @@ build_replay_recording(sentry_value_t meta, double start_sec, sentry_value_t recording = sentry_value_new_list(); sentry_value_append(recording, meta_event); sentry_value_append(recording, video_event); + append_breadcrumb_events(recording, breadcrumbs, start_sec, end_sec); return recording; } @@ -210,8 +280,9 @@ build_replay_envelope(const sentry_options_t *options, sentry_value_t meta, sentry_value_t event = build_replay_event( meta, replay_id, start_sec, end_sec, segment_id, scope_source); - sentry_value_t recording = build_replay_recording( - meta, start_sec, segment_id, (double)video_len, duration_ms); + sentry_value_t recording = build_replay_recording(meta, start_sec, end_sec, + segment_id, (double)video_len, duration_ms, + sentry_value_get_by_key(scope_source, "breadcrumbs")); sentry_envelope_t *envelope = NULL; diff --git a/tests/assertions.py b/tests/assertions.py index 7f55aed35..bb9a5609a 100644 --- a/tests/assertions.py +++ b/tests/assertions.py @@ -748,7 +748,9 @@ def stdout_text(): def assert_replay_envelope(envelope, video, replay_id=REPLAY_ID): """Validate a `replay_video` envelope built from the fixture staged by - `tests.stage_replay` (the sidecar values below match that fixture).""" + `tests.stage_replay` (the sidecar values below match that fixture) for a + crash of the example run with its default setup (the breadcrumb values + below match the crumbs that setup adds).""" assert envelope.headers["event_id"] == replay_id (item,) = envelope.items @@ -770,7 +772,8 @@ def assert_replay_envelope(envelope, video, replay_id=REPLAY_ID): header, _, rrweb = body["replay_recording"].partition(b"\n") assert json.loads(header) == {"segment_id": 0} - meta_event, video_event = json.loads(rrweb) + events = json.loads(rrweb) + meta_event, video_event = events[0], events[1] assert meta_event["type"] == 4 assert meta_event["data"]["width"] == 3864 assert meta_event["data"]["height"] == 2100 @@ -785,4 +788,34 @@ def assert_replay_envelope(envelope, video, replay_id=REPLAY_ID): assert payload["frameCount"] == 58 assert payload["frameRate"] == 30 + # the crash event's breadcrumbs falling into the replay window are + # embedded as rrweb `breadcrumb` custom events, in timestamp order + breadcrumb_events = events[2:] + assert len(breadcrumb_events) == 3 + for rrweb_event in breadcrumb_events: + assert rrweb_event["type"] == 5 + assert rrweb_event["data"]["tag"] == "breadcrumb" + # the outer rrweb timestamp is in ms, the payload timestamp in seconds + payload_ts = rrweb_event["data"]["payload"]["timestamp"] + assert abs(rrweb_event["timestamp"] - payload_ts * 1000) < 1 + assert event["replay_start_timestamp"] <= payload_ts <= event["timestamp"] + timestamps = [e["timestamp"] for e in events] + assert timestamps == sorted(timestamps) + + crumbs = [e["data"]["payload"] for e in breadcrumb_events] + assert [c.get("message") for c in crumbs] == [ + "default level is info", + "debug crumb", + "lf\ncrlf\r\nlf\n...", + ] + assert crumbs[0]["type"] == "default" + http_crumb = crumbs[1] + assert http_crumb["type"] == "http" + assert http_crumb["category"] == "example!" + assert http_crumb["level"] == "debug" + assert http_crumb["data"]["url"] == "https://example.com/api/1.0/users" + assert http_crumb["data"]["method"] == "GET" + assert http_crumb["data"]["status_code"] == 200 + assert crumbs[2]["category"] == "something else" + assert body["replay_video"] == video diff --git a/tests/unit/test_value.c b/tests/unit/test_value.c index ba0ef57c2..bbec74f73 100644 --- a/tests/unit/test_value.c +++ b/tests/unit/test_value.c @@ -1646,6 +1646,9 @@ SENTRY_TEST(value_from_msgpack_object) SENTRY_TEST(value_from_msgpack_flat_buffer) { + // buffers holding multiple concatenated msgpack values are rejected by + // `sentry__value_from_msgpack`; they must be decoded with + // `sentry__value_from_msgpack_stream` instead sentry_value_t val1 = sentry_value_new_list(); sentry_value_append(val1, sentry_value_new_int32(1)); sentry_value_append(val1, sentry_value_new_int32(2)); @@ -1656,42 +1659,83 @@ SENTRY_TEST(value_from_msgpack_flat_buffer) size_t size2 = 0; char *buf2 = sentry_value_to_msgpack(val2, &size2); - sentry_value_t val3 = sentry_value_new_string("three"); - size_t size3 = 0; - char *buf3 = sentry_value_to_msgpack(val3, &size3); - char combined[256]; size_t combined_size = 0; memcpy(combined + combined_size, buf1, size1); combined_size += size1; memcpy(combined + combined_size, buf2, size2); combined_size += size2; - memcpy(combined + combined_size, buf3, size3); - combined_size += size3; sentry_value_t result = sentry__value_from_msgpack(combined, combined_size); - TEST_CHECK(sentry_value_get_type(result) == SENTRY_VALUE_TYPE_LIST); - TEST_CHECK(sentry_value_get_length(result) == 3); + TEST_CHECK(sentry_value_is_null(result)); + + // a single value followed by trailing garbage is rejected as well + combined_size = size1; + memset(combined + combined_size, 0xC1, 4); // 0xC1 is never used in msgpack + combined_size += 4; - sentry_value_t elem0 = sentry_value_get_by_index(result, 0); - TEST_CHECK(sentry_value_get_type(elem0) == SENTRY_VALUE_TYPE_LIST); - TEST_CHECK(sentry_value_get_length(elem0) == 2); - TEST_CHECK(sentry_value_as_int32(sentry_value_get_by_index(elem0, 0)) == 1); - TEST_CHECK(sentry_value_as_int32(sentry_value_get_by_index(elem0, 1)) == 2); + result = sentry__value_from_msgpack(combined, combined_size); + TEST_CHECK(sentry_value_is_null(result)); + + sentry_free(buf1); + sentry_free(buf2); + sentry_value_decref(val1); + sentry_value_decref(val2); +} - sentry_value_t elem1 = sentry_value_get_by_index(result, 1); - TEST_CHECK(sentry_value_as_int32(elem1) == 2); +SENTRY_TEST(value_from_msgpack_stream) +{ + TEST_CHECK( + sentry_value_is_null(sentry__value_from_msgpack_stream(NULL, 0))); + TEST_CHECK(sentry_value_is_null(sentry__value_from_msgpack_stream("", 0))); - sentry_value_t elem2 = sentry_value_get_by_index(result, 2); - TEST_CHECK_STRING_EQUAL(sentry_value_as_string(elem2), "three"); + // a single value decodes to a one-element list, unlike + // `sentry__value_from_msgpack` which decodes it bare + sentry_value_t val1 = sentry_value_new_object(); + sentry_value_set_by_key(val1, "message", sentry_value_new_string("first")); + size_t size1 = 0; + char *buf1 = sentry_value_to_msgpack(val1, &size1); + + sentry_value_t single = sentry__value_from_msgpack_stream(buf1, size1); + TEST_CHECK(sentry_value_get_type(single) == SENTRY_VALUE_TYPE_LIST); + TEST_CHECK(sentry_value_get_length(single) == 1); + TEST_CHECK_STRING_EQUAL( + sentry_value_as_string(sentry_value_get_by_key( + sentry_value_get_by_index(single, 0), "message")), + "first"); + + // multiple concatenated values decode to a list of all of them + sentry_value_t val2 = sentry_value_new_object(); + sentry_value_set_by_key(val2, "message", sentry_value_new_string("second")); + size_t size2 = 0; + char *buf2 = sentry_value_to_msgpack(val2, &size2); + + char combined[256]; + size_t combined_size = 0; + memcpy(combined + combined_size, buf1, size1); + combined_size += size1; + memcpy(combined + combined_size, buf2, size2); + combined_size += size2; + + sentry_value_t multi + = sentry__value_from_msgpack_stream(combined, combined_size); + TEST_CHECK(sentry_value_get_type(multi) == SENTRY_VALUE_TYPE_LIST); + TEST_CHECK(sentry_value_get_length(multi) == 2); + TEST_CHECK_STRING_EQUAL( + sentry_value_as_string(sentry_value_get_by_key( + sentry_value_get_by_index(multi, 0), "message")), + "first"); + TEST_CHECK_STRING_EQUAL( + sentry_value_as_string(sentry_value_get_by_key( + sentry_value_get_by_index(multi, 1), "message")), + "second"); sentry_free(buf1); sentry_free(buf2); - sentry_free(buf3); sentry_value_decref(val1); sentry_value_decref(val2); - sentry_value_decref(val3); - sentry_value_decref(result); + sentry_value_decref(single); + sentry_value_decref(multi); } #define TEST_CHECK_MESSAGE_EQUAL(breadcrumbs, index, message) \ diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index be30ce476..f904da856 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -389,6 +389,7 @@ XX(value_from_msgpack_int64) XX(value_from_msgpack_list) XX(value_from_msgpack_null) XX(value_from_msgpack_object) +XX(value_from_msgpack_stream) XX(value_from_msgpack_string) XX(value_from_msgpack_uint64) XX(value_get_by_null_key)