feat(sentry-tracing): add span fields to breadcrumbs #708
Conversation
thomaseizinger
changed the title
This switches our `sentry-tracing` dependency to a fork that includes getsentry/sentry-rust#708. Recording our span fields with breadcrumbs is important to provide accurate context of the message. Without the span fields, the messages give us a lot less information. Since the last release, the open issue on `flush` having a flipped return value got fixed as well.
|
Unfortuantely, this is only one half of the story. We also need to find a solution to #617 (comment) otherwise only spans that have been sampled are present. |
|
I think this needs a rebase |
thomaseizinger
force-pushed
the
feat/add-span-context-to-breadcrumbs
branch
from
5570b24 to
789cdc1
Compare
Done! |
Any thoughts on this? I did some digging through the codebase and I think we could solve this by sampling spans later, i.e. keep them around initially and only sample and discard them when they are about to be submitted to Sentry. I don't know how much of a performance impact this has though. The problem with the current approach is that the code added in this PR does not do anything unless one also samples all spans. That on the other hand though can create A LOT of spans in Sentry which may not be desired. Ideally, I think the two features of sampling spans to be sent to Sentry and capturing the fields within spans for breadcrumbs and events are orthogonal. |
This comment was marked as outdated.
This comment was marked as outdated.
|
A test for sentry-rust/sentry-core/src/test.rs Line 120 in 05fc7de Otherwise I’m a bit out of the loop when it comes to how the SDK APIs and are supposed to work :-( |
I managed to add one based on that! :) |
This is another attempt at fixing #7386. Previous PR was #7379. The difference is, this time it works! In the following screenshot, `handle_input` is a currently active span.  I had to make some patches to Sentry, most notably: - getsentry/sentry-rust#708 - getsentry/sentry-rust#712 The way we configure Sentry is quite tricky: First and foremost, we need to understand that the `tracing` adapter for Sentry has a `span_filter` configuration. When a span gets filtered out there, the rest of `sentry-tracing` never sees the data in that span. Thus, in order to capture variables from spans, we need to have a fairly generous span filter. In this PR, we change this span filter to include all spans except those on TRACE level. Secondly, by default, the Sentry SDK doesn't send any spans to the backend, i.e. the sampling rate is 0. Previously, we set the sampling rate to 1.0 because the `span_filter` was already filtering out all non-telemetry spans. A telemetry span is a concept that we invented. It is a span that gets sampled at _creation_ time with a probability of 1%. This is useful because creating a lot of spans is also expensive, so we don't want to do it e.g. on a per-packet basis. With just these configuration options, we now have a problem: We don't want to submit all spans to Sentry but we need the `span_filter` to allow all spans otherwise we can't capture the contextual fields from the span in breadcrumbs. Luckily, the Sentry SDK has another configuration option: `traces_sampler`. The `traces_sampler` gets to compute a sampling rate for each individual span. This allows us to discard all spans from being sent to Sentry unless they are `telemetry` spans. Resolves: #7386.
The fields of a span are equally helpful in an event as well as in the breadcrumbs leading up to it. In fact, in an application that makes heavy use of spans to add contextual information to messages, the log messages themselves without the span fields are often almost useless.