Fix #73886: Handle access log & error log on Docker

[mcq8]

Seems a bit odd to set the stdout to /dev/null making it unusable for logs.
I could not find any reason why it is like this other then the commit b172c7d and 4d005a8 where @tony2001 added the FPM sapi.

[krakjoe]

It is not clear to me that this is a good idea, I require someone intimately familiar with FPM to tell me it is.

[mcq8]

I definitely need to add a close if you use syslog. The same as in #1432 but for stdout.

[schmunk42]

I don't know about the fpm internals, but unclean logging is definitely a problem in a Docker setup.

[bukka]

I'm not sure if this can be really useful if you have got multiple running workers (multiple child process running at the same time). Then if they all log to the stdout, you won't be able to see which worker (which executed script) emitted the message. It might be ok if you address this in the app somehow and put it to the message yourself but still think it would be better to handle it in the master and have the option to differentiate it. Also you shouldn't do this for demonised master anyway (you would need to do it only if it's running in the foreground).

I think that would be much better to make children stdout and stderr data processing (when catch_workers_output enabled) a bit more configurable. It means address it when doing fpm_stdio_child_said though. For example having ability to point the stdout from the child to the specific place. I plan to look into this as I'm just refactoring zlog and one of the reason for that is also address issues with catch_workers_output. So I will take this into consideration and try to come up with something better.

[php-pulls]

Comment on behalf of bukka at php.net:

Closing as it is not correct because it can lead to the interleaving of output from the workers as there is no lock on stdout. The preferred solution will be done using pipe to master as noted above.

[jaylinski]

I plan to look into this as I'm just refactoring zlog and one of the reason for that is also address issues with catch_workers_output. So I will take this into consideration and try to come up with something better.

@bukka I know this was 4 years ago, but I still wanted to ask if you came up with a solution or a PoC. If not, I may give it a try, because I would find it really helpful if a dockerized php-fpm-container could log directly to stdout.

Related to docker-library/php#358 (comment).

[bukka]

@jaylinski No progress on this. To be honest I completely forgot about it. As it was said it will require some changes in fpm_stdio_child_said and will likely require creating of a second pipe. That will need to be configurable so it happens only if requested otherwise another fd should not be introduced. It might be a bit tricky though.

[pentago]

Just wanted to check if there's new motivation and enthusiasm to fix this?

My motivation to get this fixed is to be able to run rootless PHP-FPM Docker containers.
Yes, I know FPM can run with an arbitrary user but I'd like to avoid running the master process as root and to be able to log to both stdout/stderr.

If anyone knows a workaround, I'd be eternally grateful.

[jayaddison]

@bukka to consider an alternative viewpoint: there could be cases where interleaving of output to stdout is a known issue, and PHP-FPM is intentionally configured to use a single worker process and/or run non-daemonized.

The concerns do make sense, I think - but the limitation caused by redirecting the stdout file handle to /dev/null seems to cause issues that affect more environments than are necessary.

(it's a tricky one: perhaps as a prevention for misconfiguration it makes sense, but it's frustrating in cases where the limitation may not be relevant -- and the containerized configuration which redirects access logs to stderr is probably quite widely-distributed now, and seems like a kind of oddity)

[bukka]

It's an option but I just quickly looked into this and I think it should be doable using a secondary pipe. I bumped the priority of this a bit so I should hopefully manage to try to implement it before 8.3 freeze.

[jayaddison]

No hurry, honestly - it'd be valuable, but I think (almost? :)) everyone would prefer to see it implemented safely.

From a bit of digging into the history of php-fpm and FastCGI-SAPI, I wondered whether this part of the fastcgi spec could be the reason that this file handle is closed in the worker processes.

The standard descriptors STDOUT_FILENO and STDERR_FILENO are closed when the application begins execution.

That'd kinda make sense to me -- the parent process likely doesn't have to follow the same initialization requirements, and so perhaps that's why error_log (at the global level) is fine redirecting to stderr, whereas access.log (at the pool level) cannot easily redirect to stdout? A bit of a guess.

Even so, it wouldn't entirely explain why the code seems to (and has since 0.6.0.2?) close stdin and stdout but not stderr, though.

[bukka]

So there are two parts where stderr and stdout is handled. The one that is linked and was changed in this PR is for master process init which is then inherited by children. So here it closes the the stdout but then there is fpm_stdio_child_use_pipes where it closes stderr if it's not configured to catch worker output otherwise it will redirect both stderr and stdout to a pipe handled by master process. To separate it we would need a secondary pipe for stdout and then process it in the master process in a similar way like stderr. There also few more other things that would need to be done so it's not as simple as that but it should be doable.

[jayaddison]

Thank you for the explanation 👍