-
Notifications
You must be signed in to change notification settings - Fork 516
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 logging to stdout #1489
Fix logging to stdout #1489
Conversation
Allow for setting logFile to /dev/stdout for logging to stdout. Log settings are set directly in the mongodb-agent comment instead of passing through the environment (which is not the most secure way).
Let me assign this to @lsierant for review |
Thanks! |
Hi @lsierant , Please don't hesitate to reach out to me if you need some extra info or if you would like some update to the PR. I'm happy to work with you to find a solution for this as it is blocking our upgrades to newer mongo versions. Thanks |
Noticed the unit test failed. I probably overlooked something there. I will update the PR
|
Updated unit test. Instead of testing for the env variables it now test for the construction of the agent command. |
Sorry. Missed the env count check, should be all green now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM and thanks for your contribution!
// To keep consistent with old behavior not setting the logFile in the config does not log to stdout but keeps | ||
// the default logFile as defined by DefaultAgentLogFile. Setting the logFile explictly to "/dev/stdout" will log to stdout. | ||
agentLogOptions := "" | ||
if logFile != "/dev/stdout" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From a reading perspective I prefer the inverse:
if logFile == ...{}
else:
{}
@legal90 there are plans to release in about 2-3 weeks |
Any date in sight for this release? |
Hi @nammn Any update on when this is going to be released, we need this update to be able to use the latest version of the app? Thanks |
@jlestrange @uttammeena @legal90 @laiminhtrung1997 sorry for the late reply and release, but its out now! |
Dear @nammn @schans |
|
Allow for setting logFile to /dev/stdout for logging to stdout. Log settings are set directly in the mongodb-agent comment instead of passing through the environment (which is not the most secure way).
All Submissions:
closes #XXXX
in your comment to auto-close the issue that your PR fixes (if such).