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.

Sign up for GitHub

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

Jump to bottom

Add some helpers for using Kleisli or IOLocal for logging context #676

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+134 −4
Open

Add some helpers for using Kleisli or IOLocal for logging context #676

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8709392
Add helpers for logging with some propagated ctx and kleisli implemen…
wunderk1nd-e Aug 17, 2022
d8f1164
Add ce3 module and iolocal instances
wunderk1nd-e Aug 17, 2022
d2d340d
Merge branch 'main' into ctxtual
rossabaker Feb 20, 2023
f0629f3
Regenerate workflow after merge
rossabaker Feb 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,11 +119,11 @@ jobs:

- name: Make target directories
if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main')
run: mkdir -p testing/jvm/target noop/jvm/target target .js/target core/native/target site/target testing/native/target noop/native/target core/js/target js-console/target testing/js/target noop/js/target core/jvm/target .jvm/target .native/target slf4j/target project/target
run: mkdir -p testing/jvm/target noop/jvm/target target .js/target core/native/target site/target testing/native/target ce3/js/target noop/native/target core/js/target js-console/target testing/js/target noop/js/target core/jvm/target .jvm/target .native/target slf4j/target ce3/jvm/target project/target

- name: Compress target directories
if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main')
run: tar cf targets.tar testing/jvm/target noop/jvm/target target .js/target core/native/target site/target testing/native/target noop/native/target core/js/target js-console/target testing/js/target noop/js/target core/jvm/target .jvm/target .native/target slf4j/target project/target
run: tar cf targets.tar testing/jvm/target noop/jvm/target target .js/target core/native/target site/target testing/native/target ce3/js/target noop/native/target core/js/target js-console/target testing/js/target noop/js/target core/jvm/target .jvm/target .native/target slf4j/target ce3/jvm/target project/target

- name: Upload target directories
if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main')
Expand Down
10 changes: 10 additions & 0 deletions build.sbt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,16 @@ lazy val noop = crossProject(JSPlatform, JVMPlatform, NativePlatform)
)
.nativeSettings(commonNativeSettings)

lazy val ce3 = crossProject(JSPlatform, JVMPlatform)
.settings(commonSettings)
.dependsOn(core)
.settings(
name := "log4cats-ce3",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would call this log4cats-cats-effect. Anything beyond 3 will be a breaking change, and I'd avoid colloquial abbrevations in a jar name.

libraryDependencies ++= Seq(
"org.typelevel" %%% "cats-effect" % catsEffectV
)
)

Comment on lines +79 to +88
Copy link
Member

@kubukoz kubukoz Aug 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't core already depend on CE3? oh, it doesn't. cool

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably we can move this to the slf4j module to don't create a new module. Speaking from a user experience it'd be handier - one wouldn't need to add a new dependency. WDYT?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to go with whatever you guys think is most appropriate - just didn't add it to either of the existing ones because:

  • Core doesn't depend on CE3
  • Isn't strictly speaking an slf4j concern

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With #678 we will be depending on cats-effect, and I see no other way to resolve the UUIDGen issue otherwise.

We'll have to track how the dependency on CE in core goes there as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now it depends on cats-effect-std. This still introduces the IO dependency.

lazy val slf4j = project
.settings(commonSettings)
.dependsOn(core.jvm)
Expand Down
16 changes: 16 additions & 0 deletions ce3/shared/src/main/scala/org/typelevel/log4cats/ce3/IOLocalHelpers.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package org.typelevel.log4cats.ce3

import cats.effect.{IO, IOLocal}
import org.typelevel.log4cats.{LoggerFactory, SelfAwareStructuredLogger}

object IOLocalHelpers {
def loggerWithContextFromIOLocal(
sl: SelfAwareStructuredLogger[IO],
local: IOLocal[Map[String, String]]
): SelfAwareStructuredLogger[IO] = SelfAwareStructuredLogger.withContextF(sl)(local.get)

def factoryWithContextFromIOLocal(
lf: LoggerFactory[IO],
local: IOLocal[Map[String, String]]
): LoggerFactory[IO] = LoggerFactory.withContextF(lf)(local.get)
}
22 changes: 20 additions & 2 deletions core/shared/src/main/scala/org/typelevel/log4cats/LoggerFactory.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@

package org.typelevel.log4cats

import cats.FlatMap
import cats.Functor
import cats.Monad
import cats.data.EitherT
import cats.data.Kleisli
import cats.data.OptionT
import cats.syntax.functor._
import cats.~>
import cats.data.OptionT
import cats.data.EitherT

import scala.annotation.implicitNotFound

Expand Down Expand Up @@ -60,4 +62,20 @@ object LoggerFactory extends LoggerFactoryGenCompanion {
fk(logger)
}
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I somehow missed that LoggerFactory was a thing - nice!

def withContextF[F[_]: FlatMap](
lf: LoggerFactory[F]
)(ctx: F[Map[String, String]]): LoggerFactory[F] =
new LoggerFactory[F] {
override def getLoggerFromName(name: String): SelfAwareStructuredLogger[F] =
SelfAwareStructuredLogger.withContextF(lf.getLoggerFromName(name))(ctx)

override def fromName(name: String): F[SelfAwareStructuredLogger[F]] = lf
.fromName(name)
.map(SelfAwareStructuredLogger.withContextF(_)(ctx))
}

def withContextFromKleisli[F[_]: Monad](
lf: LoggerFactory[Kleisli[F, Map[String, String], *]]
): LoggerFactory[Kleisli[F, Map[String, String], *]] = withContextF(lf)(Kleisli.ask)
}
86 changes: 86 additions & 0 deletions core/shared/src/main/scala/org/typelevel/log4cats/SelfAwareStructuredLogger.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ package org.typelevel.log4cats

import cats._
import cats.Show.Shown
import cats.data.Kleisli
import cats.implicits.{toFlatMapOps, toFunctorOps}

trait SelfAwareStructuredLogger[F[_]] extends SelfAwareLogger[F] with StructuredLogger[F] {
override def mapK[G[_]](fk: F ~> G): SelfAwareStructuredLogger[G] =
Expand Down Expand Up @@ -94,6 +96,90 @@ object SelfAwareStructuredLogger {
sl.trace(modify(ctx), t)(message)
}

def withContextFromKleisli[F[_]: Monad](
sl: SelfAwareStructuredLogger[Kleisli[F, Map[String, String], *]]
): SelfAwareStructuredLogger[Kleisli[F, Map[String, String], *]] =
withContextF(sl)(
Kleisli.ask[F, Map[String, String]]
)
def withContextF[F[_]: FlatMap](
sl: SelfAwareStructuredLogger[F]
)(ctx: F[Map[String, String]]): SelfAwareStructuredLogger[F] =
new ModifiedContextFSelfAwareStructuredLogger[F](sl)(existingCtx => ctx.map(_ ++ existingCtx))
Comment on lines +105 to +108
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what we're missing is something like this:

def fa: F[A]

def faWithContext: F[A] = StructuredLogger[F].locally(Map("k" -> "v"))(fa)

so that you can wrap an effect, not just a logger. That would help propagate the context to whatever services you're calling, without a need to pass the context or the updated logger.

Copy link
Author

@wunderk1nd-e wunderk1nd-e Aug 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I guess I was thinking of users doing that using Kleisli.local or similar for IOLocal.
e.g.

  def scope[G, T](local: IOLocal[T], value: T)(task: IO[G]): IO[G] =
    local.getAndSet(value).bracket(_ => task)(local.set)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How are you imagining something like StructuredLogger[F].locally would work?
Just specific implementations for where there's a IOLocal or Kleisli around?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I think we'll need concrete implementations

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have come to believe that almost all uses of IOLocal should have that cats.mtl.Local shape. See typelevel/cats-effect#3385. And I like the scope name: that's what Local calls it.


private class ModifiedContextFSelfAwareStructuredLogger[F[_]: FlatMap](
sl: SelfAwareStructuredLogger[F]
)(
modify: Map[String, String] => F[Map[String, String]]
) extends SelfAwareStructuredLogger[F] {
private lazy val defaultCtx: F[Map[String, String]] = modify(Map.empty)

def error(message: => String): F[Unit] = defaultCtx.flatMap(sl.error(_)(message))

def warn(message: => String): F[Unit] = defaultCtx.flatMap(sl.warn(_)(message))

def info(message: => String): F[Unit] = defaultCtx.flatMap(sl.info(_)(message))

def debug(message: => String): F[Unit] = defaultCtx.flatMap(sl.debug(_)(message))

def trace(message: => String): F[Unit] = defaultCtx.flatMap(sl.trace(_)(message))

def trace(ctx: Map[String, String])(msg: => String): F[Unit] =
modify(ctx).flatMap(sl.trace(_)(msg))

def debug(ctx: Map[String, String])(msg: => String): F[Unit] =
modify(ctx).flatMap(sl.debug(_)(msg))

def info(ctx: Map[String, String])(msg: => String): F[Unit] =
modify(ctx).flatMap(sl.info(_)(msg))

def warn(ctx: Map[String, String])(msg: => String): F[Unit] =
modify(ctx).flatMap(sl.warn(_)(msg))

def error(ctx: Map[String, String])(msg: => String): F[Unit] =
modify(ctx).flatMap(sl.error(_)(msg))

def isTraceEnabled: F[Boolean] = sl.isTraceEnabled

def isDebugEnabled: F[Boolean] = sl.isDebugEnabled

def isInfoEnabled: F[Boolean] = sl.isInfoEnabled

def isWarnEnabled: F[Boolean] = sl.isWarnEnabled

def isErrorEnabled: F[Boolean] = sl.isErrorEnabled

def error(t: Throwable)(message: => String): F[Unit] =
defaultCtx.flatMap(sl.error(_, t)(message))

def warn(t: Throwable)(message: => String): F[Unit] =
defaultCtx.flatMap(sl.warn(_, t)(message))

def info(t: Throwable)(message: => String): F[Unit] =
defaultCtx.flatMap(sl.info(_, t)(message))

def debug(t: Throwable)(message: => String): F[Unit] =
defaultCtx.flatMap(sl.debug(_, t)(message))

def trace(t: Throwable)(message: => String): F[Unit] =
defaultCtx.flatMap(sl.trace(_, t)(message))

def error(ctx: Map[String, String], t: Throwable)(message: => String): F[Unit] =
modify(ctx).flatMap(sl.error(_, t)(message))

def warn(ctx: Map[String, String], t: Throwable)(message: => String): F[Unit] =
modify(ctx).flatMap(sl.warn(_, t)(message))

def info(ctx: Map[String, String], t: Throwable)(message: => String): F[Unit] =
modify(ctx).flatMap(sl.info(_, t)(message))

def debug(ctx: Map[String, String], t: Throwable)(message: => String): F[Unit] =
modify(ctx).flatMap(sl.debug(_, t)(message))

def trace(ctx: Map[String, String], t: Throwable)(message: => String): F[Unit] =
modify(ctx).flatMap(sl.trace(_, t)(message))
}

private def withModifiedString[F[_]](
l: SelfAwareStructuredLogger[F],
f: String => String
Expand Down