hipages / php-fpm_exporter

A prometheus exporter for PHP-FPM.
Apache License 2.0
604 stars 123 forks source link

why not catch SIGTERM signal? #81

Closed stanxing closed 4 years ago

stanxing commented 4 years ago

First, thanks for your contributions,this project is very perfect. But i hava a issue why SIGTERM signal are not captured in the code?

Int fact, in many management tools,they will send SIGTERM signal to notify process to shutdown, such as docker stop or kubectl delete pod and so on.

Can SIGTERM signal processing be add? I will be willing to submit a PR.

https://github.com/hipages/php-fpm_exporter/blob/master/cmd/server.go#L98

        // We'll accept graceful shutdowns when quit via SIGINT (Ctrl+C)
        // SIGKILL, SIGQUIT or SIGTERM (Ctrl+/) will not be caught.
        signal.Notify(c, os.Interrupt)
estahn commented 4 years ago

@stanxing Do you have any particular issues with the current behaviour of php-fpm_exporter? We're running php-fpm_exporter in our Kubernetes cluster and had no issues with signal handling.

Example:

go run main.go server --phpfpm.scrape-uri tcp://127.0.0.1:9031/status
INFO[0000] Starting server on :9253 with path /metrics
kill -SIGTERM 74192
[1]    74192 terminated  go run main.go server --phpfpm.scrape-uri tcp://127.0.0.1:9031/status
stanxing commented 4 years ago

Sure,the process will be terminated because of panic. This is default behavior to recive SIGTERM signal in golang. I think it should be gracefully shutdown when reciving SIGTERM

estahn commented 4 years ago

@stanxing I see. If you could contribute a pull request with the required changes that would be welcome.

stanxing commented 4 years ago

@estahn pull request linked

estahn commented 4 years ago

This has been released: https://github.com/hipages/php-fpm_exporter/releases/tag/v1.1.1