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

allow configuring additional log appenders #139

Merged
merged 3 commits into from
Nov 26, 2019

Conversation

automaticserver
Copy link
Contributor

Add the ability to configure additional appenders in log4j.properties, useful e.g. for getting Zookeeper to log to syslog.

Copy link
Owner

@deric deric left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM, just add the .sort calls.

@@ -65,3 +65,9 @@ log4j.appender.TRACEFILE.File=${zookeeper.tracelog.dir}/${zookeeper.tracelog.fil
log4j.appender.TRACEFILE.layout=org.apache.log4j.PatternLayout
### Notice we are including log4j's NDC here (%x)
log4j.appender.TRACEFILE.layout.ConversionPattern=%d{ISO8601} [myid:%X{myid}] - %-5p [%t:%C{1}@%L][%x] - %m%n
<% @extra_appenders.each_pair do |name, data| -%>
Copy link
Owner

Choose a reason for hiding this comment

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

It might be a good idea to add a .sort here, in order to avoid unnecessary changes each puppet run.

@@ -65,3 +65,9 @@ log4j.appender.TRACEFILE.File=${zookeeper.tracelog.dir}/${zookeeper.tracelog.fil
log4j.appender.TRACEFILE.layout=org.apache.log4j.PatternLayout
### Notice we are including log4j's NDC here (%x)
log4j.appender.TRACEFILE.layout.ConversionPattern=%d{ISO8601} [myid:%X{myid}] - %-5p [%t:%C{1}@%L][%x] - %m%n
<% @extra_appenders.each_pair do |name, data| -%>

<% data.each_pair do |key, value| -%>
Copy link
Owner

Choose a reason for hiding this comment

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

Same here, data.sort.each_pair is a preferred way

@deric
Copy link
Owner

deric commented Nov 26, 2019

Looks good, thanks!

@deric deric merged commit 05f1a4b into deric:master Nov 26, 2019
cegeka-jenkins pushed a commit to cegeka/puppet-zookeeper that referenced this pull request Sep 7, 2020
* allow configuring additional log appenders
* sort() produces an array of arrays, so we need to replace each_pair() with map()
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

Successfully merging this pull request may close these issues.

3 participants