Skip to content

Conversation

@huitseeker
Copy link
Contributor

@huitseeker huitseeker commented Oct 25, 2024

Configures logging for go through an environment variable.

Example usage:

SP1_GO_LOG=false cargo test

./cargo test
SP1_GO_LOG=debug ./cargo test

This solution works because:

  • The init() function in logging.go will run before any other code in the package
  • Since gnark uses zerolog, setting the global log level will affect all logging throughout the application, including in dependencies
  • The change is minimal and contained to just one file (as long as the main.go module early-loads the sp1 package)
  • It doesn't require any changes to the Rust code or to gnark's source code
  • It's configurable through an environment variable

Configures logging for go through an environment variable.

Example usage:
```
SP1_GO_LOG=false cargo test

./cargo test
SP1_GO_LOG=true ./cargo test
```

This solution works because:

- The init() function in logging.go will run before any other code in the package
- Since gnark uses zerolog, setting the global log level will affect all logging throughout the application, including in dependencies
- The change is minimal and contained to just one file
- It doesn't require any changes to the Rust code or to gnark's source code
- It's configurable through an environment variable
wwared
wwared previously approved these changes Oct 25, 2024
Copy link
Contributor

@wwared wwared left a comment

Choose a reason for hiding this comment

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

This is a great and elegant solution! I didn't even know about Go's init functions, very nice!

There's two situations where the Go code is still printing even with logging disabled (after a couple quick tests on my end):

  • When making the artifacts, it prints information related to fetching and verifying the aztec ignition srs data. This is a non-issue we can live with
  • When verifying a proof in the FFI lurk-hs context, it seems to still print the "ignoring uninitialized slice: Vars []frontend.Variable" message 3 times. This is unfortunately a regular fmt.Printf call inside of gnark, either here or here

The latter seems a bit trickier to solve neatly. It might be easier for us to do a minimal patch to the sphinx-recursion-gnark-ffi go code to properly initialize the slices gnark is referring to in the messages rather than trying to patch out the printf call (though I don't know what exactly we should initialize them to). We can also make a minimal gnark upstream PR changing those two prints to warning log messages (which might be the best overall solution for everyone)

I've opened Consensys/gnark#1305 upstream to solve the latter issue

Adds an 'off' option

Co-authored-by: wwared <wwared@users.noreply.github.com>
@huitseeker huitseeker enabled auto-merge (squash) October 25, 2024 20:41
@huitseeker huitseeker merged commit 142bbfd into dev Oct 25, 2024
@huitseeker huitseeker deleted the go-logging branch October 25, 2024 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants