Alternative : Alternative io log4s proposal #33
Conversation
The log4cats-slf4j module now uses the macro based implementation from iolog4s: https://iolog4s.github.io/iolog4s/ This one is essentially identical to the log4s approach, but macros generate code that wrap statements in F.delay when appropriate.
This solution fixes a problem in the previous commit where the program would crash with ??? when the logger was being referenced from the `Logger[F]` interface. What this implementation effectively achives is that when the user uses the the the slf4j backend wrapper via the subtype io.chrisdavenport.log4cats.slf4j.Slf4jLogger then they get macro-based optimizations like you see in iolog4s. But when they switch to the abstract algebra of `Logger[F]` then they lose this macro-based optimization and they get something that is equivalent to the old log4cats wrapper.
| case Some(e) => List(msg, e.tree) | ||
| } | ||
| val logExpr = q"$logger.${TermName(logLevel.methodName)}(..$logValues)" | ||
| val checkExpr = q"$logger.${TermName(s"is${logLevel.name}Enabled")}" |
There was a problem hiding this comment.
This calls .isEnabled on the java logger. I didn't delay this in an F[_] because from the looks of it it seemed to be referentially transparent once the logger is created. Creation of the logger itself being the impure operation.
But I wasn't 100% sure 😊
There was a problem hiding this comment.
By what criteria is the creation of the logger not referentially transparent? As in because it loads resources. In that case do you think the methods should return F[Logger[F]]
There was a problem hiding this comment.
@ChristopherDavenport, yes I was thinking about the fact that logger creation loads resources, therefore it is unsafe.
We could leave current methods as they are (since I have a hunch that even all us FP people still use unsafe creation methods), and maybe add additional methods of the form:safe${factoryMethod} : F[Logger[F]].
This latter approach is slightly problematic since the convention is usually the other way around: adding the unsafe word to method names. Therefore, the second option would be to stick to convention, and basically ruin all current factory methods, and provide the same functionality under unsafe${factoryMethod}: Logger[F] method names.
I'm also fine with not doing anything else 🤷♂️. I have never had problems with the impurity of logger creation, only with the impurity of logging itself 😄.
There was a problem hiding this comment.
I agree. I think I'll throw these into an unsafe name, and with a corresponding equivalent one in F just so that we are totally transparent.
Alternative possible solution for mdc/awareness/logger algebras.
Creates an alternative implementation that is created in an internal modules and then exposed higher.
@lorandszakacs RFC