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

FPM worker output not going to logs #202

Closed
3 tasks
beequeue opened this issue Mar 16, 2017 · 18 comments
Closed
3 tasks

FPM worker output not going to logs #202

beequeue opened this issue Mar 16, 2017 · 18 comments

Comments

@beequeue
Copy link

beequeue commented Mar 16, 2017

With reference to #103, we've recently encountered the same issue and are considering using a forked buildpack to accomplish app log capture across our PHP CF deployments, which isn't ideal from a maintenance point of view.

Given that logging to stdout is fairly idiomatic when dealing with cloud environments (e.g. https://12factor.net/logs), is there any appetite to default the catch_workers_outputphp-fpm.conf setting to yes?

Alternatively, could some configuration be introduced to specifically target this setting which would mitigate the need to fully replace the php-fpm.conf file?

Steps to reproduce are the same as per #103.


What version of Cloud Foundry and CF CLI are you using? (i.e. What is the output of running cf curl /v2/info && cf version?

  • Cloud Foundry version is v241
  • CLI version 6.11.3

What version of the buildpack you are using?
4.3.15

If you were attempting to accomplish a task, what was it you were attempting to do?
View application logs as output by the app to stdout

What did you expect to happen?
Application logs to appear in aggregated logging

What was the actual behavior?
Application logs are not captured

Please confirm where necessary:

  • I have included a log output
  • My log includes an error message
  • I have included steps for reproduction
@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/141848233

The labels on this github issue will be updated when the story is started.

@lbayerl
Copy link

lbayerl commented Mar 16, 2017

+1. We're currently as well fully replacing the php-fpm.conf just to set this one value as it's required by many Cloud Foundry PaaS to fully support the built-in logging. Would be great to set it by default or make it configurable!

@dwytrykus
Copy link

+1. Same here: fully replacing php-fpm.conf just to set catch_workers_output to yes

@dmikusa
Copy link

dmikusa commented Mar 16, 2017

@sclevine - This is your call. It has come up before.

#103

Info about the property, which does have trade-offs (i.e. can affect peformance, not inline with FastCGI spec).

; Redirect worker stdout and stderr into main error log. If not set, stdout and
; stderr will be redirected to /dev/null according to FastCGI specs.
; Note: on highloaded environement, this can cause some delay in the page
; process time (several ms).
; Default Value: no
;catch_workers_output = yes

If we don't want to change the default, we should definitely add an easy way to set the value of this property without needing to override the whole file. I don't think it would be too hard to control that via env variable.

@lbayerl & @dwytrykus - If we're unable to change the default, would being able to set the env variable FPM_CATCH_WORKERS_OUTPUT to yes and have that update the php-fpm config file be an OK solution?

@dwytrykus
Copy link

@dmikusa-pivotal Yes, absolutely fine if I can set it via env variable.

@beequeue
Copy link
Author

Same for us, an env var would be grand.

@lbayerl
Copy link

lbayerl commented Mar 16, 2017

great!

@sclevine
Copy link
Contributor

Story to add the env var suggested by Dan: https://www.pivotaltracker.com/story/show/141880113

@dmikusa
Copy link

dmikusa commented Mar 16, 2017

@lbayerl - @beequeue - @dwytrykus: Could you try this branch?

https://github.com/dmikusa-pivotal/php-buildpack/tree/catch_workers_output

It should let you turn this feature on via options.json.

Ex:

{
    "FPM_CATCH_WORKERS_OUTPUT": "yes"
}

Is that OK? Turns out that could be done with a config only change. If this is OK, I can submit a PR.

Thanks,

Dan

@dgodd
Copy link
Contributor

dgodd commented Mar 16, 2017

@dmikusa-pivotal That looks great, I'm happy to merge either the branch or PR when you are ready

@beequeue
Copy link
Author

@dmikusa-pivotal That looks great. Have tested with our setup and I'm seeing logs 👍

@lbayerl
Copy link

lbayerl commented Mar 31, 2017

@dmikusa-pivotal thanks for your efforts. unfortunately my test failed.

  1. having "FPM_CATCH_WORKERS_OUTPUT": "yes", in my options.json
  2. removing the conf file which was overwriting the old one
  3. setting buildpack: https://github.com/dmikusa-pivotal/php-buildpack.git in my manifest.yml

results in no logging.
I can see the file app/php/etc/php-fpm.conf with the following content on my remote PaaS:
;catch_workers_output = yes which looks like the standard version.

Did I do anything wrong how to test it? Thanks

,

@sclevine
Copy link
Contributor

Hi @lbayerl,

It appears that you aren't using the catch_workers_output branch that @dmikusa-pivotal suggested.

Try this in your manifest.yml:

buildpack: https://github.com/dmikusa-pivotal/php-buildpack#catch_workers_output

@beequeue
Copy link
Author

beequeue commented Apr 3, 2017

Sorry to chase but any idea when this might land in a release?

@sclevine
Copy link
Contributor

sclevine commented Apr 3, 2017

Hi @beequeue,

We're going to implement this more generally than the solution on @dmikusa-pivotal's branch. Follow https://www.pivotaltracker.com/story/show/141979329 for details.

@lbayerl
Copy link

lbayerl commented May 4, 2017

After successfully testing the catch_worker_output configuration as the new solution is implemented in version 4.3.32 I vote for closing this issue.

@sclevine
Copy link
Contributor

sclevine commented May 5, 2017

Agreed, thanks!

@beequeue
Copy link
Author

Thank you to all those who worked on this, it's a great help to us 👍

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

No branches or pull requests

7 participants