Skip to content

Deferred log action in server Logger middleware#7744

Open
alexcardell wants to merge 15 commits into
http4s:series/0.23from
alexcardell:deferred-logger
Open

Deferred log action in server Logger middleware#7744
alexcardell wants to merge 15 commits into
http4s:series/0.23from
alexcardell:deferred-logger

Conversation

@alexcardell

@alexcardell alexcardell commented Oct 30, 2025

Copy link
Copy Markdown

Built on the work of @iRevive in #7446 but adding an intermediate config builder object to keep compatibility

I hate naming things XConfig but what else is there? Please bikeshed away

I think you could make an argument that withDeferredLogAction is not the best name as it isn't clear that withLogAction may give unintuitive behaviour, perhaps that should be withLogAction and a withSomethingLogAction indicates the non-deferred one

@mergify mergify Bot added series/0.23 PRs targeting 0.23.x module:server docs Relates to our website or tutorials labels Oct 30, 2025
* val logger: Logger[IO] = ...
* val logF: IO[String => IO[Unit]] =
* ioLocal.get.map { ctx =>
* { msg => logger.info("log msg and ctx: ...") }

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

String interpolating the values for the example fails doc generation on native and JS

* @param redactHeadersWhen Function to determine which headers should be redacted
* @param logAction The effectful log action that returns a logging function
*/
sealed abstract case class LoggerConfig[F[_]] private[middleware] (

@iRevive iRevive Nov 3, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would it be problematic to extend it in a binary-compatible way in the future?
Even though the case class is sealed.

https://mimalyzer.indoorvivants.com/comparison/a134a982-9f94-4f36-8d8f-7f7b05a0c270

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I've changed it to a sealed abstract class and mimalyzer seems happy. New version: https://mimalyzer.indoorvivants.com/comparison/e8f82479-11fd-4678-a1f3-adc8f92f3723

(Handy little tool, I hadn't come across it! Is there a way to validate that sort of thing via the sbt plugin?)

I am not sure if there is a simpler approach for extending these builders in a binary-compatible way and would be happy to simplify it

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there a way to validate that sort of thing via the sbt plugin?

Only making changes, committing them, likely creating a tag, and running mimaReportBinaryIssues, I'm afraid ☹️

I think sealed abstract class is good enough.

@alexcardell alexcardell requested a review from iRevive November 17, 2025 16:31

@iRevive iRevive left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great stuff!

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

Labels

docs Relates to our website or tutorials module:server series/0.23 PRs targeting 0.23.x

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants