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

tracing: send spans' attributes along with the event ones #629

Merged
merged 5 commits into from
Dec 6, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
31 changes: 31 additions & 0 deletions sentry-core/src/performance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,25 @@ pub struct Transaction {
pub(crate) inner: TransactionArc,
}

/// Iterable for a transaction's [`extra` field](protocol::Transaction::extra).
pub struct TransactionData<'a>(MutexGuard<'a, TransactionInner>);

impl<'a> TransactionData<'a> {
/// Iterate over the `extra` map
/// of the [transaction][protocol::Transaction].
///
/// If the transaction not sampled for sending,
/// the metadata will not be populated at all
/// so the produced iterator is empty.
pub fn iter(&self) -> Box<dyn Iterator<Item = (&String, &protocol::Value)> + '_> {
if let Some(ref rx) = self.0.transaction {
Box::new(rx.extra.iter())
} else {
Box::new(std::iter::empty())
}
}
}

impl Transaction {
#[cfg(feature = "client")]
fn new(mut client: Option<Arc<Client>>, ctx: TransactionContext) -> Self {
Expand Down Expand Up @@ -464,6 +483,18 @@ impl Transaction {
}
}

/// Returns an iterating accessor to the transaction's [`extra` field](protocol::Transaction::extra).
///
/// # Concurrency
/// In order to obtain any kind of reference to the `extra` field,
/// a `Mutex` needs to be locked. The returned `TransactionData` holds on to this lock
/// for as long as it lives. Therefore you must take care not to keep the returned
/// `TransactionData` around too long or it will never relinquish the lock and you may run into
/// a deadlock.
pub fn data(&self) -> TransactionData {
TransactionData(self.inner.lock().unwrap())
}

/// Get the TransactionContext of the Transaction.
///
/// Note that this clones the underlying value.
Expand Down
71 changes: 63 additions & 8 deletions sentry-tracing/src/converters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@ use std::collections::BTreeMap;
use std::error::Error;

use sentry_core::protocol::{Event, Exception, Mechanism, Thread, Value};
use sentry_core::{event_from_error, Breadcrumb, Level};
use sentry_core::{event_from_error, Breadcrumb, Level, TransactionOrSpan};
use tracing_core::field::{Field, Visit};
use tracing_core::{span, Subscriber};
use tracing_subscriber::layer::Context;
use tracing_subscriber::registry::LookupSpan;

use super::layer::SentrySpanData;

/// Converts a [`tracing_core::Level`] to a Sentry [`Level`]
fn convert_tracing_level(level: &tracing_core::Level) -> Level {
match level {
Expand All @@ -29,7 +31,14 @@ fn level_to_exception_type(level: &tracing_core::Level) -> &'static str {
}

/// Extracts the message and metadata from an event
fn extract_event_data(event: &tracing_core::Event) -> (Option<String>, FieldVisitor) {
/// and also optionally from its spans chain.
fn extract_event_data<S>(
event: &tracing_core::Event,
ctx: Option<Context<S>>,
) -> (Option<String>, FieldVisitor)
where
S: Subscriber + for<'a> LookupSpan<'a>,
{
// Find message of the event, if any
let mut visitor = FieldVisitor::default();
event.record(&mut visitor);
Expand All @@ -41,6 +50,40 @@ fn extract_event_data(event: &tracing_core::Event) -> (Option<String>, FieldVisi
.or_else(|| visitor.json_values.remove("error"))
.and_then(|v| v.as_str().map(|s| s.to_owned()));

// Add the context fields of every parent span.
let current_span = ctx.as_ref().and_then(|ctx| {
event
.parent()
.and_then(|id| ctx.span(id))
.or_else(|| ctx.lookup_current())
});
if let Some(span) = current_span {
for span in span.scope() {
let name = span.name();
let ext = span.extensions();
if let Some(span_data) = ext.get::<SentrySpanData>() {
match &span_data.sentry_span {
TransactionOrSpan::Span(span) => {
for (key, value) in span.data().iter() {
if key != "message" {
let key = format!("{}:{}", name, key);
visitor.json_values.insert(key, value.clone());
}
}
}
TransactionOrSpan::Transaction(transaction) => {
for (key, value) in transaction.data().iter() {
if key != "message" {
let key = format!("{}:{}", name, key);
visitor.json_values.insert(key, value.clone());
}
}
}
}
}
}
}

(message, visitor)
}

Expand Down Expand Up @@ -104,8 +147,14 @@ impl Visit for FieldVisitor {
}

/// Creates a [`Breadcrumb`] from a given [`tracing_core::Event`]
pub fn breadcrumb_from_event(event: &tracing_core::Event) -> Breadcrumb {
Copy link
Member

Choose a reason for hiding this comment

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

This is a breaking change anyway because this fn signature is gaining a new parameter. So might as well just go directly with an Option, as in impl Into is a bit heavy handed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since I don't need it anyway, I can remove this new argument from this particular breadcrumb_from_event. I put it here before only to make the uniform-looking signatures.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, restored the initial breadcrumb_from_event signature.

let (message, visitor) = extract_event_data(event);
pub fn breadcrumb_from_event<'context, S>(
event: &tracing_core::Event,
_ctx: impl Into<Option<Context<'context, S>>>,
) -> Breadcrumb
where
S: Subscriber + for<'a> LookupSpan<'a>,
{
let (message, visitor) = extract_event_data::<S>(event, None);
Breadcrumb {
category: Some(event.metadata().target().to_owned()),
ty: "log".into(),
Expand Down Expand Up @@ -174,11 +223,14 @@ fn contexts_from_event(
}

/// Creates an [`Event`] from a given [`tracing_core::Event`]
pub fn event_from_event<S>(event: &tracing_core::Event, _ctx: Context<S>) -> Event<'static>
pub fn event_from_event<'context, S>(
event: &tracing_core::Event,
ctx: impl Into<Option<Context<'context, S>>>,
) -> Event<'static>
where
S: Subscriber + for<'a> LookupSpan<'a>,
{
let (message, mut visitor) = extract_event_data(event);
let (message, mut visitor) = extract_event_data(event, ctx.into());

Event {
logger: Some(event.metadata().target().to_owned()),
Expand All @@ -191,15 +243,18 @@ where
}

/// Creates an exception [`Event`] from a given [`tracing_core::Event`]
pub fn exception_from_event<S>(event: &tracing_core::Event, _ctx: Context<S>) -> Event<'static>
pub fn exception_from_event<'context, S>(
event: &tracing_core::Event,
ctx: impl Into<Option<Context<'context, S>>>,
) -> Event<'static>
where
S: Subscriber + for<'a> LookupSpan<'a>,
{
// Exception records in Sentry need a valid type, value and full stack trace to support
// proper grouping and issue metadata generation. tracing_core::Record does not contain sufficient
// information for this. However, it may contain a serialized error which we can parse to emit
// an exception record.
let (mut message, visitor) = extract_event_data(event);
let (mut message, visitor) = extract_event_data(event, ctx.into());
let FieldVisitor {
mut exceptions,
mut json_values,
Expand Down
40 changes: 32 additions & 8 deletions sentry-tracing/src/layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ pub struct SentryLayer<S> {
event_mapper: Option<EventMapper<S>>,

span_filter: Box<dyn Fn(&Metadata) -> bool + Send + Sync>,

with_span_attributes: bool,
}

impl<S> SentryLayer<S> {
Expand Down Expand Up @@ -104,6 +106,19 @@ impl<S> SentryLayer<S> {
self.span_filter = Box::new(filter);
self
}

/// Enable every parent span's attributes to be sent along with own event's attributes.
///
/// Note that the root span is considered a [transaction][sentry_core::protocol::Transaction]
/// so its context will only be grabbed only if you set the transaction to be sampled.
/// The most straightforward way to do this is to set
/// the [traces_sample_rate][sentry_core::ClientOptions::traces_sample_rate] to `1.0`
/// while configuring your sentry client.
#[must_use]
pub fn enable_span_attributes(mut self) -> Self {
self.with_span_attributes = true;
self
}
}

impl<S> Default for SentryLayer<S>
Expand All @@ -116,15 +131,17 @@ where
event_mapper: None,

span_filter: Box::new(default_span_filter),

with_span_attributes: false,
}
}
}

/// Data that is attached to the tracing Spans `extensions`, in order to
/// `finish` the corresponding sentry span `on_close`, and re-set its parent as
/// the *current* span.
struct SentrySpanData {
sentry_span: TransactionOrSpan,
pub(super) struct SentrySpanData {
pub(super) sentry_span: TransactionOrSpan,
parent_sentry_span: Option<TransactionOrSpan>,
}

Expand All @@ -135,12 +152,19 @@ where
fn on_event(&self, event: &Event, ctx: Context<'_, S>) {
let item = match &self.event_mapper {
Some(mapper) => mapper(event, ctx),
None => match (self.event_filter)(event.metadata()) {
EventFilter::Ignore => EventMapping::Ignore,
EventFilter::Breadcrumb => EventMapping::Breadcrumb(breadcrumb_from_event(event)),
EventFilter::Event => EventMapping::Event(event_from_event(event, ctx)),
EventFilter::Exception => EventMapping::Event(exception_from_event(event, ctx)),
},
None => {
let span_ctx = self.with_span_attributes.then_some(ctx);
match (self.event_filter)(event.metadata()) {
EventFilter::Ignore => EventMapping::Ignore,
EventFilter::Breadcrumb => {
EventMapping::Breadcrumb(breadcrumb_from_event(event, span_ctx))
}
EventFilter::Event => EventMapping::Event(event_from_event(event, span_ctx)),
EventFilter::Exception => {
EventMapping::Event(exception_from_event(event, span_ctx))
}
}
}
};

match item {
Expand Down