Handle contract logs correctly #104
Conversation
TotallyNotChase
force-pushed
the
trace-logs
branch
from
d512519 to
f61f949
Compare
src/BotPlutusInterface/Effects.hs
Outdated
|
|
||
| -- | Version of "Control.Monad.Freer.Extras.handleLogTrace" that takes into account the log level setting. | ||
| handleLogTrace' :: Pretty a => LogLevel -> Eff (LogMsg a ': effs) ~> Eff effs | ||
| handleLogTrace' logLevelSetting = interpret $ \case |
There was a problem hiding this comment.
Do we have a way of disserning contract logs to BPI logs? perhaps a prefix or something?
There was a problem hiding this comment.
Should both type of logs get a prefix?
There was a problem hiding this comment.
It should be noted that while BPI logs look more or less like "bare" strings. Contract logs have the log level prepended as a prefix, e.g [INFO] Minted value: .....
There was a problem hiding this comment.
Thats a fair point, it would actually be nice to have those log level prefixes on the BPI logs, then something else for separating between the two
There was a problem hiding this comment.
I did some refactoring that should enable all of this. Added a LogContext.
src/BotPlutusInterface/Effects.hs
Outdated
| LMessage msg -> | ||
| if logLevelSetting >= toNativeLogLevel (msg ^. Freer.logLevel) | ||
| then Trace.trace (Render.renderString . layoutPretty defaultLayoutOptions . pretty $ msg) $ pure () | ||
| else pure () |
There was a problem hiding this comment.
Could use when here
src/BotPlutusInterface/Effects.hs
Outdated
| handleLogTrace' logLevelSetting = interpret $ \case | ||
| LMessage msg -> | ||
| if logLevelSetting >= toNativeLogLevel (msg ^. Freer.logLevel) | ||
| then Trace.trace (Render.renderString . layoutPretty defaultLayoutOptions . pretty $ msg) $ pure () |
There was a problem hiding this comment.
Logging with trace seems hacky - especially since PABEffect already has a logging effect, could we instead of handling this effect, reinterpret it in some way to PABEffect and use that code (which does direct putStrLns in IO)
Alternatively, we can add the LastMember constraint on this, and lift to that.
There was a problem hiding this comment.
I agree. I just got lazy and decided to use the IOG handleLogTrace. I changed this to reinterpret the effect to use PABEffect later down the line.
src/BotPlutusInterface/Effects.hs
Outdated
| import Cardano.Api qualified | ||
| import Control.Concurrent qualified as Concurrent | ||
| import Control.Concurrent.STM (atomically, modifyTVar, modifyTVar') | ||
| import Control.Lens |
There was a problem hiding this comment.
Lets use explicit imports
There was a problem hiding this comment.
agree, fixed
src/BotPlutusInterface/Effects.hs
Outdated
| import Plutus.Contract.Effects (ChainIndexQuery, ChainIndexResponse) | ||
| import Plutus.PAB.Core.ContractInstance.STM (Activity) | ||
| import PlutusTx.Builtins.Internal (BuiltinByteString (BuiltinByteString)) | ||
| import Prettyprinter |
src/BotPlutusInterface/Contract.hs
Outdated
| import Plutus.Contract.Resumable (Resumable (..)) | ||
| import Plutus.Contract.Types (Contract (..), ContractEffs) | ||
| import PlutusTx.Builtins (fromBuiltin) | ||
| import Prettyprinter |
|
CI isn't running, guessing that because you're running a fork? can you verify it passes build, formatting and lint? |
I was unable to run tests due to the missing machinery. |
|
I've merged this to chase/trace-logs under BPI, created this PR to get it to master, with CI |
2 participants
I'm unsure where to get a
Prettyprinterinstance forData.Aeson.Valueso I instantiated it inline. It should definitely be replaced by some "official" instance or moved to a separate module. Let me know what to do about that.