From d2239452a7dec43df3993a507a18a6886f5501dc Mon Sep 17 00:00:00 2001 From: Betty Da Date: Thu, 16 Dec 2021 01:51:06 -0500 Subject: [PATCH 01/23] 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 3675e5527e..d0bcc485fc 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 71cef6ed69..7ea05ad985 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 ac5c69136f..b5a8f1ab09 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 a579afa3e5..2554be1c73 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 781361644a..716be19d4b 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/23] 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 0ace0324e6..215fbdbde8 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 d0bcc485fc..a7aca164c5 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 c15493dab7..fd4df8793c 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 6eb2e84188..dc225e0d94 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 bec05ef5a0..e6f5a14533 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 5052f023fc..3edf49888a 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 8ab8d6ab4b..f0b51d6590 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 cce9bbc7fd..a21baa7b0e 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 716be19d4b..0c54353ed6 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/23] 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 215fbdbde8..c15127674b 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 a7aca164c5..275db88dd1 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 dc225e0d94..cd038455db 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 a21baa7b0e..aa8f4fdcb6 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/23] 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 275db88dd1..71c870d460 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/23] whoops --- src/sentry_core.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/sentry_core.c b/src/sentry_core.c index cd038455db..31db365024 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/23] 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 c15127674b..96b23f9076 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/23] 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 96b23f9076..5535a10ec9 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/23] 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 fd4df8793c..2318e27746 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 31db365024..3a0f24166e 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/23] 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 3a0f24166e..0f2bbc139c 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/23] 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 0f2bbc139c..4e5c6b6a64 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/23] 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 4e5c6b6a64..af500990be 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/23] 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 af500990be..270d032305 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 c71c68ecf9..d3cadf246e 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 aa8f4fdcb6..56cc49984b 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/23] docs --- include/sentry.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/sentry.h b/include/sentry.h index 71c870d460..3cddb4e718 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/23] 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 3cddb4e718..1ed7d8b9fe 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 270d032305..213e4fa858 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 56cc49984b..cd0a763cb0 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/23] 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 213e4fa858..494413fd8c 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 cd0a763cb0..6e0fee1713 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/23] 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 494413fd8c..ffe2282401 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/23] 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 e6f5a14533..4e3ec42d0e 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/23] 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 5535a10ec9..8edca36fc2 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 1ed7d8b9fe..d14ad05aff 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 d3cadf246e..9217267d8b 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 f0b51d6590..bcedcbbbb4 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 6e0fee1713..782b43d6c2 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 ceddb01439e7665448f51eb22060d76d41a18f57 Mon Sep 17 00:00:00 2001 From: Betty Da Date: Fri, 17 Dec 2021 01:36:13 -0500 Subject: [PATCH 19/23] feat(tracing): Only ever allow one active transaction, stuff transaction into scope --- examples/example.c | 4 +- include/sentry.h | 11 +++--- src/sentry_core.c | 46 ++++++++++++++--------- src/sentry_core.h | 2 +- src/sentry_scope.c | 27 ++++++++++++-- src/sentry_scope.h | 20 +++++++++- tests/unit/test_tracing.c | 77 +++++++++++++++++++++++++++++++++------ tests/unit/tests.inc | 1 + 8 files changed, 147 insertions(+), 41 deletions(-) diff --git a/examples/example.c b/examples/example.c index 8edca36fc2..06fb7fe5ac 100644 --- a/examples/example.c +++ b/examples/example.c @@ -221,8 +221,8 @@ main(int argc, char **argv) sentry_transaction_context_set_sampled(tx_ctx, 0); } - sentry_value_t tx = sentry_transaction_start(tx_ctx); - sentry_transaction_finish(tx); + sentry_transaction_start(tx_ctx); + sentry_transaction_finish(); } // make sure everything flushes diff --git a/include/sentry.h b/include/sentry.h index d14ad05aff..d5e3287843 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -1296,16 +1296,15 @@ SENTRY_EXPERIMENTAL_API void sentry_transaction_context_remove_sampled( * 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_EXPERIMENTAL_API void sentry_transaction_start( sentry_value_t transaction_context); /** - * 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. + * Finishes and sends the current 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 sentry_uuid_t sentry_transaction_finish( - sentry_value_t transaction); +SENTRY_EXPERIMENTAL_API sentry_uuid_t sentry_transaction_finish(); #ifdef __cplusplus } diff --git a/src/sentry_core.c b/src/sentry_core.c index ffe2282401..e026d766c3 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -711,12 +711,11 @@ sentry_set_level(sentry_level_t level) } } -sentry_value_t +void sentry_transaction_start(sentry_value_t tx_cxt) { sentry_value_t tx = sentry_value_new_event(); - // 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)); @@ -738,27 +737,40 @@ sentry_transaction_start(sentry_value_t tx_cxt) sentry__value_new_string_owned( sentry__msec_time_to_iso8601(sentry__msec_time()))); + sentry__scope_set_span(tx); sentry_value_decref(tx_cxt); - - return tx; } sentry_uuid_t -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_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 +sentry_transaction_finish() +{ + sentry_value_t tx = sentry_value_new_null(); + SENTRY_WITH_SCOPE (scope) { + if (sentry_value_is_null(scope->span)) { + SENTRY_DEBUG("could not find a transaction on the scope to finish"); + return sentry_uuid_nil(); + } + + // 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(scope->span, "sampled"); + if (!sentry_value_is_true(sampled)) { + SENTRY_DEBUG("throwing away transaction due to sample rate or " + "user-provided sampling value in transaction context"); + sentry__scope_remove_span(); + return sentry_uuid_nil(); + } + tx = sentry__value_clone(scope->span); + } + sentry__scope_remove_span(); + if (sentry_value_is_null(tx)) { + SENTRY_DEBUG("could not find a transaction on the scope to finish"); return sentry_uuid_nil(); } - sentry_value_decref(sampled); + + sentry_value_remove_by_key(tx, "sampled"); sentry_value_set_by_key(tx, "type", sentry_value_new_string("transaction")); sentry_value_set_by_key(tx, "timestamp", diff --git a/src/sentry_core.h b/src/sentry_core.h index 4e3ec42d0e..e540f5d2a0 100644 --- a/src/sentry_core.h +++ b/src/sentry_core.h @@ -118,7 +118,7 @@ void sentry__options_unlock(void); for (sentry_options_t *Options = sentry__options_lock(); Options; \ sentry__options_unlock(), Options = NULL) -// these for now are only needed for tests +// these for now are only needed outside of core for tests #ifdef SENTRY_UNITTEST bool sentry__roll_dice(double probability); bool sentry__should_send_transaction(sentry_value_t tx_cxt); diff --git a/src/sentry_scope.c b/src/sentry_scope.c index 3edf49888a..99012fa9bd 100644 --- a/src/sentry_scope.c +++ b/src/sentry_scope.c @@ -230,9 +230,30 @@ sentry__symbolize_stacktrace(sentry_value_t stacktrace) void sentry__scope_set_span(sentry_value_t span) { - // TODO: implement this function and get rid of this line. - (void)span; - return; + SENTRY_WITH_SCOPE_MUT (scope) { + sentry_value_decref(scope->span); + scope->span = span; + } +} + +#ifdef SENTRY_UNITTEST +sentry_value_t +sentry__scope_get_span() +{ + SENTRY_WITH_SCOPE (scope) { + return scope->span; + } + return sentry_value_new_null(); +} +#endif + +void +sentry__scope_remove_span() +{ + SENTRY_WITH_SCOPE_MUT (scope) { + sentry_value_decref(scope->span); + scope->span = sentry_value_new_null(); + } } void diff --git a/src/sentry_scope.h b/src/sentry_scope.h index 393cf349d4..a4d0ce9b47 100644 --- a/src/sentry_scope.h +++ b/src/sentry_scope.h @@ -77,10 +77,23 @@ void sentry__scope_apply_to_event(const sentry_scope_t *scope, /** * Sets the span (actually transaction) on the scope. An internal way to pass - * around contextual information needed from a transaction into other events. + * around contextual information needed from a transaction into other events. If + * the scope already contains an unfinished transaction, that transaction will + * be discarded and will not be sent to sentry. + * + * This takes ownership of the span. */ void sentry__scope_set_span(sentry_value_t span); +/** + * Removes the current span (actually transaction) on the scope. If the + * transaction has not yet finished, this does not finish the transaction + * nor does it send it to sentry; The transaction will be discarded. + * + * Remove at your own discretion. + */ +void sentry__scope_remove_span(); + /** * These are convenience macros to automatically lock/unlock a scope inside a * code block. @@ -96,3 +109,8 @@ void sentry__scope_set_span(sentry_value_t span); sentry__scope_unlock(), Scope = NULL) #endif + +// this is only used in unit tests +#ifdef SENTRY_UNITTEST +sentry_value_t sentry__scope_get_span(); +#endif diff --git a/tests/unit/test_tracing.c b/tests/unit/test_tracing.c index 782b43d6c2..59057cf843 100644 --- a/tests/unit/test_tracing.c +++ b/tests/unit/test_tracing.c @@ -1,7 +1,12 @@ +#include "sentry_scope.h" #include "sentry_testsupport.h" #include "sentry_tracing.h" #include "sentry_uuid.h" +#define CHECK_STRING_PROPERTY(Src, Field, Expected) \ + TEST_CHECK_STRING_EQUAL( \ + sentry_value_as_string(sentry_value_get_by_key(Src, Field)), Expected) + SENTRY_TEST(basic_tracing_context) { sentry_value_t span = sentry_value_new_object(); @@ -128,23 +133,23 @@ SENTRY_TEST(basic_function_transport_transaction) 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); + sentry_transaction_start(transaction); + sentry_uuid_t event_id = sentry_transaction_finish(); // 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_context("honk", "beep"); - transaction = sentry_transaction_start(transaction); - event_id = sentry_transaction_finish(transaction); + sentry_transaction_start(transaction); + event_id = sentry_transaction_finish(); TEST_CHECK(!sentry_uuid_is_nil(&event_id)); sentry_user_consent_revoke(); 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); + sentry_transaction_start(transaction); + event_id = sentry_transaction_finish(); // 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)); @@ -173,8 +178,8 @@ SENTRY_TEST(transport_sampling_transactions) for (int i = 0; i < 100; i++) { 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); + sentry_transaction_start(transaction); + sentry_uuid_t event_id = sentry_transaction_finish(); if (!sentry_uuid_is_nil(&event_id)) { sent_transactions += 1; } @@ -182,7 +187,7 @@ SENTRY_TEST(transport_sampling_transactions) sentry_close(); - // well, its random after all + // exact value is nondeterministic because of rng TEST_CHECK(called_transport > 50 && called_transport < 100); TEST_CHECK(called_transport == sent_transactions); } @@ -216,8 +221,8 @@ SENTRY_TEST(transactions_skip_before_send) 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); + sentry_transaction_start(transaction); + sentry_uuid_t event_id = sentry_transaction_finish(); TEST_CHECK(!sentry_uuid_is_nil(&event_id)); sentry_close(); @@ -225,3 +230,53 @@ SENTRY_TEST(transactions_skip_before_send) TEST_CHECK_INT_EQUAL(called_transport, 1); TEST_CHECK_INT_EQUAL(called_beforesend, 0); } + +static void +before_transport(sentry_envelope_t *envelope, void *data) +{ + uint64_t *called = data; + *called += 1; + + sentry_envelope_free(envelope); +} + +SENTRY_TEST(multiple_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(before_transport); + sentry_transport_set_state(transport, &called_transport); + sentry_options_set_transport(options, transport); + + sentry_options_set_traces_sample_rate(options, 1.0); + sentry_init(options); + + sentry_value_t tx_cxt = sentry_value_new_transaction_context("wow!", NULL); + sentry_transaction_start(tx_cxt); + + sentry_value_t scope_tx = sentry__scope_get_span(); + CHECK_STRING_PROPERTY(scope_tx, "transaction", "wow!"); + + sentry_uuid_t event_id = sentry_transaction_finish(); + scope_tx = sentry__scope_get_span(); + TEST_CHECK(sentry_value_is_null(scope_tx)); + TEST_CHECK(!sentry_uuid_is_nil(&event_id)); + + tx_cxt = sentry_value_new_transaction_context("whoa!", NULL); + sentry_transaction_start(tx_cxt); + tx_cxt = sentry_value_new_transaction_context("wowee!", NULL); + sentry_transaction_start(tx_cxt); + scope_tx = sentry__scope_get_span(); + CHECK_STRING_PROPERTY(scope_tx, "transaction", "wowee!"); + event_id = sentry_transaction_finish(); + TEST_CHECK(!sentry_uuid_is_nil(&event_id)); + + sentry_close(); + + TEST_CHECK_INT_EQUAL(called_transport, 2); +} + +#undef CHECK_STRING_PROPERTY diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index 0c54353ed6..2e79a18a80 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -29,6 +29,7 @@ XX(module_finder) XX(mpack_newlines) XX(mpack_removed_tags) XX(multiple_inits) +XX(multiple_transactions) XX(os) XX(page_allocator) XX(path_basics) From c87a8a02d97f48f13d08e4e5c72415d50155eabf Mon Sep 17 00:00:00 2001 From: Betty Da Date: Tue, 21 Dec 2021 03:12:55 -0500 Subject: [PATCH 20/23] no ambiguity allowed here --- src/sentry_scope.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry_scope.h b/src/sentry_scope.h index a4d0ce9b47..029c771f3d 100644 --- a/src/sentry_scope.h +++ b/src/sentry_scope.h @@ -90,7 +90,7 @@ void sentry__scope_set_span(sentry_value_t span); * transaction has not yet finished, this does not finish the transaction * nor does it send it to sentry; The transaction will be discarded. * - * Remove at your own discretion. + * Invoke this at your own discretion. */ void sentry__scope_remove_span(); From cbf156a9474b146a55697fcbda989cfc3f7a6d76 Mon Sep 17 00:00:00 2001 From: Betty Da Date: Fri, 17 Dec 2021 02:14:00 -0500 Subject: [PATCH 21/23] feat(tracing): Defer some transaction validation and allow creation of internal spans --- src/sentry_core.c | 19 +++++++-- src/sentry_value.c | 34 +++++++++++---- src/sentry_value.h | 7 +++ tests/unit/test_tracing.c | 89 ++++++++++++++++++++++++++------------- tests/unit/tests.inc | 1 + 5 files changed, 108 insertions(+), 42 deletions(-) diff --git a/src/sentry_core.c b/src/sentry_core.c index e026d766c3..15c65bcfd7 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -714,7 +714,13 @@ sentry_set_level(sentry_level_t level) void sentry_transaction_start(sentry_value_t tx_cxt) { + // 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. sentry_value_t tx = sentry_value_new_event(); + sentry_value_remove_by_key(tx, "timestamp"); bool should_sample = sentry__should_send_transaction(tx_cxt); sentry_value_set_by_key( @@ -731,8 +737,10 @@ sentry_transaction_start(sentry_value_t tx_cxt) 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, "transaction")); sentry_value_set_by_key( - tx, "transaction", sentry_value_get_by_key_owned(tx_cxt, "name")); + 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()))); @@ -778,10 +786,13 @@ sentry_transaction_finish() sentry__msec_time_to_iso8601(sentry__msec_time()))); sentry_value_set_by_key(tx, "level", sentry_value_new_string("info")); - // 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")); + sentry_value_t name = sentry_value_get_by_key(tx, "transaction"); + if (sentry_value_is_null(name) || sentry_value_get_length(name) == 0) { + sentry_value_set_by_key(tx, "transaction", + sentry_value_new_string("")); + } + // TODO: add tracestate 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); diff --git a/src/sentry_value.c b/src/sentry_value.c index 9217267d8b..90f21961af 100644 --- a/src/sentry_value.c +++ b/src/sentry_value.c @@ -1124,10 +1124,32 @@ sentry_value_new_stacktrace(void **ips, size_t len) return stacktrace; } +sentry_value_t +sentry__value_new_span(sentry_value_t parent, const char *operation) +{ + sentry_value_t span = sentry_value_new_object(); + + sentry_transaction_context_set_operation(span, operation); + sentry_value_set_by_key(span, "status", sentry_value_new_string("ok")); + + if (!sentry_value_is_null(parent)) { + sentry_value_set_by_key(span, "trace_id", + sentry_value_get_by_key_owned(parent, "trace_id")); + sentry_value_set_by_key(span, "parent_span_id", + sentry_value_get_by_key_owned(parent, "span_id")); + sentry_value_set_by_key( + span, "sampled", sentry_value_get_by_key_owned(parent, "sampled")); + } + + return span; +} + sentry_value_t sentry_value_new_transaction_context(const char *name, const char *operation) { - sentry_value_t transaction_context = sentry_value_new_object(); + sentry_value_t transaction_context + = sentry__value_new_span(sentry_value_new_null(), operation); + sentry_transaction_context_set_name(transaction_context, name); sentry_uuid_t trace_id = sentry_uuid_new_v4(); sentry_value_set_by_key(transaction_context, "trace_id", @@ -1147,14 +1169,8 @@ void 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 - // the transaction. - if (sentry_value_is_null(sv_name) || sentry__string_eq(name, "")) { - sentry_value_decref(sv_name); - sv_name = sentry_value_new_string(""); - } - sentry_value_set_by_key(transaction_context, "name", sv_name); + sentry_value_set_by_key( + transaction_context, "transaction", sentry_value_new_string(name)); } void diff --git a/src/sentry_value.h b/src/sentry_value.h index 2eb5004f13..cba7853643 100644 --- a/src/sentry_value.h +++ b/src/sentry_value.h @@ -61,6 +61,13 @@ sentry_value_t sentry__value_new_list_with_size(size_t size); */ sentry_value_t sentry__value_new_object_with_size(size_t size); +/** + * Constructs a new Span. + * + */ +sentry_value_t sentry__value_new_span( + sentry_value_t parent, const char *operation); + /** * This will parse the Value into a UUID, or return a `nil` UUID on error. * See also `sentry_uuid_from_string`. diff --git a/tests/unit/test_tracing.c b/tests/unit/test_tracing.c index 59057cf843..09cdc7d1d4 100644 --- a/tests/unit/test_tracing.c +++ b/tests/unit/test_tracing.c @@ -3,6 +3,8 @@ #include "sentry_tracing.h" #include "sentry_uuid.h" +#define IS_NULL(Src, Field) \ + sentry_value_is_null(sentry_value_get_by_key(Src, Field)) #define CHECK_STRING_PROPERTY(Src, Field, Expected) \ TEST_CHECK_STRING_EQUAL( \ sentry_value_as_string(sentry_value_get_by_key(Src, Field)), Expected) @@ -43,46 +45,31 @@ SENTRY_TEST(basic_transaction) { 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")); - TEST_CHECK_STRING_EQUAL(tx_name, ""); - 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"))); + CHECK_STRING_PROPERTY(tx_cxt, "transaction", ""); + CHECK_STRING_PROPERTY(tx_cxt, "op", ""); + TEST_CHECK(!IS_NULL(tx_cxt, "trace_id")); + TEST_CHECK(!IS_NULL(tx_cxt, "span_id")); sentry_value_decref(tx_cxt); 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, ""); - 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"))); + CHECK_STRING_PROPERTY(tx_cxt, "transaction", ""); + CHECK_STRING_PROPERTY(tx_cxt, "op", ""); + TEST_CHECK(!IS_NULL(tx_cxt, "trace_id")); + TEST_CHECK(!IS_NULL(tx_cxt, "span_id")); sentry_value_decref(tx_cxt); 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")); - 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"))); + CHECK_STRING_PROPERTY(tx_cxt, "transaction", "honk.beep"); + CHECK_STRING_PROPERTY(tx_cxt, "op", "beepbeep"); + TEST_CHECK(!IS_NULL(tx_cxt, "trace_id")); + TEST_CHECK(!IS_NULL(tx_cxt, "span_id")); 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, ""); + CHECK_STRING_PROPERTY(tx_cxt, "transaction", ""); 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, ""); + CHECK_STRING_PROPERTY(tx_cxt, "op", ""); sentry_transaction_context_set_sampled(tx_cxt, 1); TEST_CHECK( @@ -91,6 +78,49 @@ SENTRY_TEST(basic_transaction) sentry_value_decref(tx_cxt); } +static void +check_backfilled_name(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)); + CHECK_STRING_PROPERTY( + transaction, "transaction", ""); + + sentry_envelope_free(envelope); +} + +SENTRY_TEST(transaction_name_backfill_on_finish) +{ + 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(check_backfilled_name); + sentry_transport_set_state(transport, &called); + sentry_options_set_transport(options, transport); + + sentry_options_set_traces_sample_rate(options, 1.0); + sentry_init(options); + + sentry_value_t transaction + = sentry_value_new_transaction_context(NULL, NULL); + sentry_transaction_start(transaction); + sentry_uuid_t event_id = sentry_transaction_finish(); + TEST_CHECK(!sentry_uuid_is_nil(&event_id)); + + transaction = sentry_value_new_transaction_context("", ""); + sentry_transaction_start(transaction); + event_id = sentry_transaction_finish(); + TEST_CHECK(!sentry_uuid_is_nil(&event_id)); + + sentry_close(); + TEST_CHECK_INT_EQUAL(called, 2); +} + static void send_transaction_envelope_test_basic(sentry_envelope_t *envelope, void *data) { @@ -279,4 +309,5 @@ SENTRY_TEST(multiple_transactions) TEST_CHECK_INT_EQUAL(called_transport, 2); } +#undef IS_NULL #undef CHECK_STRING_PROPERTY diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index 2e79a18a80..0126f03944 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -49,6 +49,7 @@ XX(session_basics) XX(slice) XX(symbolizer) XX(task_queue) +XX(transaction_name_backfill_on_finish) XX(transactions_skip_before_send) XX(transport_sampling_transactions) XX(uninitialized) From d07e1f069a4b353c474fd3879c6016d9d504d41d Mon Sep 17 00:00:00 2001 From: Betty Da Date: Fri, 17 Dec 2021 03:07:41 -0500 Subject: [PATCH 22/23] feat(tracing): Basic span support with nesting --- examples/example.c | 18 ++++- include/sentry.h | 53 +++++++++++++- src/sentry_core.c | 88 +++++++++++++++++++++++ src/sentry_core.h | 1 + src/sentry_tracing.c | 51 ++++++++++--- src/sentry_tracing.h | 1 + src/sentry_value.c | 5 +- tests/unit/test_tracing.c | 146 ++++++++++++++++++++++++++++++++++++++ tests/unit/tests.inc | 3 + 9 files changed, 349 insertions(+), 17 deletions(-) diff --git a/examples/example.c b/examples/example.c index 06fb7fe5ac..99a04d2fc8 100644 --- a/examples/example.c +++ b/examples/example.c @@ -97,6 +97,10 @@ main(int argc, char **argv) sentry_options_set_traces_sample_rate(options, 1.0); } + if (has_arg(argc, argv, "child-spans")) { + sentry_options_set_max_spans(options, 5); + } + sentry_init(options); if (!has_arg(argc, argv, "no-setup")) { @@ -214,14 +218,24 @@ main(int argc, char **argv) if (has_arg(argc, argv, "capture-transaction")) { sentry_value_t tx_ctx - = sentry_value_new_transaction_context("I'm a little teapot", + = sentry_value_new_transaction_context("little.teapot", "Short and stout here is my handle and here is my spout"); if (has_arg(argc, argv, "unsample-tx")) { sentry_transaction_context_set_sampled(tx_ctx, 0); } - sentry_transaction_start(tx_ctx); + + if (has_arg(argc, argv, "child-spans")) { + sentry_value_t child_ctx = sentry_span_start_child( + sentry_value_new_null(), "littler.teapot", NULL); + sentry_value_t grandchild_ctx + = sentry_span_start_child(child_ctx, "littlest.teapot", NULL); + + sentry_span_finish(grandchild_ctx); + sentry_span_finish(child_ctx); + } + sentry_transaction_finish(); } diff --git a/include/sentry.h b/include/sentry.h index d5e3287843..a1fd9558f2 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -1295,17 +1295,64 @@ 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. + * + * `sentry_transaction_finish` should be called after this is invoked, otherwise + * the Transaction will not be sent to sentry. New spans cannot be created + * unless there exists an active Transaction. */ SENTRY_EXPERIMENTAL_API void sentry_transaction_start( sentry_value_t transaction_context); /** - * Finishes and sends the current transaction to sentry. The event ID of the - * transaction will be returned if this was successful; A nil UUID will be - * returned otherwise. + * Finishes and sends the current active Transaction to sentry. Any unfinished + * spans are removed from the Transaction before it is sent over. + * + * No new spans can be created after this is invoked unless a new Transaction is + * started via `sentry_transaction_start`. */ SENTRY_EXPERIMENTAL_API sentry_uuid_t sentry_transaction_finish(); +/** + * Starts a new Span. + * + * If `parent_span` is `sentry_value_null`, then the current active Transaction + * is used as the parent for the new Span. An active Transaction must be created + * via `sentry_transaction_start` in order for the Span to be successfully + * created. + * + * If `parent_span` is another Span, it must belong to the current active + * Transaction in order for Span creation to succeed. This will take ownership + * of any `parent_span`s that do reference non-existent Spans in the current + * active Transaction. + * + * Both operation and description can be null, but it is recommended to supply + * the former. See https://develop.sentry.dev/sdk/performance/span-operations/ + * for conventions around operations. + * + * See https://develop.sentry.dev/sdk/event-payloads/span/ for a description of + * the created Span's properties and expectations for operation and description. + * + * Returns a value that should be passed into `sentry_span_finish`. Not + * finishing the Span means it will be discarded, and will not be sent to + * sentry. `sentry_value_null` will be returned, and `parent_span`'s ownership + * will be taken if the child Span could not be created. + */ +SENTRY_EXPERIMENTAL_API sentry_value_t sentry_span_start_child( + sentry_value_t parent_span, char *operation, char *description); + +/** + * Finishes a span. + * + * Returns a value that should be passed into `sentry_span_finish`. Not + * finishing the span means it will be discarded, and will not be sent to + * sentry. + * + * This takes ownership of `span`, as child spans must always occur within the + * total duration of a parent span and cannot take a longer amount of time to + * complete than the parent span they belong to. + */ +SENTRY_EXPERIMENTAL_API void sentry_span_finish(sentry_value_t span); + #ifdef __cplusplus } #endif diff --git a/src/sentry_core.c b/src/sentry_core.c index 15c65bcfd7..b78767f200 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -784,6 +784,8 @@ sentry_transaction_finish() sentry_value_set_by_key(tx, "timestamp", sentry__value_new_string_owned( sentry__msec_time_to_iso8601(sentry__msec_time()))); + // TODO: This might not actually be necessary. Revisit after talking to + // the relay team about this. sentry_value_set_by_key(tx, "level", sentry_value_new_string("info")); sentry_value_t name = sentry_value_get_by_key(tx, "transaction"); @@ -806,6 +808,92 @@ sentry_transaction_finish() sentry_value_remove_by_key(tx, "description"); sentry_value_remove_by_key(tx, "status"); + // TODO: prune unfinished child spans // This decrefs for us, generates an event ID, merges scope return sentry__capture_event(tx); } + +sentry_value_t +sentry_span_start_child( + sentry_value_t parent_span_context, char *operation, char *description) +{ + size_t max_spans = SENTRY_SPANS_MAX; + SENTRY_WITH_OPTIONS (options) { + max_spans = options->max_spans; + } + + sentry_value_t child_span_context = sentry_value_new_null(); + size_t span_count = 0; + SENTRY_WITH_SCOPE_MUT (scope) { + // There isn't an active transaction. This span has nothing to attach + // to. + if (sentry_value_is_null(scope->span)) { + return sentry_value_new_null(); + } + // Aggressively discard spans if a transaction is unsampled to avoid + // wasting memory + sentry_value_t sampled + = sentry_value_get_by_key(scope->span, "sampled"); + if (!sentry_value_is_true(sampled)) { + return sentry_value_new_null(); + } + sentry_value_t spans = sentry_value_get_by_key(scope->span, "spans"); + span_count = sentry_value_get_length(spans); + if (span_count >= max_spans) { + return sentry_value_new_null(); + } + // TODO: if the parent span can't be found in the current active + // transaction, take ownership of the parent span context and return + // null. + + sentry_value_t parent; + if (sentry_value_is_null(parent_span_context)) { + parent = scope->span; + } else { + parent = parent_span_context; + } + + sentry_value_t child = sentry__value_new_span(parent, operation); + sentry_uuid_t span_id = sentry_uuid_new_v4(); + sentry_value_set_by_key( + child, "span_id", sentry__value_new_span_uuid(&span_id)); + sentry_value_set_by_key( + child, "description", sentry_value_new_string(description)); + sentry_value_set_by_key(child, "start_timestamp", + sentry__value_new_string_owned( + sentry__msec_time_to_iso8601(sentry__msec_time()))); + + if (sentry_value_is_null(spans)) { + spans = sentry_value_new_list(); + sentry_value_set_by_key(scope->span, "spans", spans); + } + child_span_context = sentry__span_get_span_context(child); + sentry_value_append(spans, child); + } + sentry_value_set_by_key( + child_span_context, "index", sentry_value_new_int32((int)span_count)); + + return child_span_context; +} + +void +sentry_span_finish(sentry_value_t span_context) +{ + sentry_value_t sv_index = sentry_value_get_by_key(span_context, "index"); + if (sentry_value_is_null(sv_index)) { + sentry_value_decref(span_context); + return; + } + int index = sentry_value_as_int32(sv_index); + + SENTRY_WITH_SCOPE_MUT (scope) { + sentry_value_t spans = sentry_value_get_by_key(scope->span, "spans"); + // TODO: maybe validate that to_update.span_id == span.span_id + sentry_value_t to_update = sentry_value_get_by_index(spans, index); + sentry_value_set_by_key(to_update, "timestamp", + sentry__value_new_string_owned( + sentry__msec_time_to_iso8601(sentry__msec_time()))); + } + + sentry_value_decref(span_context); +} diff --git a/src/sentry_core.h b/src/sentry_core.h index e540f5d2a0..39c83492d5 100644 --- a/src/sentry_core.h +++ b/src/sentry_core.h @@ -5,6 +5,7 @@ #include "sentry_logger.h" #define SENTRY_BREADCRUMBS_MAX 100 +#define SENTRY_SPANS_MAX 1000 #if defined(__GNUC__) && (__GNUC__ >= 4) # define MUST_USE __attribute__((warn_unused_result)) diff --git a/src/sentry_tracing.c b/src/sentry_tracing.c index 73e23c0d88..42a3f3af74 100644 --- a/src/sentry_tracing.c +++ b/src/sentry_tracing.c @@ -1,4 +1,5 @@ #include "sentry_sync.h" +#include "sentry_value.h" sentry_value_t sentry__span_get_trace_context(sentry_value_t span) @@ -11,23 +12,53 @@ sentry__span_get_trace_context(sentry_value_t span) sentry_value_t trace_context = sentry_value_new_object(); -#define PLACE_VALUE(Key, Source) \ +#define PLACE_CLONED_VALUE(Key, Source) \ do { \ sentry_value_t src = sentry_value_get_by_key(Source, Key); \ if (!sentry_value_is_null(src)) { \ - sentry_value_incref(src); \ - sentry_value_set_by_key(trace_context, Key, src); \ + sentry_value_set_by_key( \ + trace_context, Key, sentry__value_clone(src)); \ } \ } while (0) - PLACE_VALUE("trace_id", span); - PLACE_VALUE("span_id", span); - PLACE_VALUE("parent_span_id", span); - PLACE_VALUE("op", span); - PLACE_VALUE("description", span); - PLACE_VALUE("status", span); + PLACE_CLONED_VALUE("trace_id", span); + PLACE_CLONED_VALUE("span_id", span); + PLACE_CLONED_VALUE("parent_span_id", span); + PLACE_CLONED_VALUE("op", span); + PLACE_CLONED_VALUE("description", span); + PLACE_CLONED_VALUE("status", span); + // TODO: freeze this return trace_context; -#undef PLACE_VALUE +#undef PLACE_CLONED_VALUE +} + +sentry_value_t +sentry__span_get_span_context(sentry_value_t span) +{ + if (sentry_value_is_null(span) + || sentry_value_is_null(sentry_value_get_by_key(span, "trace_id")) + || sentry_value_is_null(sentry_value_get_by_key(span, "span_id"))) { + return sentry_value_new_null(); + } + + sentry_value_t span_context = sentry_value_new_object(); + +#define PLACE_CLONED_VALUE(Key, Source) \ + do { \ + sentry_value_t src = sentry_value_get_by_key(Source, Key); \ + if (!sentry_value_is_null(src)) { \ + sentry_value_set_by_key( \ + span_context, Key, sentry__value_clone(src)); \ + } \ + } while (0) + + PLACE_CLONED_VALUE("trace_id", span); + PLACE_CLONED_VALUE("span_id", span); + PLACE_CLONED_VALUE("status", span); + + return span_context; + +#undef PLACE_CLONED_VALUE } diff --git a/src/sentry_tracing.h b/src/sentry_tracing.h index d99afe828f..aecb7ab084 100644 --- a/src/sentry_tracing.h +++ b/src/sentry_tracing.h @@ -11,4 +11,5 @@ */ sentry_value_t sentry__span_get_trace_context(sentry_value_t span); +sentry_value_t sentry__span_get_span_context(sentry_value_t span); #endif diff --git a/src/sentry_value.c b/src/sentry_value.c index 90f21961af..98416cc287 100644 --- a/src/sentry_value.c +++ b/src/sentry_value.c @@ -1132,13 +1132,14 @@ sentry__value_new_span(sentry_value_t parent, const char *operation) sentry_transaction_context_set_operation(span, operation); sentry_value_set_by_key(span, "status", sentry_value_new_string("ok")); + // Span creation is currently aggressively pruned prior to this function so + // once we're in here we definitely know that the span and its parent + // transaction are sampled. if (!sentry_value_is_null(parent)) { sentry_value_set_by_key(span, "trace_id", sentry_value_get_by_key_owned(parent, "trace_id")); sentry_value_set_by_key(span, "parent_span_id", sentry_value_get_by_key_owned(parent, "span_id")); - sentry_value_set_by_key( - span, "sampled", sentry_value_get_by_key_owned(parent, "sampled")); } return span; diff --git a/tests/unit/test_tracing.c b/tests/unit/test_tracing.c index 09cdc7d1d4..e505f11eb9 100644 --- a/tests/unit/test_tracing.c +++ b/tests/unit/test_tracing.c @@ -309,5 +309,151 @@ SENTRY_TEST(multiple_transactions) TEST_CHECK_INT_EQUAL(called_transport, 2); } +SENTRY_TEST(basic_spans) +{ + sentry_options_t *options = sentry_options_new(); + sentry_options_set_traces_sample_rate(options, 1.0); + sentry_options_set_max_spans(options, 3); + sentry_init(options); + + // Starting a child with no active transaction should fail + sentry_value_t parentless_child + = sentry_span_start_child(sentry_value_new_null(), NULL, NULL); + TEST_CHECK(sentry_value_is_null(parentless_child)); + + sentry_value_t tx_cxt = sentry_value_new_transaction_context("wow!", NULL); + sentry_transaction_start(tx_cxt); + + sentry_value_t child + = sentry_span_start_child(sentry_value_new_null(), "honk", "goose"); + TEST_CHECK(!sentry_value_is_null(child)); + + // Peek into the transaction's span list and make sure everything is + // good + sentry_value_t scope_tx = sentry__scope_get_span(); + const char *trace_id + = sentry_value_as_string(sentry_value_get_by_key(scope_tx, "trace_id")); + const char *parent_span_id + = sentry_value_as_string(sentry_value_get_by_key(scope_tx, "span_id")); + TEST_CHECK(!IS_NULL(scope_tx, "spans")); + TEST_CHECK_INT_EQUAL( + sentry_value_get_length(sentry_value_get_by_key(scope_tx, "spans")), 1); + + // Make sure the span inherited everything correctly + sentry_value_t stored_child = sentry_value_get_by_index( + sentry_value_get_by_key(scope_tx, "spans"), 0); + CHECK_STRING_PROPERTY(stored_child, "trace_id", trace_id); + CHECK_STRING_PROPERTY(stored_child, "parent_span_id", parent_span_id); + CHECK_STRING_PROPERTY(stored_child, "op", "honk"); + CHECK_STRING_PROPERTY(stored_child, "description", "goose"); + // Not finished yet + TEST_CHECK(IS_NULL(stored_child, "timestamp")); + // Span contexts carry indices in this SDK to make it easier to find and + // update them, make sure they don't leak into the transaction + TEST_CHECK(IS_NULL(stored_child, "index")); + + sentry_span_finish(child); + stored_child = sentry_value_get_by_index( + sentry_value_get_by_key(scope_tx, "spans"), 0); + // Should be finished + TEST_CHECK(!IS_NULL(stored_child, "timestamp")); + + sentry_close(); +} + +SENTRY_TEST(child_spans) +{ + sentry_options_t *options = sentry_options_new(); + sentry_options_set_traces_sample_rate(options, 1.0); + sentry_options_set_max_spans(options, 3); + sentry_init(options); + + // Starting a child with no active transaction should fail + sentry_value_t parentless_child + = sentry_span_start_child(sentry_value_new_null(), NULL, NULL); + TEST_CHECK(sentry_value_is_null(parentless_child)); + + // Finishing a nonexistent span doesn't explode anything + sentry_value_t fake_span + = sentry__value_new_span(sentry_value_new_null(), NULL); + sentry_span_finish(fake_span); + + sentry_value_t tx_cxt = sentry_value_new_transaction_context("wow!", NULL); + sentry_transaction_start(tx_cxt); + + sentry_value_t child + = sentry_span_start_child(sentry_value_new_null(), "honk", "goose"); + TEST_CHECK(!sentry_value_is_null(child)); + + // Peek into the transaction's span list and make sure everything is + // good + sentry_value_t scope_tx = sentry__scope_get_span(); + const char *trace_id + = sentry_value_as_string(sentry_value_get_by_key(scope_tx, "trace_id")); + TEST_CHECK_INT_EQUAL( + sentry_value_get_length(sentry_value_get_by_key(scope_tx, "spans")), 1); + + const char *parent_span_id + = sentry_value_as_string(sentry_value_get_by_key(child, "span_id")); + + sentry_value_t grandchild = sentry_span_start_child(child, "beep", "car"); + sentry_span_finish(grandchild); + + // Make sure everything on the transaction looks good, check grandchild + TEST_CHECK_INT_EQUAL( + sentry_value_get_length(sentry_value_get_by_key(scope_tx, "spans")), 2); + + sentry_value_t stored_grandchild = sentry_value_get_by_index( + sentry_value_get_by_key(scope_tx, "spans"), 1); + CHECK_STRING_PROPERTY(stored_grandchild, "trace_id", trace_id); + CHECK_STRING_PROPERTY(stored_grandchild, "parent_span_id", parent_span_id); + CHECK_STRING_PROPERTY(stored_grandchild, "op", "beep"); + CHECK_STRING_PROPERTY(stored_grandchild, "description", "car"); + // No span context-exclusive values leaking into transaction's spans + TEST_CHECK(IS_NULL(stored_grandchild, "index")); + // Should be finished + TEST_CHECK(!IS_NULL(stored_grandchild, "timestamp")); + + sentry_span_finish(child); + + sentry_close(); +} + +SENTRY_TEST(overflow_spans) +{ + sentry_options_t *options = sentry_options_new(); + sentry_options_set_traces_sample_rate(options, 1.0); + sentry_options_set_max_spans(options, 1); + sentry_init(options); + + sentry_value_t tx_cxt = sentry_value_new_transaction_context("wow!", NULL); + sentry_transaction_start(tx_cxt); + + sentry_value_t child + = sentry_span_start_child(sentry_value_new_null(), "honk", "goose"); + const char *child_span_id + = sentry_value_as_string(sentry_value_get_by_key(child, "span_id")); + + sentry_value_t scope_tx = sentry__scope_get_span(); + TEST_CHECK_INT_EQUAL( + sentry_value_get_length(sentry_value_get_by_key(scope_tx, "spans")), 1); + + sentry_value_t overflow_child + = sentry_span_start_child(child, "beep", "car"); + TEST_CHECK(sentry_value_is_null(overflow_child)); + + TEST_CHECK_INT_EQUAL( + sentry_value_get_length(sentry_value_get_by_key(scope_tx, "spans")), 1); + + sentry_value_t stored_child = sentry_value_get_by_index( + sentry_value_get_by_key(scope_tx, "spans"), 0); + CHECK_STRING_PROPERTY(stored_child, "span_id", child_span_id); + + sentry_value_decref(child); + sentry_value_decref(overflow_child); + + sentry_close(); +} + #undef IS_NULL #undef CHECK_STRING_PROPERTY diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index 0126f03944..69fd32a580 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -6,9 +6,11 @@ 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_spans) XX(basic_tracing_context) XX(basic_transaction) XX(buildid_fallback) +XX(child_spans) XX(concurrent_init) XX(count_sampled_events) XX(custom_logger) @@ -31,6 +33,7 @@ XX(mpack_removed_tags) XX(multiple_inits) XX(multiple_transactions) XX(os) +XX(overflow_spans) XX(page_allocator) XX(path_basics) XX(path_current_exe) From 8f8ee903b04719209e76d42603c1e0e44a716071 Mon Sep 17 00:00:00 2001 From: Betty Da Date: Fri, 17 Dec 2021 03:35:26 -0500 Subject: [PATCH 23/23] feat(tracing): Drop unfinished spans from a transaction --- examples/example.c | 4 +++ src/sentry_core.c | 19 ++++++++++++- tests/unit/test_tracing.c | 56 +++++++++++++++++++++++++++++++++++++++ tests/unit/tests.inc | 1 + 4 files changed, 79 insertions(+), 1 deletion(-) diff --git a/examples/example.c b/examples/example.c index 99a04d2fc8..29e5fd1404 100644 --- a/examples/example.c +++ b/examples/example.c @@ -232,6 +232,10 @@ main(int argc, char **argv) sentry_value_t grandchild_ctx = sentry_span_start_child(child_ctx, "littlest.teapot", NULL); + sentry_value_t unfinished_ctx + = sentry_span_start_child(child_ctx, "large.teapot", NULL); + + sentry_value_decref(unfinished_ctx); sentry_span_finish(grandchild_ctx); sentry_span_finish(child_ctx); } diff --git a/src/sentry_core.c b/src/sentry_core.c index b78767f200..4054fe0f7f 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -808,7 +808,24 @@ sentry_transaction_finish() sentry_value_remove_by_key(tx, "description"); sentry_value_remove_by_key(tx, "status"); - // TODO: prune unfinished child spans + sentry_value_t spans = sentry_value_get_by_key(tx, "spans"); + size_t span_count = sentry_value_get_length(spans); + // Go backwards to avoid accidentally skipping elements + for (size_t i = span_count; i > 0; i--) { + // TODO: Assume that tags and data from scope do not need to be merged + // into spans. This may be completely wrong. + bool should_remove = false; + { + sentry_value_t span = sentry_value_get_by_index(spans, i - 1); + should_remove = sentry_value_is_null( + sentry_value_get_by_key(span, "timestamp")); + } + if (should_remove) { + SENTRY_DEBUG("dropped an unfinished span from transaction"); + sentry_value_remove_by_index(spans, i - 1); + } + } + // This decrefs for us, generates an event ID, merges scope return sentry__capture_event(tx); } diff --git a/tests/unit/test_tracing.c b/tests/unit/test_tracing.c index e505f11eb9..f11e25cc0c 100644 --- a/tests/unit/test_tracing.c +++ b/tests/unit/test_tracing.c @@ -455,5 +455,61 @@ SENTRY_TEST(overflow_spans) sentry_close(); } +static void +check_spans(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)); + + size_t span_count = sentry_value_get_length( + sentry_value_get_by_key(transaction, "spans")); + TEST_CHECK_INT_EQUAL(span_count, 1); + + sentry_envelope_free(envelope); +} + +SENTRY_TEST(drop_unfinished_spans) +{ + 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(check_spans); + 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_max_spans(options, 2); + sentry_init(options); + + sentry_value_t tx_cxt = sentry_value_new_transaction_context("wow!", NULL); + sentry_transaction_start(tx_cxt); + + sentry_value_t child + = sentry_span_start_child(sentry_value_new_null(), "honk", "goose"); + TEST_CHECK(!sentry_value_is_null(child)); + + sentry_value_t grandchild = sentry_span_start_child(child, "beep", "car"); + TEST_CHECK(!sentry_value_is_null(grandchild)); + sentry_span_finish(grandchild); + + sentry_value_t scope_tx = sentry__scope_get_span(); + TEST_CHECK_INT_EQUAL( + sentry_value_get_length(sentry_value_get_by_key(scope_tx, "spans")), 2); + + sentry_uuid_t event_id = sentry_transaction_finish(); + TEST_CHECK(!sentry_uuid_is_nil(&event_id)); + + sentry_value_decref(child); + + sentry_close(); + + TEST_CHECK_INT_EQUAL(called_transport, 1); +} + #undef IS_NULL #undef CHECK_STRING_PROPERTY diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index 69fd32a580..de9b83afa4 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -14,6 +14,7 @@ XX(child_spans) XX(concurrent_init) XX(count_sampled_events) XX(custom_logger) +XX(drop_unfinished_spans) XX(dsn_parsing_complete) XX(dsn_parsing_invalid) XX(dsn_store_url_with_path)