Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(tracing): Allow manual creation and sending of spanless Transactions #631

Merged
merged 22 commits into from
Dec 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
d223945
feat(tracing): Add in basic Envelope support for Transactions
relaxolotl Dec 16, 2021
0ddd751
feat(tracing): Allow manual creation and sending of spanless Transact…
relaxolotl Dec 16, 2021
204649f
rename `sentry_start_transaction` to `sentry_transaction_start`
relaxolotl Dec 16, 2021
61c6194
brush up docstrings
relaxolotl Dec 16, 2021
f47625d
whoops
relaxolotl Dec 16, 2021
49ec6d8
made a mistake with no-setup
relaxolotl Dec 17, 2021
c63c07b
no longer needed
relaxolotl Dec 21, 2021
6b5d9fb
todo cleanup
relaxolotl Dec 21, 2021
298c4a6
improve debug message
relaxolotl Dec 21, 2021
8cdd5c3
no more attachments for now
relaxolotl Dec 21, 2021
3d44eeb
accidentally forgot to copy over a useful comment
relaxolotl Dec 21, 2021
10ecee5
move uuid initialization into transactions
relaxolotl Dec 21, 2021
ff9250e
docs
relaxolotl Dec 21, 2021
d967e36
return the event id of the transaction if successfully sent
relaxolotl Dec 21, 2021
eec1502
handle one subtle bug in capture event, note another possible bug
relaxolotl Dec 21, 2021
41f55a2
this is already taken care of earlier in the pipeline
relaxolotl Dec 21, 2021
5c9d703
document ownership stealing
relaxolotl Dec 21, 2021
18d60d0
let's not pretend transaction contexts are transactions to avoid conf…
relaxolotl Dec 21, 2021
68a0530
manage refs correctly
relaxolotl Dec 22, 2021
356e077
note when ownership is being stolen
relaxolotl Dec 22, 2021
f108235
Merge remote-tracking branch 'origin/master' into tracing/the-rest
relaxolotl Dec 22, 2021
3e716bc
missed a spot
relaxolotl Dec 22, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions examples/example.c
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@ 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")) {
Expand Down Expand Up @@ -208,6 +212,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_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_context_set_sampled(tx_ctx, 0);
}

sentry_value_t tx = sentry_transaction_start(tx_ctx);
sentry_transaction_finish(tx);
}

// make sure everything flushes
sentry_close();

Expand Down
56 changes: 42 additions & 14 deletions include/sentry.h
Original file line number Diff line number Diff line change
Expand Up @@ -1238,8 +1238,8 @@ 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
* into `sentry_start_transaction` in order to be recorded and sent to sentry.
* 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
* https://docs.sentry.io/platforms/native/enriching-events/transaction-name/
Expand All @@ -1251,37 +1251,65 @@ 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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Transaction constructed off of the context.
* Transaction constructed off of the context.
*
* Borrows `transaction`.

*/
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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* conventions on `operation`s.
* conventions on `operation`s.
*
* Borrows `transaction`.

*/
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 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 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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* child spans will never be sent to sentry.
* child spans will never be sent to sentry.
*
* Borrows `transaction`.

