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

feat(native): enhance traceability with additional logs for better debugging #197

Merged
merged 6 commits into from
Dec 20, 2024

Conversation

goshawk-3
Copy link
Contributor

@goshawk-3 goshawk-3 commented Dec 18, 2024

Summary

Simplifies debugging by adding trace logs for key actions such as execute, commit, queue, flush, and compute.

Changes

  • Most traces are now logged at the log.debug level with a few in log.info.
  • In Geth, enable debug-level logging using the --verbosity 4 CLI parameter.

Examples for Filtering Logs

  • grep fhevm: Display all FHEVM-related logs.
  • grep handle=59b4f9: Filter logs by specific handle.
  • grep 'sync compute': Display input and output of grpc.SyncCompute.
DEBUG[12-18|12:21:25.859] Add operation                            module="fhevm:sync compute" op=FHE_TRIVIAL_ENCRYPT handle=59b4f9..500300
INFO [12-18|12:21:25.859] Sending grpc request                     module="fhevm:sync compute" computations=1 "compressed ciphertexts"=0
INFO [12-18|12:21:25.864] Response                                 module="fhevm:sync compute" "ciphertexts count"=1
DEBUG[12-18|12:21:25.864] Response ciphertext                      module="fhevm:sync compute" handle=59b4f9..500300 len=1866
DEBUG[12-18|12:21:25.864] Computations completed                   module="fhevm:sync compute" duration=5ms
  • grep 'Insert computation' fhevm.log | grep 'CommitBlockId: 15' - Display computations inserted in Block height 10

@goshawk-3 goshawk-3 marked this pull request as ready for review December 18, 2024 10:47
Copy link
Collaborator

@david-zk david-zk left a comment

Choose a reason for hiding this comment

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

I think we need to ensure that whatever logger the integrator uses it should work for him

"sync"
"time"

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/log"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we know that this is the logging library that every integrator will use?

We had logging before in avalanche integration, so we had to expose in the api so user could pass in his own logger interface and we'd use that. I'm not sure we can assume logging implementation users will use, that's why this PR was not trivial to implement right away

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is how we did the logger before https://github.com/zama-ai/fhevm-go/blob/53d5dae4d6a4c9d887ce6cb484b642b964efc690/fhevm/interface.go#L26 we allow user to provide interface object as part of the environment

@goshawk-3 goshawk-3 requested a review from david-zk December 20, 2024 08:12
Copy link
Collaborator

@david-zk david-zk left a comment

Choose a reason for hiding this comment

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

Maybe the perfect place to pass in the logger would be once in adding argument to func InitExecutor ? CreateSession happens many times in every EVM call, we could keep one pointer to logger after InitExecutor and keep that for the entirety of application

@goshawk-3
Copy link
Contributor Author

goshawk-3 commented Dec 20, 2024

Maybe the perfect place to pass in the logger would be once in adding argument to func InitExecutor ? CreateSession happens many times in every EVM call, we could keep one pointer to logger after InitExecutor and keep that for the entirety of application

Makes sense!

For the record, InitLogger( hostLogger) is now called during the blockchain host initialization.

@goshawk-3 goshawk-3 merged commit 9e31ae6 into main Dec 20, 2024
3 checks passed
@goshawk-3 goshawk-3 self-assigned this Dec 20, 2024
@dartdart26 dartdart26 deleted the georgi/logging-improvements branch December 20, 2024 12:11
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.

2 participants