Fast logger migration #274

RiugaBachi · 2023-03-29T13:54:00Z

This PR implements the changes discussed in #257 as well as some concerns discussed privately, namely:

Configurable logging to stderr (via rotate-logs config option)
- This can be useful i.e. when keter is managed via a systemd, which automatically captures and tails stderr output.
Implementation of a KeterM cfg a monad to encapsulate a (logging + config) context, which crops up prominently.
- For contexts that only require the logging capability, we can simply leave cfg polymorphic!
Refactoring of relevant modules to use the new KeterM monad to:
- Allow access to MonadLogger -related capabilities via the internal LoggingT representation.
- Cut down on redundant passing of the same configs and loggers everywhere.
Changing of logging calls to use TH-based logging from monad-logger, giving us access to Loc (module+line number) info at each call site.
Reformatting of internal log messages to make use of the new info with format: {keter|}$time|$module$:$line_num|$log_level> $msg
Where {...} signifies a part strictly only present when logging to stderr.
Reformatting of external (process monitor, individual apps) logs to be tagged with their name:
{process-monitor> }$msg
and
{app-$name> }$msg
respectively. (Same {...} semantics as above)
- This was a minor design compromise as we would have had to incur a big lift IO / unlift IO mess to propagate LoggingT to the whole Keter.Conduit.Process.Unix module. The extra info here is also not as useful as it is for internal logs. Perhaps in the future log formatting can be streamlined in a separate PR.
Removal of the LogMessage ADT and its gigantic Show instance, which was a bit overkill and clunky for logging.
Removal of the old in-house logging solution.
Removal of the old logEx utility.

As was discussed privately, log rotation naming and directory scheme have had to be changed due to limitations in fast-logger. Namely, that it dictates the naming scheme of rotated logs with no straightforward way of customizing it to preserve backwards compatibility. On top of the public keter API changes due to the KeterM refactor, package version has been bumped to 2.1.0 to indicate a breaking change.

Relevant parts of the test suite have also been rewritten to comply with the API changes, but overall semantics are still retained.

TODO:

Resolve individual todo comments (@jappeace could use your input on some assumptions 🙏) ✔️
Version bounds on monad-logger / fast-logger / unliftio-core? ✔️
Refactoring ✔️
- Possibly some things we can move to LogFile module ✔️
  -> Generalized logger creation in Keter.Main and Keter.App to Keter.Logger.createLoggerViaConfig
- Rename/change dir of LogFile module? ✔️
  -> Moved to Keter.Logger
- ~~Investigate if we really still need a TVar for RotatingLog since fast-logger was implemented with the assumption of multithreaded contexts.~~
- ~~Formatting~~
Propagate a logging context around ✔️
Change individual log calls to source line log splices based on the show instance for the log message ADT. ✔️
Then remove said ADT. ✔️
Fix stubbed out parts of the test suite ✔️
Update changelog again ✔️
Bump version in cabal file ✔️

based on config

jappeace · 2023-03-29T15:34:24Z

ChangeLog.md

+  Rotated logs will now simply have `.1` `.2` ascending appended to the name of the base logs 
+  rather than be named after the date and time they were rotated at:  
+  `logs/keter/20230413_231415.log` -> `logs/keter/keter.log.1`  
+  Please update anything that depended on the old log naming and directory conventions accordingly.


how does this work with several apps?

Noticed a typo here actually 🙏 Added some examples for app log changes just now.

jappeace · 2023-03-29T16:21:18Z

I think deleting that log message datatype would be nice

keter/src/Keter/Common.hs

Line 59 in 1bd7e25

data LogMessage

I've no idea why this was made typesafe, I hardly can imagine a situation you want to parse and re-use log messages.

anyway, like I said you don't need an instance because you can use LoggingT: https://hackage.haskell.org/package/monad-logger-0.3.39/docs/Control-Monad-Logger.html#t:LoggingT

fast logger provides you with LogStr -> IO (), so you just have to tell it how to deal with those other args and Loc -> LogSource -> LogLevel (eg format them into a LogStr for example and concat the given LogStr, or just ignore them).
Then we can use:

x :: IO ()
x = runLoggingT (myFormatting fastlogHandle) $ $logInfo "messages from LogMessage as normal text"

myFormatting :: (LogStr -> IO ()) -> (Loc -> LogSource -> LogLevel -> LogStr -> IO ())

RiugaBachi · 2023-03-30T02:04:16Z

Oh lol my eyes completely skipped over the unwrapper being exposed. Thanks for the heads up 👍

Though, I was considering my options and still think the best path forward would be to propagate a new KeterM monad context to everything. This would allow for extensibility if at some point we decide to throw in a ReaderT KeterConfig layer etc. for other resources that seem to be passed around to everything along with the logger. I couldn't see a benefit to generalizing everything to MonadLogger/IO/etc. constraints since keter (main) stuff forms a pretty linear call graph and the polymorphism wouldn't be useful in practice.

jappeace · 2023-03-31T12:41:46Z

src/Keter/Context.hs

+import           Control.Monad.Reader      (MonadReader, ReaderT, runReaderT, ask
+                                           , withReaderT)
+
+-- | The top-level keter context monad, carrying around the main logger and some locally relevant configuration structure.


Please also refer to a relevant blogpost if you have one in mind for this design.
for example https://www.fpcomplete.com/blog/2017/06/readert-design-pattern/

this allows newer people to figure out what goes on with all the boilerplate :)

I don't think I know of one off the top of my head more specialized than that, so if you don't mind I'll steal that link lol. Added

jappeace · 2023-03-31T14:03:06Z

keter.cabal

@@ -62,6 +62,9 @@ Library
                     , attoparsec                >= 0.10
                     , http-client               >= 0.5.0
                     , http-conduit              >= 2.1
+                     , fast-logger
+                     , monad-logger
+                     , unliftio-core


I guess we should add upper bounds

jappeace · 2023-03-31T14:03:58Z

src/Keter/AppManager.hs

            Terminate -> return noWorker

-launchWorker :: AppManager
-             -> AppId
+-- | TODO: Propagate `KeterM` up through here?


this appears to be done already

jappeace · 2023-03-31T14:05:27Z

src/Keter/Common.hs

+--             , show c
+--             ]
+--     loc <- fmap showLoc TH.qLocation
+--     [|(. ExceptionThrown (pack $(TH.lift loc)))|]


yeah, just delete it. we've git to conjure up past code, which rarely happens.

jappeace · 2023-03-31T14:08:02Z

src/Keter/Main.hs

+                e' <-
+                    case e of
+                        FSN.Removed fp _ _ -> do
+                            --log $ WatchedFile "removed" (fromFilePath fp)


shouldn't we re-add thes log lines as monad logger ones?

Good catch! I used my fuzzy find plugin fearing this but I guess a few escaped my eyes

jappeace

oki, I'll merge this tonight, you may fixup whatever you want meanwhile. 🎉

jappeace · 2023-03-31T16:44:27Z

thank you!

RiugaBachi added 13 commits March 28, 2023 20:23

Add rotate log config option

53bcb66

Add monad-logger and fast-logger deps

d2e30d6

Remove (unused)

38897c4

Update RotatingLog to wrap a FastLogger and its release callback

194015e

Add defaultRotationSpec

1a6e828

Update imports/exports

f6ca7cc

Refactor withLogger to use a FastLogger to rotating files or stderr

7e3ed8c

based on config

Update withRotatingLogger to use a FastLogger to a rotating file

db54ccc

Rename for clarity

6805a6e

Change addHunk to fast logging function

2f7c736

Change rotating log close fn

7cfb8fc

Add yaml parsing for rotate-logs

1e5c35d

Update changelog

29a673f

RiugaBachi mentioned this pull request Mar 29, 2023

Use fastlogger instead of homegrown logging system #257

Closed

jappeace reviewed Mar 29, 2023

View reviewed changes

RiugaBachi added 2 commits March 29, 2023 10:39

Fix typo

ab6cf90

Added log name change examples for apps

8306f4c

RiugaBachi added 11 commits March 30, 2023 11:39

Implement KeterM monad

2c2dbd0

Add unliftio-core dependency

954411b

Expose Keter.Context

397aed8

Refactor Keter.Main to use KeterM

c0e9dc8

Refactor Keter.App to use KeterM

1725181

Refactor Keter.AppManager to use KeterM

801e0e4

Refactor Keter.Cli to use KeterM

839700f

Remove ascLog field (unused)

8fc1dd1

Remove again

c0a668f

Refactor Keter.HostManager to use KeterM

b51b51d

Refactor Keter.PortPool to use KeterM

7b9ec22

RiugaBachi added 2 commits March 31, 2023 07:22

Rename rotating log references

7887b89

Use createLoggerViaConfig

921cea2

jappeace reviewed Mar 31, 2023

View reviewed changes

RiugaBachi added 5 commits March 31, 2023 07:49

Add blog post link to KeterM doc

bb6bfd3

Add 'keter|' tag to internal log format

8ed4fe5

Remove LogLevel from process monitor log fn

07337ed

Refactor app & process-monitor log format

d586327

Imports

dd1b71c

jappeace reviewed Mar 31, 2023

View reviewed changes

Unquote app name

02c0623

jappeace reviewed Mar 31, 2023

View reviewed changes

Fix keter tag

15a6cfd

jappeace reviewed Mar 31, 2023

View reviewed changes

jappeace approved these changes Mar 31, 2023

View reviewed changes

RiugaBachi added 12 commits March 31, 2023 09:32

Remove logEx

95cef93

Remove todo

5b47603

Reintroduce commented out log calls

a6bb6a4

Add upper dependency bounds

6847ba4

Rename LogFile to Log

3255e86

Move Keter.Conduit.LogFile to Keter.Logger

8a18651

Update exposed modules

40fcca6

Rename imports

fd4905c

Store LogType in Logger

cd9e110

Update format comment

aa22c8d

Refactor log formatter to emit tags only when logging to stderr

57f5c0d

Refactor to use formatTag

d58364f

jappeace merged commit dcdfd80 into snoyberg:master Mar 31, 2023

ulidtko mentioned this pull request Jul 24, 2024

Reloading is broken in v2.1 with rotate-logs: True #296

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fast logger migration #274

Fast logger migration #274

RiugaBachi commented Mar 29, 2023 •

edited

Loading

jappeace Mar 29, 2023

RiugaBachi Mar 29, 2023

jappeace commented Mar 29, 2023 •

edited

Loading

RiugaBachi commented Mar 30, 2023 •

edited

Loading

jappeace Mar 31, 2023

RiugaBachi Mar 31, 2023

jappeace Mar 31, 2023

RiugaBachi Mar 31, 2023

jappeace Mar 31, 2023

RiugaBachi Mar 31, 2023

jappeace Mar 31, 2023 •

edited

Loading

RiugaBachi Mar 31, 2023

jappeace Mar 31, 2023 •

edited

Loading

RiugaBachi Mar 31, 2023

jappeace left a comment •

edited

Loading

jappeace commented Mar 31, 2023

Fast logger migration #274

Fast logger migration #274

Conversation

RiugaBachi commented Mar 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jappeace commented Mar 29, 2023 • edited Loading

RiugaBachi commented Mar 30, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jappeace Mar 31, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jappeace Mar 31, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jappeace left a comment • edited Loading

Choose a reason for hiding this comment

jappeace commented Mar 31, 2023

RiugaBachi commented Mar 29, 2023 •

edited

Loading

jappeace commented Mar 29, 2023 •

edited

Loading

RiugaBachi commented Mar 30, 2023 •

edited

Loading

jappeace Mar 31, 2023 •

edited

Loading

jappeace Mar 31, 2023 •

edited

Loading

jappeace left a comment •

edited

Loading