diff --git a/src/sentry_core.c b/src/sentry_core.c index 8bfaa2d5d..09a038b1a 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -735,37 +735,25 @@ sentry_transaction_start(sentry_transaction_context_t *opaque_tx_cxt) { sentry_value_t tx_cxt = opaque_tx_cxt->inner; - // TODO: it would be nice if we could just merge tx_cxt into tx. - // `sentry_value_new_transaction_event()` is also an option, but risks - // causing more confusion as there's already a - // `sentry_value_new_transaction`. The ending timestamp is stripped as well - // to avoid misleading ourselves later down the line. + // If the parent span ID is some empty-ish value, just remove it + sentry_value_t parent_span + = sentry_value_get_by_key(tx_cxt, "parent_span_id"); + if (sentry_value_get_length(parent_span) < 1) { + sentry_value_remove_by_key(tx_cxt, "parent_span_id"); + } + + // The ending timestamp is stripped to avoid misleading ourselves later + // down the line, as it is the only way to determine whether a transaction + // has ended or not. sentry_value_t tx = sentry_value_new_event(); sentry_value_remove_by_key(tx, "timestamp"); + sentry__value_merge_objects(tx, tx_cxt); + bool should_sample = sentry__should_send_transaction(tx_cxt); sentry_value_set_by_key( tx, "sampled", sentry_value_new_bool(should_sample)); - // Avoid having this show up in the payload at all if it doesn't have a - // valid value - sentry_value_t parent_span - = sentry_value_get_by_key_owned(tx_cxt, "parent_span_id"); - if (sentry_value_get_length(parent_span) > 0) { - sentry_value_set_by_key(tx, "parent_span_id", parent_span); - } else { - sentry_value_decref(parent_span); - } - sentry_value_set_by_key( - tx, "trace_id", sentry_value_get_by_key_owned(tx_cxt, "trace_id")); - sentry_value_set_by_key( - tx, "span_id", sentry_value_get_by_key_owned(tx_cxt, "span_id")); - sentry_value_set_by_key(tx, "transaction", - sentry_value_get_by_key_owned(tx_cxt, "transaction")); - sentry_value_set_by_key( - tx, "op", sentry_value_get_by_key_owned(tx_cxt, "op")); - sentry_value_set_by_key( - tx, "status", sentry_value_get_by_key_owned(tx_cxt, "status")); sentry_value_set_by_key(tx, "start_timestamp", sentry__value_new_string_owned( sentry__msec_time_to_iso8601(sentry__msec_time()))); diff --git a/src/sentry_scope.c b/src/sentry_scope.c index 1d3ec4d73..b2b732470 100644 --- a/src/sentry_scope.c +++ b/src/sentry_scope.c @@ -1,4 +1,5 @@ #include "sentry_scope.h" +#include "sentry.h" #include "sentry_backend.h" #include "sentry_core.h" #include "sentry_database.h" @@ -7,6 +8,7 @@ #include "sentry_string.h" #include "sentry_symbolizer.h" #include "sentry_sync.h" +#include "sentry_value.h" #include @@ -297,18 +299,43 @@ sentry__scope_apply_to_event(const sentry_scope_t *scope, PLACE_STRING("transaction", scope->transaction); PLACE_VALUE("sdk", scope->client_sdk); - // TODO: these should merge - PLACE_CLONED_VALUE("tags", scope->tags); - PLACE_CLONED_VALUE("extra", scope->extra); + sentry_value_t event_tags = sentry_value_get_by_key(event, "tags"); + if (sentry_value_is_null(event_tags)) { + if (!sentry_value_is_null(scope->tags)) { + PLACE_CLONED_VALUE("tags", scope->tags); + } + } else { + sentry__value_merge_objects(event_tags, scope->tags); + } + sentry_value_t event_extra = sentry_value_get_by_key(event, "extra"); + if (sentry_value_is_null(event_extra)) { + if (!sentry_value_is_null(scope->extra)) { + PLACE_CLONED_VALUE("extra", scope->extra); + } + } else { + sentry__value_merge_objects(event_extra, scope->extra); + } #ifdef SENTRY_PERFORMANCE_MONITORING - // TODO: better, more thorough deep merging sentry_value_t contexts = sentry__value_clone(scope->contexts); - sentry_value_t trace = sentry__transaction_get_trace_context(scope->span); - if (!sentry_value_is_null(trace)) { - sentry_value_set_by_key(contexts, "trace", trace); + // prep contexts sourced from scope; data about transaction on scope needs + // to be extracted and inserted + sentry_value_t scope_trace + = sentry__transaction_get_trace_context(scope->span); + if (!sentry_value_is_null(scope_trace)) { + if (sentry_value_is_null(contexts)) { + contexts = sentry_value_new_object(); + } + sentry_value_set_by_key(contexts, "trace", scope_trace); + } + + // merge contexts sourced from scope into the event + sentry_value_t event_contexts = sentry_value_get_by_key(event, "contexts"); + if (sentry_value_is_null(event_contexts)) { + PLACE_VALUE("contexts", contexts); + } else { + sentry__value_merge_objects(event_contexts, contexts); } - PLACE_VALUE("contexts", contexts); sentry_value_decref(contexts); #endif diff --git a/src/sentry_value.c b/src/sentry_value.c index 2cfa86f5f..cbb367df9 100644 --- a/src/sentry_value.c +++ b/src/sentry_value.c @@ -806,6 +806,41 @@ sentry_value_is_null(sentry_value_t value) return value._bits == CONST_NULL; } +int +sentry__value_merge_objects(sentry_value_t dst, sentry_value_t src) +{ + if (sentry_value_is_null(src)) { + return 0; + } + if (sentry_value_get_type(dst) != SENTRY_VALUE_TYPE_OBJECT + || sentry_value_get_type(src) != SENTRY_VALUE_TYPE_OBJECT + || sentry_value_is_frozen(dst)) { + return 1; + } + thing_t *thing = value_as_thing(src); + if (!thing) { + return 1; + } + obj_t *obj = thing->payload._ptr; + for (size_t i = 0; i < obj->len; i++) { + char *key = obj->pairs[i].k; + sentry_value_t src_val = obj->pairs[i].v; + sentry_value_t dst_val = sentry_value_get_by_key(dst, key); + if (sentry_value_get_type(dst_val) == SENTRY_VALUE_TYPE_OBJECT + && sentry_value_get_type(src_val) == SENTRY_VALUE_TYPE_OBJECT) { + if (sentry__value_merge_objects(dst_val, src_val) != 0) { + return 1; + } + } else { + if (sentry_value_set_by_key(dst, key, src_val) != 0) { + return 1; + } + sentry_value_incref(src_val); + } + } + return 0; +} + void sentry__jsonwriter_write_value(sentry_jsonwriter_t *jw, sentry_value_t value) { diff --git a/src/sentry_value.h b/src/sentry_value.h index abe57cc85..7dfa6eb8c 100644 --- a/src/sentry_value.h +++ b/src/sentry_value.h @@ -89,6 +89,22 @@ sentry_value_t sentry__value_clone(sentry_value_t value); int sentry__value_append_bounded( sentry_value_t value, sentry_value_t v, size_t max); +/** + * Deep-merges object src into dst. + * + * For each key-value pair in the src object the same key in the dst object + * will be set to the value from src. If both the dst value and the src value + * are objects themselves they are stepped into recursively instead of + * overriding the entire dst object. + * + * If src is null nothing needs to be merged and this is handled gracefully, + * otherwise if dst is any other type than an object or src is neither an + * object nor null an error is returned. + * + * Returns 0 on success. + */ +int sentry__value_merge_objects(sentry_value_t dst, sentry_value_t src); + /** * Parse the given JSON string into a new Value. */ diff --git a/tests/unit/test_value.c b/tests/unit/test_value.c index 783567f40..46170b145 100644 --- a/tests/unit/test_value.c +++ b/tests/unit/test_value.c @@ -229,6 +229,62 @@ SENTRY_TEST(value_object) sentry_value_decref(val); } +SENTRY_TEST(value_object_merge) +{ + sentry_value_t dst = sentry_value_new_object(); + sentry_value_set_by_key(dst, "a", sentry_value_new_int32(1)); + sentry_value_set_by_key(dst, "b", sentry_value_new_int32(2)); + + sentry_value_t src = sentry_value_new_object(); + sentry_value_set_by_key(src, "b", sentry_value_new_int32(20)); + sentry_value_set_by_key(src, "c", sentry_value_new_int32(30)); + + int rv = sentry__value_merge_objects(dst, src); + TEST_CHECK_INT_EQUAL(rv, 0); + sentry_value_decref(src); + + sentry_value_t a = sentry_value_get_by_key(dst, "a"); + sentry_value_t b = sentry_value_get_by_key(dst, "b"); + sentry_value_t c = sentry_value_get_by_key(dst, "c"); + TEST_CHECK_INT_EQUAL(sentry_value_as_int32(a), 1); + TEST_CHECK_INT_EQUAL(sentry_value_as_int32(b), 20); + TEST_CHECK_INT_EQUAL(sentry_value_as_int32(c), 30); + + sentry_value_decref(dst); +} + +SENTRY_TEST(value_object_merge_nested) +{ + sentry_value_t dst = sentry_value_new_object(); + sentry_value_set_by_key(dst, "a", sentry_value_new_int32(1)); + sentry_value_t dst_nested = sentry_value_new_object(); + sentry_value_set_by_key(dst_nested, "ba", sentry_value_new_int32(1)); + sentry_value_set_by_key(dst_nested, "bb", sentry_value_new_int32(2)); + sentry_value_set_by_key(dst, "b", dst_nested); + + sentry_value_t src = sentry_value_new_object(); + sentry_value_t src_nested = sentry_value_new_object(); + sentry_value_set_by_key(src_nested, "bb", sentry_value_new_int32(20)); + sentry_value_set_by_key(src_nested, "bc", sentry_value_new_int32(30)); + sentry_value_set_by_key(src, "b", src_nested); + + int rv = sentry__value_merge_objects(dst, src); + TEST_CHECK_INT_EQUAL(rv, 0); + sentry_value_decref(src); + + sentry_value_t a = sentry_value_get_by_key(dst, "a"); + sentry_value_t nested = sentry_value_get_by_key(dst, "b"); + sentry_value_t ba = sentry_value_get_by_key(nested, "ba"); + sentry_value_t bb = sentry_value_get_by_key(nested, "bb"); + sentry_value_t bc = sentry_value_get_by_key(nested, "bc"); + TEST_CHECK_INT_EQUAL(sentry_value_as_int32(a), 1); + TEST_CHECK_INT_EQUAL(sentry_value_as_int32(ba), 1); + TEST_CHECK_INT_EQUAL(sentry_value_as_int32(bb), 20); + TEST_CHECK_INT_EQUAL(sentry_value_as_int32(bc), 30); + + sentry_value_decref(dst); +} + SENTRY_TEST(value_freezing) { sentry_value_t val = sentry_value_new_list(); diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index 42cbed1c6..2341c3358 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -79,6 +79,8 @@ XX(value_json_surrogates) XX(value_list) XX(value_null) XX(value_object) +XX(value_object_merge) +XX(value_object_merge_nested) XX(value_string) XX(value_unicode) XX(value_wrong_type)