Add SLF4J MDC support for JUL #79
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
|
| if (copyOfContextMap != null ) { | ||
| return copyOfContextMap; | ||
| } | ||
| return new HashMap<String, String>(); |
There was a problem hiding this comment.
return Collections.emptyMap()
|
|
||
| @Override | ||
| public Map<String, String> getMDC() { | ||
| return new HashMap<String, String>(); |
There was a problem hiding this comment.
return Collections.emptyMap()
There was a problem hiding this comment.
Done. I somehow assumed that this was not possible as I first had return Objects.requireNonNullElse(copyOfContextMap, Collections.emptyMap()); but maven wouldn't compile that.
On a side note, is it intended that maven compiles to java 6 (legacy support or something else)? My idea hints me towards code style improvements that don't compile. e.g. multi catch blocks like this: catch (Exception | LinkageError e) { return Unavailable.INSTANCE; }
jul-ecs-formatter/src/main/java/co/elastic/logging/jul/MdcSupplier.java
Outdated
Show resolved
Hide resolved
…ng MDC in some JVM implementations.
Pull request for issue: #78.
This pull requests adds mdc support for JUL which is useful for legacy applications that want to use APM.
Before merging please review the MdcSupplier interface. I implemented it similar to the
ObjectMessageJacksonSerializerinterface but I noticed that theObjectMessageJacksonSerializerdoes not have unit tests for when there is no Jackson Objectmapper on the classpath. Currently this is similar to my implementation ofMdcSupplierwhere theMdcSupplier.Unavailable, which is used if slf4j is not on the classpath, is not tested with unit tests. I am not really sure how to proceed with this issue.