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

fix(inputs.phpfpm): Add timeout for fcgi #15036

Merged
merged 3 commits into from
Mar 22, 2024

Conversation

PierreF
Copy link
Contributor

@PierreF PierreF commented Mar 21, 2024

Summary

The input PHP-FPM don't have timeout when talking (F)CGI protocol, which could lead to input blocked forever.

The timeout exists and was used for HTTP, but not for fcgi.

Checklist

  • No AI generated code was used in this PR

Related issues

resolves #15035

@PierreF PierreF force-pushed the phpfpm-fcgi-timeout branch 2 times, most recently from 6cb1744 to 668fb06 Compare March 21, 2024 10:54
Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PierreF thanks for fixing this issue! Could you please add a unit-test for the timeout?!?

@srebhan srebhan self-assigned this Mar 21, 2024
@srebhan srebhan changed the title Add timeout to Phpfpm using fcgi fix(inputs.phpfpm): Add timeout for fcgi Mar 21, 2024
@telegraf-tiger telegraf-tiger bot added fix pr to fix corresponding bug plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins labels Mar 21, 2024
Use the timeout of php-fpm with fgci protocol

Signed-off-by: Pierre Fersing <pierre.fersing@bleemeo.com>
@PierreF PierreF force-pushed the phpfpm-fcgi-timeout branch from 668fb06 to 449e44c Compare March 21, 2024 12:53
@PierreF
Copy link
Contributor Author

PierreF commented Mar 21, 2024

I've added the test in the same commit (I'm unsure if you prefer a single commit with code & test or if you prefer avoid push force).
The error seems unrelated to my change: it's on syslog input and phpfpm input test passed.

Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments regarding the unit-test...

plugins/inputs/phpfpm/phpfpm_test.go Outdated Show resolved Hide resolved
plugins/inputs/phpfpm/phpfpm_test.go Show resolved Hide resolved
plugins/inputs/phpfpm/phpfpm_test.go Outdated Show resolved Hide resolved
plugins/inputs/phpfpm/phpfpm_test.go Outdated Show resolved Hide resolved
PierreF and others added 2 commits March 21, 2024 15:44
Co-authored-by: Sven Rebhan <36194019+srebhan@users.noreply.github.com>
@telegraf-tiger
Copy link
Contributor

Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Thanks @PierreF!

@srebhan srebhan added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Mar 21, 2024
@srebhan srebhan assigned powersj and DStrand1 and unassigned srebhan Mar 21, 2024
@powersj powersj merged commit 4c1aa59 into influxdata:master Mar 22, 2024
26 checks passed
@github-actions github-actions bot added this to the v1.30.1 milestone Mar 22, 2024
@PierreF PierreF deleted the phpfpm-fcgi-timeout branch March 22, 2024 12:51
powersj pushed a commit that referenced this pull request Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix pr to fix corresponding bug plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PHP-FPM input never timeout with FCGI protocol
4 participants