[opentelemetry-php-contrib] Pre hook exception Symfony\Component\HttpKernel\Controller\ErrorController could not be converted to string

[trigrab]

Describe your environment

I am running a symfony 6.4 app with opentelemetry-auto-symfony installed. In some subrequests I get this Warning in my nginx error logs:

*41 FastCGI sent in stderr: "PHP message: PHP Warning:  Symfony\Component\HttpKernel\HttpKernel::handle(): OpenTelemetry: pre hook threw exception, class=Symfony\Component\HttpKernel\HttpKernel function=handle message=Object of class Symfony\Component\HttpKernel\Controller\ErrorController could not be converted to string in Unknown on line 0" while reading response header from upstream, client: 172.110.1.1, server: someserver $, request: "GET someWrongUrl

To me it seems like either the symfony instrumentation for HttpKernel handle should not expect the _controller attribute, which in my case is the HttpKernel ErrorController, to be a string SymfonyInstrumentation.php#L50 or the ErrorController of the HttpKernel component should implment a __toString() function. I am not a php developer and I have no experience in symfony development. Therefor I don't know the right behavior for this. But I expect the exception should not be thrown.

Steps to reproduce I expect any symfony app (6.4 or above) would have the same error when HttpKernel ErrorController is part of a subrequest. The assumption that _controller is a string is the problem here. As I am not a php developer I cannot tell how to reproduce in a minimal setup and I cannot share my symfony app. If necessary I will ask a collegue who is php developer to provide a minimal setup.

What is the expected behavior? There should be no exception. My error should be handled by the ErrorController.

What is the actual behavior? Warning about an exception in the opentelemetry symfony pre hook for HttpKernel handle

Additional context I don't think it is connected, but I am running a container based symfony app in version 6.8 with php 8.4.6

[brettmc]

Thanks for the bug report. Sorry I don't know the symfony auto-instrumentation very well. In your case, what type is the _controller attribute? Perhaps a closure, or invokable?

[trigrab]

The _controller attribute is of type \Symfony\Component\HttpKernel\Controller\ErrorController it has an attribute controller which is the string error_controller. That might be the right attribute in this case for the name.

[bobstrecansky]

@trigrab - are you planning on submitting a pull request for this? Thanks in advance!

[trigrab]

@bobstrecansky - as I said, I am not sure which solution would be right. But with some hints on the right solution I will definitely submit a PR.

At the moment I don't know if this is an error in the sub request handling of the symfony instrumentation or if \Symfony\Component\HttpKernel\Controller\ErrorController should implement a __toString() function.

[brettmc]

If what you're observing is "pure" symfony (ie, the error controller and the key it uses in attributes is out-of-box functionality), then we should be able to add an integration test to replicate it. From there, if we can make the test green it will be a great start. Based on your report, we could possibly just make a is_string check on _controller and controller? I'd like to understand when symfony uses which attribute.

[brettmc]

or if \Symfony\Component\HttpKernel\Controller\ErrorController should implement a __toString() function

Do symfony controllers usually implement Stringable? If not, then I think let's just update this library to fix the issue from our end.

[umulmrum]

Hi, Symfony developer and colleague of @trigrab here. I think I can help on that part:

Symfony allows multiple data types here: strings, arrays, and objects (basically anything that is either a callable controller or can be resolved into one by either built-in or custom controller resolvers). So you should take all 3 types into account (@trigrab's case originates from the case where an exception in application code is handled by Symfony, so it's built-in, not custom logic).

Controllers do usually NOT implement Stringable and I think it doesn't make that much sense, because a controller is usually identified from framework configuration (by its FQCN); a controller is called by the framework and then just passes the control flow to business logic, so it's normally not a code entity that needs to be identifiable by itself.

The controller property is specific to the ErrorController and is not suitable as a general solution.

My best guess would be to check the type of _controller, and do the following:

• If it's a string, keep the current solution.
• If it's an object, use get_class($controller) to get the FQCN, and be done.
• If It's an array, things get a bit dirty, as both PHP and Symfony are quite flexible on what is allowed here. See \Symfony\Component\HttpKernel\Controller\ControllerResolver::getController for the different cases. Tbh I'm not sure if the check for function_exists means that there might be something else in the _controller attribute than what I wrote above.

I hope this helps a bit - thank you for your work!

[trigrab]

I tried to implement the type check here: https://github.com/reservix/opentelemetry-php-contrib/commit/bdec4e52ba7befb5cccb4396c7e91a26b8599c7c

Should I create a PR with that patch?

[bobstrecansky]

Please and thank you!