(SERVER-1069) add logback json encoder in jar file

[mfournier]

We're using this extension to logback to have puppet-server generate JSON-encoded logs, which makes them easier to process downstream (indexing into elasticsearch for instance).

This doesn't alter the way puppet-server works, it would only give the user more configuration options available in logback.xml and request-logging.xml.

I'm not sure this is the right way to submit changes to puppet-server. Should I also open an issue in JIRA ?

Should this get accepted, documentation/configuration.markdown should probably be amended too, to mention the additional logging capabilities.

[mfournier]

Not sure how this error relates to my change. Should I rebase on the master branch ?

lein test :only puppetlabs.services.jruby.jruby-pool-int-test/admin-api-flush-jruby-pool-test

FAIL in (admin-api-flush-jruby-pool-test) (jruby_pool_int_test.clj:140)
Flushing the pool results in all new JRuby instances
expected: (true? (verify-no-constants pool-context 4))
  actual: (not (true? false))

[rlinehan]

@mfournier I'm pretty sure that's a race condition in our tests, as I ran the tests locally off your branch and didn't encounter it. :/ I'll bump travis to try again.

Regarding process around submitting PRs... it might be a bit easier for our internal tracking if you create a SERVER ticket (and then I believe if you update the commit message/PR with (SERVER-<xyz>) at the beginning it might do some automagical jira/git integration), but especially for something not super big it's not a huge deal.

Thanks for the contribution! Just FYI, most of the team working on Puppet Server is out this week because it's American Thanksgiving, so you might not get more feedback right away on this, but we'll try to get back to you more on this soon.

[camlow325]

@mfournier One other option might be to just add the logstash jars to the classpath which is used by the Puppet Server Java process. Currently, the ability to do this is a bit scattered. We have a ticket in our backlog that would hopefully allow for this to be done in a more consistent way across packaging systems - see https://tickets.puppetlabs.com/browse/SERVER-249. For now, you'd basically want to look for where the "-cp" argument is defined in the package init scripts - which differs based on the OS/packaging mechanism. For packages built to work with systemd, I'd expect to see that in the ExecStart directive. For that you could append the logback jars to the -cp argument - for example, ${INSTALL_DIR}/puppet-server-release.jar:/path/to/logstash1.jar:.... Does this make sense and would it work for your use case? What would you think of this approach?

[cprice404]

I think sometime before long we'll probably be integrating this library into puppet-server:

https://github.com/puppetlabs/structured-logging/blob/master/project.clj

At that point we'd have a dependency on the net.stash.logback/logstash-logback-encoder library anyway, so I think it'd probably be OK to go ahead introduce that dependency now if it will help other use cases. I'm not quite clear on what the other dependency brings to the table that isn't available in the logstash-logback-encoder, though?

[mfournier]

Sorry for leaving this aside for so long !

I just took a couple of minutes to test what "logstash-logback-layout" brings. And it appears it isn't required. I think I got confused by part of logstash-logback-encoder's documentation, which mention logback "layouts", but this is all bundled in the "logstash-logback-encoded" jar.

Thanks for paying attention to this @cprice404 !

@camlow325, thanks for the advice ! This is how we're finally doing it on our puppet-server setup. It turns out to be less complicated to maintain for us than having our custom build of puppet-server.

So FYI I just rebased my patch against current stable branch + dropped the superfluous library + added a reference to the JIRA issue.

We've been running with this in production for a few weeks now, so far so good.

[cprice404]

@mfournier ping! Left a message on the Jira ticket; I'm OK with merging this but would just like to consider adding a blurb to the docs explaining how people could use it. We can handle that on our end, but maybe you can give us a few pointers on the Jira ticket?