Skip to content

Commit c700177

Browse files
committed
Restructure logger implementation to reduce published API surface
1 parent dfdf128 commit c700177

File tree

8 files changed

+79
-99
lines changed

8 files changed

+79
-99
lines changed

api/devlog-kotlin.api

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ public final class dev/hermannm/devlog/LogBuilder {
2020
public static final fun addLogFieldOfType-impl (Ldev/hermannm/devlog/LogEvent;Ljava/lang/String;Ljava/lang/Object;Lkotlin/reflect/KType;)V
2121
public static final fun addStringLogField-impl (Ldev/hermannm/devlog/LogEvent;Ljava/lang/String;Ljava/lang/Object;)V
2222
public static final synthetic fun box-impl (Ldev/hermannm/devlog/LogEvent;)Ldev/hermannm/devlog/LogBuilder;
23-
public static fun constructor-impl (Ldev/hermannm/devlog/LogEvent;)Ldev/hermannm/devlog/LogEvent;
2423
public fun equals (Ljava/lang/Object;)Z
2524
public static fun equals-impl (Ldev/hermannm/devlog/LogEvent;Ljava/lang/Object;)Z
2625
public static final fun equals-impl0 (Ldev/hermannm/devlog/LogEvent;Ldev/hermannm/devlog/LogEvent;)Z
@@ -29,22 +28,13 @@ public final class dev/hermannm/devlog/LogBuilder {
2928
public static fun hashCode-impl (Ldev/hermannm/devlog/LogEvent;)I
3029
public static final fun rawJsonField-impl (Ldev/hermannm/devlog/LogEvent;Ljava/lang/String;Ljava/lang/String;Z)V
3130
public static synthetic fun rawJsonField-impl$default (Ldev/hermannm/devlog/LogEvent;Ljava/lang/String;Ljava/lang/String;ZILjava/lang/Object;)V
32-
public static final fun setCause-impl (Ldev/hermannm/devlog/LogEvent;Ljava/lang/Throwable;Lorg/slf4j/Logger;)V
3331
public fun toString ()Ljava/lang/String;
3432
public static fun toString-impl (Ldev/hermannm/devlog/LogEvent;)Ljava/lang/String;
3533
public final synthetic fun unbox-impl ()Ldev/hermannm/devlog/LogEvent;
3634
}
3735

38-
public abstract interface class dev/hermannm/devlog/LogEvent {
39-
public abstract fun addJsonField (Ljava/lang/String;Ljava/lang/String;)V
40-
public abstract fun addStringField (Ljava/lang/String;Ljava/lang/String;)V
41-
public abstract fun handlesExceptionTreeTraversal ()Z
42-
public abstract fun log (Ljava/lang/String;Lorg/slf4j/Logger;)V
43-
public abstract fun setCause-ARkn0dU (Ljava/lang/Throwable;Lorg/slf4j/Logger;Ldev/hermannm/devlog/LogEvent;)V
44-
}
45-
46-
public final class dev/hermannm/devlog/LogEventJvm {
47-
public static final fun createLogEvent (Ldev/hermannm/devlog/LogLevel;Lorg/slf4j/Logger;)Ldev/hermannm/devlog/LogEvent;
36+
public final class dev/hermannm/devlog/LogBuilderKt {
37+
public static final fun createLogBuilder-kpOQkOI (Ldev/hermannm/devlog/LogLevel;Lorg/slf4j/Logger;)Ldev/hermannm/devlog/LogEvent;
4838
}
4939

5040
public final class dev/hermannm/devlog/LogField {
@@ -97,7 +87,7 @@ public final class dev/hermannm/devlog/Logger {
9787
public static final fun isInfoEnabled-impl (Lorg/slf4j/Logger;)Z
9888
public static final fun isTraceEnabled-impl (Lorg/slf4j/Logger;)Z
9989
public static final fun isWarnEnabled-impl (Lorg/slf4j/Logger;)Z
100-
public static final fun log-impl (Lorg/slf4j/Logger;Ldev/hermannm/devlog/LogLevel;Ljava/lang/Throwable;Lkotlin/jvm/functions/Function1;)V
90+
public static final fun log-aaf-xeU (Lorg/slf4j/Logger;Ldev/hermannm/devlog/LogEvent;Ljava/lang/String;Ljava/lang/Throwable;)V
10191
public fun toString ()Ljava/lang/String;
10292
public static fun toString-impl (Lorg/slf4j/Logger;)Ljava/lang/String;
10393
public static final fun trace-impl (Lorg/slf4j/Logger;Ljava/lang/Throwable;Lkotlin/jvm/functions/Function1;)V

src/commonMain/kotlin/dev/hermannm/devlog/LogBuilder.kt

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,8 @@ import kotlinx.serialization.serializer
3535
*/
3636
@kotlin.jvm.JvmInline // Inline value class, to wrap the underlying log event without overhead
3737
public value class LogBuilder
38-
@PublishedApi
3938
internal constructor(
40-
@PublishedApi internal val logEvent: LogEvent,
39+
internal val logEvent: LogEvent,
4140
) {
4241
/**
4342
* Adds a [log field][LogField] (structured key-value data) to the log.
@@ -336,7 +335,6 @@ internal constructor(
336335
* Checks if the log cause exception (or any of its own cause exceptions) implements the
337336
* [HasLoggingContext] interface, and if so, adds those fields to the log.
338337
*/
339-
@PublishedApi
340338
internal fun setCause(cause: Throwable, logger: PlatformLogger) {
341339
logEvent.setCause(cause, logger, this)
342340

@@ -368,3 +366,8 @@ internal constructor(
368366
}
369367
}
370368
}
369+
370+
@PublishedApi
371+
internal fun createLogBuilder(level: LogLevel, logger: Logger): LogBuilder {
372+
return LogBuilder(createLogEvent(level, logger.underlyingLogger))
373+
}

src/commonMain/kotlin/dev/hermannm/devlog/LogEvent.kt

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ package dev.hermannm.devlog
1919
* `Slf4jLogEvent`. [LogEvent] is the common interface between the two, so that [LogBuilder] can
2020
* call this interface without having to care about the underlying implementation.
2121
*/
22-
@PublishedApi
2322
internal interface LogEvent {
2423
/**
2524
* @param logger We pass the logger so that the implementation has access to it if necessary (our
@@ -36,11 +35,11 @@ internal interface LogEvent {
3635
fun log(message: String, logger: PlatformLogger)
3736

3837
/**
39-
* Normally, [LogBuilder.setCause] would traverse the tree of exceptions from a cause exception,
40-
* checking for [ExceptionWithLoggingContext] and [LoggingContextProvider]. But some `LogEvent`
41-
* implementations, namely our `LogbackLogEvent`, does its own exception traversal already.
42-
* Instead of traversing exceptions twice, we let the log event implementation set this flag to
43-
* true if it takes care of it itself.
38+
* Normally, [LogBuilder.traverseExceptionTreeForLogFields] would traverse the tree of exceptions
39+
* from a cause exception, checking for [ExceptionWithLoggingContext] and
40+
* [LoggingContextProvider]. But some `LogEvent` implementations, namely our `LogbackLogEvent`,
41+
* does its own exception traversal already. Instead of traversing exceptions twice, we let the
42+
* log event implementation set this flag to true if it takes care of it itself.
4443
*/
4544
fun handlesExceptionTreeTraversal(): Boolean
4645
}
@@ -51,4 +50,4 @@ internal interface LogEvent {
5150
* On the JVM, this returns an SLF4J `LoggingEvent`, or a specialized optimized version for Logback
5251
* if Logback is used as the logging backend.
5352
*/
54-
@PublishedApi internal expect fun createLogEvent(level: LogLevel, logger: PlatformLogger): LogEvent
53+
internal expect fun createLogEvent(level: LogLevel, logger: PlatformLogger): LogEvent

src/commonMain/kotlin/dev/hermannm/devlog/Logger.kt

Lines changed: 44 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ import kotlin.reflect.KClass
5353
@kotlin.jvm.JvmInline // Inline value class, to wrap the underlying platform logger without overhead
5454
public value class Logger
5555
internal constructor(
56-
@PublishedApi internal val underlyingLogger: PlatformLogger,
56+
internal val underlyingLogger: PlatformLogger,
5757
) {
5858
/**
5959
* Calls the given lambda to build a log message, and logs it at [LogLevel.INFO], if enabled.
@@ -96,8 +96,10 @@ internal constructor(
9696
cause: Throwable? = null,
9797
buildLog: LogBuilder.() -> String,
9898
) {
99-
if (underlyingLogger.isInfoEnabled()) {
100-
log(LogLevel.INFO, cause, buildLog)
99+
if (isInfoEnabled) {
100+
val builder = createLogBuilder(LogLevel.INFO, this)
101+
val message = builder.buildLog()
102+
log(builder, message, cause)
101103
}
102104
}
103105

@@ -146,8 +148,10 @@ internal constructor(
146148
cause: Throwable? = null,
147149
buildLog: LogBuilder.() -> String,
148150
) {
149-
if (underlyingLogger.isWarnEnabled()) {
150-
log(LogLevel.WARN, cause, buildLog)
151+
if (isWarnEnabled) {
152+
val builder = createLogBuilder(LogLevel.WARN, this)
153+
val message = builder.buildLog()
154+
log(builder, message, cause)
151155
}
152156
}
153157

@@ -196,8 +200,10 @@ internal constructor(
196200
cause: Throwable? = null,
197201
buildLog: LogBuilder.() -> String,
198202
) {
199-
if (underlyingLogger.isErrorEnabled()) {
200-
log(LogLevel.ERROR, cause, buildLog)
203+
if (isErrorEnabled) {
204+
val builder = createLogBuilder(LogLevel.ERROR, this)
205+
val message = builder.buildLog()
206+
log(builder, message, cause)
201207
}
202208
}
203209

@@ -242,8 +248,10 @@ internal constructor(
242248
cause: Throwable? = null,
243249
buildLog: LogBuilder.() -> String,
244250
) {
245-
if (underlyingLogger.isDebugEnabled()) {
246-
log(LogLevel.DEBUG, cause, buildLog)
251+
if (isDebugEnabled) {
252+
val builder = createLogBuilder(LogLevel.DEBUG, this)
253+
val message = builder.buildLog()
254+
log(builder, message, cause)
247255
}
248256
}
249257

@@ -288,8 +296,10 @@ internal constructor(
288296
cause: Throwable? = null,
289297
buildLog: LogBuilder.() -> String,
290298
) {
291-
if (underlyingLogger.isTraceEnabled()) {
292-
log(LogLevel.TRACE, cause, buildLog)
299+
if (isTraceEnabled) {
300+
val builder = createLogBuilder(LogLevel.TRACE, this)
301+
val message = builder.buildLog()
302+
log(builder, message, cause)
293303
}
294304
}
295305

@@ -344,24 +354,23 @@ internal constructor(
344354
buildLog: LogBuilder.() -> String,
345355
) {
346356
if (isEnabledFor(level)) {
347-
log(level, cause, buildLog)
357+
val builder = createLogBuilder(level, this)
358+
val message = builder.buildLog()
359+
log(builder, message, cause)
348360
}
349361
}
350362

351363
@PublishedApi
352-
internal inline fun log(
353-
level: LogLevel,
364+
internal fun log(
365+
logBuilder: LogBuilder,
366+
message: String,
354367
cause: Throwable?,
355-
buildLog: LogBuilder.() -> String,
356368
) {
357-
val builder = LogBuilder(createLogEvent(level, underlyingLogger))
358-
val message = builder.buildLog()
359369
if (cause != null) {
360-
// Call this after buildLog(), so cause exception fields don't overwrite LogBuilder fields
361-
builder.setCause(cause, underlyingLogger)
370+
logBuilder.setCause(cause, underlyingLogger)
362371
}
363372

364-
builder.logEvent.log(message, underlyingLogger)
373+
logBuilder.logEvent.log(message, underlyingLogger)
365374
}
366375

367376
/**
@@ -372,7 +381,7 @@ internal constructor(
372381
* [Logback configuration docs](https://logback.qos.ch/manual/configuration.html#loggerElement)).
373382
*/
374383
public val isErrorEnabled: Boolean
375-
inline get() = underlyingLogger.isErrorEnabled()
384+
get() = underlyingLogger.isErrorEnabled()
376385

377386
/**
378387
* Returns true if [LogLevel.WARN] is enabled for this logger.
@@ -382,7 +391,7 @@ internal constructor(
382391
* [Logback configuration docs](https://logback.qos.ch/manual/configuration.html#loggerElement)).
383392
*/
384393
public val isWarnEnabled: Boolean
385-
inline get() = underlyingLogger.isWarnEnabled()
394+
get() = underlyingLogger.isWarnEnabled()
386395

387396
/**
388397
* Returns true if [LogLevel.INFO] is enabled for this logger.
@@ -392,7 +401,7 @@ internal constructor(
392401
* [Logback configuration docs](https://logback.qos.ch/manual/configuration.html#loggerElement)).
393402
*/
394403
public val isInfoEnabled: Boolean
395-
inline get() = underlyingLogger.isInfoEnabled()
404+
get() = underlyingLogger.isInfoEnabled()
396405

397406
/**
398407
* Returns true if [LogLevel.DEBUG] is enabled for this logger.
@@ -402,7 +411,7 @@ internal constructor(
402411
* [Logback configuration docs](https://logback.qos.ch/manual/configuration.html#loggerElement)).
403412
*/
404413
public val isDebugEnabled: Boolean
405-
inline get() = underlyingLogger.isDebugEnabled()
414+
get() = underlyingLogger.isDebugEnabled()
406415

407416
/**
408417
* Returns true if [LogLevel.TRACE] is enabled for this logger.
@@ -412,7 +421,7 @@ internal constructor(
412421
* [Logback configuration docs](https://logback.qos.ch/manual/configuration.html#loggerElement)).
413422
*/
414423
public val isTraceEnabled: Boolean
415-
inline get() = underlyingLogger.isTraceEnabled()
424+
get() = underlyingLogger.isTraceEnabled()
416425

417426
/**
418427
* Returns true if the given log level is enabled for this logger.
@@ -525,7 +534,6 @@ public expect fun getLogger(name: String): Logger
525534
*
526535
* On the JVM, we use SLF4J as the underlying logger.
527536
*/
528-
@PublishedApi
529537
internal expect interface PlatformLogger {
530538
fun getName(): String
531539

@@ -561,3 +569,13 @@ internal fun normalizeLoggerName(name: String?): String {
561569
else -> name
562570
}
563571
}
572+
573+
/**
574+
* SLF4J has the concept of a "caller boundary": the fully qualified class name of the logger class
575+
* that made the log. This is used by logger implementations, such as Logback, when the user enables
576+
* "caller info": showing the location in the source code where the log was made. Logback then knows
577+
* to exclude stack trace elements up to this caller boundary, since the user wants to see where in
578+
* _their_ code the log was made, not the location in the logging library. In our case, our boundary
579+
* is the [Logger] class.
580+
*/
581+
internal const val LOGGER_CLASS_NAME = "dev.hermannm.devlog.Logger"

src/commonTest/kotlin/dev/hermannm/devlog/LoggerTest.kt

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import dev.hermannm.devlog.testutils.loggerInOtherFile
99
import dev.hermannm.devlog.testutils.parameterizedTest
1010
import io.kotest.matchers.booleans.shouldBeFalse
1111
import io.kotest.matchers.booleans.shouldBeTrue
12+
import io.kotest.matchers.nulls.shouldNotBeNull
1213
import io.kotest.matchers.shouldBe
1314
import kotlin.test.AfterTest
1415
import kotlin.test.Test
@@ -341,6 +342,12 @@ internal class LoggerTest {
341342
return
342343
}
343344
}
345+
346+
@Test
347+
fun `LOGGER_CLASS_NAME has expected value`() {
348+
val expectedName: String = Logger::class.qualifiedName.shouldNotBeNull()
349+
LOGGER_CLASS_NAME shouldBe expectedName
350+
}
344351
}
345352

346353
private val loggerOutsideClass = getLogger()

src/jvmMain/kotlin/dev/hermannm/devlog/LogEvent.jvm.kt

Lines changed: 11 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import org.slf4j.event.Level as Slf4jLevel
2222
import org.slf4j.spi.LocationAwareLogger
2323
import org.slf4j.spi.LoggingEventAware
2424

25-
@PublishedApi
2625
internal actual fun createLogEvent(level: LogLevel, logger: Slf4jLogger): LogEvent {
2726
if (LOGBACK_IS_ON_CLASSPATH && logger is LogbackLogger) {
2827
return LogbackLogEvent(level, logger)
@@ -55,7 +54,7 @@ internal class Slf4jLogEvent(
5554
logger: Slf4jLogger,
5655
) : LogEvent, BaseSlf4jEvent(level.toSlf4j(), logger) {
5756
init {
58-
this.callerBoundary = FULLY_QUALIFIED_CLASS_NAME
57+
this.callerBoundary = LOGGER_CLASS_NAME
5958
this.timeStamp = System.currentTimeMillis()
6059
}
6160

@@ -152,11 +151,6 @@ internal class Slf4jLogEvent(
152151

153152
/** We don't traverse the cause exception tree in this log event implementation. */
154153
override fun handlesExceptionTreeTraversal(): Boolean = false
155-
156-
internal companion object {
157-
/** See [LogbackLogEvent.FULLY_QUALIFIED_CLASS_NAME]. */
158-
@JvmField internal val FULLY_QUALIFIED_CLASS_NAME = Slf4jLogEvent::class.java.name
159-
}
160154
}
161155

162156
internal fun LogLevel.toSlf4j(): Slf4jLevel {
@@ -173,7 +167,7 @@ internal fun LogLevel.toSlf4j(): Slf4jLevel {
173167
internal class LogbackLogEvent(level: LogLevel, logger: LogbackLogger) :
174168
LogEvent,
175169
BaseLogbackEvent(
176-
FULLY_QUALIFIED_CLASS_NAME,
170+
LOGGER_CLASS_NAME,
177171
logger,
178172
level.toLogback(),
179173
null, // message (we set this when finalizing the log)
@@ -223,33 +217,16 @@ internal class LogbackLogEvent(level: LogLevel, logger: LogbackLogger) :
223217
* [setCause].
224218
*/
225219
override fun handlesExceptionTreeTraversal(): Boolean = true
220+
}
226221

227-
internal companion object {
228-
/**
229-
* When a [LogbackLogEvent] is constructed, we know the `logger` is a [LogbackLogger]. So when
230-
* we receive the logger again in [setCause] and [log], we can safely cast it. It's fast to cast
231-
* to a concrete class, so we'd rather do that than increase the allocated size of the event by
232-
* adding a field.
233-
*/
234-
@JvmStatic
235-
private fun PlatformLogger.asLogbackLogger(): LogbackLogger {
236-
return this as LogbackLogger
237-
}
238-
239-
/**
240-
* SLF4J has the concept of a "caller boundary": the fully qualified class name of the logger
241-
* class that made the log. This is used by logger implementations, such as Logback, when the
242-
* user enables "caller info": showing the location in the source code where the log was made.
243-
* Logback then knows to exclude stack trace elements up to this caller boundary, since the user
244-
* wants to see where in _their_ code the log was made, not the location in the logging library.
245-
*
246-
* In our case, the caller boundary is in fact not [dev.hermannm.devlog.Logger], but our
247-
* [LogEvent] implementations. This is because all the methods on `Logger` are `inline` - so the
248-
* logger method actually called by user code at runtime is [LogbackLogEvent.log] /
249-
* [Slf4jLogEvent.log].
250-
*/
251-
@JvmField internal val FULLY_QUALIFIED_CLASS_NAME = LogbackLogEvent::class.java.name
252-
}
222+
/**
223+
* When a [LogbackLogEvent] is constructed, we know the `logger` is a [LogbackLogger]. So when we
224+
* receive the logger again in [LogbackLogEvent.log], we can safely cast it. It's fast to cast to a
225+
* concrete class, so we'd rather do that than increase the allocated size of the event by adding a
226+
* field.
227+
*/
228+
private fun PlatformLogger.asLogbackLogger(): LogbackLogger {
229+
return this as LogbackLogger
253230
}
254231

255232
internal fun LogLevel.toLogback(): LogbackLevel {

src/jvmMain/kotlin/dev/hermannm/devlog/Logger.jvm.kt

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,6 @@
11
@file:JvmName("LoggerJvm")
2-
@file:Suppress(
3-
// We want `getLogger()` to be inline, so that `lookupClass()` is called in the caller's scope
4-
"NOTHING_TO_INLINE",
5-
// The `expect` declaration of the `PlatformLogger` interface is annotated with `@PublishedApi`,
6-
// so we can use it in inline methods. Kotlin warns us that the `actual` definition should have
7-
// the same annotations, but here we use a `typealias`, which doesn't work with `@PublishedApi`.
8-
// But this is fine here, because we typealias `PlatformLogger` to the public SLF4J Logger,
9-
// which works like `@PublishedApi`.
10-
"ACTUAL_ANNOTATIONS_NOT_MATCH_EXPECT",
11-
)
2+
// We want `getLogger()` to be inline, so that `lookupClass()` is called in the caller's scope
3+
@file:Suppress("NOTHING_TO_INLINE")
124

135
package dev.hermannm.devlog
146

0 commit comments

Comments
 (0)