*/
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`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* The Transaction will use the sampling rate as defined in `sentry_options`.
* The Transaction will use the sampling rate as defined in `sentry_options`.
*
* Borrows `transaction`.

*/
SENTRY_EXPERIMENTAL_API void sentry_transaction_context_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.
relaxolotl marked this conversation as resolved.
Show resolved Hide resolved
*
* Takes ownership of `transaction_context`.
*/
SENTRY_EXPERIMENTAL_API sentry_value_t 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.
relaxolotl marked this conversation as resolved.
Show resolved Hide resolved
*
* Always takes ownership of `transaction`, regardless of whether the operation
* was successful or not.
*/
SENTRY_EXPERIMENTAL_API void sentry_transaction_remove_sampled(
SENTRY_EXPERIMENTAL_API sentry_uuid_t sentry_transaction_finish(
sentry_value_t transaction);

#ifdef __cplusplus
Expand Down
2 changes: 2 additions & 0 deletions src/backends/sentry_backend_inproc.c
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,8 @@ handle_ucontext(const sentry_ucontext_t *uctx)

sentry_envelope_t *envelope
= sentry__prepare_event(options, event, NULL);
// TODO(tracing): Revisit when investigating transaction flushing during
// hard crashes.

sentry_session_t *session = sentry__end_current_session_with_status(
SENTRY_SESSION_STATUS_CRASHED);
Expand Down
144 changes: 121 additions & 23 deletions src/sentry_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -377,10 +378,11 @@ sentry__capture_event(sentry_value_t event)
sentry_envelope_t *envelope = NULL;

bool was_captured = false;
bool was_sent = false;
relaxolotl marked this conversation as resolved.
Show resolved Hide resolved
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);
}
Expand All @@ -395,14 +397,14 @@ sentry__capture_event(sentry_value_t event)
mut_options->session->init = false;
sentry__options_unlock();
}

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
Expand All @@ -414,32 +416,20 @@ 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);
// 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);
send = sentry__roll_dice(options->traces_sample_rate);
// TODO(tracing): Run through traces sampler function if rate is
// unavailable.
}
return send;
}

sentry_envelope_t *
Expand All @@ -452,7 +442,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;
}
Expand Down Expand Up @@ -507,6 +498,36 @@ 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;

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;
}

// TODO(tracing): Revisit when adding attachment support for transactions.

return envelope;

fail:
sentry_envelope_free(envelope);
sentry_value_decref(transaction);
return NULL;
}

void
sentry_handle_exception(const sentry_ucontext_t *uctx)
{
Expand Down Expand Up @@ -688,3 +709,80 @@ sentry_set_level(sentry_level_t level)
scope->level = level;
}
}

sentry_value_t
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));

// 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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rather minor as the semantics end up being the same, but i was kind of expecting a !sentry_value_is_null(parent_span) here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, i was really relying on the fact that .._get_length() handles nulls already here. a lot of the helper functions for sentry_value_t do so already, so i'm trying to remove any unnecessary null checks whereever i can as a result.

sentry_value_set_by_key(tx, "parent_span_id", parent_span);
} else {
sentry_value_decref(parent_span);
}
relaxolotl marked this conversation as resolved.
Show resolved Hide resolved
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"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwiw these three may end up setting null values if the passed in transaction context was not correct. this is fine i think, there's little point in doing much validation here already.

Copy link
Contributor Author

@relaxolotl relaxolotl Dec 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's a future PR that validates the contents of these values later on in the transaction lifecycle: #633

sentry_value_set_by_key(tx, "start_timestamp",
sentry__value_new_string_owned(
sentry__msec_time_to_iso8601(sentry__msec_time())));

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(tx);
// TODO(tracing): remove from scope
return sentry_uuid_nil();
}

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(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 trace_context = sentry__span_get_trace_context(tx);
sentry_value_t contexts = sentry_value_new_object();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we absolutely sure there is no contexts yet on the transaction value?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

by this point, yes. unless a user deliberately tries to insert a context properly via the sentry_value API instead of using the provided setters for transactions in the public API, the context field should not exist on a transaction yet during this method.

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 takes ownership of the transaction, generates an event ID, merges
// scope
return sentry__capture_event(tx);
}
25 changes: 21 additions & 4 deletions src/sentry_core.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -56,6 +57,24 @@ 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. This takes ownership of the transaction, which
* means that the caller no longer needs to call `sentry_value_decref` on the
* transaction.
relaxolotl marked this conversation as resolved.
Show resolved Hide resolved
*/
sentry_envelope_t *sentry__prepare_transaction(const sentry_options_t *options,
sentry_value_t transaction, sentry_uuid_t *event_id);

/**
* This function will submit the `envelope` to the given `transport`, first
* checking for consent.
Expand Down Expand Up @@ -98,9 +117,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
3 changes: 2 additions & 1 deletion src/sentry_scope.c
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

Expand Down
Loading