[contrib-auto-symfony] Subrequests
I'd like to start a discussion on how to handle symfony subrequests.
One great example of subrequests is the ErrorController that renders the red exception page with a stack trace when you have an uncaught exception in your application.
Because the ErrorController has no route set, it's span name is just "GET", which is not helpful at all:
Discussion points
- Should
HttpKernel::handle()span names contain the controller as well? - Does it make sense that a subrequest gets
SpanKind::KIND_SERVER? - Should the
ErrorControllerspan really be marked as failed even though it correctly output a stack trace?
@DrLuke sorry I'd missed this issue. You might also like to ask this on our slack channel where it might get more eyes on it.
Should HttpKernel::handle() span names contain the controller as well?
Not if they're treated like other requests, since the spec tells us how root spans should be named. It's quite possible that we just haven't considered sub-requests and they should be handled differently though - can you tell from the Request whether it's a main or subrequest? I think it would be fine to do that so that sub requests are more meaningfully named.
Does it make sense that a subrequest gets SpanKind::KIND_SERVER?
Maybe INTERNAL? see https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#spankind
Should the ErrorController span really be marked as failed even though it correctly output a stack trace?
That probably depends on the response code of the controller? It it a 20x or a 50x? I don't have a strong opinion, though - what makes the most sense?
@brettmc
can you tell from the Request whether it's a main or subrequest?
Yes, when the request is being handled, the request type is passed into the method.
@dkarlovi @DrLuke Can you please review the linked PR for this? I don't have a symfony app to test this against, but it looks like you do. I added tests and they pass, which is a start.
I do not have a Symfony app with subrequests which is also OTEL instrumented available ATM, sorry. Hopefully @DrLuke can help.
@brettmc Great work, this is what it looks like in my APM:
The sub-request is displayed as internal 👍
@DrLuke what APM app / UI is that?
@DrLuke what APM app / UI is that?
Elastic APM
@DrLuke @dkarlovi I've just released a new beta of the symfony auto-instrumentation. If you're happy with it, report back as I plan to release 1.0.0 if there are no other issues.