Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP error_log configuration is missing #662

Closed
mxr576 opened this issue Jan 18, 2023 · 6 comments
Closed

PHP error_log configuration is missing #662

mxr576 opened this issue Jan 18, 2023 · 6 comments

Comments

@mxr576
Copy link

mxr576 commented Jan 18, 2023

error_log configuration is not set in PHP images. It should be set to /proc/self/fd/2 like in the official PHP image.
This is basically the default configuration for 12fa PHP application.

@rocketeerbkw
Copy link
Member

I was doing some research around this and I'm seeing conflicting information. I see issues saying this needs to be added in order for docker to get error logs, but also some issues (admittedly old ones) are saying this won't work for php-fpm because of child process log mixing.

It would be helpful to get some more information so I don't waste time looking at outdated info. Is something not working that making this change would fix? Do you have any other background on why it should be added other than the upstream image has it?

@mxr576
Copy link
Author

mxr576 commented Jan 19, 2023

This is the most extensive guide that I have found related to this topic: https://rtfm.co.ua/en/linux-php-fpm-docker-stdout-and-stderr-no-an-applications-error-logs/

This comment is also fairly detailed about related configuration bits: docker-library/php#878 (comment)

but also some issues (admittedly old ones) are saying this won't work for php-fpm because of child process log mixing.

Maybe you have read this in GH issues or in the Stackoverflow post but it looks like all problems have been solved around these with PHP 8.0 (with decorate_workers_output = no) and catch_workers_output = yes.

My ultimate goal with this change is to see fatal errors at least in container logs on Kibana when the application crashes and not even Monolog was booted that could have distributed log messages to Logtash or other places.

And for the record, what triggered this change request is that Monolog failed to send a log to Logstash due to some networking issues therefore it got tracked nowhere... So even if Monolog is working, other issues (like I/O, and networking) could happen but we cannot lose logs. So my idea is using setting up error_log() as an exception handler in Monolog (see related PR to ensure logs are caught and available in Kibana (via container logs) even if Logstash connection failed. (Not to mention after this change, we could also see if a Monolog handler regularly fails to track logs.)

@mxr576
Copy link
Author

mxr576 commented Jan 19, 2023

In addition, Symfony's logging doc also says:

In the prod environment, logs are written to STDERR PHP stream, which works best in modern containerized applications deployed to servers without disk write permissions.

https://symfony.com/doc/current/logging.html#where-logs-are-stored

@rocketeerbkw
Copy link
Member

I had some time to look into this some more. It looks to me that everything is already configured correctly? When I test our images with php code that throws errors, everything is getting logged correctly for a docker architecture.

I also checkout our upstream images and it's already being set as you suggested https://github.com/docker-library/php/blob/9e9fd53ae5107ce1904e9bb6b428b93b9519037c/8.2/alpine3.16/fpm/Dockerfile#L230.

Are there specific error that you aren't seeing in the logs? Can you provide a reproducible test?

@rocketeerbkw
Copy link
Member

@mxr576 do you have any more info on this? I'll close it as not reproducible if not.

@rocketeerbkw
Copy link
Member

We think this is working as expected.

@rocketeerbkw rocketeerbkw closed this as not planned Won't fix, can't repro, duplicate, stale May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants