Skip to content

feat(coprocessor): add log level configurations#263

Merged
dartdart26 merged 1 commit intomainfrom
petar/conf-log-level
Jun 16, 2025
Merged

feat(coprocessor): add log level configurations#263
dartdart26 merged 1 commit intomainfrom
petar/conf-log-level

Conversation

@dartdart26
Copy link
Copy Markdown
Collaborator

@dartdart26 dartdart26 commented Jun 13, 2025

Add the log-level cmd line option to configure the log level.

Also, make sure gw-listener and transaction-sender retry connecting to the node infinitely at startup as the WsConnect options don't apply to connect during provider creation.

Finally, add structured logging to fhevm-listener instead of println!s. Also, added some TODOs to remove panics and, instead, propagate errors. Will be done in a separate PR.

As a side note, using structured logging in all other services where it is applicable will be done in a separate PR as well (as we usually only utilize the message field in the log macros).

Resolves https://github.com/zama-ai/fhevm-internal/issues/88

@dartdart26 dartdart26 self-assigned this Jun 13, 2025
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a --log-level option for configuring log verbosity, switches WebSocket connection retries to indefinite loops with logging, and replaces ad-hoc println!/eprintln! calls in the fhevm-listener service with structured tracing macros.

  • Added log_level: Level to CLI args and propagated it into tracing_subscriber setup.
  • Changed get_chain_id to retry indefinitely with logged errors.
  • Replaced all println!/eprintln! in fhevm-listener with info!, warn!, and error!.

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
transaction-sender/tests/common.rs Simplified get_chain_id invocation in tests.
transaction-sender/src/lib.rs Removed retry params, implemented infinite retry with logs.
sns-executor/src/lib.rs Added log_level field to Config.
fhevm-listener/src/cmd/mod.rs Introduced log_level CLI arg and structured logging.
fhevm-listener/Cargo.toml Added tracing and tracing-subscriber dependencies.
coprocessor/src/lib.rs (async_main) Passed args.log_level into tracing_subscriber.
coprocessor/src/daemon_cli.rs Added log_level to daemon CLI args.
Comments suppressed due to low confidence (1)

coprocessor/fhevm-engine/transaction-sender/src/lib.rs:93

  • [nitpick] Use Rust doc comments (///) instead of // so these lines appear in generated docs and IDE tooltips.
// Gets the chain ID from the given WebSocket URL.

Comment thread coprocessor/fhevm-engine/fhevm-listener/src/cmd/mod.rs Outdated
Comment thread coprocessor/fhevm-engine/coprocessor/src/lib.rs
@dartdart26 dartdart26 force-pushed the petar/conf-log-level branch 2 times, most recently from bbcf8ed to 67e1a47 Compare June 13, 2025 13:33
Comment thread coprocessor/fhevm-engine/fhevm-listener/src/cmd/mod.rs
Comment thread coprocessor/fhevm-engine/fhevm-listener/src/cmd/mod.rs
Comment thread coprocessor/fhevm-engine/gw-listener/src/bin/gw_listener.rs
Comment thread coprocessor/fhevm-engine/transaction-sender/src/lib.rs Outdated
Comment thread coprocessor/fhevm-engine/transaction-sender/src/lib.rs Outdated
@dartdart26 dartdart26 force-pushed the petar/conf-log-level branch 13 times, most recently from cebd0a3 to 50f58fa Compare June 16, 2025 13:23
Add the `log-level` cmd line option to configure the log level.

Also, make sure gw-listener and transaction-sender retry connecting to
the node infinitely at startup as the `WsConnect` options don't apply
to connect during provider creation.

Finally, add structured logging to fhevm-listener instead of println!s.
Also, added some TODOs to remove panics and, instead, propagate errors.
Will be done in a separate PR.

As a side note, using structured logging in all other services where it
is applicable will be done in a separate PR as well (as we usually only
utilize the `message` field in the log macros).
@dartdart26 dartdart26 force-pushed the petar/conf-log-level branch from 50f58fa to e86a45c Compare June 16, 2025 14:31
@dartdart26 dartdart26 merged commit 7039b64 into main Jun 16, 2025
62 checks passed
@dartdart26 dartdart26 deleted the petar/conf-log-level branch June 16, 2025 19:55
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