-
Notifications
You must be signed in to change notification settings - Fork 7
PLT-7497: restructure Marconi with a focus on logging #212
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
82020a4
23225e1
e3a706f
25059f0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ module Marconi.ChainIndex.Logging ( | |
| chainSyncEventStreamLogging, | ||
| MarconiTrace, | ||
| mkMarconiTrace, | ||
| logMInfo, | ||
|
|
||
| -- * Exported for testing purposes | ||
| marconiFormatting, | ||
|
|
@@ -22,7 +23,11 @@ import Cardano.Api.Extended.Streaming ( | |
| ) | ||
| import Cardano.BM.Trace (Trace, logInfo) | ||
| import Cardano.BM.Tracing (contramap) | ||
| import Control.Lens.Getter qualified as Lens | ||
| import Control.Monad (when) | ||
| import Control.Monad.IO.Class (MonadIO (liftIO)) | ||
| import Control.Monad.Reader (asks) | ||
| import Control.Monad.Trans (lift) | ||
| import Data.IORef (IORef, modifyIORef', newIORef, readIORef) | ||
| import Data.Text (Text) | ||
| import Data.Time ( | ||
|
|
@@ -36,7 +41,7 @@ import Data.Time ( | |
| import Data.Word (Word64) | ||
| import GHC.Generics (Generic) | ||
| import Marconi.ChainIndex.Orphans () | ||
| import Marconi.ChainIndex.Types (MarconiTrace) | ||
| import Marconi.ChainIndex.Types (ChainIndexerT, MarconiTrace, runIndexerConfigTrace) | ||
| import Prettyprinter (Pretty (pretty), (<+>)) | ||
| import Prettyprinter qualified as Pretty | ||
| import Prettyprinter.Render.Text qualified as Pretty | ||
|
|
@@ -54,6 +59,14 @@ marconiFormatting = | |
| Pretty.renderStrict | ||
| . Pretty.layoutPretty Pretty.defaultLayoutOptions | ||
|
|
||
| logMInfo :: MonadIO m => Pretty.Doc () -> ChainIndexerT m () | ||
| logMInfo msg = do | ||
| trace <- Lens.view runIndexerConfigTrace | ||
| lift $ logInfo trace msg | ||
|
|
||
| -- TODO: this should also throw an exception | ||
| logMError = undefined | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel like log functions shouldn't be throwing exceptions, despite what we said earlier today about having a unified framework for both. Perhaps it would be OK to have a function that does both, but have that reflected in the name. Something like In another system I worked on, we also included a "context" parameter which was just a string that indicated where the error was thrown from. This helped a lot when looking at logs.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree, I think we need to have the concrete requirements regarding errors and exceptions in order to make any changes here. |
||
|
|
||
| -- | Chain synchronisation statistics measured starting from previously measured 'LastSyncStats'. | ||
| data LastSyncStats = LastSyncStats | ||
| { syncStatsNumBlocks :: !Word64 | ||
|
|
@@ -87,7 +100,7 @@ instance Pretty LastSyncLog where | |
| <+> pretty cp | ||
| <+> "and current node tip is" | ||
| <+> pretty nt | ||
| <> "." | ||
| <> "." | ||
|
|
||
| processingSummaryMsg timeSinceLastMsg = | ||
| "Processed" | ||
|
|
@@ -96,12 +109,12 @@ instance Pretty LastSyncLog where | |
| <+> pretty numRollBackwards | ||
| <+> "rollbacks in the last" | ||
| <+> pretty (formatTime defaultTimeLocale "%s" timeSinceLastMsg) | ||
| <> "s" | ||
| <> "s" | ||
| in case (timeSinceLastMsgM, cp, nt) of | ||
| (Nothing, _, _) -> | ||
| "Starting from" | ||
| <+> pretty cp | ||
| <> "." | ||
| <> "." | ||
| <+> currentTipMsg timeSinceLastMsgM | ||
| (Just _, _, C.ChainTipAtGenesis) -> | ||
| "Not syncing. Node tip is at Genesis" | ||
|
|
@@ -110,7 +123,7 @@ instance Pretty LastSyncLog where | |
| "Synchronising (0%)." | ||
| <+> currentTipMsg timeSinceLastMsgM | ||
| <+> processingSummaryMsg timeSinceLastMsg | ||
| <> "." | ||
| <> "." | ||
| ( Just timeSinceLastMsg | ||
| , C.ChainPoint (C.SlotNo chainSyncSlot) _ | ||
| , C.ChainTip (C.SlotNo nodeTipSlot) _ _ | ||
|
|
@@ -119,7 +132,7 @@ instance Pretty LastSyncLog where | |
| "Fully synchronised." | ||
| <+> currentTipMsg timeSinceLastMsgM | ||
| <+> processingSummaryMsg timeSinceLastMsg | ||
| <> "." | ||
| <> "." | ||
| ( Just timeSinceLastMsg | ||
| , C.ChainPoint (C.SlotNo chainSyncSlot) _ | ||
| , C.ChainTip (C.SlotNo nodeTipSlot) _ _ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,6 +33,9 @@ data RetryState = RetryState | |
| , secondsBeforeNextRetry :: !Word64 | ||
| } | ||
|
|
||
| -- TODO: this function is problematic because it is used both inside the | ||
| -- ChainIndexT context and outside to initialize the config | ||
| -- see Marconi.ChainIndex.Run.run | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you make the monad be a type parameter Or have two versions of the function, one of which wraps the other and lives in
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure about using
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, still, for the second solution I believe you end up explicitly implementing the |
||
| withNodeConnectRetry :: forall a. MarconiTrace IO -> RetryConfig -> FilePath -> IO a -> IO a | ||
| withNodeConnectRetry stdoutTrace retryConfig socketPath action = do | ||
| let initialRetryState = RetryState 0 (baseTimeBeforeNextRetry retryConfig) | ||
|
|
@@ -66,7 +69,7 @@ withNodeConnectRetry stdoutTrace retryConfig socketPath action = do | |
| <+> pretty socketPath | ||
| <+> "does not exist. Retrying in" | ||
| <+> pretty (secondsBeforeNextRetry retryState) | ||
| <> "s ..." | ||
| <> "s ..." | ||
|
|
||
| threadDelay $ fromIntegral $ secondsBeforeNextRetry retryState * 1_000_000 | ||
| runActionWithRetries | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.