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

add net.logstash.log4j2.JSONEventLayoutV1 for log4j2 support #56

Closed
wants to merge 4 commits into from
Closed

add net.logstash.log4j2.JSONEventLayoutV1 for log4j2 support #56

wants to merge 4 commits into from

Conversation

michaelkuechler
Copy link

  • moved HostData to new package as it seemed misplaced with the new log4j2 package

- moved HostData to new package as it seemed misplaced with the new log4j2 package
@vsnguyen
Copy link

I noticed when using this. You'd loose the logging level because its always an empty map.

{"@timestamp":"2015-11-18T15:08:24.843Z","source_host":"CKSCLP008.local","level":{},.....}

@maartenbosteels
Copy link

Hi Michael,

Thanks for this !
I think I found an issue in your code: the log-level doesn't end up in my log-files: the key is present but the value is always an empty object, like this "level":{}

I tried to reproduce this with your unit tests (net.logstash.log4j2.JSONEventLayoutV1Test)
but it seem that these tests are actually using the log4j 1.x appender and layout.

Are you actively using your patch ?
Or did you find another solution ?

@michaelkuechler
Copy link
Author

Are you actively using your patch?

I was to, but then priorities shifted 😐 - so I guess one has to look at the issue sometime…

@maartenbosteels
Copy link

I fixed the issue with the log-level and discovered that your unit tests were using the log4j 1.x code.
So I removed the log4j 1.x code and fixed the unit tests as well
=> https://github.com/DNSBelgium/log4j-jsonevent-layout

@callumcodes
Copy link

Will this PR be actioned and released anytime soon? The DNSBelgium fork appears to work.

@cmpsoares91
Copy link

cmpsoares91 commented May 26, 2020

Hello,
Are these changes planned to be merged anytime soon?
Kind regards

@michaelkuechler
Copy link
Author

@cmpsoares91 as i wrote above: this is more than stale and AFAIR i never even ended up really using the code. so i consider this stale and probably outdated…

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.

5 participants