-
Notifications
You must be signed in to change notification settings - Fork 107
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
[Reverted] add logrotate config for pgbouncer and filebeat input #921 #1823
[Reverted] add logrotate config for pgbouncer and filebeat input #921 #1823
Conversation
enabled: true | ||
paths: | ||
- /var/log/pgbouncer/*.log* | ||
exclude_files: [".gz$"] | ||
{% endif %} |
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.
Please add a posibility to use multiline for pgbouncer as well. Dont forget about update docs. Thanks.
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.
Pgbouncer logs do not seem to generate multilines. Do you think it is neccessary anyways then?
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.
Hmm I don't know the structure of pgbouncer logs. It would be good to have this as an option (not enabled by default) as we already provided it for postgresql logs. So maybe let's involve someone else in discussion.
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.
I've checked on Ubuntu, there is no any multi-line entry in log, even after changing verbose = 0
to verbose = 2
.
core/src/epicli/data/common/defaults/configuration/postgresql.yml
Outdated
Show resolved
Hide resolved
IDK if someone is going to review that, but for me it's ok to add multiline support later when/if it will be necessary. |
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
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
core/src/epicli/data/common/ansible/playbooks/roles/filebeat/templates/filebeat.yml.j2
Show resolved
Hide resolved
copytruncate | ||
size 10M | ||
daily | ||
create 0640 pgbouncer pgbouncer |
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.
On Ubuntu we do not have such user:
id pgbouncer
id: ‘pgbouncer’: no such user
create 0640 pgbouncer pgbouncer | ||
nodateext | ||
postrotate | ||
/bin/kill -HUP cat /run/pgbouncer/pgbouncer.pid 2>/dev/null 2> /dev/null || true |
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.
On Ubuntu the pidfile path is different:
grep pidfile /etc/pgbouncer/pgbouncer.ini
pidfile = /var/run/postgresql/pgbouncer.pid
@@ -121,3 +121,16 @@ specification: | |||
# to have multiple unique filenames per day when dateext option is set | |||
dateformat -%Y%m%dH%H | |||
} | |||
/var/log/pgbouncer/pgbouncer.log { |
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.
On RHEL there is already existing configuration in /etc/logrotate.d/pgbouncer
so this one duplicates it.
Guise, pls take into account @to-bar 's review and create another PR with fixes. 🙄 |
This PR has to be rewritten so I decided to revert it. See comments. |
… (hitachienergy#1823) * add logrotate config for pgbouncer and filebeat input
… (hitachienergy#1823) * add logrotate config for pgbouncer and filebeat input
No description provided.