You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2022/08/27 09:35:07 UTC
[GitHub] [logging-log4j2] ppkarwasz commented on a diff in pull request #1022: SLF4J2
ppkarwasz commented on code in PR #1022:
URL: https://github.com/apache/logging-log4j2/pull/1022#discussion_r956561066
##########
log4j-slf4j20-impl/src/main/java/org/apache/logging/slf4j/Log4jMDCAdapter.java:
##########
@@ -27,57 +29,229 @@
*/
public class Log4jMDCAdapter implements MDCAdapter {
+ private final MDCAdapter adapterDelegate;
+
+ public Log4jMDCAdapter(boolean enableSlf4jStack) {
+ if (enableSlf4jStack) {
+ adapterDelegate = new ExtendedMDCAdaptor();
+ } else {
+ adapterDelegate = new StandardMDCAdaptor();
+ }
+ }
+
@Override
- public void put(final String key, final String val) {
- ThreadContext.put(key, val);
+ public void put(String key, String val) {
+ adapterDelegate.put(key, val);
}
@Override
- public String get(final String key) {
- return ThreadContext.get(key);
+ public String get(String key) {
+ return adapterDelegate.get(key);
}
@Override
- public void remove(final String key) {
- ThreadContext.remove(key);
+ public void remove(String key) {
+ adapterDelegate.remove(key);
}
@Override
public void clear() {
- ThreadContext.clearMap();
+ adapterDelegate.clear();
}
@Override
public Map<String, String> getCopyOfContextMap() {
- return ThreadContext.getContext();
+ return adapterDelegate.getCopyOfContextMap();
}
@Override
- @SuppressWarnings("unchecked") // nothing we can do about this, restricted by SLF4J API
- public void setContextMap(@SuppressWarnings("rawtypes") final Map map) {
- ThreadContext.clearMap();
- ThreadContext.putAll(map);
+ public void setContextMap(Map<String, String> contextMap) {
+ adapterDelegate.setContextMap(contextMap);
}
@Override
public void pushByKey(String key, String value) {
- // not implemented yet
+ adapterDelegate.pushByKey(key, value);
}
@Override
public String popByKey(String key) {
- // not implemented yet
- return null;
+ return adapterDelegate.popByKey(key);
}
@Override
public Deque<String> getCopyOfDequeByKey(String key) {
- // not implemented yet
- return null;
+ return adapterDelegate.getCopyOfDequeByKey(key);
}
@Override
public void clearDequeByKey(String key) {
- // not implemented yet
+ adapterDelegate.clearDequeByKey(key);
+ }
+
+ private static class StandardMDCAdaptor implements MDCAdapter {
+
+ @Override
+ public void put(final String key, final String val) {
+ ThreadContext.put(key, val);
+ }
+
+ @Override
+ public String get(final String key) {
+ return ThreadContext.get(key);
+ }
+
+ @Override
+ public void remove(final String key) {
+ ThreadContext.remove(key);
+ }
+
+ @Override
+ public void clear() {
+ ThreadContext.clearMap();
+ }
+
+ @Override
+ public Map<String, String> getCopyOfContextMap() {
+ return ThreadContext.getContext();
+ }
+
+ @Override
+ public void setContextMap(final Map<String, String> map) {
+ ThreadContext.clearMap();
+ ThreadContext.putAll(map);
+ }
+
+ @Override
+ public void pushByKey(String key, String value) {
+ // NOP
+ }
+
+ @Override
+ public String popByKey(String key) {
+ // NOP
+ return null;
+ }
+
+ @Override
+ public Deque<String> getCopyOfDequeByKey(String key) {
+ // NOP
+ return null;
+ }
+
+ @Override
+ public void clearDequeByKey(String key) {
+ // NOP
+ }
+ }
+
+ /**
+ * Wires the top of the stack to the MDC current value
+ */
+ private static final class ExtendedMDCAdaptor extends StandardMDCAdaptor {
+
+ private final ThreadLocalMapOfStacks threadLocalMapOfDeques = new ThreadLocalMapOfStacks();
+
+ @Override
+ public void clear() {
+ super.clear();
+ threadLocalMapOfDeques.clear();
+ }
+
+ @Override
+ public void pushByKey(String key, String value) {
+ threadLocalMapOfDeques.pushByKey(key, value);
+ put(key, value);
+ }
+
+ @Override
+ public String popByKey(String key) {
+ String value = threadLocalMapOfDeques.popByKey(key);
+ String head = threadLocalMapOfDeques.peekByKey(key);
+ if (head != null) {
+ put(key, head);
+ }
+ return value;
+ }
+
+ @Override
+ public Deque<String> getCopyOfDequeByKey(String key) {
+ return threadLocalMapOfDeques.getCopyOfDequeByKey(key);
+ }
+
+ @Override
+ public void clearDequeByKey(String key) {
+ threadLocalMapOfDeques.clearDequeByKey(key);
+ }
+ }
+
+ /**
+ * A simple implementation of ThreadLocal backed Map containing values of type
+ * Deque<String>. This class is inspired from SLF4J version 2.0.0
+ */
+ private static final class ThreadLocalMapOfStacks {
+
+ private final ThreadLocal<Map<String, Deque<String>>> tlMapOfStacks = new ThreadLocal<>();
+
+ void pushByKey(String key, String value) {
+ if (key == null)
+ return;
+
+ Map<String, Deque<String>> map = tlMapOfStacks.get();
+
+ if (map == null) {
+ map = new HashMap<>();
+ tlMapOfStacks.set(map);
+ }
+
+ Deque<String> deque = map.get(key);
+ if (deque == null) {
+ deque = new ArrayDeque<>();
+ }
+ deque.push(value);
+ map.put(key, deque);
Review Comment:
We could issue a warning if the `key` is already used in the classical MDC (`ThreadContext.get() != null`) (cf. [SLF4J-531](https://jira.qos.ch/browse/SLF4J-531) and return.
There is really no reason to allow users to use the same `key` in both the classical and stack-based MDC.
##########
log4j-slf4j20-impl/src/main/java/org/apache/logging/slf4j/Log4jMDCAdapter.java:
##########
@@ -27,57 +29,229 @@
*/
public class Log4jMDCAdapter implements MDCAdapter {
+ private final MDCAdapter adapterDelegate;
+
+ public Log4jMDCAdapter(boolean enableSlf4jStack) {
+ if (enableSlf4jStack) {
+ adapterDelegate = new ExtendedMDCAdaptor();
+ } else {
+ adapterDelegate = new StandardMDCAdaptor();
+ }
+ }
+
@Override
- public void put(final String key, final String val) {
- ThreadContext.put(key, val);
+ public void put(String key, String val) {
+ adapterDelegate.put(key, val);
}
@Override
- public String get(final String key) {
- return ThreadContext.get(key);
+ public String get(String key) {
+ return adapterDelegate.get(key);
}
@Override
- public void remove(final String key) {
- ThreadContext.remove(key);
+ public void remove(String key) {
+ adapterDelegate.remove(key);
}
@Override
public void clear() {
- ThreadContext.clearMap();
+ adapterDelegate.clear();
}
@Override
public Map<String, String> getCopyOfContextMap() {
- return ThreadContext.getContext();
+ return adapterDelegate.getCopyOfContextMap();
}
@Override
- @SuppressWarnings("unchecked") // nothing we can do about this, restricted by SLF4J API
- public void setContextMap(@SuppressWarnings("rawtypes") final Map map) {
- ThreadContext.clearMap();
- ThreadContext.putAll(map);
+ public void setContextMap(Map<String, String> contextMap) {
+ adapterDelegate.setContextMap(contextMap);
}
@Override
public void pushByKey(String key, String value) {
- // not implemented yet
+ adapterDelegate.pushByKey(key, value);
}
@Override
public String popByKey(String key) {
- // not implemented yet
- return null;
+ return adapterDelegate.popByKey(key);
}
@Override
public Deque<String> getCopyOfDequeByKey(String key) {
- // not implemented yet
- return null;
+ return adapterDelegate.getCopyOfDequeByKey(key);
}
@Override
public void clearDequeByKey(String key) {
- // not implemented yet
+ adapterDelegate.clearDequeByKey(key);
+ }
+
+ private static class StandardMDCAdaptor implements MDCAdapter {
+
+ @Override
+ public void put(final String key, final String val) {
+ ThreadContext.put(key, val);
+ }
+
+ @Override
+ public String get(final String key) {
+ return ThreadContext.get(key);
+ }
+
+ @Override
+ public void remove(final String key) {
+ ThreadContext.remove(key);
+ }
+
+ @Override
+ public void clear() {
+ ThreadContext.clearMap();
+ }
+
+ @Override
+ public Map<String, String> getCopyOfContextMap() {
+ return ThreadContext.getContext();
+ }
+
+ @Override
+ public void setContextMap(final Map<String, String> map) {
+ ThreadContext.clearMap();
+ ThreadContext.putAll(map);
+ }
+
+ @Override
+ public void pushByKey(String key, String value) {
+ // NOP
+ }
+
+ @Override
+ public String popByKey(String key) {
+ // NOP
+ return null;
+ }
+
+ @Override
+ public Deque<String> getCopyOfDequeByKey(String key) {
+ // NOP
+ return null;
+ }
+
+ @Override
+ public void clearDequeByKey(String key) {
+ // NOP
+ }
+ }
+
+ /**
+ * Wires the top of the stack to the MDC current value
+ */
+ private static final class ExtendedMDCAdaptor extends StandardMDCAdaptor {
+
+ private final ThreadLocalMapOfStacks threadLocalMapOfDeques = new ThreadLocalMapOfStacks();
+
+ @Override
+ public void clear() {
+ super.clear();
+ threadLocalMapOfDeques.clear();
+ }
+
+ @Override
+ public void pushByKey(String key, String value) {
+ threadLocalMapOfDeques.pushByKey(key, value);
+ put(key, value);
+ }
+
+ @Override
+ public String popByKey(String key) {
+ String value = threadLocalMapOfDeques.popByKey(key);
+ String head = threadLocalMapOfDeques.peekByKey(key);
+ if (head != null) {
+ put(key, head);
+ }
+ return value;
+ }
+
+ @Override
+ public Deque<String> getCopyOfDequeByKey(String key) {
+ return threadLocalMapOfDeques.getCopyOfDequeByKey(key);
+ }
+
+ @Override
+ public void clearDequeByKey(String key) {
+ threadLocalMapOfDeques.clearDequeByKey(key);
+ }
+ }
+
+ /**
+ * A simple implementation of ThreadLocal backed Map containing values of type
+ * Deque<String>. This class is inspired from SLF4J version 2.0.0
+ */
+ private static final class ThreadLocalMapOfStacks {
Review Comment:
I wouldn't copy this class, but just use the one Ceki provides in `slf4j-api`.
##########
log4j-slf4j20-impl/src/main/java/org/apache/logging/slf4j/Log4jMDCAdapter.java:
##########
@@ -27,57 +29,229 @@
*/
public class Log4jMDCAdapter implements MDCAdapter {
+ private final MDCAdapter adapterDelegate;
+
+ public Log4jMDCAdapter(boolean enableSlf4jStack) {
+ if (enableSlf4jStack) {
+ adapterDelegate = new ExtendedMDCAdaptor();
+ } else {
+ adapterDelegate = new StandardMDCAdaptor();
+ }
+ }
+
@Override
- public void put(final String key, final String val) {
- ThreadContext.put(key, val);
+ public void put(String key, String val) {
+ adapterDelegate.put(key, val);
}
@Override
- public String get(final String key) {
- return ThreadContext.get(key);
+ public String get(String key) {
+ return adapterDelegate.get(key);
}
@Override
- public void remove(final String key) {
- ThreadContext.remove(key);
+ public void remove(String key) {
+ adapterDelegate.remove(key);
}
@Override
public void clear() {
- ThreadContext.clearMap();
+ adapterDelegate.clear();
}
@Override
public Map<String, String> getCopyOfContextMap() {
- return ThreadContext.getContext();
+ return adapterDelegate.getCopyOfContextMap();
}
@Override
- @SuppressWarnings("unchecked") // nothing we can do about this, restricted by SLF4J API
- public void setContextMap(@SuppressWarnings("rawtypes") final Map map) {
- ThreadContext.clearMap();
- ThreadContext.putAll(map);
+ public void setContextMap(Map<String, String> contextMap) {
+ adapterDelegate.setContextMap(contextMap);
}
@Override
public void pushByKey(String key, String value) {
- // not implemented yet
+ adapterDelegate.pushByKey(key, value);
}
@Override
public String popByKey(String key) {
- // not implemented yet
- return null;
+ return adapterDelegate.popByKey(key);
}
@Override
public Deque<String> getCopyOfDequeByKey(String key) {
- // not implemented yet
- return null;
+ return adapterDelegate.getCopyOfDequeByKey(key);
}
@Override
public void clearDequeByKey(String key) {
- // not implemented yet
+ adapterDelegate.clearDequeByKey(key);
+ }
+
+ private static class StandardMDCAdaptor implements MDCAdapter {
+
+ @Override
+ public void put(final String key, final String val) {
+ ThreadContext.put(key, val);
+ }
+
+ @Override
+ public String get(final String key) {
+ return ThreadContext.get(key);
+ }
+
+ @Override
+ public void remove(final String key) {
+ ThreadContext.remove(key);
+ }
+
+ @Override
+ public void clear() {
+ ThreadContext.clearMap();
+ }
+
+ @Override
+ public Map<String, String> getCopyOfContextMap() {
+ return ThreadContext.getContext();
+ }
+
+ @Override
+ public void setContextMap(final Map<String, String> map) {
+ ThreadContext.clearMap();
+ ThreadContext.putAll(map);
+ }
+
+ @Override
+ public void pushByKey(String key, String value) {
+ // NOP
+ }
+
+ @Override
+ public String popByKey(String key) {
+ // NOP
+ return null;
+ }
+
+ @Override
+ public Deque<String> getCopyOfDequeByKey(String key) {
+ // NOP
+ return null;
+ }
+
+ @Override
+ public void clearDequeByKey(String key) {
+ // NOP
+ }
+ }
+
+ /**
+ * Wires the top of the stack to the MDC current value
+ */
+ private static final class ExtendedMDCAdaptor extends StandardMDCAdaptor {
+
+ private final ThreadLocalMapOfStacks threadLocalMapOfDeques = new ThreadLocalMapOfStacks();
+
+ @Override
+ public void clear() {
+ super.clear();
+ threadLocalMapOfDeques.clear();
+ }
+
+ @Override
+ public void pushByKey(String key, String value) {
+ threadLocalMapOfDeques.pushByKey(key, value);
+ put(key, value);
+ }
+
+ @Override
+ public String popByKey(String key) {
+ String value = threadLocalMapOfDeques.popByKey(key);
+ String head = threadLocalMapOfDeques.peekByKey(key);
+ if (head != null) {
+ put(key, head);
+ }
+ return value;
+ }
+
+ @Override
+ public Deque<String> getCopyOfDequeByKey(String key) {
+ return threadLocalMapOfDeques.getCopyOfDequeByKey(key);
+ }
+
+ @Override
+ public void clearDequeByKey(String key) {
+ threadLocalMapOfDeques.clearDequeByKey(key);
+ }
+ }
+
+ /**
+ * A simple implementation of ThreadLocal backed Map containing values of type
+ * Deque<String>. This class is inspired from SLF4J version 2.0.0
+ */
+ private static final class ThreadLocalMapOfStacks {
+
+ private final ThreadLocal<Map<String, Deque<String>>> tlMapOfStacks = new ThreadLocal<>();
+
+ void pushByKey(String key, String value) {
+ if (key == null)
+ return;
Review Comment:
If the `key` is `null` I would delegate to `ThreadContext.push` to take advantage of our NDC.
##########
log4j-slf4j20-impl/src/main/java/org/apache/logging/slf4j/Log4jMDCAdapter.java:
##########
@@ -27,57 +29,229 @@
*/
public class Log4jMDCAdapter implements MDCAdapter {
+ private final MDCAdapter adapterDelegate;
+
+ public Log4jMDCAdapter(boolean enableSlf4jStack) {
+ if (enableSlf4jStack) {
+ adapterDelegate = new ExtendedMDCAdaptor();
+ } else {
+ adapterDelegate = new StandardMDCAdaptor();
+ }
+ }
+
@Override
- public void put(final String key, final String val) {
- ThreadContext.put(key, val);
+ public void put(String key, String val) {
+ adapterDelegate.put(key, val);
}
@Override
- public String get(final String key) {
- return ThreadContext.get(key);
+ public String get(String key) {
+ return adapterDelegate.get(key);
}
@Override
- public void remove(final String key) {
- ThreadContext.remove(key);
+ public void remove(String key) {
+ adapterDelegate.remove(key);
}
@Override
public void clear() {
- ThreadContext.clearMap();
+ adapterDelegate.clear();
}
@Override
public Map<String, String> getCopyOfContextMap() {
- return ThreadContext.getContext();
+ return adapterDelegate.getCopyOfContextMap();
}
@Override
- @SuppressWarnings("unchecked") // nothing we can do about this, restricted by SLF4J API
- public void setContextMap(@SuppressWarnings("rawtypes") final Map map) {
- ThreadContext.clearMap();
- ThreadContext.putAll(map);
+ public void setContextMap(Map<String, String> contextMap) {
+ adapterDelegate.setContextMap(contextMap);
}
@Override
public void pushByKey(String key, String value) {
- // not implemented yet
+ adapterDelegate.pushByKey(key, value);
}
@Override
public String popByKey(String key) {
- // not implemented yet
- return null;
+ return adapterDelegate.popByKey(key);
}
@Override
public Deque<String> getCopyOfDequeByKey(String key) {
- // not implemented yet
- return null;
+ return adapterDelegate.getCopyOfDequeByKey(key);
}
@Override
public void clearDequeByKey(String key) {
- // not implemented yet
+ adapterDelegate.clearDequeByKey(key);
+ }
+
+ private static class StandardMDCAdaptor implements MDCAdapter {
+
+ @Override
+ public void put(final String key, final String val) {
+ ThreadContext.put(key, val);
+ }
+
+ @Override
+ public String get(final String key) {
+ return ThreadContext.get(key);
+ }
+
+ @Override
+ public void remove(final String key) {
+ ThreadContext.remove(key);
+ }
+
+ @Override
+ public void clear() {
+ ThreadContext.clearMap();
+ }
+
+ @Override
+ public Map<String, String> getCopyOfContextMap() {
+ return ThreadContext.getContext();
+ }
+
+ @Override
+ public void setContextMap(final Map<String, String> map) {
+ ThreadContext.clearMap();
+ ThreadContext.putAll(map);
+ }
+
+ @Override
+ public void pushByKey(String key, String value) {
+ // NOP
+ }
+
+ @Override
+ public String popByKey(String key) {
+ // NOP
+ return null;
+ }
+
+ @Override
+ public Deque<String> getCopyOfDequeByKey(String key) {
+ // NOP
+ return null;
+ }
+
+ @Override
+ public void clearDequeByKey(String key) {
+ // NOP
+ }
+ }
+
+ /**
+ * Wires the top of the stack to the MDC current value
+ */
+ private static final class ExtendedMDCAdaptor extends StandardMDCAdaptor {
Review Comment:
Personally I am fine with this implementation being the only one.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org