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

chore: changing log level to trace for sending message print #217

Conversation

gabrielmer
Copy link

In applications with recurring RPC requests, this print can pollute the logs way too much.

As a result, propose to have this log set to trace :)

@gabrielmer gabrielmer requested a review from etan-status March 4, 2024 17:33
@gabrielmer gabrielmer self-assigned this Mar 4, 2024
@etan-status etan-status requested a review from tersec March 4, 2024 18:26
Copy link
Contributor

@Ivansete-status Ivansete-status left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!

@gabrielmer
Copy link
Author

@etan-status @tersec WDYT?

@@ -142,7 +142,7 @@ method call*(client: RpcHttpClient, name: string,
id = client.getNextId()
reqBody = requestTxEncode(name, params, id)

debug "Sending message to RPC server",
trace "Sending message to RPC server",
Copy link
Member

Choose a reason for hiding this comment

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

broadly, this logging is currently needed for nimbus - what could be done is to implement a hook that would be called before/after each request - in such a world, we could remove logging entirely from json-rpc (notably, logging is used when the API does not expose enough information for the consumer of the api to log it by themselves - it's a shortcut to avoid complicating the API)

@tersec
Copy link
Contributor

tersec commented Mar 24, 2024

It's already debug, so someone opting into that log level is already opting into a fairly sizable volume of logs.

@gabrielmer
Copy link
Author

Thanks for your input! The hooks approach is actually great, but it's true that if it's in debug we're already opting for extensive logging. So will close the PR so we can keep it simple :)

Apologies and thank you!

@gabrielmer gabrielmer closed this Apr 9, 2024
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.

4 participants