Track subrequests with scopes

[Jean85]

This should resolve #186. I preferred to approach this with a separate listener, which seems a much cleaner solution.

[Jean85]

I would like a review from @ste93cry too.

[mcfedr]

Should you not be adding the scope variables, such as route, in the same way as is done in https://github.com/getsentry/sentry-symfony/blob/master/src/EventListener/RequestListener.php#L88

[Jean85]

It may depend on what we want to report. The main route is still the one of the master request, and I'm still thinking about if we need to add more data in that scope, or override the previous information.

IMHO, the subrequest info shouldn't override, but I'm not sure if it's in this bundle's scope to add that, or if I should leave it to the end user.

[mcfedr]

Yes, I see what you mean, you still want to see the actual request,

[Jean85]

Exactly. You'll still free to add tags, extra context or whatever to the scope, and if you do it inside the subrequest, it will be popped out and reverted at the end of it.

Does it make sense to you? Do you think it's a good approach?

[mcfedr]

Yes, though like you say, would be nice to automatically add something about the subrequest, like $scope->setTag('route', $matchedRoute); is done for master, maybe just with sub_request_route or something.

[ste93cry]

I agree, for example at work we use subrequests and we add some information to the extra context such as:

• current_request_url
• current_request_method
• master_request_url
• master_request_method
• is_master_request

This way we know if an event got generated from a request or a sub request

[mcfedr]

If you remove the checks for master request in the main RequestListener, haven't you achieved the same thing? without adding another listener into the system that has to be called for every request.

[Jean85]

If you remove the checks for master request in the main RequestListener, haven't you achieved the same thing? without adding another listener into the system that has to be called for every request.

The performance impact of a listener is negligible thanks to the compiled container, it's just a single method call.

Also, removing that check would just override the info, and not revert it back when the subrequest is complete, so the reported info would be wrong. Popping and pushing scopes is a lot more efficient and useful in this respect.

[mcfedr]

You make good points, I take back my comments :)

[stayallive]

For as far as I can judge this it looks good 👍

[HazAT]

Look goooood 👌