Skip to content

Commit 34bfc72

Browse files
morgen-peschkemorgen-peschke
authored
Merge pull request #910 from morgen-peschke/support-slf4j-dynamic-is-enabled
Expose MDC to Slf4jLoggerInternal isEnabled
2 parents 6ee980c + 47e741e commit 34bfc72

File tree

4 files changed

+286
-236
lines changed

4 files changed

+286
-236
lines changed

‎slf4j/src/main/scala/org/typelevel/log4cats/slf4j/Slf4jLogger.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,10 @@ object Slf4jLogger extends Slf4jLoggerCompat {
3333
getLoggerFromSlf4j[F](org.slf4j.LoggerFactory.getLogger(clazz))
3434

3535
def getLoggerFromSlf4j[F[_]: Sync](logger: JLogger): SelfAwareStructuredLogger[F] =
36-
new Slf4jLoggerInternal.Slf4jLogger(logger, Sync.Type.Delay)
36+
new Slf4jLoggerInternal.Slf4jLogger(logger, Sync.Type.Delay)(Sync[F])
3737

3838
def getLoggerFromBlockingSlf4j[F[_]: Sync](logger: JLogger): SelfAwareStructuredLogger[F] =
39-
new Slf4jLoggerInternal.Slf4jLogger(logger, Sync.Type.Blocking)
39+
new Slf4jLoggerInternal.Slf4jLogger(logger, Sync.Type.Blocking)(Sync[F])
4040

4141
def create[F[_]: Sync](implicit name: LoggerName): F[SelfAwareStructuredLogger[F]] =
4242
Sync[F].delay(getLoggerFromName(name.value))

‎slf4j/src/main/scala/org/typelevel/log4cats/slf4j/internal/Slf4jLoggerInternal.scala

Lines changed: 90 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,9 @@
1616

1717
package org.typelevel.log4cats.slf4j.internal
1818

19-
import org.typelevel.log4cats.*
20-
import cats.syntax.all.*
2119
import cats.effect.*
22-
import org.slf4j.Logger as JLogger
23-
import org.slf4j.MDC
24-
25-
import scala.annotation.nowarn
20+
import org.slf4j.{Logger as JLogger, MDC}
21+
import org.typelevel.log4cats.*
2622

2723
private[slf4j] object Slf4jLoggerInternal {
2824

@@ -36,106 +32,127 @@ private[slf4j] object Slf4jLoggerInternal {
3632
def apply(t: Throwable)(msg: => String): F[Unit]
3733
}
3834

39-
// Need this to make sure MDC is correctly cleared before logging
40-
private[this] def noContextLog[F[_]](isEnabled: F[Boolean], logging: () => Unit)(implicit
41-
F: Sync[F]
42-
): F[Unit] =
43-
contextLog[F](isEnabled, Map.empty, logging)
44-
45-
private[this] def contextLog[F[_]](
46-
isEnabled: F[Boolean],
47-
ctx: Map[String, String],
48-
logging: () => Unit
49-
)(implicit F: Sync[F]): F[Unit] = {
50-
51-
val ifEnabled = F.delay {
52-
val backup =
53-
try MDC.getCopyOfContextMap()
54-
catch {
55-
case e: IllegalStateException =>
56-
// MDCAdapter is missing, no point in doing anything with
57-
// the MDC, so just hope the logging backend can salvage
58-
// something.
59-
logging()
60-
throw e
61-
}
62-
63-
try {
64-
// Once 2.12 is no longer supported, change this to MDC.setContextMap(ctx.asJava)
65-
MDC.clear()
66-
ctx.foreach { case (k, v) => MDC.put(k, v) }
67-
logging()
68-
} finally
69-
if (backup eq null) MDC.clear()
70-
else MDC.setContextMap(backup)
71-
}
72-
73-
isEnabled.ifM(
74-
ifEnabled,
75-
F.unit
76-
)
77-
}
78-
79-
@nowarn("msg=used")
80-
final class Slf4jLogger[F[_]](val logger: JLogger, sync: Sync.Type = Sync.Type.Delay)(implicit
35+
final class Slf4jLogger[F[_]](
36+
val logger: JLogger,
37+
sync: Sync.Type,
38+
defaultCtx: Map[String, String]
39+
)(implicit
8140
F: Sync[F]
8241
) extends SelfAwareStructuredLogger[F] {
8342

43+
def this(logger: JLogger, sync: Sync.Type = Sync.Type.Delay)(F: Sync[F]) =
44+
this(logger, sync, Map.empty)(F)
45+
8446
@deprecated("Use constructor with sync", "2.6.0")
8547
def this(logger: JLogger)(
8648
F: Sync[F]
8749
) =
8850
this(logger, Sync.Type.Delay)(F)
8951

90-
override def isTraceEnabled: F[Boolean] = F.delay(logger.isTraceEnabled)
91-
override def isDebugEnabled: F[Boolean] = F.delay(logger.isDebugEnabled)
92-
override def isInfoEnabled: F[Boolean] = F.delay(logger.isInfoEnabled)
93-
override def isWarnEnabled: F[Boolean] = F.delay(logger.isWarnEnabled)
94-
override def isErrorEnabled: F[Boolean] = F.delay(logger.isErrorEnabled)
52+
private val isTraceEnabledUnsafe = () => logger.isTraceEnabled
53+
private val isDebugEnabledUnsafe = () => logger.isDebugEnabled
54+
private val isInfoEnabledUnsafe = () => logger.isInfoEnabled
55+
private val isWarnEnabledUnsafe = () => logger.isWarnEnabled
56+
private val isErrorEnabledUnsafe = () => logger.isErrorEnabled
57+
58+
override def addContext(ctx: Map[String, String]): SelfAwareStructuredLogger[F] =
59+
new Slf4jLogger[F](logger, sync, defaultCtx ++ ctx)
60+
61+
override def isTraceEnabled: F[Boolean] =
62+
F.delay(withPreparedMDCUnsafe(Map.empty, isTraceEnabledUnsafe))
63+
override def isDebugEnabled: F[Boolean] =
64+
F.delay(withPreparedMDCUnsafe(Map.empty, isDebugEnabledUnsafe))
65+
override def isInfoEnabled: F[Boolean] =
66+
F.delay(withPreparedMDCUnsafe(Map.empty, isInfoEnabledUnsafe))
67+
override def isWarnEnabled: F[Boolean] =
68+
F.delay(withPreparedMDCUnsafe(Map.empty, isWarnEnabledUnsafe))
69+
override def isErrorEnabled: F[Boolean] =
70+
F.delay(withPreparedMDCUnsafe(Map.empty, isErrorEnabledUnsafe))
9571

9672
override def trace(t: Throwable)(msg: => String): F[Unit] =
97-
noContextLog(isTraceEnabled, () => logger.trace(msg, t))
73+
noContextLog(isTraceEnabledUnsafe, () => logger.trace(msg, t))
9874
override def trace(msg: => String): F[Unit] =
99-
noContextLog(isTraceEnabled, () => logger.trace(msg))
75+
noContextLog(isTraceEnabledUnsafe, () => logger.trace(msg))
10076
override def trace(ctx: Map[String, String])(msg: => String): F[Unit] =
101-
contextLog(isTraceEnabled, ctx, () => logger.trace(msg))
77+
contextLog(isTraceEnabledUnsafe, ctx, () => logger.trace(msg))
10278
override def trace(ctx: Map[String, String], t: Throwable)(msg: => String): F[Unit] =
103-
contextLog(isTraceEnabled, ctx, () => logger.trace(msg, t))
79+
contextLog(isTraceEnabledUnsafe, ctx, () => logger.trace(msg, t))
10480

10581
override def debug(t: Throwable)(msg: => String): F[Unit] =
106-
noContextLog(isDebugEnabled, () => logger.debug(msg, t))
82+
noContextLog(isDebugEnabledUnsafe, () => logger.debug(msg, t))
10783
override def debug(msg: => String): F[Unit] =
108-
noContextLog(isDebugEnabled, () => logger.debug(msg))
84+
noContextLog(isDebugEnabledUnsafe, () => logger.debug(msg))
10985
override def debug(ctx: Map[String, String])(msg: => String): F[Unit] =
110-
contextLog(isDebugEnabled, ctx, () => logger.debug(msg))
86+
contextLog(isDebugEnabledUnsafe, ctx, () => logger.debug(msg))
11187
override def debug(ctx: Map[String, String], t: Throwable)(msg: => String): F[Unit] =
112-
contextLog(isDebugEnabled, ctx, () => logger.debug(msg, t))
88+
contextLog(isDebugEnabledUnsafe, ctx, () => logger.debug(msg, t))
11389

11490
override def info(t: Throwable)(msg: => String): F[Unit] =
115-
noContextLog(isInfoEnabled, () => logger.info(msg, t))
91+
noContextLog(isInfoEnabledUnsafe, () => logger.info(msg, t))
11692
override def info(msg: => String): F[Unit] =
117-
noContextLog(isInfoEnabled, () => logger.info(msg))
93+
noContextLog(isInfoEnabledUnsafe, () => logger.info(msg))
11894
override def info(ctx: Map[String, String])(msg: => String): F[Unit] =
119-
contextLog(isInfoEnabled, ctx, () => logger.info(msg))
95+
contextLog(isInfoEnabledUnsafe, ctx, () => logger.info(msg))
12096
override def info(ctx: Map[String, String], t: Throwable)(msg: => String): F[Unit] =
121-
contextLog(isInfoEnabled, ctx, () => logger.info(msg, t))
97+
contextLog(isInfoEnabledUnsafe, ctx, () => logger.info(msg, t))
12298

12399
override def warn(t: Throwable)(msg: => String): F[Unit] =
124-
noContextLog(isWarnEnabled, () => logger.warn(msg, t))
100+
noContextLog(isWarnEnabledUnsafe, () => logger.warn(msg, t))
125101
override def warn(msg: => String): F[Unit] =
126-
noContextLog(isWarnEnabled, () => logger.warn(msg))
102+
noContextLog(isWarnEnabledUnsafe, () => logger.warn(msg))
127103
override def warn(ctx: Map[String, String])(msg: => String): F[Unit] =
128-
contextLog(isWarnEnabled, ctx, () => logger.warn(msg))
104+
contextLog(isWarnEnabledUnsafe, ctx, () => logger.warn(msg))
129105
override def warn(ctx: Map[String, String], t: Throwable)(msg: => String): F[Unit] =
130-
contextLog(isWarnEnabled, ctx, () => logger.warn(msg, t))
106+
contextLog(isWarnEnabledUnsafe, ctx, () => logger.warn(msg, t))
131107

132108
override def error(t: Throwable)(msg: => String): F[Unit] =
133-
noContextLog(isErrorEnabled, () => logger.error(msg, t))
109+
noContextLog(isErrorEnabledUnsafe, () => logger.error(msg, t))
134110
override def error(msg: => String): F[Unit] =
135-
noContextLog(isErrorEnabled, () => logger.error(msg))
111+
noContextLog(isErrorEnabledUnsafe, () => logger.error(msg))
136112
override def error(ctx: Map[String, String])(msg: => String): F[Unit] =
137-
contextLog(isErrorEnabled, ctx, () => logger.error(msg))
113+
contextLog(isErrorEnabledUnsafe, ctx, () => logger.error(msg))
138114
override def error(ctx: Map[String, String], t: Throwable)(msg: => String): F[Unit] =
139-
contextLog(isErrorEnabled, ctx, () => logger.error(msg, t))
115+
contextLog(isErrorEnabledUnsafe, ctx, () => logger.error(msg, t))
116+
117+
private def withPreparedMDCUnsafe[A](extraCtx: Map[String, String], body: () => A): A = {
118+
val ctx = defaultCtx ++ extraCtx
119+
val backup =
120+
try MDC.getCopyOfContextMap()
121+
catch {
122+
case e: IllegalStateException =>
123+
// MDCAdapter is missing, no point in doing anything with
124+
// the MDC, so just hope the logging backend can salvage
125+
// something.
126+
body()
127+
throw e
128+
}
129+
130+
try {
131+
// Once 2.12 is no longer supported, change this to MDC.setContextMap(ctx.asJava)
132+
MDC.clear()
133+
ctx.foreach { case (k, v) => MDC.put(k, v) }
134+
body()
135+
} finally
136+
if (backup eq null) MDC.clear()
137+
else MDC.setContextMap(backup)
138+
}
139+
140+
private def noContextLog(isEnabledUnsafe: () => Boolean, logging: () => Unit): F[Unit] =
141+
contextLog(isEnabledUnsafe, Map.empty, logging)
142+
143+
private def contextLog(
144+
isEnabledUnsafe: () => Boolean,
145+
ctx: Map[String, String],
146+
logging: () => Unit
147+
): F[Unit] =
148+
F.delay(
149+
withPreparedMDCUnsafe(
150+
ctx,
151+
() =>
152+
if (isEnabledUnsafe()) {
153+
logging()
154+
}
155+
)
156+
)
140157
}
141158
}

‎slf4j/src/test/scala/org/typelevel/log4cats/slf4j/internal/JTestLogger.java

Lines changed: 43 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,14 @@
2121
import org.slf4j.Marker;
2222
import org.typelevel.log4cats.extras.LogLevel;
2323
import scala.Option;
24+
import scala.collection.immutable.Map$;
25+
import scala.collection.mutable.Builder;
2426

25-
import java.util.ArrayList;
26-
import java.util.HashMap;
27-
import java.util.List;
28-
import java.util.Map;
27+
import java.util.*;
2928
import java.util.concurrent.atomic.AtomicReference;
29+
import java.util.function.BooleanSupplier;
3030
import java.util.function.Function;
31+
import java.util.function.Predicate;
3132
import java.util.function.Supplier;
3233

3334
public class JTestLogger implements Logger {
@@ -41,6 +42,23 @@ public class JTestLogger implements Logger {
4142
private static final LogLevel.Warn$ Warn = LogLevel.Warn$.MODULE$;
4243
private static final LogLevel.Error$ Error = LogLevel.Error$.MODULE$;
4344

45+
public static BooleanSupplier Enabled = () -> true;
46+
public static BooleanSupplier Disabled = () -> false;
47+
public static BooleanSupplier dynamicUsingMDC(Predicate<scala.collection.immutable.Map<String, String>> f) {
48+
return () -> {
49+
Map<String, String> map = MDC.getCopyOfContextMap();
50+
if (Objects.isNull(map)) {
51+
map = new HashMap<>();
52+
}
53+
54+
Builder<scala.Tuple2<String, String>, scala.collection.immutable.Map<String, String>> builder =
55+
Map$.MODULE$.newBuilder();;
56+
57+
map.forEach((k,v) -> builder.$plus$eq(scala.Tuple2$.MODULE$.apply(k,v)));
58+
return f.test(builder.result());
59+
};
60+
}
61+
4462
private Map<String, String> captureContext () {
4563
java.util.Map<String, String> mdc = MDC.getCopyOfContextMap();
4664
if (mdc == null) {
@@ -95,20 +113,19 @@ static TestLogMessage of(LogLevel logLevel,
95113
}
96114

97115
private final String loggerName;
98-
private final boolean traceEnabled;
99-
private final boolean debugEnabled;
100-
private final boolean infoEnabled;
101-
private final boolean warnEnabled;
102-
private final boolean errorEnabled;
116+
private final BooleanSupplier traceEnabled;
117+
private final BooleanSupplier debugEnabled;
118+
private final BooleanSupplier infoEnabled;
119+
private final BooleanSupplier warnEnabled;
120+
private final BooleanSupplier errorEnabled;
103121
private final AtomicReference<List<TestLogMessage>> loggedMessages;
104122

105-
106123
public JTestLogger(String loggerName,
107-
boolean traceEnabled,
108-
boolean debugEnabled,
109-
boolean infoEnabled,
110-
boolean warnEnabled,
111-
boolean errorEnabled) {
124+
BooleanSupplier traceEnabled,
125+
BooleanSupplier debugEnabled,
126+
BooleanSupplier infoEnabled,
127+
BooleanSupplier warnEnabled,
128+
BooleanSupplier errorEnabled) {
112129
this.loggerName = loggerName;
113130
this.traceEnabled = traceEnabled;
114131
this.debugEnabled = debugEnabled;
@@ -126,22 +143,22 @@ private void save(Function<Map<String, String>, TestLogMessage> mkLogMessage) {
126143
}
127144

128145
public List<TestLogMessage> logs() { return loggedMessages.get(); }
129-
public void reset() { loggedMessages.set(new ArrayList<>()); }
146+
public void clearLogs() { loggedMessages.set(new ArrayList<>()); }
130147

131148
@Override public String getName() { return loggerName;}
132149

133-
@Override public boolean isTraceEnabled() { return traceEnabled; }
134-
@Override public boolean isDebugEnabled() { return debugEnabled; }
135-
@Override public boolean isInfoEnabled() { return infoEnabled; }
136-
@Override public boolean isWarnEnabled() { return warnEnabled; }
137-
@Override public boolean isErrorEnabled() { return errorEnabled; }
150+
@Override public boolean isTraceEnabled() { return traceEnabled.getAsBoolean(); }
151+
@Override public boolean isDebugEnabled() { return debugEnabled.getAsBoolean(); }
152+
@Override public boolean isInfoEnabled() { return infoEnabled.getAsBoolean(); }
153+
@Override public boolean isWarnEnabled() { return warnEnabled.getAsBoolean(); }
154+
@Override public boolean isErrorEnabled() { return errorEnabled.getAsBoolean(); }
138155

139156
// We don't use them, so we're going to ignore Markers
140-
@Override public boolean isTraceEnabled(Marker marker) { return traceEnabled; }
141-
@Override public boolean isDebugEnabled(Marker marker) { return debugEnabled; }
142-
@Override public boolean isInfoEnabled(Marker marker) { return infoEnabled; }
143-
@Override public boolean isWarnEnabled(Marker marker) { return warnEnabled; }
144-
@Override public boolean isErrorEnabled(Marker marker) { return errorEnabled; }
157+
@Override public boolean isTraceEnabled(Marker marker) { return traceEnabled.getAsBoolean(); }
158+
@Override public boolean isDebugEnabled(Marker marker) { return debugEnabled.getAsBoolean(); }
159+
@Override public boolean isInfoEnabled(Marker marker) { return infoEnabled.getAsBoolean(); }
160+
@Override public boolean isWarnEnabled(Marker marker) { return warnEnabled.getAsBoolean(); }
161+
@Override public boolean isErrorEnabled(Marker marker) { return errorEnabled.getAsBoolean(); }
145162

146163
@Override public void trace(String msg) { save(ctx -> TestLogMessage.of(Trace, ctx, () -> msg)); }
147164
@Override public void trace(String msg, Throwable t) { save(ctx -> TestLogMessage.of(Trace, ctx, t, () -> msg)); }

0 commit comments

Comments
 (0)