Skip to content

Add JSON logging to Kore #3824

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

Merged
merged 34 commits into from
Apr 24, 2024
Merged

Add JSON logging to Kore #3824

merged 34 commits into from
Apr 24, 2024

Conversation

geo2a
Copy link
Contributor

@geo2a geo2a commented Apr 22, 2024

Fixes #3817
Fixes #3815

  • the Entry type class gets a new oneLineJson method. Log entries will now need to implement this method if they are to be logged as JSON. The default implementation if just the type constructor name.
  • The DebugAttemptEquation log entry gets an a oneLineJson implementation that serialised the successful/failed equation applications in the same form as the Booster, i.e. following the RPC log format. The terms are not included to keep the log size manageable.
  • The LogSomeAction constructor of the LogType (a filed of KoreLogOptions) is repurposed. Instead of holding one log action, it now holds two, plus a predicate that specifies which log entries to log using which action. Then, the makeKoreLogger1 function uses the selector function to filter the logs, render them with the appropriate rendered (json or standard) and log them using the appropriate log action.
  • booster/Server.hs is modified to initialize LogSomeAction with the appropriate log actions. If -l SimplifyJson log level is activated, we enable DebugAttemptEquation and DebugApplyEquation (as before). If --simplification-log-file is passed too, then Kore's simplification will be redirected there, otherwise they will end-up on stderr. Booster's simplilfications behave in the same way.
  • Finally, booster/Proxy.hs does not modify the request parameters when falling back to Kore or making internal simplify requests. We do not output logs received form Kore at -l SimplifJson anymore: Kore will now do it itself.

Example

Put simplification logs into a file

kore-rpc-booster def.kore --module DEF --server-port 12341 -l SimplifyJson --simplification-log-file logs.json

Or simply to stderr:

kore-rpc-booster def.kore --module DEF --server-port 12341 -l SimplifyJson

where there will be prepended with [SimplifyJson]

@geo2a geo2a changed the title Add --log-format json to kore-rpc Add --log-format=json to kore-rpc Apr 22, 2024
@geo2a geo2a force-pushed the georgy/kore-json-logging branch from ca1af7e to 03608f3 Compare April 22, 2024 09:47
@geo2a geo2a force-pushed the georgy/kore-json-logging branch from c393fa4 to 8b2ecf7 Compare April 22, 2024 10:49
@geo2a geo2a changed the title Add --log-format=json to kore-rpc Add JSON logging to Kore Apr 23, 2024
@geo2a geo2a marked this pull request as ready for review April 24, 2024 08:02
@geo2a geo2a force-pushed the georgy/kore-json-logging branch from 6cdaaf1 to eaf8d16 Compare April 24, 2024 08:19
Comment on lines +441 to +451
prettyContext =
\case
[] -> []
xs -> ("Context" <> Pretty.colon) : (indent <$> mkContext xs)

mkContext =
\case
[] -> []
[(doc, typeName)] ->
[Pretty.hsep [Pretty.parens typeName, doc]]
(doc, typeName) : xs -> (Pretty.hsep [Pretty.parens typeName, doc]) : (indent <$> (mkContext xs))
Copy link
Contributor

Choose a reason for hiding this comment

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

i guess we will want to tweak this in a separate PR to resemble the booster contexts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, certainly.

failureDescription err = shortenText . Pretty.renderText . Pretty.layoutOneLine . Pretty.pretty $ err

shortenText :: Text -> Text
shortenText msg = Text.take 200 msg <> ("...truncated" :: Text)
Copy link
Contributor

Choose a reason for hiding this comment

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

should we have this here? I notice that we still emit stuff like /* T Fn D Spa */ in kore terms. Perhaps once those are removed, we can remove this truncation without things blowing up too much?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added the truncation because the golden file got to 140mb... though that was a bit too much to put into git.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#3823 did not improve matters much unfortunately. I've settled on a slightly longer cutoff of 500 characters.

Copy link
Contributor

@goodlyrottenapple goodlyrottenapple left a comment

Choose a reason for hiding this comment

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

left a couple of comments, looks good otherwise

@geo2a
Copy link
Contributor Author

geo2a commented Apr 24, 2024

Thanks @goodlyrottenapple! I'll be merging this then.

As we've discussed, we should come up with a minimal test for the JSON logging. I've created an issue #3830.

@geo2a geo2a added automerge and removed automerge labels Apr 24, 2024
@rv-jenkins rv-jenkins merged commit c391d6b into master Apr 24, 2024
@rv-jenkins rv-jenkins deleted the georgy/kore-json-logging branch April 24, 2024 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants