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 integration #180

Closed
khionu opened this issue Feb 6, 2020 · 27 comments
Closed

tracing integration #180

khionu opened this issue Feb 6, 2020 · 27 comments

Comments

@khionu
Copy link

khionu commented Feb 6, 2020

Yet Another Integration Issue. As with #160, support for tracing would be fantastic.

@nCrazed
Copy link
Contributor

nCrazed commented Aug 17, 2020

Rocket (a popular web framework) is currently in the process of migrating to tracing, having this integration for when that happens would be very useful.

rwf2/Rocket#1410

@kellpossible
Copy link

kellpossible commented Sep 26, 2020

It would also be good to have support for eyre/color-eyre too along with this.

@khionu
Copy link
Author

khionu commented Sep 26, 2020

@kellpossible should open a separate issue for a separate crate to support.

@kellpossible
Copy link

I've started working on this https://github.com/kellpossible/sentry-tracing I'll make it into a pull request here when I've tested it out more and sure about the feature set. Happy for any comments, or if someone else wants to rip it out and make a PR of it now.

@Swatinem
Copy link
Member

Some preliminary typedefs related to tracing just landed in #276

@dashed
Copy link
Member

dashed commented Oct 26, 2020

@kellpossible were you able to generate any useful spans/traces?

@kellpossible
Copy link

kellpossible commented Oct 27, 2020

@dashed I've got events working as breadcrumbs, and issues. Span support is limited at the moment I think, haven't tested it much.

@gakonst
Copy link

gakonst commented Apr 18, 2021

@kellpossible thank you for the work so far! are you still working on the feature?

@kellpossible
Copy link

kellpossible commented Apr 18, 2021

@gakonst not really, it was working well enough for my purposes at the time, and I'm currently reassigned on a different project which doesn't involve using sentry, but perhaps will come back to it.

@leops
Copy link
Contributor

leops commented Apr 20, 2021

I've forked @kellpossible's repository to https://github.com/levels3d/sentry-tracing and I'm currently working on adding transaction collection. I've implemented the conversion of tracing spans to sentry span data, but I'm not too sure on how to proceed to submit the transaction to Sentry as wrapping it in an enveloppe and sending that doesn't seem to work properly (there might be an issue in the Sentry SDK as the span IDs are getting serialized as full 32 bytes UUID strings when it looks like they should only be 16 characters long)

@jplatte
Copy link

jplatte commented Jun 7, 2021

@leops Idk what transaction collection is, but am I right in assuming that I have to use your fork to get span support? It doesn't seem like the official sentry-tracing from this repo currently reports spans to sentry, which makes the reported errors much less useful than they should be.

@leops
Copy link
Contributor

leops commented Jun 7, 2021

Well my fork is sort of halfway there, while my version does collect spans from the tracing library since the Sentry SDK doesn't really support transactions I tried to pass that data into the SDK anyway in the best way I could find but the resulting transactions never show up in Sentry, and unfortunately at the moment I don't have more time to spend on debugging this.

@onelson
Copy link

onelson commented Jun 30, 2021

I'm very excited about the possibility of getting spans to show up on the performance tab. Other than the size alignment issue for the IDs, are there any other barriers? Maybe we won't know until that issue is cleared.

@leops
Copy link
Contributor

leops commented Jul 1, 2021

I finally could free up some time to work on this, and I think I've fixed all the outstanding issues. On the latest revision of my fork and using the changes I've pushed in #349 tracing spans show up correctly as transactions on Sentry. With @kellpossible's authorization since I started from their code I could start working on another PR to contribute all this to the official sentry-tracing crate

@onelson
Copy link

onelson commented Jul 1, 2021

I've been playing around with what's in master here, and @leops's fork, trying to put it together with sentry-actix and tracing-actix-web. Seems like there's a deadlock when errors are sent to sentry. I'll... try and boil this down to a simple repro 😬

@onelson
Copy link

onelson commented Jul 8, 2021

Maybe this is better tracked as a separate issue, but here's the deadlock repro: https://github.com/LaikaStudios/actix-tracing-sentry-repro

The repro depends on the newly released 0.23 sdk, but I expect the topic branch here to also have the same issue.

It might be there's a problem with the subscriber configuration, but even so I'd bet anyone working in actix-web will stumble into this as things are today.

@Swatinem
Copy link
Member

@onelson I just ran your repro on mac with a normal debug build (cargo run), and that worked fine. On which system are you?
Otherwise, the only thing that I noticed was that the /err case is reported twice, once through sentry itself, and again through the logger from within actix.

@onelson
Copy link

onelson commented Jul 12, 2021

@Swatinem This is good news yet unfortunate for me, I suppose. In this case, I was on a Centos 7 system - my company furnished workstation. Maybe I need to loop my IT department in.

@onelson
Copy link

onelson commented Jul 12, 2021

I just ran the repro project on my home machine (Ubuntu 20.04) and it worked fine, so I don't know what's wrong with my Centos box at work...

@onelson
Copy link

onelson commented Jul 14, 2021

RE: the deadlock - I built the repro as a docker container and found that the deadlock happens when I run the container on my Centos box seemingly all of our Centos 7 machines, but not on another fedora machine so I'm not sure if it points to the specific kernel or perhaps some part of the network configuration. At any rate, I'm glad this isn't widespread and I'll follow up with our IT dept to try and figure this out.

@seanpianka
Copy link
Contributor

seanpianka commented Jul 16, 2021

What's the likelihood that @leops tracing integration will be brought in upstream to this crate? I'm looking to demo the integration between an application instrumented with tracing and the visual representation in Sentry.

Thanks @leops for making these code improvements!

@dmitrymur
Copy link
Contributor

Thank you @leops for the latest updates! One more question that is left: "How to propagate the trace_id from other services/systems?". As a workaround we can pass trace_id and span_id as parameters to span! macro and parse them in default_span_mapper function. Also the decision to sample or not, in that case, should be taken from the very root transaction that might be started on another system. Consistency within a trace

@seanpianka
Copy link
Contributor

It seems like this integration creates a single issue in sentry and lumps all events/issues together under a single issue titled <unlabeled event>.

Any idea how to add a label to the event/issue, so that issues for different errors are created separately?

Thank you!

@DmitrySamoylov
Copy link
Contributor

I just played with tracing spans and I can see some trace/span details showing up in Sentry (thanks @leops)

image

but it seems that tracing span fields aren't reported to Sentry. Trying to figure out if I misconfigured something or it's just unsupported.

Here is my example application that sends an event to Sentry.

What I expect is that function argument (span_field) appears somewhere on the event page at sentry.io:

fn main() {
    ...
    make_an_error("span_field_value");
}

#[instrument]
fn make_an_error(span_field: &str) {                   // span_field IS NOT sent to sentry
    error!(event_field = "event_field_value", "test"); // event_field IS sent to sentry
}

.. but it isn't:

image

As a workaround, I don't mind extending sentry event additional data with fields from the span right in the event_mapper callback. But is there some way to get span fields from event_mapper callback?

@seanpianka
Copy link
Contributor

@DmitrySamoylov I am working on additional support for tracing with sentry here: #359

@svenstaro
Copy link

Can't this issue be closed as sentry-tracing exists?

@Swatinem
Copy link
Member

Yes, lets do that :-D I think we can file followups for anything that is missing from the integration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests