usabilla / php-docker-template

Docker images for PHP applications, CLI and FPM with shared socket
https://hub.docker.com/r/usabillabv/php
MIT License
177 stars 15 forks source link

Make FPM logging format configurable #153

Closed WyriHaximus closed 4 years ago

WyriHaximus commented 4 years ago

By making the logging format configurable users of this image can use their own desired format such as JSON, CSV, or something else.

Usabilla PHP Docker Template

Closes: #152 Reviewers: @usabilla/oss-docker

Type

Please specify the type of changes being proposed:

Q A
Documentation? yes
Dockerfile change? yes
Build feature? no
Apply CVE Patch? no
Remove CVE Patch? no

Changelog

WyriHaximus commented 4 years ago

Opened an issue with hadolint for the linting error: https://github.com/hadolint/hadolint/issues/428

WyriHaximus commented 4 years ago

Opened an issue with hadolint for the linting error: hadolint/hadolint#428

Looks like hadolint is getting a new Dockerfile parser and the linting issue this PR has will be solved by it. E.T.A. for the parser is somewhere this week according to https://github.com/hadolint/language-docker/pull/53#issuecomment-634653154

WyriHaximus commented 4 years ago

Where do we centralize this in Usabilla-land?

I have some ideas, will open an internal issue about it.

WyriHaximus commented 4 years ago

@renatomefi the bug is in the Dockerfile parser, AFAIK there is no ignore for that

renatomefi commented 4 years ago

@renatomefi the bug is in the Dockerfile parser, AFAIK there is no ignore for that

Uh fair, is there any scaping we could do that won't change the effective value?

WyriHaximus commented 4 years ago

@renatomefi the bug is in the Dockerfile parser, AFAIK there is no ignore for that

Uh fair, is there any scaping we could do that won't change the effective value?

@renatomefi the irony here is that \" inside the " is the cause of the parser breaking. Will look into that.

WyriHaximus commented 4 years ago

@renatomefi sooo funny story, the current format might be correct in the code, it doesn't translate correctly into the FPM conf. And as far as escaping goes, it's complicated. We can always consider consider doing a small BC break and drop the "'s

WyriHaximus commented 4 years ago

I know the hadolint docker language author said they'd release your patch somewhere last week, but given the cadence of the project I have a feeling it might take much longer, do we have a workaround?

@renatomefi they just released a new version with the new parser 🎉🎉🎉

WyriHaximus commented 4 years ago

@renatomefi yup, also managed to get the correct escaping in the Dockerfile, one of the culprits was not putting the format between quotes in the FPM pool .conf 😂 . Will fiddle with the example a bit, and come up with a working and tested JSON example. Not sure how to unit test the output of FPM yet

WyriHaximus commented 4 years ago

Updated it with a working JSON example and also added an example log line to the commit message.

{"cpu_usage":84.03,"memory_usage":8388608,"duration_microsecond":0.179,"script":"/opt/project/public/index.php","content_length":0,"request_method":"GET","pool_name":"www","process_id":"11","request_query_string":"key=value","request_uri_query_string_glue":"?","request_uri":"/index.php","request_url":"/index.php?key=value","remote_ip_address":"127.0.0.1","response_status_code":200,"time":"07/Jun/2020:21:24:20 +0000","remote_user":""}