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

Logging improvements (Node definition overhaul) #13

Merged
merged 12 commits into from
Jun 30, 2024

Conversation

bilal-fazlani
Copy link
Owner

@bilal-fazlani bilal-fazlani commented Feb 21, 2024

The logger currently is a custom implementation and a layer inside MaelstromRuntime. The apis to use logging methods are exposed at top level.

When there is an unhandled error, ZIO's runtime logs it using ZIO's default logger which prints data on STDOUT. This is breaks maelstrom because it uses STDOUT and network IO and can't makes sense of this log.

Also, one thought is - it does not makes sense to have another logging interface for ZIO-Maelstrom when ZIO does provides a native interface.

Somehow, we need to create logger extending ZIO's interface and provide an easy mechanism to replace default logger with Maelstrom logger.


Tasks

  • implementation
  • tests
  • documentation
  • examples

Update: 27 Feb 2024

Using a custom logger as ZIO logger requires overriding bootstrap method in a ZIO app. Overriding was not possible in the older approach there was no parent class to extend from. So this turned out to be a lot bigger change than expected. I had to change the Node definition approach from a simple ZIO effect to an object which extends from a new trait called MaelstromNode. Extending this trait now installs the new ZIOMaelstromLogger. Because logLevel and logFormat were part of settings which was part of layer construction, I also had to break that down make it into an overridable val/def.

Copy link

netlify bot commented Feb 21, 2024

Deploy Preview for zio-maelstrom ready!

Name Link
🔨 Latest commit 40b8f42
🔍 Latest deploy log https://app.netlify.com/sites/zio-maelstrom/deploys/66813bc0be836c000825b221
😎 Deploy Preview https://deploy-preview-13--zio-maelstrom.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@bilal-fazlani bilal-fazlani force-pushed the logging-improvements branch 2 times, most recently from 31fc5f8 to 68364e0 Compare February 27, 2024 09:44
@bilal-fazlani bilal-fazlani changed the title Logging improvements Logging improvements (Node definition overhaul) Feb 27, 2024
@bilal-fazlani bilal-fazlani self-assigned this Feb 27, 2024
@bilal-fazlani bilal-fazlani added the enhancement New feature or request label Feb 27, 2024
@bilal-fazlani bilal-fazlani merged commit e3db9fc into main Jun 30, 2024
5 checks passed
@bilal-fazlani bilal-fazlani deleted the logging-improvements branch June 30, 2024 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant