Upgrade ZIO Logging #782
Upgrade ZIO Logging #782
Conversation
vagroz
requested review from
catostrophe,
danslapman,
FunFunFine,
Odomontois and
oskin1
as code owners
| } | ||
| ``` | ||
| Run the program and look at the output: | ||
| ```json lines |
There was a problem hiding this comment.
Perhaps it's better to write this example using ConsoleContextLayout for readability. Even on 23'' screen I have to scroll it right now to see the main stuff.
There was a problem hiding this comment.
I think it's ok just remove extra fields
docs/tofu.logging.recipes.zio.md
Outdated
| @derive(loggable) | ||
| final case class Context(requestId: Int) | ||
| ``` | ||
| Here we use [Tofu Derevo](https://github.com/tofu-tf/derevo) for automatic derivation `Loggable` instance. |
There was a problem hiding this comment.
It would be nice to see the link to Loggable page here.
docs/tofu.logging.recipes.zio.md
Outdated
| ``` | ||
| Here we use [Tofu Derevo](https://github.com/tofu-tf/derevo) for automatic derivation `Loggable` instance. | ||
|
|
||
| The main idea of this approach is to store your in ZIO [FiberRef](https://zio.dev/docs/datatypes/fiber/fiberref). |
There was a problem hiding this comment.
What if someone would want to store the Context not in FiberRef, but in R itself (which is blasphemy of course)? Should we have a note about this approach here?
There was a problem hiding this comment.
Yes, we should. But I've never use the R as a context. It may be useful with TF and WithContext monad, but I'm not good in TF, so I'm not able to write a good example with that approach.
Now I can note that this is one of possible ways
| import tofu.logging.{LoggedValue, Logging} | ||
| import zio._ | ||
|
|
||
| class ZUniversalContextLogging[R](name: String, fctx: (LoggedValue => Unit) => URIO[R, Unit]) |
There was a problem hiding this comment.
fctx: (LoggedValue => Unit) => URIO[R, Unit]
We definitely need a scaladoc with a note on unsafety of this argument
There was a problem hiding this comment.
I feel we can simplify that down to
ctxLog: URIO[R, LoggedValue]
and use direct flatMap in the write methods instead of callback approach
There was a problem hiding this comment.
Oh, I've got it. I'll fix
| @@ -37,6 +39,34 @@ class ZLogsSuite extends AnyFlatSpec with Matchers { | |||
| LogTree(ctx) | |||
| } should ===(List.fill(2)(expected)) | |||
| } | |||
|
|
|||
| "ZLogs plain with context" should "log the context" in { | |||
| type CtxService = FiberRef[(FooService, BarService)] | |||
There was a problem hiding this comment.
Is it possible to rework this spec? It's kinda confusing when I see context which is named as some service or something.
There was a problem hiding this comment.
context which is named as some service
Look, actually FiberRef[(FooService, BarService)] is a service providing the context (FooService, BarService), right?
From ZIO docs:
Has[A] represents a dependency on a service of type A
And val logLayer: URLayer[Has[CtxService], Has[ZLogging.Make]] below depends on this
In production the service would be more complex like
trait CtxService {
def getContext: UIO[(GlobalSate, FiberState)]
def setReadiness(v: Boolean): UIO[Unit]
//... etc
}
53c7e5c
to
04272a7
Compare
| import zio._ | ||
|
|
||
| class ZUniversalContextLogs[R: Loggable] extends ZLogging.ZMake[R] { | ||
| private def useContextValue(f: LoggedValue => Unit): URIO[R, Unit] = ZIO.access[R](f(_)) |
There was a problem hiding this comment.
This approach is a little bit troublesome, since we imply some unexeptional effect. Call of f should probably be wrapped by the effectTotal. I suppose this is the main concern of @FunFunFine towards LoggedValue => Unit in the previous thread.
Still URIO[R, LoggedValue] should work safely
This PR brings changes of tofu-zio-logging according to the new Module Pattern 2.0. Plain logging with encapsulated FiberRef context will be especially useful.
I've written the documentation how to use it. Pls check my grammar.
Also fix logging documentation: