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

[RFC] HoistLogAction with MVar #44

Open
yhuangsh opened this issue Jan 8, 2025 · 3 comments
Open

[RFC] HoistLogAction with MVar #44

yhuangsh opened this issue Jan 8, 2025 · 3 comments

Comments

@yhuangsh
Copy link

yhuangsh commented Jan 8, 2025

Case use:

  • Have “global” log action that is mutable at run time. For example, the default log action is at a low verbosity level at normal time, a system admin could switch to a higher verbosity level logs without having to restart a running server
  • MVar allows another (admin) thread to change the default log action on the fly via the MVar
  • Do not use any of the performUnsafeIO hacks
module Colog.Core.Action (hoistLogActionMVar , hoistLogActionIOMVar) where

import Colog (LogAction(..), hoistLogAction)
import Control.Concurrent.MVar (MVar, readMVar)
import Control.Monad.IO.Class (MonadIO(..))

hoistLogActionMVar ::  MonadIO n => (forall a. m a -> n a) -> MVar (LogAction m msg) -> LogAction n msg
hoistLogActionMVar f v =
   LogAction
     \msg -> do
        m <- liftIO $ readMVar v
        unLogAction (hoistLogAction f m) msg

hoistLogActionIOMVar :: MonadIO m => MVar (LogAction IO msg) -> LogAction m msg
hoistLogActionIOMVar = hoistLogActionMVar liftIO

Every log action will involve additional IO (readMVar), performance impact not assessed.

Appreciate comments

@alaendle
Copy link
Member

alaendle commented Jan 8, 2025

Hi @yhuangsh, maybe I'm not experienced enough or didn't fully understand the intended usage - or need some more time to think about this, but at a first look the design seems somehow strange to me. I can understand the first bullet point, but this could be a simple cfilterM, or not? Regarding point two - the thread - do we really want this (nondeterministic) behaviour? At the moment I personally couldn't imagine a useful scenario where we really want to change the underlying logAction completely. We also have cmapM at our disposal, so we can influence the content of the message and therefore also the concrete sink if needed. So maybe I'm reluctant regarding anything mutable 😉, but if really needed I would suggest that this is up to the client app using the already provided primitives. But maybe I've missed something important - if so, maybe you can underpin your point, by providing some pseudocode how you plan to use this feature?

@yhuangsh
Copy link
Author

hi @alaendle, my use case boils down to one admin thread and a number of other threads for various server functions, each implemented in its own unique Monad transformer stack which in turn includes a LogAction, conceptually wrapped in a ReaderT layer of the stack. The admin thread supplies the LogActions to these server threads at launch, and may want to change level of verbosity for all or some of the servers. For example, turning on Debug if some server isn’t working properly. Using MVar is just standard way to transport a LogAction from the admin thread to the server threads, nothing fancy here. The admin thread does use some of the combinators (I used filterBySeverity) to filter logs or apply other transforms on the base LogAction, then transport the modified LogAction to target servers via MVars. Hope this explain my use case. I’m sure it’s not the only way to do it.

@alaendle
Copy link
Member

Thanks @yhuangsh for the additional explanation. However I'm not sure about your intention. Did you just seek feedback regarding your usage of the library? Or was the intention that we should included something like the mentioned functions in the library?

Without having all the details of your code it's really hard fully assess your approach; I would say, if it works for you it's fine - at least I see no obvious reason why it could be problematic. Personally I still tend to think that the complete switch of the action might be too powerful approach to be promoted as a general pattern.

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

No branches or pull requests

2 participants