-
-
Notifications
You must be signed in to change notification settings - Fork 161
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
Conversation
use sentry::{
integrations::tracing::{self as sentry_tracing, EventFilter},
ClientOptions,
};
use tracing::{span, warn, Level};
use tracing_subscriber::{layer::SubscriberExt as _, util::SubscriberInitExt as _};
const DSN: &str = "MY_DSN";
fn main() {
let _guard = sentry::init((
DSN,
ClientOptions {
traces_sample_rate: 1.0,
..Default::default()
},
));
let sentry_layer = sentry_tracing::layer()
.enable_span_attributes()
.event_filter(|md| match *md.level() {
Level::ERROR | Level::WARN => EventFilter::Event,
_ => EventFilter::Ignore,
});
tracing_subscriber::registry()
.with(sentry_layer)
.with(tracing_subscriber::fmt::layer())
.init();
// create very simple spans hierarchy
let span = span!(Level::ERROR, "root span", span_int = 42, span_str = "hello");
span.in_scope(|| {
span!(
Level::WARN,
"child span",
span_num = 144,
span_str = "world"
)
.in_scope(|| {
warn!(event_data = 3.1417, "The event message");
})
});
} gives the following output in the dashboard |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
sentry-tracing/src/converters.rs
Outdated
@@ -174,11 +220,11 @@ 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<S>(event: &tracing_core::Event, ctx: Option<Context<S>>) -> Event<'static> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this breaking change is a bit unfortunate. I’m also a bit surprised why we had Context<S>
there in the first place, if it was unused in all of these functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the type from Option<Context>
to impl Into<Option<Context>>
. This modification restores the capability to provide either a direct Context
or an Option<Context>
. As a result, the breaking change has been mitigated.
sentry-tracing/src/converters.rs
Outdated
@@ -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 { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #629 +/- ##
==========================================
- Coverage 73.03% 72.75% -0.29%
==========================================
Files 59 59
Lines 6658 6706 +48
==========================================
+ Hits 4863 4879 +16
- Misses 1795 1827 +32 |
* master: tracing: send spans' attributes along with the event ones (#629) release: 0.32.0 Update crate dependencies (#628) feat: bump http to 1.0.0 (#627) release: 0.31.8 MonitorSchedule constructor that validates crontab syntax (#625) fix(docs): Fix some doc errors that slipped in (#623) docs(tower): Mention how to enable http feature from sentry crate (#622) build(deps): bump rustix from 0.37.23 to 0.37.25 (#619)
Implementing #617
Adds a
enable_span_attributes()
method for thesentry_tracing::SentryLayer
that activates the desired behaviour.The root span's attributes can only be propagated once the traces enabled (e.g. through
ClientOptions::traces_sample_rate
).