brefphp / bref

Serverless PHP on AWS Lambda
https://bref.sh
MIT License
3.15k stars 365 forks source link

PHP-FPM is adding a prefix warning to stderr #432

Closed tlfbrito closed 4 years ago

tlfbrito commented 5 years ago

According to https://github.com/docker-library/php/issues/207 this is a known issue, but in PHP 7.3 seems to have a solution according to https://www.php.net/ChangeLog-7.php#7.3.0 using decorate_workers_output.

Motivation:

This prefix will make very difficult to process the log entry and correctly parse the log entry.

Example:

php_1  | [26-Aug-2019 16:26:59] WARNING: [pool default] child 6 said into stderr: "[2019-08-26 16:26:59] app.INFO: Response 200 for "GET /ping" {"method":"GET","path":"","uri":"/ping","content-type":null,"latency":708,"client-ip":"172.22.0.1","status_code":200,"user-agent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/76.0.3809.100 Safari/537.36"} []"
footballencarta commented 5 years ago

Is there a timeline for this? Will the decorate_workers_output option be changed upon release of PHP 7.4?

mnapoli commented 5 years ago

@footballencarta it is possible that the PHP 7.4 build forces us to define separate build processes per version. If we do that, then it's easier to solve this problem. But no timeline or no guarantee that this happens, unfortunately.

tlfbrito commented 4 years ago

Is there anything that we can do to avoid this on PHP 7.4? Maybe a extension layer?

mnapoli commented 4 years ago

I think this could be doable to fix that.

Here is where we import the config file:

https://github.com/brefphp/bref/blob/10aca10ddb5aeddbe584d4cab31bf8b5b49a3b99/runtime/layers/fpm/Dockerfile#L6

Here is the line that disables the prefix (currently commented out):

https://github.com/brefphp/bref/blob/b7db4c4bdf4da8727c5301b6f782154706e836ea/runtime/layers/fpm/php-fpm.conf#L20

What we could do :

alepieser commented 4 years ago

Hi everyone,

Any news about this issue ? Is there a timeline to solve it ?

Thanks

mnapoli commented 4 years ago

@alepieser nothing new AFAIK. Personally I haven't prioritized this issue over other things. Maybe someone else (or you) could try to implement a feature to solve that? I tried to add some pointers in my previous comment, that may help.

tlfbrito commented 4 years ago

I think this could be doable to fix that.

Here is where we import the config file:

https://github.com/brefphp/bref/blob/10aca10ddb5aeddbe584d4cab31bf8b5b49a3b99/runtime/layers/fpm/Dockerfile#L6

Here is the line that disables the prefix (currently commented out):

https://github.com/brefphp/bref/blob/b7db4c4bdf4da8727c5301b6f782154706e836ea/runtime/layers/fpm/php-fpm.conf#L20

What we could do :

  • uncomment the line
  • COPY the config file in the Dockerfile
  • if PHP = 7.2, remove the line (command in the Dockerfile)

Let me know that you think @mnapoli : https://github.com/brefphp/bref/pull/577

alepieser commented 4 years ago

@tiagobrito I thought to propose the same solution. Thanks !