From d2239452a7dec43df3993a507a18a6886f5501dc Mon Sep 17 00:00:00 2001 From: Betty Da Date: Thu, 16 Dec 2021 01:51:06 -0500 Subject: [PATCH 01/21] feat(tracing): Add in basic Envelope support for Transactions --- include/sentry.h | 12 +++++-- src/sentry_envelope.c | 62 ++++++++++++++++++++++++++++++++++++- src/sentry_envelope.h | 6 ++++ tests/unit/test_envelopes.c | 32 +++++++++++++++++++ tests/unit/tests.inc | 1 + 5 files changed, 110 insertions(+), 3 deletions(-) diff --git a/include/sentry.h b/include/sentry.h index 3675e5527..d0bcc485f 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -554,13 +554,21 @@ typedef struct sentry_envelope_s sentry_envelope_t; SENTRY_API void sentry_envelope_free(sentry_envelope_t *envelope); /** - * Given an envelope returns the embedded event if there is one. + * Given an Envelope, returns the embedded Event if there is one. * - * This returns a borrowed value to the event in the envelope. + * This returns a borrowed value to the Event in the Envelope. */ SENTRY_API sentry_value_t sentry_envelope_get_event( const sentry_envelope_t *envelope); +/** + * Given an Envelope, returns the embedded Transaction if there is one. + * + * This returns a borrowed value to the Transaction in the Envelope. + */ +SENTRY_EXPERIMENTAL_API sentry_value_t sentry_envelope_get_transaction( + const sentry_envelope_t *envelope); + /** * Serializes the envelope. * diff --git a/src/sentry_envelope.c b/src/sentry_envelope.c index 71cef6ed6..7ea05ad98 100644 --- a/src/sentry_envelope.c +++ b/src/sentry_envelope.c @@ -41,6 +41,9 @@ envelope_add_item(sentry_envelope_t *envelope) if (envelope->contents.items.item_count >= SENTRY_MAX_ENVELOPE_ITEMS) { return NULL; } + // TODO: Envelopes may have at most one event item or one transaction item, + // and not one of both. Some checking should be done here or in + // `sentry__envelope_add_[transaction|event]` to ensure this can't happen. sentry_envelope_item_t *rv = &envelope->contents.items @@ -197,7 +200,25 @@ sentry_envelope_get_event(const sentry_envelope_t *envelope) return sentry_value_new_null(); } for (size_t i = 0; i < envelope->contents.items.item_count; i++) { - if (!sentry_value_is_null(envelope->contents.items.items[i].event)) { + if (!sentry_value_is_null(envelope->contents.items.items[i].event) + && !sentry__event_is_transaction( + envelope->contents.items.items[i].event)) { + return envelope->contents.items.items[i].event; + } + } + return sentry_value_new_null(); +} + +sentry_value_t +sentry_envelope_get_transaction(const sentry_envelope_t *envelope) +{ + if (envelope->is_raw) { + return sentry_value_new_null(); + } + for (size_t i = 0; i < envelope->contents.items.item_count; i++) { + if (!sentry_value_is_null(envelope->contents.items.items[i].event) + && sentry__event_is_transaction( + envelope->contents.items.items[i].event)) { return envelope->contents.items.items[i].event; } } @@ -234,6 +255,45 @@ sentry__envelope_add_event(sentry_envelope_t *envelope, sentry_value_t event) return item; } +sentry_envelope_item_t * +sentry__envelope_add_transaction( + sentry_envelope_t *envelope, sentry_value_t transaction) +{ + sentry_envelope_item_t *item = envelope_add_item(envelope); + if (!item) { + return NULL; + } + + sentry_jsonwriter_t *jw = sentry__jsonwriter_new(NULL); + if (!jw) { + return NULL; + } + + sentry_value_t event_id = sentry__ensure_event_id(transaction, NULL); + + item->event = transaction; + sentry__jsonwriter_write_value(jw, transaction); + item->payload = sentry__jsonwriter_into_string(jw, &item->payload_len); + + sentry__envelope_item_set_header( + item, "type", sentry_value_new_string("transaction")); + sentry_value_t length = sentry_value_new_int32((int32_t)item->payload_len); + sentry__envelope_item_set_header(item, "length", length); + + sentry_value_incref(event_id); + sentry__envelope_set_header(envelope, "event_id", event_id); + +#ifdef SENTRY_UNITTEST + sentry_value_t now = sentry_value_new_string("2021-12-16T05:53:59.343Z"); +#else + sentry_value_t now = sentry__value_new_string_owned( + sentry__msec_time_to_iso8601(sentry__msec_time())); +#endif + sentry__envelope_set_header(envelope, "sent_at", now); + + return item; +} + sentry_envelope_item_t * sentry__envelope_add_session( sentry_envelope_t *envelope, const sentry_session_t *session) diff --git a/src/sentry_envelope.h b/src/sentry_envelope.h index ac5c69136..b5a8f1ab0 100644 --- a/src/sentry_envelope.h +++ b/src/sentry_envelope.h @@ -36,6 +36,12 @@ sentry_uuid_t sentry__envelope_get_event_id(const sentry_envelope_t *envelope); sentry_envelope_item_t *sentry__envelope_add_event( sentry_envelope_t *envelope, sentry_value_t event); +/** + * Add a transaction to this envelope. + */ +sentry_envelope_item_t *sentry__envelope_add_transaction( + sentry_envelope_t *envelope, sentry_value_t transaction); + /** * Add a session to this envelope. */ diff --git a/tests/unit/test_envelopes.c b/tests/unit/test_envelopes.c index a579afa3e..2554be1c7 100644 --- a/tests/unit/test_envelopes.c +++ b/tests/unit/test_envelopes.c @@ -32,6 +32,38 @@ SENTRY_TEST(basic_http_request_preparation_for_event) sentry__dsn_decref(dsn); } +SENTRY_TEST(basic_http_request_preparation_for_transaction) +{ + sentry_dsn_t *dsn = sentry__dsn_new("https://foo@sentry.invalid/42"); + + sentry_uuid_t event_id + = sentry_uuid_from_string("c993afb6-b4ac-48a6-b61b-2558e601d65d"); + sentry_envelope_t *envelope = sentry__envelope_new(); + sentry_value_t transaction = sentry_value_new_object(); + sentry_value_set_by_key( + transaction, "event_id", sentry__value_new_uuid(&event_id)); + sentry_value_set_by_key( + transaction, "type", sentry_value_new_string("transaction")); + sentry__envelope_add_transaction(envelope, transaction); + + sentry_prepared_http_request_t *req + = sentry__prepare_http_request(envelope, dsn, NULL); + TEST_CHECK_STRING_EQUAL(req->method, "POST"); + TEST_CHECK_STRING_EQUAL( + req->url, "https://sentry.invalid:443/api/42/envelope/"); + TEST_CHECK_STRING_EQUAL(req->body, + "{\"event_id\":\"c993afb6-b4ac-48a6-b61b-2558e601d65d\",\"sent_at\":" + "\"2021-12-16T05:53:59.343Z\"}\n" + "{\"type\":\"transaction\",\"length\":72}\n" + "{\"event_id\":\"c993afb6-b4ac-48a6-b61b-2558e601d65d\",\"type\":" + "\"transaction\"}"); + + sentry__prepared_http_request_free(req); + sentry_envelope_free(envelope); + + sentry__dsn_decref(dsn); +} + SENTRY_TEST(basic_http_request_preparation_for_event_with_attachment) { sentry_dsn_t *dsn = sentry__dsn_new("https://foo@sentry.invalid/42"); diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index 781361644..716be19d4 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -4,6 +4,7 @@ XX(basic_function_transport) XX(basic_http_request_preparation_for_event) XX(basic_http_request_preparation_for_event_with_attachment) XX(basic_http_request_preparation_for_minidump) +XX(basic_http_request_preparation_for_transaction) XX(basic_tracing_context) XX(basic_transaction) XX(buildid_fallback) From 0ddd751f071c32d2d0f8bf1f56e487452f972b80 Mon Sep 17 00:00:00 2001 From: Betty Da Date: Thu, 16 Dec 2021 01:53:00 -0500 Subject: [PATCH 02/21] feat(tracing): Allow manual creation and sending of spanless Transactions --- examples/example.c | 21 +++- include/sentry.h | 14 +++ src/backends/sentry_backend_inproc.c | 1 + src/sentry_core.c | 161 +++++++++++++++++++++++---- src/sentry_core.h | 23 +++- src/sentry_scope.c | 3 +- tests/unit/test_sampling.c | 38 +------ tests/unit/test_tracing.c | 126 +++++++++++++++++++++ tests/unit/tests.inc | 4 +- 9 files changed, 326 insertions(+), 65 deletions(-) diff --git a/examples/example.c b/examples/example.c index 0ace0324e..215fbdbde 100644 --- a/examples/example.c +++ b/examples/example.c @@ -93,9 +93,15 @@ main(int argc, char **argv) options, sentry_transport_new(print_envelope)); } + if (has_arg(argc, argv, "capture-transaction")) { + sentry_options_set_traces_sample_rate(options, 1.0); + } + sentry_init(options); - if (!has_arg(argc, argv, "no-setup")) { + if (!has_arg(argc, argv, "no-setup") + || has_arg(argc, argv, "capture-transaction")) { + sentry_set_transaction("test-transaction"); sentry_set_level(SENTRY_LEVEL_WARNING); sentry_set_extra("extra stuff", sentry_value_new_string("some value")); @@ -208,6 +214,19 @@ main(int argc, char **argv) sentry_capture_event(event); } + if (has_arg(argc, argv, "capture-transaction")) { + sentry_value_t tx_ctx + = sentry_value_new_transaction("I'm a little teapot", + "Short and stout here is my handle and here is my spout"); + + if (has_arg(argc, argv, "unsample-tx")) { + sentry_transaction_set_sampled(tx_ctx, 0); + } + + sentry_value_t tx = sentry_start_transaction(tx_ctx); + sentry_transaction_finish(tx); + } + // make sure everything flushes sentry_close(); diff --git a/include/sentry.h b/include/sentry.h index d0bcc485f..a7aca164c 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -1284,6 +1284,20 @@ SENTRY_EXPERIMENTAL_API void sentry_transaction_set_sampled( SENTRY_EXPERIMENTAL_API void sentry_transaction_remove_sampled( sentry_value_t transaction); +/** + * Starts a new transaction based on the provided context, restored + * from an external integration (i.e. a span from a different SDK) + * or manually constructed by a user. + */ +SENTRY_EXPERIMENTAL_API sentry_value_t sentry_start_transaction( + sentry_value_t transaction); + +/** + * Finishes a transaction. + */ +SENTRY_EXPERIMENTAL_API void sentry_transaction_finish( + sentry_value_t transaction); + #ifdef __cplusplus } #endif diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index c15493dab..fd4df8793 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -260,6 +260,7 @@ handle_ucontext(const sentry_ucontext_t *uctx) sentry_envelope_t *envelope = sentry__prepare_event(options, event, NULL); + // TODO: flush transaction sitting in scope? sentry_session_t *session = sentry__end_current_session_with_status( SENTRY_SESSION_STATUS_CRASHED); diff --git a/src/sentry_core.c b/src/sentry_core.c index 6eb2e8418..dc225e0d9 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -16,6 +16,7 @@ #include "sentry_session.h" #include "sentry_string.h" #include "sentry_sync.h" +#include "sentry_tracing.h" #include "sentry_transport.h" #include "sentry_value.h" @@ -380,7 +381,7 @@ sentry__capture_event(sentry_value_t event) SENTRY_WITH_OPTIONS (options) { was_captured = true; if (sentry__event_is_transaction(event)) { - return sentry_uuid_nil(); + envelope = sentry__prepare_transaction(options, event, &event_id); } else { envelope = sentry__prepare_event(options, event, &event_id); } @@ -389,10 +390,10 @@ sentry__capture_event(sentry_value_t event) SENTRY_WITH_OPTIONS_MUT (mut_options) { sentry__envelope_add_session( envelope, mut_options->session); - // we're assuming that if a session is added to an envelope - // it will be sent onwards. This means we now need to set - // the init flag to false because we're no longer the - // initial session update. + // we're assuming that if a session is added to an + // envelope it will be sent onwards. This means we now + // need to set the init flag to false because we're no + // longer the initial session update. mut_options->session->init = false; } } @@ -415,32 +416,21 @@ sentry__roll_dice(double probability) } bool -sentry__should_skip_transaction(sentry_value_t tx_cxt) +sentry__should_send_transaction(sentry_value_t tx_cxt) { sentry_value_t context_setting = sentry_value_get_by_key(tx_cxt, "sampled"); if (!sentry_value_is_null(context_setting)) { - return !sentry_value_is_true(context_setting); + return sentry_value_is_true(context_setting); } - bool skip = true; + bool send = false; SENTRY_WITH_OPTIONS (options) { - skip = !sentry__roll_dice(options->traces_sample_rate); + SENTRY_DEBUG("rolling dice"); + send = sentry__roll_dice(options->traces_sample_rate); + SENTRY_DEBUGF("result: %d", send); // TODO: run through traces sampler function if rate is unavailable } - return skip; -} - -bool -sentry__should_skip_event(const sentry_options_t *options, sentry_value_t event) -{ - if (sentry__event_is_transaction(event)) { - // The sampling decision should already be made for transactions - // during their construction. No need to recalculate here. - // See `sentry__should_skip_transaction`. - return !sentry_value_is_true(sentry_value_get_by_key(event, "sampled")); - } else { - return !sentry__roll_dice(options->sample_rate); - } + return send; } sentry_envelope_t * @@ -453,7 +443,8 @@ sentry__prepare_event(const sentry_options_t *options, sentry_value_t event, sentry__record_errors_on_current_session(1); } - if (sentry__should_skip_event(options, event)) { + bool should_skip = !sentry__roll_dice(options->sample_rate); + if (should_skip) { SENTRY_DEBUG("throwing away event due to sample rate"); goto fail; } @@ -508,6 +499,60 @@ sentry__prepare_event(const sentry_options_t *options, sentry_value_t event, return NULL; } +sentry_envelope_t * +sentry__prepare_transaction(const sentry_options_t *options, + sentry_value_t transaction, sentry_uuid_t *event_id) +{ + sentry_envelope_t *envelope = NULL; + + bool should_skip = !sentry_value_is_true( + sentry_value_get_by_key(transaction, "sampled")); + if (should_skip) { + SENTRY_DEBUG("throwing away transaction due to sample rate"); + goto fail; + } + // Field is superfluous, strip so it doesn't leak into the payload + sentry_value_remove_by_key(transaction, "sampled"); + + SENTRY_WITH_SCOPE (scope) { + SENTRY_TRACE("merging scope into event"); + // Don't include debugging info + sentry_scope_mode_t mode = SENTRY_SCOPE_ALL & ~SENTRY_SCOPE_MODULES + & ~SENTRY_SCOPE_STACKTRACES; + sentry__scope_apply_to_event(scope, options, transaction, mode); + } + + sentry__ensure_event_id(transaction, event_id); + envelope = sentry__envelope_new(); + if (!envelope || !sentry__envelope_add_transaction(envelope, transaction)) { + goto fail; + } + + SENTRY_TRACE("adding attachments to envelope"); + for (sentry_attachment_t *attachment = options->attachments; attachment; + attachment = attachment->next) { + sentry_envelope_item_t *item = sentry__envelope_add_from_path( + envelope, attachment->path, "attachment"); + if (!item) { + continue; + } + sentry__envelope_item_set_header(item, "filename", +#ifdef SENTRY_PLATFORM_WINDOWS + sentry__value_new_string_from_wstr( +#else + sentry_value_new_string( +#endif + sentry__path_filename(attachment->path))); + } + + return envelope; + +fail: + sentry_envelope_free(envelope); + sentry_value_decref(transaction); + return NULL; +} + void sentry_handle_exception(const sentry_ucontext_t *uctx) { @@ -689,3 +734,71 @@ sentry_set_level(sentry_level_t level) scope->level = level; } } + +sentry_value_t +sentry_start_transaction(sentry_value_t tx_cxt) +{ + sentry_value_t tx = sentry_value_new_event(); + + // TODO: stuff transaction into the scope + bool should_sample = sentry__should_send_transaction(tx_cxt); + sentry_value_set_by_key( + tx, "sampled", sentry_value_new_bool(should_sample)); + + sentry_value_set_by_key( + tx, "transaction", sentry_value_get_by_key_owned(tx_cxt, "name")); + sentry_value_set_by_key(tx, "start_timestamp", + sentry__value_new_string_owned( + sentry__msec_time_to_iso8601(sentry__msec_time()))); + + sentry_uuid_t trace_id = sentry_uuid_new_v4(); + sentry_value_set_by_key( + tx, "trace_id", sentry__value_new_internal_uuid(&trace_id)); + + sentry_uuid_t span_id = sentry_uuid_new_v4(); + sentry_value_set_by_key( + tx, "span_id", sentry__value_new_span_uuid(&span_id)); + + sentry_value_decref(tx_cxt); + + return tx; +} + +void +sentry_transaction_finish(sentry_value_t tx) +{ + sentry_value_t sampled = sentry_value_get_by_key(tx, "sampled"); + if (!sentry_value_is_null(sampled) && !sentry_value_is_true(sampled)) { + sentry_value_decref(sampled); + sentry_value_decref(tx); + // TODO: remove from scope + return; + } + sentry_value_decref(sampled); + + sentry_value_set_by_key(tx, "type", sentry_value_new_string("transaction")); + sentry_value_set_by_key(tx, "timestamp", + sentry__value_new_string_owned( + sentry__msec_time_to_iso8601(sentry__msec_time()))); + sentry_value_set_by_key(tx, "level", sentry_value_new_string("info")); + + // TODO: add tracestate + // set up trace context so it mirrors the final json value + sentry_value_set_by_key(tx, "status", sentry_value_new_string("ok")); + + sentry_value_t trace_context = sentry__span_get_trace_context(tx); + sentry_value_t contexts = sentry_value_new_object(); + sentry_value_set_by_key(contexts, "trace", trace_context); + sentry_value_set_by_key(tx, "contexts", contexts); + + // clean up trace context fields + sentry_value_remove_by_key(tx, "trace_id"); + sentry_value_remove_by_key(tx, "span_id"); + sentry_value_remove_by_key(tx, "parent_span_id"); + sentry_value_remove_by_key(tx, "op"); + sentry_value_remove_by_key(tx, "description"); + sentry_value_remove_by_key(tx, "status"); + + // This decrefs for us, generates an event ID, merges scope + sentry__capture_event(tx); +} diff --git a/src/sentry_core.h b/src/sentry_core.h index bec05ef5a..e6f5a1453 100644 --- a/src/sentry_core.h +++ b/src/sentry_core.h @@ -35,7 +35,8 @@ bool sentry__should_skip_upload(void); bool sentry__event_is_transaction(sentry_value_t event); /** - * Convert the given event into an envelope. + * Convert the given event into an envelope. This assumes that the event + * being passed in is not a transaction. * * More specifically, it will do the following things: * - sample the event, possibly discarding it, @@ -56,6 +57,22 @@ sentry_envelope_t *sentry__prepare_event(const sentry_options_t *options, */ sentry_uuid_t sentry__capture_event(sentry_value_t event); +/** + * Convert the given transaction into an envelope. This assumes that the + * event being passed in is a transaction. + * + * It will do the following things: + * - discard the transaction if it is unsampled + * - apply the scope to the transaction + * - add the transaction to a new envelope + * - add any attachments to the envelope + * + * The function will ensure the transaction has a UUID and write it into the + * `event_id` out-parameter. + */ +sentry_envelope_t *sentry__prepare_transaction(const sentry_options_t *options, + sentry_value_t event, sentry_uuid_t *event_id); + /** * This function will submit the `envelope` to the given `transport`, first * checking for consent. @@ -102,9 +119,7 @@ void sentry__options_unlock(void); // these for now are only needed for tests #ifdef SENTRY_UNITTEST bool sentry__roll_dice(double probability); -bool sentry__should_skip_transaction(sentry_value_t tx_cxt); -bool sentry__should_skip_event( - const sentry_options_t *options, sentry_value_t event); +bool sentry__should_send_transaction(sentry_value_t tx_cxt); #endif #endif diff --git a/src/sentry_scope.c b/src/sentry_scope.c index 5052f023f..3edf49888 100644 --- a/src/sentry_scope.c +++ b/src/sentry_scope.c @@ -268,7 +268,8 @@ sentry__scope_apply_to_event(const sentry_scope_t *scope, PLACE_STRING("dist", options->dist); PLACE_STRING("environment", options->environment); - if (IS_NULL("level")) { + // is not transaction and has no level + if (IS_NULL("type") && IS_NULL("level")) { SET("level", sentry__value_new_level(scope->level)); } diff --git a/tests/unit/test_sampling.c b/tests/unit/test_sampling.c index 8ab8d6ab4..f0b51d659 100644 --- a/tests/unit/test_sampling.c +++ b/tests/unit/test_sampling.c @@ -13,54 +13,24 @@ SENTRY_TEST(sampling_transaction) sentry_options_t *options = sentry_options_new(); TEST_CHECK(sentry_init(options) == 0); - // TODO: replace with proper construction of a transaction, e.g. - // new_transaction -> transaction_set_sampled -> start_transaction sentry_value_t tx_cxt = sentry_value_new_transaction("honk", NULL); sentry_transaction_set_sampled(tx_cxt, 0); - TEST_CHECK(sentry__should_skip_transaction(tx_cxt)); + TEST_CHECK(sentry__should_send_transaction(tx_cxt) == false); sentry_transaction_set_sampled(tx_cxt, 1); - TEST_CHECK(sentry__should_skip_transaction(tx_cxt) == false); + TEST_CHECK(sentry__should_send_transaction(tx_cxt)); // fall back to default in sentry options (0.0) if sampled isn't there sentry_transaction_remove_sampled(tx_cxt); - TEST_CHECK(sentry__should_skip_transaction(tx_cxt)); + TEST_CHECK(sentry__should_send_transaction(tx_cxt) == false); options = sentry_options_new(); sentry_options_set_traces_sample_rate(options, 1.0); TEST_CHECK(sentry_init(options) == 0); - TEST_CHECK(sentry__should_skip_transaction(tx_cxt) == false); + TEST_CHECK(sentry__should_send_transaction(tx_cxt)); sentry_value_decref(tx_cxt); sentry_close(); } - -SENTRY_TEST(sampling_event) -{ - // default is to sample all (error) events, and to not sample any - // transactions - sentry_options_t *options = sentry_options_new(); - - sentry_value_t event = sentry_value_new_object(); - sentry_value_set_by_key(event, "sampled", sentry_value_new_bool(0)); - - // events ignore sampled field if they're not transactions - TEST_CHECK(sentry__should_skip_event(options, event) == false); - - // respect sampled field if it is a transaction - sentry_value_set_by_key( - event, "type", sentry_value_new_string("transaction")); - TEST_CHECK(sentry__should_skip_event(options, event)); - - // if the sampled field isn't set on a transaction, don't ever send - // transactions even if the option says to do so - sentry_value_remove_by_key(event, "sampled"); - TEST_CHECK(sentry__should_skip_event(options, event)); - sentry_options_set_traces_sample_rate(options, 1.0); - TEST_CHECK(sentry__should_skip_event(options, event)); - - sentry_value_decref(event); - sentry_options_free(options); -} diff --git a/tests/unit/test_tracing.c b/tests/unit/test_tracing.c index cce9bbc7f..a21baa7b0 100644 --- a/tests/unit/test_tracing.c +++ b/tests/unit/test_tracing.c @@ -73,3 +73,129 @@ SENTRY_TEST(basic_transaction) sentry_value_decref(tx_cxt); } + +static void +send_transaction_envelope_test_basic(sentry_envelope_t *envelope, void *data) +{ + uint64_t *called = data; + *called += 1; + + sentry_value_t transaction = sentry_envelope_get_transaction(envelope); + TEST_CHECK(!sentry_value_is_null(transaction)); + const char *event_id = sentry_value_as_string( + sentry_value_get_by_key(transaction, "event_id")); + TEST_CHECK_STRING_EQUAL(event_id, "4c035723-8638-4c3a-923f-2ab9d08b4018"); + + if (*called == 1) { + const char *type = sentry_value_as_string( + sentry_value_get_by_key(transaction, "type")); + TEST_CHECK_STRING_EQUAL(type, "transaction"); + const char *name = sentry_value_as_string( + sentry_value_get_by_key(transaction, "transaction")); + TEST_CHECK_STRING_EQUAL(name, "honk"); + } + + sentry_envelope_free(envelope); +} + +SENTRY_TEST(basic_function_transport_transaction) +{ + uint64_t called = 0; + + sentry_options_t *options = sentry_options_new(); + sentry_options_set_dsn(options, "https://foo@sentry.invalid/42"); + + sentry_transport_t *transport + = sentry_transport_new(send_transaction_envelope_test_basic); + sentry_transport_set_state(transport, &called); + sentry_options_set_transport(options, transport); + + sentry_options_set_traces_sample_rate(options, 1.0); + sentry_options_set_require_user_consent(options, true); + sentry_init(options); + + sentry_value_t transaction + = sentry_value_new_transaction("How could you", "Don't capture this."); + transaction = sentry_start_transaction(transaction); + sentry_transaction_finish(transaction); + sentry_user_consent_give(); + + transaction = sentry_value_new_transaction("honk", "beep"); + transaction = sentry_start_transaction(transaction); + sentry_transaction_finish(transaction); + + sentry_user_consent_revoke(); + transaction = sentry_value_new_transaction( + "How could you again", "Don't capture this either."); + transaction = sentry_start_transaction(transaction); + sentry_transaction_finish(transaction); + + sentry_close(); + + TEST_CHECK_INT_EQUAL(called, 1); +} + +SENTRY_TEST(transport_sampling_transactions) +{ + uint64_t called_transport = 0; + + sentry_options_t *options = sentry_options_new(); + sentry_options_set_dsn(options, "https://foo@sentry.invalid/42"); + + sentry_transport_t *transport + = sentry_transport_new(send_transaction_envelope_test_basic); + sentry_transport_set_state(transport, &called_transport); + sentry_options_set_transport(options, transport); + + sentry_options_set_traces_sample_rate(options, 0.75); + sentry_init(options); + + for (int i = 0; i < 100; i++) { + sentry_value_t transaction + = sentry_value_new_transaction("honk", "beep"); + transaction = sentry_start_transaction(transaction); + sentry_transaction_finish(transaction); + } + + sentry_close(); + + // well, its random after all + TEST_CHECK(called_transport > 50 && called_transport < 100); +} + +static sentry_value_t +before_send(sentry_value_t event, void *UNUSED(hint), void *data) +{ + uint64_t *called = data; + *called += 1; + + sentry_value_decref(event); + return sentry_value_new_null(); +} + +SENTRY_TEST(transactions_skip_before_send) +{ + uint64_t called_beforesend = 0; + uint64_t called_transport = 0; + + sentry_options_t *options = sentry_options_new(); + sentry_options_set_dsn(options, "https://foo@sentry.invalid/42"); + + sentry_transport_t *transport + = sentry_transport_new(send_transaction_envelope_test_basic); + sentry_transport_set_state(transport, &called_transport); + sentry_options_set_transport(options, transport); + + sentry_options_set_traces_sample_rate(options, 1.0); + sentry_options_set_before_send(options, before_send, &called_beforesend); + sentry_init(options); + + sentry_value_t transaction = sentry_value_new_transaction("honk", "beep"); + transaction = sentry_start_transaction(transaction); + sentry_transaction_finish(transaction); + + sentry_close(); + + TEST_CHECK_INT_EQUAL(called_transport, 1); + TEST_CHECK_INT_EQUAL(called_beforesend, 0); +} diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index 716be19d4..0c54353ed 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -1,6 +1,7 @@ XX(background_worker) XX(basic_consent_tracking) XX(basic_function_transport) +XX(basic_function_transport_transaction) XX(basic_http_request_preparation_for_event) XX(basic_http_request_preparation_for_event_with_attachment) XX(basic_http_request_preparation_for_minidump) @@ -41,13 +42,14 @@ XX(rate_limit_parsing) XX(recursive_paths) XX(sampling_before_send) XX(sampling_decision) -XX(sampling_event) XX(sampling_transaction) XX(serialize_envelope) XX(session_basics) XX(slice) XX(symbolizer) XX(task_queue) +XX(transactions_skip_before_send) +XX(transport_sampling_transactions) XX(uninitialized) XX(unwinder) XX(url_parsing_complete) From 204649f72a795c22a71b37c5b3b9ab2bf73dc49a Mon Sep 17 00:00:00 2001 From: Betty Da Date: Thu, 16 Dec 2021 03:19:26 -0500 Subject: [PATCH 03/21] rename `sentry_start_transaction` to `sentry_transaction_start` --- examples/example.c | 2 +- include/sentry.h | 4 ++-- src/sentry_core.c | 2 +- tests/unit/test_tracing.c | 10 +++++----- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/examples/example.c b/examples/example.c index 215fbdbde..c15127674 100644 --- a/examples/example.c +++ b/examples/example.c @@ -223,7 +223,7 @@ main(int argc, char **argv) sentry_transaction_set_sampled(tx_ctx, 0); } - sentry_value_t tx = sentry_start_transaction(tx_ctx); + sentry_value_t tx = sentry_transaction_start(tx_ctx); sentry_transaction_finish(tx); } diff --git a/include/sentry.h b/include/sentry.h index a7aca164c..275db88dd 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -1239,7 +1239,7 @@ SENTRY_EXPERIMENTAL_API double sentry_options_get_traces_sample_rate( /** * Constructs a new inert Transaction. The returned value needs to be passed - * into `sentry_start_transaction` in order to be recorded and sent to sentry. + * into `sentry_transaction_start` in order to be recorded and sent to sentry. * * See * https://docs.sentry.io/platforms/native/enriching-events/transaction-name/ @@ -1289,7 +1289,7 @@ SENTRY_EXPERIMENTAL_API void sentry_transaction_remove_sampled( * from an external integration (i.e. a span from a different SDK) * or manually constructed by a user. */ -SENTRY_EXPERIMENTAL_API sentry_value_t sentry_start_transaction( +SENTRY_EXPERIMENTAL_API sentry_value_t sentry_transaction_start( sentry_value_t transaction); /** diff --git a/src/sentry_core.c b/src/sentry_core.c index dc225e0d9..cd038455d 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -736,7 +736,7 @@ sentry_set_level(sentry_level_t level) } sentry_value_t -sentry_start_transaction(sentry_value_t tx_cxt) +sentry_transaction_start(sentry_value_t tx_cxt) { sentry_value_t tx = sentry_value_new_event(); diff --git a/tests/unit/test_tracing.c b/tests/unit/test_tracing.c index a21baa7b0..aa8f4fdcb 100644 --- a/tests/unit/test_tracing.c +++ b/tests/unit/test_tracing.c @@ -116,18 +116,18 @@ SENTRY_TEST(basic_function_transport_transaction) sentry_value_t transaction = sentry_value_new_transaction("How could you", "Don't capture this."); - transaction = sentry_start_transaction(transaction); + transaction = sentry_transaction_start(transaction); sentry_transaction_finish(transaction); sentry_user_consent_give(); transaction = sentry_value_new_transaction("honk", "beep"); - transaction = sentry_start_transaction(transaction); + transaction = sentry_transaction_start(transaction); sentry_transaction_finish(transaction); sentry_user_consent_revoke(); transaction = sentry_value_new_transaction( "How could you again", "Don't capture this either."); - transaction = sentry_start_transaction(transaction); + transaction = sentry_transaction_start(transaction); sentry_transaction_finish(transaction); sentry_close(); @@ -153,7 +153,7 @@ SENTRY_TEST(transport_sampling_transactions) for (int i = 0; i < 100; i++) { sentry_value_t transaction = sentry_value_new_transaction("honk", "beep"); - transaction = sentry_start_transaction(transaction); + transaction = sentry_transaction_start(transaction); sentry_transaction_finish(transaction); } @@ -191,7 +191,7 @@ SENTRY_TEST(transactions_skip_before_send) sentry_init(options); sentry_value_t transaction = sentry_value_new_transaction("honk", "beep"); - transaction = sentry_start_transaction(transaction); + transaction = sentry_transaction_start(transaction); sentry_transaction_finish(transaction); sentry_close(); From 61c61947895b0fbcdabe78c1de6882f3d57cca73 Mon Sep 17 00:00:00 2001 From: Betty Da Date: Thu, 16 Dec 2021 03:24:55 -0500 Subject: [PATCH 04/21] brush up docstrings --- include/sentry.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/sentry.h b/include/sentry.h index 275db88dd..71c870d46 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -1270,9 +1270,9 @@ SENTRY_EXPERIMENTAL_API void sentry_transaction_set_operation( sentry_value_t transaction, const char *operation); /** - * Sets the `sampled` field on a Transaction. When turned on, the Transaction - * will bypass all sampling options and always be sent to sentry. If this is - * explicitly turned off in the Transaction, it will never be sent to sentry. + * Sets the `sampled` field on a Transaction. When passed any value above 0, the + * Transaction will bypass all sampling options and always be sent to sentry. If + * passed 0, this Transaction and its child spans will never be sent to sentry. */ SENTRY_EXPERIMENTAL_API void sentry_transaction_set_sampled( sentry_value_t transaction, int sampled); @@ -1293,7 +1293,7 @@ SENTRY_EXPERIMENTAL_API sentry_value_t sentry_transaction_start( sentry_value_t transaction); /** - * Finishes a transaction. + * Finishes and sends a transaction to sentry. */ SENTRY_EXPERIMENTAL_API void sentry_transaction_finish( sentry_value_t transaction); From f47625d7ab32173614e7c9c8bfcaed045a573413 Mon Sep 17 00:00:00 2001 From: Betty Da Date: Thu, 16 Dec 2021 03:39:02 -0500 Subject: [PATCH 05/21] whoops --- src/sentry_core.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/sentry_core.c b/src/sentry_core.c index cd038455d..31db36502 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -425,9 +425,7 @@ sentry__should_send_transaction(sentry_value_t tx_cxt) bool send = false; SENTRY_WITH_OPTIONS (options) { - SENTRY_DEBUG("rolling dice"); send = sentry__roll_dice(options->traces_sample_rate); - SENTRY_DEBUGF("result: %d", send); // TODO: run through traces sampler function if rate is unavailable } return send; From 49ec6d8b1ca8cdd1384f4a1aabbcc271ed974e23 Mon Sep 17 00:00:00 2001 From: Betty Da Date: Fri, 17 Dec 2021 01:28:09 -0500 Subject: [PATCH 06/21] made a mistake with no-setup --- examples/example.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/example.c b/examples/example.c index c15127674..96b23f907 100644 --- a/examples/example.c +++ b/examples/example.c @@ -100,7 +100,7 @@ main(int argc, char **argv) sentry_init(options); if (!has_arg(argc, argv, "no-setup") - || has_arg(argc, argv, "capture-transaction")) { + && !has_arg(argc, argv, "capture-transaction")) { sentry_set_transaction("test-transaction"); sentry_set_level(SENTRY_LEVEL_WARNING); From c63c07b0b843e11052a364cf7f40601c515e1414 Mon Sep 17 00:00:00 2001 From: Betty Da Date: Mon, 20 Dec 2021 23:36:31 -0500 Subject: [PATCH 07/21] no longer needed --- examples/example.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/examples/example.c b/examples/example.c index 96b23f907..5535a10ec 100644 --- a/examples/example.c +++ b/examples/example.c @@ -99,9 +99,7 @@ main(int argc, char **argv) sentry_init(options); - if (!has_arg(argc, argv, "no-setup") - && !has_arg(argc, argv, "capture-transaction")) { - + if (!has_arg(argc, argv, "no-setup")) { sentry_set_transaction("test-transaction"); sentry_set_level(SENTRY_LEVEL_WARNING); sentry_set_extra("extra stuff", sentry_value_new_string("some value")); From 6b5d9fb446dbf980cfe1c3854362d691a8b4b261 Mon Sep 17 00:00:00 2001 From: Betty Da Date: Tue, 21 Dec 2021 00:41:06 -0500 Subject: [PATCH 08/21] todo cleanup --- src/backends/sentry_backend_inproc.c | 3 ++- src/sentry_core.c | 9 +++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index fd4df8793..2318e2774 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -260,7 +260,8 @@ handle_ucontext(const sentry_ucontext_t *uctx) sentry_envelope_t *envelope = sentry__prepare_event(options, event, NULL); - // TODO: flush transaction sitting in scope? + // TODO(tracing): Revisit when investigating transaction flushing during + // hard crashes. sentry_session_t *session = sentry__end_current_session_with_status( SENTRY_SESSION_STATUS_CRASHED); diff --git a/src/sentry_core.c b/src/sentry_core.c index 31db36502..3a0f24166 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -426,7 +426,8 @@ sentry__should_send_transaction(sentry_value_t tx_cxt) bool send = false; SENTRY_WITH_OPTIONS (options) { send = sentry__roll_dice(options->traces_sample_rate); - // TODO: run through traces sampler function if rate is unavailable + // TODO(tracing): Run through traces sampler function if rate is + // unavailable. } return send; } @@ -738,7 +739,7 @@ sentry_transaction_start(sentry_value_t tx_cxt) { sentry_value_t tx = sentry_value_new_event(); - // TODO: stuff transaction into the scope + // TODO(tracing): stuff transaction into the scope bool should_sample = sentry__should_send_transaction(tx_cxt); sentry_value_set_by_key( tx, "sampled", sentry_value_new_bool(should_sample)); @@ -769,7 +770,7 @@ sentry_transaction_finish(sentry_value_t tx) if (!sentry_value_is_null(sampled) && !sentry_value_is_true(sampled)) { sentry_value_decref(sampled); sentry_value_decref(tx); - // TODO: remove from scope + // TODO(tracing): remove from scope return; } sentry_value_decref(sampled); @@ -780,7 +781,7 @@ sentry_transaction_finish(sentry_value_t tx) sentry__msec_time_to_iso8601(sentry__msec_time()))); sentry_value_set_by_key(tx, "level", sentry_value_new_string("info")); - // TODO: add tracestate + // TODO(tracing): add tracestate // set up trace context so it mirrors the final json value sentry_value_set_by_key(tx, "status", sentry_value_new_string("ok")); From 298c4a6fff59c85e538d289752d257b319e79191 Mon Sep 17 00:00:00 2001 From: Betty Da Date: Tue, 21 Dec 2021 00:42:49 -0500 Subject: [PATCH 09/21] improve debug message --- src/sentry_core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/sentry_core.c b/src/sentry_core.c index 3a0f24166..0f2bbc139 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -507,7 +507,8 @@ sentry__prepare_transaction(const sentry_options_t *options, bool should_skip = !sentry_value_is_true( sentry_value_get_by_key(transaction, "sampled")); if (should_skip) { - SENTRY_DEBUG("throwing away transaction due to sample rate"); + SENTRY_DEBUG("throwing away transaction due to sample rate or " + "user-provided sampling value in transaction context"); goto fail; } // Field is superfluous, strip so it doesn't leak into the payload From 8cdd5c31282fb49d5fced64efb6e091fc794a6ed Mon Sep 17 00:00:00 2001 From: Betty Da Date: Tue, 21 Dec 2021 00:43:40 -0500 Subject: [PATCH 10/21] no more attachments for now --- src/sentry_core.c | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/src/sentry_core.c b/src/sentry_core.c index 0f2bbc139..4e5c6b6a6 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -528,22 +528,7 @@ sentry__prepare_transaction(const sentry_options_t *options, goto fail; } - SENTRY_TRACE("adding attachments to envelope"); - for (sentry_attachment_t *attachment = options->attachments; attachment; - attachment = attachment->next) { - sentry_envelope_item_t *item = sentry__envelope_add_from_path( - envelope, attachment->path, "attachment"); - if (!item) { - continue; - } - sentry__envelope_item_set_header(item, "filename", -#ifdef SENTRY_PLATFORM_WINDOWS - sentry__value_new_string_from_wstr( -#else - sentry_value_new_string( -#endif - sentry__path_filename(attachment->path))); - } + // TODO(tracing): Revisit when adding attachment support for transactions. return envelope; From 3d44eeb335df0e783ee9c77e1ed031e4e31c4fae Mon Sep 17 00:00:00 2001 From: Betty Da Date: Tue, 21 Dec 2021 00:52:07 -0500 Subject: [PATCH 11/21] accidentally forgot to copy over a useful comment --- src/sentry_core.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/sentry_core.c b/src/sentry_core.c index 4e5c6b6a6..af500990b 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -752,6 +752,9 @@ sentry_transaction_start(sentry_value_t tx_cxt) void sentry_transaction_finish(sentry_value_t tx) { + // The sampling decision should already be made for transactions during + // their construction. No need to recalculate here. See + // `sentry__should_skip_transaction`. sentry_value_t sampled = sentry_value_get_by_key(tx, "sampled"); if (!sentry_value_is_null(sampled) && !sentry_value_is_true(sampled)) { sentry_value_decref(sampled); From 10ecee514514fd2f4286a7d18c2416ec54c2b9bd Mon Sep 17 00:00:00 2001 From: Betty Da Date: Tue, 21 Dec 2021 00:57:47 -0500 Subject: [PATCH 12/21] move uuid initialization into transactions --- src/sentry_core.c | 19 +++++++++++-------- src/sentry_value.c | 8 ++++++++ tests/unit/test_tracing.c | 12 ++++++++++++ 3 files changed, 31 insertions(+), 8 deletions(-) diff --git a/src/sentry_core.c b/src/sentry_core.c index af500990b..270d03230 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -730,20 +730,23 @@ sentry_transaction_start(sentry_value_t 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); + } + 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, "trace_id")); sentry_value_set_by_key( tx, "transaction", sentry_value_get_by_key_owned(tx_cxt, "name")); sentry_value_set_by_key(tx, "start_timestamp", sentry__value_new_string_owned( sentry__msec_time_to_iso8601(sentry__msec_time()))); - sentry_uuid_t trace_id = sentry_uuid_new_v4(); - sentry_value_set_by_key( - tx, "trace_id", sentry__value_new_internal_uuid(&trace_id)); - - sentry_uuid_t span_id = sentry_uuid_new_v4(); - sentry_value_set_by_key( - tx, "span_id", sentry__value_new_span_uuid(&span_id)); - sentry_value_decref(tx_cxt); return tx; diff --git a/src/sentry_value.c b/src/sentry_value.c index c71c68ecf..d3cadf246 100644 --- a/src/sentry_value.c +++ b/src/sentry_value.c @@ -1129,6 +1129,14 @@ sentry_value_new_transaction(const char *name, const char *operation) { sentry_value_t transaction = sentry_value_new_object(); + sentry_uuid_t trace_id = sentry_uuid_new_v4(); + sentry_value_set_by_key( + transaction, "trace_id", sentry__value_new_internal_uuid(&trace_id)); + + sentry_uuid_t span_id = sentry_uuid_new_v4(); + sentry_value_set_by_key( + transaction, "span_id", sentry__value_new_span_uuid(&span_id)); + sentry_transaction_set_name(transaction, name); sentry_transaction_set_operation(transaction, operation); diff --git a/tests/unit/test_tracing.c b/tests/unit/test_tracing.c index aa8f4fdcb..56cc49984 100644 --- a/tests/unit/test_tracing.c +++ b/tests/unit/test_tracing.c @@ -44,6 +44,10 @@ SENTRY_TEST(basic_transaction) const char *tx_op = sentry_value_as_string(sentry_value_get_by_key(tx_cxt, "op")); TEST_CHECK_STRING_EQUAL(tx_op, ""); + TEST_CHECK( + !sentry_value_is_null(sentry_value_get_by_key(tx_cxt, "trace_id"))); + TEST_CHECK( + !sentry_value_is_null(sentry_value_get_by_key(tx_cxt, "span_id"))); sentry_value_decref(tx_cxt); tx_cxt = sentry_value_new_transaction("", ""); @@ -51,6 +55,10 @@ SENTRY_TEST(basic_transaction) tx_name = sentry_value_as_string(sentry_value_get_by_key(tx_cxt, "name")); TEST_CHECK_STRING_EQUAL(tx_name, ""); TEST_CHECK_STRING_EQUAL(tx_op, ""); + TEST_CHECK( + !sentry_value_is_null(sentry_value_get_by_key(tx_cxt, "trace_id"))); + TEST_CHECK( + !sentry_value_is_null(sentry_value_get_by_key(tx_cxt, "span_id"))); sentry_value_decref(tx_cxt); tx_cxt = sentry_value_new_transaction("honk.beep", "beepbeep"); @@ -58,6 +66,10 @@ SENTRY_TEST(basic_transaction) TEST_CHECK_STRING_EQUAL(tx_name, "honk.beep"); tx_op = sentry_value_as_string(sentry_value_get_by_key(tx_cxt, "op")); TEST_CHECK_STRING_EQUAL(tx_op, "beepbeep"); + TEST_CHECK( + !sentry_value_is_null(sentry_value_get_by_key(tx_cxt, "trace_id"))); + TEST_CHECK( + !sentry_value_is_null(sentry_value_get_by_key(tx_cxt, "span_id"))); sentry_transaction_set_name(tx_cxt, ""); tx_name = sentry_value_as_string(sentry_value_get_by_key(tx_cxt, "name")); From ff9250e3c949185a6952d3cf521d2e0acafa4f7f Mon Sep 17 00:00:00 2001 From: Betty Da Date: Tue, 21 Dec 2021 01:05:36 -0500 Subject: [PATCH 13/21] docs --- include/sentry.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/sentry.h b/include/sentry.h index 71c870d46..3cddb4e71 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -1285,9 +1285,9 @@ SENTRY_EXPERIMENTAL_API void sentry_transaction_remove_sampled( sentry_value_t transaction); /** - * Starts a new transaction based on the provided context, restored - * from an external integration (i.e. a span from a different SDK) - * or manually constructed by a user. + * Starts a new Transaction based on the provided context, restored from an + * external integration (i.e. a span from a different SDK) or manually + * constructed by a user. */ SENTRY_EXPERIMENTAL_API sentry_value_t sentry_transaction_start( sentry_value_t transaction); From d967e36b6413736cd76e3446e58b77023f0bac9b Mon Sep 17 00:00:00 2001 From: Betty Da Date: Tue, 21 Dec 2021 01:06:20 -0500 Subject: [PATCH 14/21] return the event id of the transaction if successfully sent --- include/sentry.h | 6 ++++-- src/sentry_core.c | 6 +++--- tests/unit/test_tracing.c | 17 ++++++++++++----- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/include/sentry.h b/include/sentry.h index 3cddb4e71..1ed7d8b9f 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -1293,9 +1293,11 @@ SENTRY_EXPERIMENTAL_API sentry_value_t sentry_transaction_start( sentry_value_t transaction); /** - * Finishes and sends a transaction to sentry. + * Finishes and sends a transaction to sentry. The event ID of the transaction + * will be returned if this was successful; A nil UUID will be returned + * otherwise. */ -SENTRY_EXPERIMENTAL_API void sentry_transaction_finish( +SENTRY_EXPERIMENTAL_API sentry_uuid_t sentry_transaction_finish( sentry_value_t transaction); #ifdef __cplusplus diff --git a/src/sentry_core.c b/src/sentry_core.c index 270d03230..213e4fa85 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -752,7 +752,7 @@ sentry_transaction_start(sentry_value_t tx_cxt) return tx; } -void +sentry_uuid_t sentry_transaction_finish(sentry_value_t tx) { // The sampling decision should already be made for transactions during @@ -763,7 +763,7 @@ sentry_transaction_finish(sentry_value_t tx) sentry_value_decref(sampled); sentry_value_decref(tx); // TODO(tracing): remove from scope - return; + return sentry_uuid_nil(); } sentry_value_decref(sampled); @@ -791,5 +791,5 @@ sentry_transaction_finish(sentry_value_t tx) sentry_value_remove_by_key(tx, "status"); // This decrefs for us, generates an event ID, merges scope - sentry__capture_event(tx); + return sentry__capture_event(tx); } diff --git a/tests/unit/test_tracing.c b/tests/unit/test_tracing.c index 56cc49984..cd0a763cb 100644 --- a/tests/unit/test_tracing.c +++ b/tests/unit/test_tracing.c @@ -129,18 +129,21 @@ SENTRY_TEST(basic_function_transport_transaction) sentry_value_t transaction = sentry_value_new_transaction("How could you", "Don't capture this."); transaction = sentry_transaction_start(transaction); - sentry_transaction_finish(transaction); + sentry_uuid_t event_id = sentry_transaction_finish(transaction); + TEST_CHECK(sentry_uuid_is_nil(&event_id)); sentry_user_consent_give(); transaction = sentry_value_new_transaction("honk", "beep"); transaction = sentry_transaction_start(transaction); - sentry_transaction_finish(transaction); + sentry_uuid_t event_id = sentry_transaction_finish(transaction); + TEST_CHECK(!sentry_uuid_is_nil(&event_id)); sentry_user_consent_revoke(); transaction = sentry_value_new_transaction( "How could you again", "Don't capture this either."); transaction = sentry_transaction_start(transaction); - sentry_transaction_finish(transaction); + sentry_uuid_t event_id = sentry_transaction_finish(transaction); + TEST_CHECK(sentry_uuid_is_nil(&event_id)); sentry_close(); @@ -166,7 +169,10 @@ SENTRY_TEST(transport_sampling_transactions) sentry_value_t transaction = sentry_value_new_transaction("honk", "beep"); transaction = sentry_transaction_start(transaction); - sentry_transaction_finish(transaction); + sentry_uuid_t event_id = sentry_transaction_finish(transaction); + TEST_CHECK(!sentry_uuid_is_nil(&event_id)); + + TEST_CHECK(!sentry_uuid_is_nil(&event_id)); } sentry_close(); @@ -204,7 +210,8 @@ SENTRY_TEST(transactions_skip_before_send) sentry_value_t transaction = sentry_value_new_transaction("honk", "beep"); transaction = sentry_transaction_start(transaction); - sentry_transaction_finish(transaction); + sentry_uuid_t event_id = sentry_transaction_finish(transaction); + TEST_CHECK(!sentry_uuid_is_nil(&event_id)); sentry_close(); From eec15025ed060c4fec152f826926d60ed11047ca Mon Sep 17 00:00:00 2001 From: Betty Da Date: Tue, 21 Dec 2021 02:34:26 -0500 Subject: [PATCH 15/21] handle one subtle bug in capture event, note another possible bug --- src/sentry_core.c | 5 +++-- tests/unit/test_tracing.c | 20 +++++++++++++------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/sentry_core.c b/src/sentry_core.c index 213e4fa85..494413fd8 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -378,6 +378,7 @@ sentry__capture_event(sentry_value_t event) sentry_envelope_t *envelope = NULL; bool was_captured = false; + bool was_sent = false; SENTRY_WITH_OPTIONS (options) { was_captured = true; if (sentry__event_is_transaction(event)) { @@ -397,14 +398,14 @@ sentry__capture_event(sentry_value_t event) mut_options->session->init = false; } } - sentry__capture_envelope(options->transport, envelope); + was_sent = true; } } if (!was_captured) { sentry_value_decref(event); } - return was_captured ? event_id : sentry_uuid_nil(); + return was_sent ? event_id : sentry_uuid_nil(); } bool diff --git a/tests/unit/test_tracing.c b/tests/unit/test_tracing.c index cd0a763cb..6e0fee171 100644 --- a/tests/unit/test_tracing.c +++ b/tests/unit/test_tracing.c @@ -130,20 +130,24 @@ SENTRY_TEST(basic_function_transport_transaction) = sentry_value_new_transaction("How could you", "Don't capture this."); transaction = sentry_transaction_start(transaction); sentry_uuid_t event_id = sentry_transaction_finish(transaction); - TEST_CHECK(sentry_uuid_is_nil(&event_id)); + // TODO: `sentry_capture_event` acts as if the event was sent if user + // consent was not given + TEST_CHECK(!sentry_uuid_is_nil(&event_id)); sentry_user_consent_give(); transaction = sentry_value_new_transaction("honk", "beep"); transaction = sentry_transaction_start(transaction); - sentry_uuid_t event_id = sentry_transaction_finish(transaction); + event_id = sentry_transaction_finish(transaction); TEST_CHECK(!sentry_uuid_is_nil(&event_id)); sentry_user_consent_revoke(); transaction = sentry_value_new_transaction( "How could you again", "Don't capture this either."); transaction = sentry_transaction_start(transaction); - sentry_uuid_t event_id = sentry_transaction_finish(transaction); - TEST_CHECK(sentry_uuid_is_nil(&event_id)); + event_id = sentry_transaction_finish(transaction); + // TODO: `sentry_capture_event` acts as if the event was sent if user + // consent was not given + TEST_CHECK(!sentry_uuid_is_nil(&event_id)); sentry_close(); @@ -165,20 +169,22 @@ SENTRY_TEST(transport_sampling_transactions) sentry_options_set_traces_sample_rate(options, 0.75); sentry_init(options); + uint64_t sent_transactions = 0; for (int i = 0; i < 100; i++) { sentry_value_t transaction = sentry_value_new_transaction("honk", "beep"); transaction = sentry_transaction_start(transaction); sentry_uuid_t event_id = sentry_transaction_finish(transaction); - TEST_CHECK(!sentry_uuid_is_nil(&event_id)); - - TEST_CHECK(!sentry_uuid_is_nil(&event_id)); + if (!sentry_uuid_is_nil(&event_id)) { + sent_transactions += 1; + } } sentry_close(); // well, its random after all TEST_CHECK(called_transport > 50 && called_transport < 100); + TEST_CHECK(called_transport == sent_transactions); } static sentry_value_t From 41f55a2bf36011b9d3d11ead00d1e5575395f732 Mon Sep 17 00:00:00 2001 From: Betty Da Date: Tue, 21 Dec 2021 01:09:32 -0500 Subject: [PATCH 16/21] this is already taken care of earlier in the pipeline --- src/sentry_core.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/sentry_core.c b/src/sentry_core.c index 494413fd8..ffe228240 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -505,16 +505,6 @@ sentry__prepare_transaction(const sentry_options_t *options, { sentry_envelope_t *envelope = NULL; - bool should_skip = !sentry_value_is_true( - sentry_value_get_by_key(transaction, "sampled")); - if (should_skip) { - SENTRY_DEBUG("throwing away transaction due to sample rate or " - "user-provided sampling value in transaction context"); - goto fail; - } - // Field is superfluous, strip so it doesn't leak into the payload - sentry_value_remove_by_key(transaction, "sampled"); - SENTRY_WITH_SCOPE (scope) { SENTRY_TRACE("merging scope into event"); // Don't include debugging info @@ -761,6 +751,8 @@ sentry_transaction_finish(sentry_value_t tx) // `sentry__should_skip_transaction`. sentry_value_t sampled = sentry_value_get_by_key(tx, "sampled"); if (!sentry_value_is_null(sampled) && !sentry_value_is_true(sampled)) { + SENTRY_DEBUG("throwing away transaction due to sample rate or " + "user-provided sampling value in transaction context"); sentry_value_decref(sampled); sentry_value_decref(tx); // TODO(tracing): remove from scope From 5c9d7034a33396f9e9d6dbe38d038f3b11211ee9 Mon Sep 17 00:00:00 2001 From: Betty Da Date: Tue, 21 Dec 2021 01:18:08 -0500 Subject: [PATCH 17/21] document ownership stealing --- src/sentry_core.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/sentry_core.h b/src/sentry_core.h index e6f5a1453..4e3ec42d0 100644 --- a/src/sentry_core.h +++ b/src/sentry_core.h @@ -68,10 +68,12 @@ sentry_uuid_t sentry__capture_event(sentry_value_t event); * - add any attachments to the envelope * * The function will ensure the transaction has a UUID and write it into the - * `event_id` out-parameter. + * `event_id` out-parameter. This takes ownership of the transaction, which + * means that the caller no longer needs to call `sentry_value_decref` on the + * transaction. */ sentry_envelope_t *sentry__prepare_transaction(const sentry_options_t *options, - sentry_value_t event, sentry_uuid_t *event_id); + sentry_value_t transaction, sentry_uuid_t *event_id); /** * This function will submit the `envelope` to the given `transport`, first From 18d60d09672583c5b7387d4df23f25b24588feed Mon Sep 17 00:00:00 2001 From: Betty Da Date: Tue, 21 Dec 2021 00:26:30 -0500 Subject: [PATCH 18/21] let's not pretend transaction contexts are transactions to avoid confusion --- examples/example.c | 4 ++-- include/sentry.h | 35 +++++++++++++++++++++-------------- src/sentry_value.c | 36 +++++++++++++++++++----------------- tests/unit/test_sampling.c | 8 ++++---- tests/unit/test_tracing.c | 25 +++++++++++++------------ 5 files changed, 59 insertions(+), 49 deletions(-) diff --git a/examples/example.c b/examples/example.c index 5535a10ec..8edca36fc 100644 --- a/examples/example.c +++ b/examples/example.c @@ -214,11 +214,11 @@ main(int argc, char **argv) if (has_arg(argc, argv, "capture-transaction")) { sentry_value_t tx_ctx - = sentry_value_new_transaction("I'm a little teapot", + = sentry_value_new_transaction_context("I'm a little teapot", "Short and stout here is my handle and here is my spout"); if (has_arg(argc, argv, "unsample-tx")) { - sentry_transaction_set_sampled(tx_ctx, 0); + sentry_transaction_context_set_sampled(tx_ctx, 0); } sentry_value_t tx = sentry_transaction_start(tx_ctx); diff --git a/include/sentry.h b/include/sentry.h index 1ed7d8b9f..d14ad05af 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -1238,7 +1238,7 @@ SENTRY_EXPERIMENTAL_API double sentry_options_get_traces_sample_rate( /* -- Performance Monitoring/Tracing APIs -- */ /** - * Constructs a new inert Transaction. The returned value needs to be passed + * Constructs a new Transaction Context. The returned value needs to be passed * into `sentry_transaction_start` in order to be recorded and sent to sentry. * * See @@ -1251,37 +1251,44 @@ SENTRY_EXPERIMENTAL_API double sentry_options_get_traces_sample_rate( * for an explanation of `operation`, in addition to other properties and * actions that can be performed on a Transaction. */ -SENTRY_EXPERIMENTAL_API sentry_value_t sentry_value_new_transaction( +SENTRY_EXPERIMENTAL_API sentry_value_t sentry_value_new_transaction_context( const char *name, const char *operation); /** - * Sets the `name` of a Transaction. + * Sets the `name` on a Transaction Context, which will be used in the + * Transaction constructed off of the context. */ -SENTRY_EXPERIMENTAL_API void sentry_transaction_set_name( +SENTRY_EXPERIMENTAL_API void sentry_transaction_context_set_name( sentry_value_t transaction, const char *name); /** - * Sets the `operation` of a Transaction. + * Sets the `operation` on a Transaction Context, which will be used in the + * Transaction constructed off of the context * * See https://develop.sentry.dev/sdk/performance/span-operations/ for * conventions on `operation`s. */ -SENTRY_EXPERIMENTAL_API void sentry_transaction_set_operation( +SENTRY_EXPERIMENTAL_API void sentry_transaction_context_set_operation( sentry_value_t transaction, const char *operation); /** - * Sets the `sampled` field on a Transaction. When passed any value above 0, the - * Transaction will bypass all sampling options and always be sent to sentry. If - * passed 0, this Transaction and its child spans will never be sent to sentry. + * Sets the `sampled` field on a Transaction Context, which will be used in the + * Transaction constructed off of the context. + * + * When passed any value above 0, the Transaction will bypass all sampling + * options and always be sent to sentry. If passed 0, this Transaction and its + * child spans will never be sent to sentry. */ -SENTRY_EXPERIMENTAL_API void sentry_transaction_set_sampled( +SENTRY_EXPERIMENTAL_API void sentry_transaction_context_set_sampled( sentry_value_t transaction, int sampled); /** - * Removes the sampled field on a Transaction. The Transaction will use the - * sampling rate as defined in `sentry_options`. + * Removes the sampled field on a Transaction Context, which will be used in the + * Transaction constructed off of the context. + * + * The Transaction will use the sampling rate as defined in `sentry_options`. */ -SENTRY_EXPERIMENTAL_API void sentry_transaction_remove_sampled( +SENTRY_EXPERIMENTAL_API void sentry_transaction_context_remove_sampled( sentry_value_t transaction); /** @@ -1290,7 +1297,7 @@ SENTRY_EXPERIMENTAL_API void sentry_transaction_remove_sampled( * constructed by a user. */ SENTRY_EXPERIMENTAL_API sentry_value_t sentry_transaction_start( - sentry_value_t transaction); + sentry_value_t transaction_context); /** * Finishes and sends a transaction to sentry. The event ID of the transaction diff --git a/src/sentry_value.c b/src/sentry_value.c index d3cadf246..9217267d8 100644 --- a/src/sentry_value.c +++ b/src/sentry_value.c @@ -1125,26 +1125,27 @@ sentry_value_new_stacktrace(void **ips, size_t len) } sentry_value_t -sentry_value_new_transaction(const char *name, const char *operation) +sentry_value_new_transaction_context(const char *name, const char *operation) { - sentry_value_t transaction = sentry_value_new_object(); + sentry_value_t transaction_context = sentry_value_new_object(); sentry_uuid_t trace_id = sentry_uuid_new_v4(); - sentry_value_set_by_key( - transaction, "trace_id", sentry__value_new_internal_uuid(&trace_id)); + sentry_value_set_by_key(transaction_context, "trace_id", + sentry__value_new_internal_uuid(&trace_id)); sentry_uuid_t span_id = sentry_uuid_new_v4(); sentry_value_set_by_key( - transaction, "span_id", sentry__value_new_span_uuid(&span_id)); + transaction_context, "span_id", sentry__value_new_span_uuid(&span_id)); - sentry_transaction_set_name(transaction, name); - sentry_transaction_set_operation(transaction, operation); + sentry_transaction_context_set_name(transaction_context, name); + sentry_transaction_context_set_operation(transaction_context, operation); - return transaction; + return transaction_context; } void -sentry_transaction_set_name(sentry_value_t transaction, const char *name) +sentry_transaction_context_set_name( + sentry_value_t transaction_context, const char *name) { sentry_value_t sv_name = sentry_value_new_string(name); // TODO: Consider doing this checking right before sending or flushing @@ -1153,28 +1154,29 @@ sentry_transaction_set_name(sentry_value_t transaction, const char *name) sentry_value_decref(sv_name); sv_name = sentry_value_new_string(""); } - sentry_value_set_by_key(transaction, "name", sv_name); + sentry_value_set_by_key(transaction_context, "name", sv_name); } void -sentry_transaction_set_operation( - sentry_value_t transaction, const char *operation) +sentry_transaction_context_set_operation( + sentry_value_t transaction_context, const char *operation) { sentry_value_set_by_key( - transaction, "op", sentry_value_new_string(operation)); + transaction_context, "op", sentry_value_new_string(operation)); } void -sentry_transaction_set_sampled(sentry_value_t transaction, int sampled) +sentry_transaction_context_set_sampled( + sentry_value_t transaction_context, int sampled) { sentry_value_set_by_key( - transaction, "sampled", sentry_value_new_bool(sampled)); + transaction_context, "sampled", sentry_value_new_bool(sampled)); } void -sentry_transaction_remove_sampled(sentry_value_t transaction) +sentry_transaction_context_remove_sampled(sentry_value_t transaction_context) { - sentry_value_remove_by_key(transaction, "sampled"); + sentry_value_remove_by_key(transaction_context, "sampled"); } static sentry_value_t diff --git a/tests/unit/test_sampling.c b/tests/unit/test_sampling.c index f0b51d659..bcedcbbbb 100644 --- a/tests/unit/test_sampling.c +++ b/tests/unit/test_sampling.c @@ -13,16 +13,16 @@ SENTRY_TEST(sampling_transaction) sentry_options_t *options = sentry_options_new(); TEST_CHECK(sentry_init(options) == 0); - sentry_value_t tx_cxt = sentry_value_new_transaction("honk", NULL); + sentry_value_t tx_cxt = sentry_value_new_transaction_context("honk", NULL); - sentry_transaction_set_sampled(tx_cxt, 0); + sentry_transaction_context_set_sampled(tx_cxt, 0); TEST_CHECK(sentry__should_send_transaction(tx_cxt) == false); - sentry_transaction_set_sampled(tx_cxt, 1); + sentry_transaction_context_set_sampled(tx_cxt, 1); TEST_CHECK(sentry__should_send_transaction(tx_cxt)); // fall back to default in sentry options (0.0) if sampled isn't there - sentry_transaction_remove_sampled(tx_cxt); + sentry_transaction_context_remove_sampled(tx_cxt); TEST_CHECK(sentry__should_send_transaction(tx_cxt) == false); options = sentry_options_new(); diff --git a/tests/unit/test_tracing.c b/tests/unit/test_tracing.c index 6e0fee171..782b43d6c 100644 --- a/tests/unit/test_tracing.c +++ b/tests/unit/test_tracing.c @@ -36,7 +36,7 @@ SENTRY_TEST(basic_tracing_context) SENTRY_TEST(basic_transaction) { - sentry_value_t tx_cxt = sentry_value_new_transaction(NULL, NULL); + sentry_value_t tx_cxt = sentry_value_new_transaction_context(NULL, NULL); TEST_CHECK(!sentry_value_is_null(tx_cxt)); const char *tx_name = sentry_value_as_string(sentry_value_get_by_key(tx_cxt, "name")); @@ -50,7 +50,7 @@ SENTRY_TEST(basic_transaction) !sentry_value_is_null(sentry_value_get_by_key(tx_cxt, "span_id"))); sentry_value_decref(tx_cxt); - tx_cxt = sentry_value_new_transaction("", ""); + tx_cxt = sentry_value_new_transaction_context("", ""); TEST_CHECK(!sentry_value_is_null(tx_cxt)); tx_name = sentry_value_as_string(sentry_value_get_by_key(tx_cxt, "name")); TEST_CHECK_STRING_EQUAL(tx_name, ""); @@ -61,7 +61,7 @@ SENTRY_TEST(basic_transaction) !sentry_value_is_null(sentry_value_get_by_key(tx_cxt, "span_id"))); sentry_value_decref(tx_cxt); - tx_cxt = sentry_value_new_transaction("honk.beep", "beepbeep"); + tx_cxt = sentry_value_new_transaction_context("honk.beep", "beepbeep"); tx_name = sentry_value_as_string(sentry_value_get_by_key(tx_cxt, "name")); TEST_CHECK_STRING_EQUAL(tx_name, "honk.beep"); tx_op = sentry_value_as_string(sentry_value_get_by_key(tx_cxt, "op")); @@ -71,15 +71,15 @@ SENTRY_TEST(basic_transaction) TEST_CHECK( !sentry_value_is_null(sentry_value_get_by_key(tx_cxt, "span_id"))); - sentry_transaction_set_name(tx_cxt, ""); + sentry_transaction_context_set_name(tx_cxt, ""); tx_name = sentry_value_as_string(sentry_value_get_by_key(tx_cxt, "name")); TEST_CHECK_STRING_EQUAL(tx_name, ""); - sentry_transaction_set_operation(tx_cxt, ""); + sentry_transaction_context_set_operation(tx_cxt, ""); tx_op = sentry_value_as_string(sentry_value_get_by_key(tx_cxt, "op")); TEST_CHECK_STRING_EQUAL(tx_op, ""); - sentry_transaction_set_sampled(tx_cxt, 1); + sentry_transaction_context_set_sampled(tx_cxt, 1); TEST_CHECK( sentry_value_is_true(sentry_value_get_by_key(tx_cxt, "sampled")) == 1); @@ -126,8 +126,8 @@ SENTRY_TEST(basic_function_transport_transaction) sentry_options_set_require_user_consent(options, true); sentry_init(options); - sentry_value_t transaction - = sentry_value_new_transaction("How could you", "Don't capture this."); + sentry_value_t transaction = sentry_value_new_transaction_context( + "How could you", "Don't capture this."); transaction = sentry_transaction_start(transaction); sentry_uuid_t event_id = sentry_transaction_finish(transaction); // TODO: `sentry_capture_event` acts as if the event was sent if user @@ -135,13 +135,13 @@ SENTRY_TEST(basic_function_transport_transaction) TEST_CHECK(!sentry_uuid_is_nil(&event_id)); sentry_user_consent_give(); - transaction = sentry_value_new_transaction("honk", "beep"); + transaction = sentry_value_new_transaction_context("honk", "beep"); transaction = sentry_transaction_start(transaction); event_id = sentry_transaction_finish(transaction); TEST_CHECK(!sentry_uuid_is_nil(&event_id)); sentry_user_consent_revoke(); - transaction = sentry_value_new_transaction( + transaction = sentry_value_new_transaction_context( "How could you again", "Don't capture this either."); transaction = sentry_transaction_start(transaction); event_id = sentry_transaction_finish(transaction); @@ -172,7 +172,7 @@ SENTRY_TEST(transport_sampling_transactions) uint64_t sent_transactions = 0; for (int i = 0; i < 100; i++) { sentry_value_t transaction - = sentry_value_new_transaction("honk", "beep"); + = sentry_value_new_transaction_context("honk", "beep"); transaction = sentry_transaction_start(transaction); sentry_uuid_t event_id = sentry_transaction_finish(transaction); if (!sentry_uuid_is_nil(&event_id)) { @@ -214,7 +214,8 @@ SENTRY_TEST(transactions_skip_before_send) sentry_options_set_before_send(options, before_send, &called_beforesend); sentry_init(options); - sentry_value_t transaction = sentry_value_new_transaction("honk", "beep"); + sentry_value_t transaction + = sentry_value_new_transaction_context("honk", "beep"); transaction = sentry_transaction_start(transaction); sentry_uuid_t event_id = sentry_transaction_finish(transaction); TEST_CHECK(!sentry_uuid_is_nil(&event_id)); From 68a05304d36dc8211a423314edcbb9b05914ebdb Mon Sep 17 00:00:00 2001 From: Betty Da Date: Wed, 22 Dec 2021 18:48:55 -0500 Subject: [PATCH 19/21] manage refs correctly --- src/sentry_core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sentry_core.c b/src/sentry_core.c index ffe228240..13acd7af7 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -727,6 +727,8 @@ sentry_transaction_start(sentry_value_t tx_cxt) = 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")); @@ -753,12 +755,10 @@ sentry_transaction_finish(sentry_value_t tx) if (!sentry_value_is_null(sampled) && !sentry_value_is_true(sampled)) { SENTRY_DEBUG("throwing away transaction due to sample rate or " "user-provided sampling value in transaction context"); - sentry_value_decref(sampled); sentry_value_decref(tx); // TODO(tracing): remove from scope return sentry_uuid_nil(); } - sentry_value_decref(sampled); sentry_value_set_by_key(tx, "type", sentry_value_new_string("transaction")); sentry_value_set_by_key(tx, "timestamp", From 356e0772f5730b782a8cabb5dd549883cdf5ba0b Mon Sep 17 00:00:00 2001 From: Betty Da Date: Wed, 22 Dec 2021 18:49:04 -0500 Subject: [PATCH 20/21] note when ownership is being stolen --- include/sentry.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/include/sentry.h b/include/sentry.h index d14ad05af..4ee29a958 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -1295,6 +1295,8 @@ SENTRY_EXPERIMENTAL_API void sentry_transaction_context_remove_sampled( * Starts a new Transaction based on the provided context, restored from an * external integration (i.e. a span from a different SDK) or manually * constructed by a user. + * + * Takes ownership of `transaction_context`. */ SENTRY_EXPERIMENTAL_API sentry_value_t sentry_transaction_start( sentry_value_t transaction_context); @@ -1303,6 +1305,9 @@ SENTRY_EXPERIMENTAL_API sentry_value_t sentry_transaction_start( * Finishes and sends a transaction to sentry. The event ID of the transaction * will be returned if this was successful; A nil UUID will be returned * otherwise. + * + * Always takes ownership of `transaction`, regardless of whether the operation + * was successful or not. */ SENTRY_EXPERIMENTAL_API sentry_uuid_t sentry_transaction_finish( sentry_value_t transaction); From 3e716bcb5aeb3991d8599b6ab089de2cfecbb186 Mon Sep 17 00:00:00 2001 From: Betty Da Date: Wed, 22 Dec 2021 18:51:56 -0500 Subject: [PATCH 21/21] missed a spot --- src/sentry_core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/sentry_core.c b/src/sentry_core.c index 8a57e0305..1d9380ce9 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -782,6 +782,7 @@ sentry_transaction_finish(sentry_value_t tx) sentry_value_remove_by_key(tx, "description"); sentry_value_remove_by_key(tx, "status"); - // This decrefs for us, generates an event ID, merges scope + // This takes ownership of the transaction, generates an event ID, merges + // scope return sentry__capture_event(tx); }