opentelemetry-php icon indicating copy to clipboard operation
opentelemetry-php copied to clipboard

[contrib-auto-symfony] Subrequests

Open DrLuke opened this issue 2 years ago • 3 comments

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:

Example trace from APM showing the main HttpKernel::handle() span labelled GET app_wip_wip, and a second trace below that for the error controller that is just labelled as GET

Discussion points

  1. Should HttpKernel::handle() span names contain the controller as well?
  2. Does it make sense that a subrequest gets SpanKind::KIND_SERVER?
  3. Should the ErrorController span really be marked as failed even though it correctly output a stack trace?

DrLuke avatar Nov 29 '23 11:11 DrLuke

@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.

brettmc avatar Dec 06 '23 23:12 brettmc

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 avatar Jan 30 '24 03:01 brettmc

@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 avatar Mar 12 '24 09:03 dkarlovi

@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.

brettmc avatar Apr 30 '24 23:04 brettmc

I do not have a Symfony app with subrequests which is also OTEL instrumented available ATM, sorry. Hopefully @DrLuke can help.

dkarlovi avatar May 01 '24 09:05 dkarlovi

@brettmc Great work, this is what it looks like in my APM: image

The sub-request is displayed as internal 👍

DrLuke avatar May 01 '24 15:05 DrLuke

@DrLuke what APM app / UI is that?

dkarlovi avatar May 01 '24 15:05 dkarlovi

@DrLuke what APM app / UI is that?

Elastic APM

DrLuke avatar May 01 '24 15:05 DrLuke

@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.

brettmc avatar May 01 '24 23:05 brettmc