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

Add Request Parameters to RPC logging #22025

Open
MysticRyuujin opened this issue Dec 15, 2020 · 4 comments
Open

Add Request Parameters to RPC logging #22025

MysticRyuujin opened this issue Dec 15, 2020 · 4 comments

Comments

@MysticRyuujin
Copy link
Contributor

Rationale

When a JSON-RPC request is sent to the node, it would be nice to be able to capture a log of the parameters that were sent.

Currently, even at maximum verbosity level, all JSON-RPC requests seem to be logged at DEBUG level at the time the request is served:

DEBUG[12-15|16:41:42.323] Served eth_getBlockByNumber         conn=192.168.15.20:56660 reqid=1 t="573.915µs"
DEBUG[12-15|16:43:52.926] Served eth_getTransactionByHash     conn=192.168.10.50:37144 reqid=1 t=2.481519ms

And as you can see, there's no indication what block was requested, what transaction hash was requested, etc.

Implementation

I think it would make sense to add the rpc request parameters under DETAIL level (or DEBUG, whatever makes sense)

@holiman
Copy link
Contributor

holiman commented Dec 16, 2020

One small caveat here.. When we implemented clef, we added an auditlog, dumping all input and output via the untrusted external api into a log file (and also using a machine-readable output format). That's fine for clef, because there's never any sensitive information passing through the API.

For geth, it's not as easy, since there are endpoints which accept keystore password. And people are not always as careful about logs as they are about their keystore, so if we start dumping highly sensitive data into logs, it may well wind up in places such as

  • centralized logging facilities (like papertrail),
  • go-ethereum issue tickets,
  • unprotected shared backup folders, etc.

I agree it's a super useful feature, and maybe we can reuse the same auditlog mechanism for certian namespace, like eth, but not have it enabled on personal, admin and debug.

@MysticRyuujin
Copy link
Contributor Author

MysticRyuujin commented Dec 16, 2020

I'd be happy with that compromise.

Also another reason for #21963

@ligi
Copy link
Member

ligi commented Dec 17, 2020

It will need to be behind a parameter. Also maybe not go to the console but to a dedicated file. Maybe alongside #21926
Also @MysticRyuujin can you write more about your use-case?

@MysticRyuujin
Copy link
Contributor Author

Typically I'm trying to figure out why some applications work against one client but not another (often times without access to the application itself - or a steep learning curve to go figure out how to reproduce something). The first thing I want to do here is grab the body of the POST(s) hitting the node, this makes it easy to just re-send some curl commands to various nodes.

While there are certainly things to consider for logging (e.g. passwords) - I feel detailed logging should be available.

My alternatives are to packet capture, stick a proxy in front of it, or rely on debugs from other applications making the calls, all of which I may or may not have access to depending on the environment, or even be allowed to do within the environment.

I'm much more advocating in favor of having the functionality available than I am in dire need of a solution - I understand there are plenty of workaround available to capture this data, but I feel it should be native.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants