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

Log to stderr by turning off the log file collector #968

Merged
merged 1 commit into from
Jun 6, 2023

Conversation

jrafanie
Copy link
Member

@jrafanie jrafanie commented Jun 2, 2023

Fixes ManageIQ/container-postgresql#40

Drop the no longer used options for logging to a file.

Related: CrunchyData/postgres-operator#1141 (comment)

Technically, it's an enhancement, but you can't look at pg logs in podified without visiting the pg pod so users would probably say this is a bug.

EDIT: We saw some possibly no longer used/wrong settings so I opened an issue to start that discussion: #969

@jrafanie jrafanie added the bug label Jun 2, 2023
@jrafanie jrafanie requested review from Fryguy and bdunne June 2, 2023 18:35
@miq-bot
Copy link
Member

miq-bot commented Jun 2, 2023

Checked commit jrafanie@9eaf5da with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint
0 files checked, 0 offenses detected
Everything looks fine. 🍪

@@ -38,8 +38,10 @@ hot_standby = on
# ERROR REPORTING AND LOGGING
#------------------------------------------------------------------------------

log_filename = 'postgresql.log'
log_rotation_age = 0
log_destination = 'stderr'
Copy link
Member Author

Choose a reason for hiding this comment

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

Technically, this is the default... but I added stderr to ensure this is explicit.

Copy link
Member

Choose a reason for hiding this comment

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

Any reason not to use stdout?

Copy link
Member

Choose a reason for hiding this comment

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

That's not an option in the settings: https://www.postgresql.org/docs/current/runtime-config-logging.html#RUNTIME-CONFIG-LOGGING-WHERE

Ultimately it's probably preferable to use jsonlog, but then I'm not sure you can do jsonlog and do to stderr.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not an option (stderr, csvlog, jsonlog, and syslog) according to https://www.postgresql.org/docs/current/runtime-config-logging.html#RUNTIME-CONFIG-LOGGING-WHERE. Perhaps it's to simplify things as log_collector "on" would need to redirect both stdout and stderr to logs.

log_collector

This parameter enables the logging collector, which is a background process that captures log messages sent to stderr and redirects them into log files

Copy link
Member Author

Choose a reason for hiding this comment

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

@Fryguy you beat me to it

log_rotation_age = 0
log_destination = 'stderr'
# This is used when logging to stderr:
logging_collector = off
Copy link
Member Author

Choose a reason for hiding this comment

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

This is what actually turns off logging to files and logs to stderr.

Comment on lines 46 to 47
log_connections = on
log_disconnections = on
Copy link
Member

Choose a reason for hiding this comment

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

Should we drop these? It seemed very chatty in the demo env.

Copy link
Member Author

Choose a reason for hiding this comment

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

Possibly? But, not here... I opened #969 to track the autovacuum settings but we can certainly review them all to see if we need those logged.

@bdunne bdunne merged commit be93f41 into ManageIQ:master Jun 6, 2023
@jrafanie jrafanie deleted the log_to_stderr branch June 6, 2023 21:26
@Fryguy
Copy link
Member

Fryguy commented Jun 7, 2023

Backported to petrosian in commit 881c4fb.

commit 881c4fbe64b85e373c452038984e557a51b2756a
Author: Brandon Dunne <[email protected]>
Date:   Tue Jun 6 17:25:28 2023 -0400

    Merge pull request #968 from jrafanie/log_to_stderr
    
    Log to stderr by turning off the log file collector
    
    (cherry picked from commit be93f4155e4c2e09fbad85e10258cbbe94ee2860)

Fryguy pushed a commit that referenced this pull request Jun 7, 2023
Log to stderr by turning off the log file collector

(cherry picked from commit be93f41)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

postgresql logs should go to STDOUT as well
4 participants