Update #676: Adopt cats.mtl.Local for Logging Context and IOLocal

[Jay-Lokhande]

this PR is continuation of #676, which have some modification since CE v3.6.0 was released dependency on Cats MTL and a Local instance for IOLocal
I have removed ce3/shared/src/main/scala/org/typelevel/log4cats/ce3/IOLocalHelpers.scala
and moved implementation to the core module

[Jay-Lokhande]

@armanbilge would you please look at my changes. 😊

[armanbilge]

@morgen-peschke would you be able to take a look? 🙏

[Jay-Lokhande]

@morgen-peschke would you please review my changes 😊

[kubukoz]

question: given this operates on Local[IO, can we make it an F[_] and move the whole thing back to the core module?

[kubukoz]

also, would love to see these as extension functions. .withLocalContext, maybe? (they can both be named the same, as they'd be called on different types)

[Jay-Lokhande]

Thank you for the review.
Based on your review
I must delete this file
and create one in the core/src/main/scala/org/typelevel/log4cats/LocalHelpers.scala (core module)

[kubukoz]

sounds about right :)

[Jay-Lokhande]

Hey, I have made some changes as you suggested

[kubukoz]

Just build.sbt I think, looks like CI is generated from that.

[Jay-Lokhande]

Yes, I will update the build.sbt, actually, while doing upstream with the latest, I accidentally removed ce3 from ci.yml. Then, looking at the previous commit, I added it manually. Thank you for the clarification.

[Jay-Lokhande]

I see there is a command for generating workflow sbt githubWorkflowGenerate, now I learned it😊

[kubukoz]

Indeed - if you run inspect githubWorkflowGenerate in sbt, it should tell you about the name of the file it comes from. And based on that you might be able to infer that it's from https://github.com/typelevel/sbt-typelevel/ - good to know in the future, in case you need to work with GH workflows via sbt more.

[Jay-Lokhande]

Thanks for pointing that out! I’ll check out inspect githubWorkflowGenerate in sbt for more details. Good to know about the connection to sbt-typelevel, definitely helpful for working with GitHub workflows in the future. Appreciate the tip! 😊

[NthPortal]

I have a few issues with this API design:

1. the Local is not bundled with the Logger, so it is easy to forget to propagate/expose it with the Logger. also with the type Local[F, Map[String, String]], it is not necessarily obvious what it is with it not being bundled with the Logger.
2. it is possible to forget to attach the Local to the Logger, or to pass the wrong Local along with the Logger and have the Local that is passed around not affect the Logger.
3. it is difficult (though not impossible) to have additional context that's not in the Local automatically added to every logging operation, such as one might want to do with traceId and spanId for a span.
4. exposing the Local directly allows users to remove existing context, which was up until now not possible and is almost certainly undesirable. it would be better to expose only methods that allow users to have certain context added for a certain effect/scope.
5. this is a more opinionated concern, but: this design encourages the use of the inflexible StructuredLogger API—the API whose inflexibility is the primary reason for this change. the user is always passing around a SelfAwareStructuredLogger, and particularly users who are not familiar with the local semantics are likely to ignore the Local and use the sub-optimal addContext methods on the Logger. I would much prefer if there was a separate LocalLogger API that exposed a (deprecated) method to access the underlying SelfAwareStructuredLogger, for gradual migration purposes.

there is also an implementation issue that the logging methods should check is<Level>Enabled before logging so that they're nearly a no-op when disabled, but that's fairly easy (if tedious) to fix and is not externally visible.

I designed #909 to solve these API shortcomings.

[Jay-Lokhande]

Hello @NthPortal ,
Thank you so much for the detailed review
I would like to study your observation/suggestions on this PR, and would like to give a try to make changes based on that. Your guidance will be very helpful for me as I proceed....

[NthPortal]

@Jay-Lokhande I'll try

[Jay-Lokhande]

Hello,
I have created this PR by fork of a already forked log4cats. I need to create a fork of original repo to begin with fresh code as project assigned but I am unable to do so because of earlier fork.
so, I need a permission to close this PR, then I will delete the fork and create a new one with original repo and will re-open the PR from a new fork