Skip to content

PHP-FPM : Option to suppress "/ping" from logs -- patch file given #8174

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

Closed
Julien75013 opened this issue Mar 6, 2022 · 3 comments
Closed

Comments

@Julien75013
Copy link

Description

SysAdmin and DevOps don't need to have logs when healthcheck are set up for checking the availability.

To understand why this is relevant, see how today's softwares performs in many high-load environments.

Apache permit full conditionnal logs :

https://httpd.apache.org/docs/trunk/en/logs.html#conditional

Haproxy permit some conditionnal logs :

Haproxy - Options dontlog-normal / dontlognull :
https://cbonte.github.io/haproxy-dconv/2.4/configuration.html#4.2-option%20dontlog-normal
https://cbonte.github.io/haproxy-dconv/2.4/configuration.html#4.2-option%20dontlognull
Haproxy - Disabling logging of successful connections :
https://cbonte.github.io/haproxy-dconv/2.4/configuration.html#chapter-8.3.3

The actual saving of data space could be several megas, for each host. For thousands of servers, it can be a significant saving.

$> tail -n 5 /var/log/php8/pings/access.log
- -  27/Nov/2021:02:12:57 +0100 "GET /ping" 200 - 0.238 2048 0.00%
- -  27/Nov/2021:02:12:58 +0100 "GET /ping" 200 - 0.309 2048 0.00%
- -  27/Nov/2021:02:13:01 +0100 "GET /ping" 200 - 0.258 2048 0.00%
- -  27/Nov/2021:02:13:04 +0100 "GET /ping" 200 - 0.190 2048 0.00%
- -  27/Nov/2021:02:13:04 +0100 "GET /ping" 200 - 0.166 2048 0.00%

$> gunzip -c /var/log/php8/pings/access.log.*.gz | wc -l
2838920

$> du -h /var/log/php8/pings/
32M     /var/log/php8/pings/

So, we need an option to disable logs when the "/ping" path is reached.

; This directive is used to apply (or prevent) the "GET /ping" to be logged.
; Directive "ping.path" have to be set, before this setting to be effective.
; Default Value: yes
;ping.dontlog = yes

Main thread from bugs.php.net : https://bugs.php.net/bug.php?id=80428
Patch file was issued in 2020 but has never been seen : php-fpm-ping_dontlog.txt

@cmb69
Copy link
Member

cmb69 commented Mar 7, 2022

Patch file was issued in 2020 but has never been seen

Consider to submit a pull request for better visibility.

Julien75013 added a commit to Julien75013/php-src that referenced this issue Mar 9, 2022
maaarghk added a commit to maaarghk/php-src that referenced this issue Mar 21, 2022
maaarghk added a commit to maaarghk/php-src that referenced this issue Mar 29, 2022
maaarghk added a commit to maaarghk/php-src that referenced this issue Mar 29, 2022
maaarghk added a commit to maaarghk/php-src that referenced this issue Apr 6, 2022
maaarghk added a commit to maaarghk/php-src that referenced this issue Jul 1, 2022
maaarghk added a commit to maaarghk/php-src that referenced this issue Jul 4, 2022
Adds a setting "access.suppress_path" to php-fpm pool configurations
which causes successful GET requests to the specified URIs to be
excluded from the access log. This is to reduce noise caused by
automated health checks.

Requests with response codes outwith the successful range 200 - 299,
requests made with query parameters and requests which have a
Content-Length other than 0 will ignore this setting as a security
precaution.

Closes phpGH-8174, #80428 [1]

[1] https://bugs.php.net/bug.php?id=80428
@bukka bukka closed this as completed in 327bb21 Jul 10, 2022
@xepozz
Copy link

xepozz commented Jul 12, 2022

@bukka could you apply that patch for 8.1 branch and release it?

@bukka
Copy link
Member

bukka commented Jul 13, 2022

@xepozz 8.1 is just for bug fixes and this is a feature so it cannot be applied there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants