dfa1 commented on a change in pull request #770:
URL: https://github.com/apache/logging-log4j2/pull/770#discussion_r828802299
##########
File path:
log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jMarkerFactory.java
##########
@@ -113,26 +125,19 @@ public boolean exists(final String name) {
return markerMap.containsKey(name);
}
- /**
- * Log4j does not support detached Markers. This method always returns
false.
- * @param name The Marker name.
- * @return {@code false}
- */
@Override
public boolean detachMarker(final String name) {
- return false;
+ if (name == null) {
+ return false;
+ }
+ detachedMarkers.add(name);
+ return true;
}
- /**
- * Log4j does not support detached Markers for performance reasons. The
returned Marker is attached.
- * @param name The Marker name.
- * @return The named Marker (unmodified).
- */
@Override
public Marker getDetachedMarker(final String name) {
- LOGGER.warn("Log4j does not support detached Markers. Returned Marker
[{}] will be unchanged.", name);
- return getMarker(name);
+ final org.apache.logging.log4j.Marker log4jMarker =
MarkerManager.getMarker(name);
Review comment:
@carterkozak thanks for the review! :)
Yes, this is exactly the point: *I tried to keep Log4j unaware of detached
markers*.
Since Log4j doesn't support detached markers, while SLF4J does, the proposed
implementation is wrapping a Lo4j's marker (that is cached internally as you
mentioned) with a fresh instance of SLF4J's marker.
Unless I'm missing something big, there are also few newly written unit
tests that can be used to disproof my understanding of the problem.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
