Log4j config with module

[jyaworski]

This is a WIP to solve #85.

Tag @wcooley

[liamjbennett]

This seems to be ok. It just needs to be squashed and pass tests.

[liamjbennett]

As a bit of a side note, I'm slightly concerned with the growing number of parameters. Might need to raise another issue about that.

[jyaworski]

@liamjbennett I'll get on that.

Re: params number, I think that's the sign of a healthy and extensible module. Have you looked at https://github.com/jfryman/puppet-nginx? The curse of being easily-modifiable is a lot of code it seems.

[jyaworski]

Tests fail because the module expects the file to be XML. I wonder if this will be harder than I thought. Rundeck may be using a nonstandard config.

[wcooley]

Tests fail because the module expects the file to be XML.

Yuck. I didn't notice that danielgil/log4j was only for the XML-formatted log4j.

I wonder if this will be harder than I thought.

Not necessarily; the Java properties format is much simpler than XML and, I suspect, the puppetlabs/inifile should suffice.

Rundeck may be using a nonstandard config.

No, Java properties like this are very common (possibly more common than XML, for logging, at least).

As far as the interface goes, I was thinking of something more like a few defined types, simliar to the log4j::logger type (but with path coming from something like $rundeck::config::log4j_properties_file).

rundeck::log4j::logger { 'org.codehaus.groovy.grails':
  level => 'info',
  appender => ['stdout', 'server-logger'],
  additivity => false,
}

I would probably also have a generic appender config that used the 3rd-level component (e.g., "cmd-logger" in "log4j.appender.cmd-logger.file") as the title and the 4th-level as parameters (with a special "classname" or "appenderclass" or "appender" for the appender Java class name, since that's otherwise at the 3rd level; I'm uncertain about using "class" as a parameter name).

$logdir = $rundeck::config::service_logs_dir # lazy abbrev
rundeck::log4j::appender { 'server-logger':
  classname => 'org.apache.log4j.DailyRollingFileAppender',
  file => "${logdir}/rundeck.log",
  datePattern => "'.'.yyyy-MM-dd",
  ...
}

Given that almost all of the default appenders have basically the same configuration, aside from file and ConversionPattern, it probably makes sense to make all that the default.

You might even start with a generic rundeck::log4j::setting defined type that's just a wrapper around ini_setting, to set the path to $rundeck::config::log4j_properties_file, section to "" ("global"-no section) and to declare the dependency/notification relationships, so that the other defines just have to provide the key/setting and value:

rundeck::log4j::setting { 'log4j.rootLogger':
  value => 'warn, stdout, server-logger',
}

(I started calling this rundeck::log4j::config, but thought that might make it confusing with rundeck::config; setting also maps it more closely to the ini_setting parameter.)

I probably wouldn't have anything in rundeck::config or rundeck, aside maybe from managing $rundeck::config::log4j_properties_file as a File resource (even that I am hesitant about). My opinion is that, since the distributed file works without changes, it should be let alone unless the user specifically requests it be changed. Making as few changes to mainline as possible makes support easier for everyone, unless the vendor can't be trusted to no break things.

(Wow, didn't quite mean for this to go on so long; I hope this isn't too much.)

[jyaworski]

@wcooley I'm quite against making rundeck manage log4j itself; using an external module, even if it's complex, would be preferable. I would rather be ready for the Rundeck devs to pivot to shipping an XML format, or the log4j devs changing their formatting, than to update some custom logic.

[igalic]

+1

[jyaworski]

Closing as I don't have time to work on this at the moment.