Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Logging #56

Open
allantl opened this issue Jan 1, 2018 · 10 comments
Open

Implement Logging #56

allantl opened this issue Jan 1, 2018 · 10 comments

Comments

@allantl
Copy link

allantl commented Jan 1, 2018

Hi, do you think it would be a good idea to add logging since its such a common use case in every project?

@pauljamescleary
Copy link
Owner

pauljamescleary commented Jan 1, 2018

@allantl I think it is a good idea. Honestly, I would be open to PRs for this one.

When I have done logging I typically just accept it as side effecting and live with the "logger.info" calls. Obviously that is not very functional.

The alternative I have seen is to use something like a State or Writer monad. I haven't done that myself yet, but would be an interesting exercise to see how it materializes in the app.

I will leave this as an issue as it is valid and see if we get any PRs. When I get done some of the other issues, I will do some digging.

@pauljamescleary
Copy link
Owner

I would say that an advantage of separation of interpretation as well as abstracting away the effect type makes introducing a writer monad easier

@tbrown1979
Copy link
Contributor

The Writer implementation for logging has some drawbacks from the implementations I've seen. Basically people use WriterT[F[_], ?, ?]s but if your F[_] (IO, Task, etc...) fails then you'd lose your logs that are in your Writer. Maybe there's a better way to do that.

People have had discussions about this on the scala sub-reddit before. My company has followed suit with the [deleted] poster in that we log and consider it RT. Almost always that happens within the context of some monad like IO or Task.

@pauljamescleary
Copy link
Owner

Interesting, boo. Thanks for that info.

Funny you mention that reddit post because I read it as well when I created this issue :)

@nolledge
Copy link

@pauljamescleary would you be interested in an integration of Log4Cats for the pet store?

@pauljamescleary
Copy link
Owner

@nolledge That looks like an interesting library. This doesn't follow the Writer pattern as discussed above, but rather accepts some side effecting in your effect type using logger.

tbh, I do this alot today, without a library, but it is done in the interpreters themselves...

for {
  _ <- IO(logger.warn(xxx))
  x <- IO(doSomethingReal)
} yield x

Kinda curious to see a PR

@nebtrx
Copy link
Contributor

nebtrx commented Dec 18, 2018

@pauljamescleary @nolledge The main benefit of using Log4Cats is you can use it with several existing backends. BTW, it says it has log4cats-extras with apparently works with mapK and Writer. Perhaps it worths to take a look..

@nolledge
Copy link

Hi @nebtrx @pauljamescleary,
I started to work on an implementation of log4cats. Currently I am not sure if I am moving into the right direction: I added a logger to the constructor of logging classes, mostrly because creating a logger inside of those classes would require an instance of Sync for every class creating its own logger. Do you think this is the right approach?

Also the current logging mechanism in Doobie does not use an F[_] encoding.

Christof Nolle
@nolledge
Nov 25 2018 15:25
Hi, I am kind of stuck trying to use a LogHandler in conjunction with log4cats. Has anyone of you tried to bring those two together?
_
Rob Norris
@tpolecat
Nov 25 2018 19:06
@nolledge Yeah LogHandler defines an unsafeRun method that's lifted into PreparedStatementIO via .delay so right now there's not a great way to do it. What you need to do is create an SLF4J logger explicitly, then use it to construct your log4cats logger and a doobie LogHandler so they both end up with the same underlying firehose.
The tagless encoding of doobie that's coming soon(ish) will likely use log4cats explicitly.

https://github.com/nolledge/scala-pet-store/tree/feat/log4cats

What do you guys think?

@nebtrx
Copy link
Contributor

nebtrx commented Jan 10, 2019

@nolledge right! I haven't thought about that. I'll gladly take a look at it.

@tpolecat
Copy link

doobie 0.7.x will probably have Log4Cats support through the transactor, FYI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants