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 trace to web socket JSON/RPC responses #6285

Merged
merged 2 commits into from
Dec 13, 2023

Conversation

matthew1001
Copy link
Contributor

@matthew1001 matthew1001 commented Dec 12, 2023

PR description

Trace WS JSON/RPC responses (successes and failures)

Fixed Issue(s)

Fixes #6276

Example trace output

Successful eth_getBlockByNumber:

2023-12-12 10:43:41.553+00:00 | vert.x-eventloop-thread-1 | TRACE | WebSocketMessageHandler | {"jsonrpc":"2.0","id":51,"result":{"number":"0x0","hash":"0x74f92c171677a9f8b1eac53fc4f2c2e0d6bbc7a7b9b259fd0067f7b573951e16","mixHash":"0x63746963616c2062797a616e74696e65206661756c7420746f6c6572616e6365","parentHash":"0x0000000000000000000000000000000000000000000000000000000000000000","nonce":"0x0000000000000000","sha3Uncles":"0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347","logsBloom":"0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000","transactionsRoot":"0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421","stateRoot":"0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421","receiptsRoot":"0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421","miner":"0x0000000000000000000000000000000000000000","difficulty":"0x1","totalDifficulty":"0x1","extraData":"0xf88fa00000000000000000000000000000000000000000000000000000000000000000f869945c9437de876531c73c9618d13872f7508869f2539474474cf92e9b74df7a2ce2c4fbc0830c773b7a769462f241db855d439cc79f5d2bd4a58eeb9c5d01cd946d544f03911b74425dd4112e95f93b82ac6302f19477fd5cb2aea0b91e7e4faf7625bf9dbaee39ee47c080c0","size":"0x292","gasLimit":"0x1fffffffffffff","gasUsed":"0x0","timestamp":"0x58ee40ba","uncles":[],"transactions":[]}}

Failed eth_getBlockByNumber:

2023-12-12 10:44:02.881+00:00 | vert.x-eventloop-thread-1 | TRACE | WebSocketMessageHandler | {"jsonrpc":"2.0","id":51,"error":{"code":-32602,"message":"Invalid params"}}

Copy link

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.
  • I thought about the changelog and included a changelog update if required.
  • If my PR includes database changes (e.g. KeyValueSegmentIdentifier) I have thought about compatibility and performed forwards and backwards compatibility tests

try {
LOG.trace(jsonObjectMapper.writeValueAsString(response));
} catch (JsonProcessingException e) {
throw new RuntimeException(e);
Copy link
Contributor

Choose a reason for hiding this comment

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

do we want to throw if we can't log? where does this exception come out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I think you're right, logging it is safer. We do have places where we throw a runtime exception for the same JSON-parsing issue e.g.

Perhaps that should be a log as well, instead of a runtime exception?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah I'd say the same thing for there

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
@macfarla macfarla merged commit 328aaf7 into hyperledger:main Dec 13, 2023
18 checks passed
gfukushima pushed a commit to gfukushima/besu that referenced this pull request Dec 15, 2023
* Add trace to web socket JSON/RPC responses

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Don't throw runtime exception if we can't parse JSON for trace

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

---------

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
gfukushima pushed a commit to gfukushima/besu that referenced this pull request Dec 15, 2023
* Add trace to web socket JSON/RPC responses

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Don't throw runtime exception if we can't parse JSON for trace

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

---------

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
jflo pushed a commit to jflo/besu that referenced this pull request Dec 18, 2023
* Add trace to web socket JSON/RPC responses

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Don't throw runtime exception if we can't parse JSON for trace

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

---------

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
Signed-off-by: jflo <justin+github@florentine.us>
jflo pushed a commit to jflo/besu that referenced this pull request Dec 18, 2023
* Add trace to web socket JSON/RPC responses

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Don't throw runtime exception if we can't parse JSON for trace

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

---------

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
jflo pushed a commit to jflo/besu that referenced this pull request Dec 18, 2023
* Add trace to web socket JSON/RPC responses

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Don't throw runtime exception if we can't parse JSON for trace

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

---------

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
Signed-off-by: jflo <justin+github@florentine.us>
jflo added a commit that referenced this pull request Dec 19, 2023
* Increase scope of reference tests (#6287)

Add new forks to transaction tests, add bad rlp to rlp ref tests.

Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
Signed-off-by: jflo <justin+github@florentine.us>

* Add trace to web socket JSON/RPC responses (#6285)

* Add trace to web socket JSON/RPC responses

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Don't throw runtime exception if we can't parse JSON for trace

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

---------

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
Signed-off-by: jflo <justin+github@florentine.us>

* [MINOR] Include Enode URL in error message if parsing exception (#6288)

* include the actual Enode URL in the error message if IP can't be resolved

---------

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: jflo <justin+github@florentine.us>

* Execute the terminatedImmediately method only once (#6284)

Signed-off-by: David Lutzardo <jdlutzardo@izertis.com>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: jflo <justin+github@florentine.us>

* move forest class to a specific package (#6293)

cleaning up the worldstate section, moving and renaming everything that is forest into a specific package in order to clarify the difference between Bonsai and Forest in the code.

---------

Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: matkt <karim.t2am@gmail.com>
Signed-off-by: jflo <justin+github@florentine.us>

* removed vintage junit dependency (#6296)

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: jflo <justin+github@florentine.us>

* updated cli and chainimport/export tests to junit 5 (#6281)

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: jflo <justin+github@florentine.us>

* [MINOR] CLI Subcommands migrate to junit 5 (#6282)

* updated cli and chainimport/export tests to junit 5

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>

* subcommands and besucommand test to junit5

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>

* removed duplicate line

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>

* removed vintage junit dep

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>

---------

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
Co-authored-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: jflo <justin+github@florentine.us>

* [MINOR] add genesis file name to config overview (#6297)

* add genesis file name to config overview

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>

---------

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: jflo <justin+github@florentine.us>

* create trie package for bonsai and forest (#6300)

Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: matkt <karim.t2am@gmail.com>
Signed-off-by: jflo <justin+github@florentine.us>

* [MINOR] More cli tests to junit 5 (#6283)

* more CLI and services tests to junit5

---------

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
Co-authored-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: jflo <justin+github@florentine.us>

* Update Gradle plugins and replace unmaintained license plugin (#6275)

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: jflo <justin+github@florentine.us>

* Quick fix to avoid a tight loop when processing added blocks in txpool (#6309)

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: jflo <justin+github@florentine.us>

* uprev to version 23.10.3-RC4

Signed-off-by: jflo <justin+github@florentine.us>

* mark deleted slot during clear storage step

Signed-off-by: Karim Taam <karim.t2am@gmail.com>

---------

Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
Signed-off-by: jflo <justin+github@florentine.us>
Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: David Lutzardo <jdlutzardo@izertis.com>
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: matkt <karim.t2am@gmail.com>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Co-authored-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
Co-authored-by: Matt Whitehead <matthew1001@gmail.com>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
Co-authored-by: David Lutzardo <jdlutzardo@izertis.com>
Co-authored-by: Karim TAAM <karim.t2am@gmail.com>
Co-authored-by: Fabio Di Fabio <fabio.difabio@consensys.net>
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.

JSON/RPC responses not traced when using web sockets
2 participants