Dropwizard: Json Log File Appender

[jamisonhyatt]

We have a requirement of including json logs creation in production. I realize this has been brought up before, but I felt like this was a little bit of a different take on the issue.

I was looking for a simple way of implementing this in dropwizard, which was straight forward in prior versions, it seemed like less of a hack.

Unfortunately, it feels like the shared interface added in 1415 between access and event logging made adding your own appenders feel much more like a hack.

It is still manageable, but If I want to add a new file appender that does _not_ implement PatternLayout (e.g.,a JsonLayout) the previous changes force new implementations to ignore the LoggingFactory altogether, because we still need to meet the build() contract of the AppenderFactory because of the way DefaultLoggingFactory() is implemented.

The 2nd option is modifying LayoutFactory to implement a secondary build method, and changing some of the deeper return types from PatternLayout to Layout as downstream consumers don't actually need anything the higher level layouts offer. See the branch where I did this here.

Unfortunately, this brings up a secondary issue...where implementers of LayoutFactory must provide implementations for AccessEvent's even if there's no desire for that functionality. I believe this is 'solved' here, and will fail fast...but it's still a hack.

In short, I'm not certain tying all future appenders hooked in through the build() of DefaultLoggingFactory to a PatternLayout for both AccessEvents and LoggingEvents leaves much wiggle room for people to cleanly extend their logging solutions.

This is all assuming I didn't miss something glaring for easily implementing the JsonLayout without ignoring the factory.

[jplock]

I think we should ship with a JSON logger out of the box as this is definitely becoming more common with tools like the ELK stack. Any thoughts @ghenkes on how best to implement one and address @jamisonhyatt's concerns? /cc @dropwizard/committers

[nickbabcock]

Preferably, I would like to see if it could be done as a Dropwizard module (ie, not in core, (but core work may have to be done to make it extensible?)).

[ghenkes]

@jplock the easiest way is probably to change LayoutFactory to return a Layout, rather than a PatternLayoutBase. I would have done that originally, however all use cases within dropwizard used a PatternLayout and this will require casting to a PatternLayout in buildLayout in order to set the pattern like this:

if (!Strings.isNullOrEmpty(logFormat) && formatter instanceof PatternLayout) {
    ((PatternLayout) formatter).setPattern(logFormat);
}

[saem]

@jamisonhyatt out of curiosity have you got a newer solution, or which solution have you settled on for JSON logging?

[jamisonhyatt]

I haven't done any more work on the implementation; what's in my fork is
the current state. I was waiting for the 1.0 release, but since then I've
had other work that's a higher priority. I think it's at least a 4-5 hours
of design and refactor away from being ready for a review.

Feel free to take it and run with it.

On Wednesday, October 5, 2016, Saem Ghani [email protected] wrote:

@jamisonhyatt https://github.com/jamisonhyatt out of curiosity have you
got a newer solution, or which solution have you settled on for JSON
logging?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/dropwizard/dropwizard/issues/1451#issuecomment-251817361,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFJZITtmPxotNZsdcbfRPUeSTg0uTAGqks5qxCO5gaJpZM4HVAMT
.

-Jamison-

[sabarivasan]

I have taken JamisonHyatt's branch as a starting point and added support for ILoggingEvent and IAccessLog: https://github.com/dropwizard/dropwizard/compare/master...jamisonhyatt:AddJsonFileAppender?expand=1

I have created the following PR: https://github.com/dropwizard/dropwizard/pull/2180

Please take a look

[toadzky]

would anyone object to a config option that flattens the MDC into the root object? i opened a PR (#2293) that would enable this. it's not a big change.

[joschi]

Closed via #2232.