sentry-symfony icon indicating copy to clipboard operation
sentry-symfony copied to clipboard

Not connecting issues (errors) with performance traces

Open askold123 opened this issue 4 years ago • 12 comments

Environment

Using https://github.com/getsentry/sentry-symfony with Symfony 5.2

Steps to Reproduce

Enable tracing as in sentry doc https://docs.sentry.io/platforms/php/guides/symfony/performance/#custom-instrumentation using this options in config:

sentry:
  dsn: "%env(SENTRY_DSN)%"
  options:
    traces_sample_rate: 1.0
    sample_rate: 1

Expected Result

Exceptions are connected with transaction as it shown in this video https://youtu.be/sA1rED-zb6w?t=375

Actual Result

Exceptions are sent in separate event and are not connected to transaction in Sentry UI.

askold123 avatar Mar 03 '21 09:03 askold123

Quoting @ste93cry from the original issue:

[...] cannot determine if this is either a bug or something that is working as expected, regardless of the video, and maybe there is something else missing that must be done manually to link together a transaction and an error. @stayallive @HazAT can you please help us?

In short, we don't know enough to see if we need to do something SDK-wise do bind events to tracing transactions, or if all is done server-side.

Jean85 avatar Mar 03 '21 11:03 Jean85

There are 2 dings to connect them that needs to be added depending on how your app works:


For traditional server side apps:

You need to connect the PHP traces to the JS SDK by setting a meta tag on your page the JS SDK can read.

<meta name="sentry-trace" content="49879feb76c84743ba5034bd2d3f1ca3-7cb5535c930d4666-1"/>

Example from Laravel: https://docs.sentry.io/platforms/php/guides/laravel/performance/#connect-your-frontend.

This is how that header is built: https://github.com/getsentry/sentry-laravel/blob/be9658a57d554f8bfe587facc1492a45c6bd0e1c/src/Sentry/Laravel/Integration.php#L175-L187.


For SPA's or where the JavaScript initiates the first request:

The JavaScript clients adds the sentry-trace header to requests that are within the tracingOrigins: https://docs.sentry.io/platforms/javascript/performance/included-instrumentation/#enable-instrumentation

You can do this to setup the transaction context for that header:

    $sentryTraceHeader = $request->header('sentry-trace');

    $context = $sentryTraceHeader
        ? TransactionContext::fromSentryTrace($sentryTraceHeader)
        : new TransactionContext;

Example from Laravel: https://github.com/getsentry/sentry-laravel/blob/be9658a57d554f8bfe587facc1492a45c6bd0e1c/src/Sentry/Laravel/Tracing/Middleware.php#L83-L87


Some more info: https://docs.sentry.io/platforms/javascript/performance/connect-services/.


I don't think how much of this should just be documented and how much should be implemented in the SDK (and/or framework integrations). For Laravel it's fully functional with a helper for the HTML tag and we read the header from the request if it's available. So maybe these 2 pieces (HTML meta tag and reading the sentry-trace header) should be added to the Symfony integration to get it rolling.

stayallive avatar Mar 03 '21 13:03 stayallive

The second feature is already included in the upcoming release of the Symfony bundle, thanks to @ste93cry: https://github.com/getsentry/sentry-symfony/blob/8f7a7375cbd2979df1a177e64504c7529b75dcba/src/EventListener/TracingRequestListener.php#L36

As for the meta tag, it would be a little more complicated... Maybe we can provide a Twig partial or function?

[EDIT] created https://github.com/getsentry/sentry-symfony/issues/458 to track the feature request.

Jean85 avatar Mar 03 '21 14:03 Jean85

In that case I don't think there is much to do then here in the PHP SDK itself?

stayallive avatar Mar 03 '21 15:03 stayallive

Nope, not here!

Jean85 avatar Mar 03 '21 15:03 Jean85

I'm not talking about distributed tracing. It's fine and working. What I'm talking about is that an exception that occurs in any span from php backend is not connected to this span. You can see the expected behavior in video from topic. You can see from this line https://github.com/getsentry/sentry-php/blob/master/src/Client.php#L146 that exceptions are sent as separate events and no matter if tracing enabled or not.

askold123 avatar Mar 05 '21 10:03 askold123

@stayallive do you have an answer for this case too?

Jean85 avatar Mar 05 '21 11:03 Jean85

Yeah it's normal those are separate events, however from looking into it it does look like we are adding the correct context that needs to be present for events to be linked to the active transaction.

https://github.com/getsentry/sentry-php/blob/d9fdc40728dc58936f59e9b2e1104163fdcafe24/src/State/Scope.php#L355-L358

From checking this is also present on events and shows up in the issue UI like this:

image

So issue -> transaction link is working.

However the transaction -> issue link is not it seems because when coming from the transaction summary the list of related events is empty.

I am checking with the Sentry server folks what would be expected and if we can do something to remedy this or that is's a bug somewhere, to be continued!

stayallive avatar Mar 05 '21 11:03 stayallive

So I found the issue, or at least what could cause it, @Jean85 and @ste93cry will hopefully help me out with tracking it down 💪

Events in Sentry have a transaction key and for a performance tracing transactions to be matched to issues like exceptions they both need to have the same value in the transaction key.

I'm not sure if Symfony is setting that transaction key at all on the Event (for exceptions or the tracing span) but that is what is needed to link them together (GitHub search did not uncover it being done at least).

I have moved this issue back to the Symfony repo since this is all working in the base SDK. It might need some changes on the Symfony side to properly have the transaction key populated.

stayallive avatar Mar 06 '21 20:03 stayallive

I'm not sure, but maybe this is already fixed in the upcoming 4.1 release? @askold123 can you try again using the develop branch?

Jean85 avatar Mar 07 '21 21:03 Jean85

@Jean85 it's not working in develop

askold123 avatar Mar 09 '21 10:03 askold123

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

github-actions[bot] avatar Jan 12 '22 21:01 github-actions[bot]