Trigger `OperationTracer` on contexts enter & exit #5756

delehef · 2023-08-07T22:25:44Z

PR description

Extends the OperationTracer interface to be triggered on contexts enter & exit.

@daniellehrner @shemnon

Fixed Issue(s)

Fixes #5728

github-actions · 2023-08-07T22:25:54Z

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

Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu>

evm/src/main/java/org/hyperledger/besu/evm/processor/AbstractMessageProcessor.java

evm/src/main/java/org/hyperledger/besu/evm/tracing/OperationTracer.java

Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu>

shemnon

This change needs tests. Either directly such as how ExtendedOperationTracerTest tested the code or indirectly like how traceEndTransaction is integrated into the testing of the T8nExecutor.

If we are mimicing Geth's behavior on their tracer some of their samples should provide the skeleton for how to test it is working as intended.

evm/src/main/java/org/hyperledger/besu/evm/processor/AbstractMessageProcessor.java

Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu>

evm/src/main/java/org/hyperledger/besu/evm/processor/AbstractMessageProcessor.java

Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu>

Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net>

Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu>

Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net>

evm/src/main/java/org/hyperledger/besu/evm/tracing/OperationTracer.java

shemnon · 2023-08-21T23:38:41Z

I was hoping for a test more like

besu/evm/src/test/java/org/hyperledger/besu/evm/StandardJsonTracerTest.java

Line 33 in 532797f

class StandardJsonTracerTest {

, where there are no mocks. A custom tracer that outputs the tracing results into a data structure or text would then be verified.

With mocks-only testing I just don't feel it proves that it's not behaving in surprising ways.

Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net>

…niellehrner/besu into feat/issue-5728/trace-context-changes

Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net>

daniellehrner · 2023-08-23T20:45:00Z

@shemnon I added another test without mocks, based on EIP-3155. Could you please review again?

evm/src/main/java/org/hyperledger/besu/evm/tracing/OperationTracer.java

+   *
+   * @param frame the frame
+   */
+  default void traceContextExit(final MessageFrame frame) {}


evm/src/main/java/org/hyperledger/besu/evm/tracing/OperationTracer.java

+   *
+   * @param frame the frame
+   */
+  default void traceContextEnter(final MessageFrame frame) {}


@shemnon

* Trigger `OperationTracer` on contexts enter & exit Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Update CHANGELOG.md Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Spotless Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Daniel's comments Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Ensure `OperationTracer` is not null before calling it Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Move back hook calls into `process` Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Fix @shemnon comments Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * added test for context enter and context exit Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> * spotless Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> * Trigger `OperationTracer` on contexts enter & exit Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Update CHANGELOG.md Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Spotless Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Daniel's comments Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Ensure `OperationTracer` is not null before calling it Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Move back hook calls into `process` Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Fix @shemnon comments Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * added test for context enter and context exit Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> * spotless Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> * added a test without mocking Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> * fixed unit tests Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> --------- Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> Co-authored-by: Daniel Lehrner <daniel.lehrner@consensys.net>

@shemnon

* Trigger `OperationTracer` on contexts enter & exit Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Update CHANGELOG.md Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Spotless Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Daniel's comments Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Ensure `OperationTracer` is not null before calling it Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Move back hook calls into `process` Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Fix @shemnon comments Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * added test for context enter and context exit Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> * spotless Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> * Trigger `OperationTracer` on contexts enter & exit Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Update CHANGELOG.md Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Spotless Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Daniel's comments Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Ensure `OperationTracer` is not null before calling it Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Move back hook calls into `process` Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Fix @shemnon comments Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * added test for context enter and context exit Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> * spotless Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> * added a test without mocking Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> * fixed unit tests Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> --------- Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> Co-authored-by: Daniel Lehrner <daniel.lehrner@consensys.net>

@shemnon

* Trigger `OperationTracer` on contexts enter & exit Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Update CHANGELOG.md Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Spotless Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Daniel's comments Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Ensure `OperationTracer` is not null before calling it Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Move back hook calls into `process` Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Fix @shemnon comments Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * added test for context enter and context exit Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> * spotless Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> * Trigger `OperationTracer` on contexts enter & exit Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Update CHANGELOG.md Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Spotless Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Daniel's comments Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Ensure `OperationTracer` is not null before calling it Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Move back hook calls into `process` Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Fix @shemnon comments Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * added test for context enter and context exit Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> * spotless Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> * added a test without mocking Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> * fixed unit tests Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> --------- Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> Co-authored-by: Daniel Lehrner <daniel.lehrner@consensys.net> Signed-off-by: garyschulte <garyschulte@gmail.com>

@shemnon

* Trigger `OperationTracer` on contexts enter & exit Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Update CHANGELOG.md Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Spotless Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Daniel's comments Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Ensure `OperationTracer` is not null before calling it Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Move back hook calls into `process` Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Fix @shemnon comments Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * added test for context enter and context exit Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> * spotless Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> * Trigger `OperationTracer` on contexts enter & exit Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Update CHANGELOG.md Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Spotless Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Daniel's comments Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Ensure `OperationTracer` is not null before calling it Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Move back hook calls into `process` Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Fix @shemnon comments Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * added test for context enter and context exit Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> * spotless Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> * added a test without mocking Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> * fixed unit tests Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> --------- Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> Co-authored-by: Daniel Lehrner <daniel.lehrner@consensys.net>

@shemnon

* Trigger `OperationTracer` on contexts enter & exit Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Update CHANGELOG.md Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Spotless Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Daniel's comments Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Ensure `OperationTracer` is not null before calling it Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Move back hook calls into `process` Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Fix @shemnon comments Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * added test for context enter and context exit Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> * spotless Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> * Trigger `OperationTracer` on contexts enter & exit Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Update CHANGELOG.md Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Spotless Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Daniel's comments Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Ensure `OperationTracer` is not null before calling it Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Move back hook calls into `process` Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * Fix @shemnon comments Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> * added test for context enter and context exit Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> * spotless Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> * added a test without mocking Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> * fixed unit tests Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> --------- Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net> Co-authored-by: Daniel Lehrner <daniel.lehrner@consensys.net>

delehef added 3 commits August 8, 2023 00:32

Trigger OperationTracer on contexts enter & exit

65a428d

Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu>

Update CHANGELOG.md

b64e7fb

Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu>

Spotless

776d24f

Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu>

daniellehrner requested changes Aug 9, 2023

View reviewed changes

evm/src/main/java/org/hyperledger/besu/evm/processor/AbstractMessageProcessor.java Outdated Show resolved Hide resolved

evm/src/main/java/org/hyperledger/besu/evm/tracing/OperationTracer.java Outdated Show resolved Hide resolved

daniellehrner mentioned this pull request Aug 9, 2023

Added traceContextEnter & traceContextExit #5741

Closed

Daniel's comments

813118b

Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu>

delehef requested a review from daniellehrner August 9, 2023 11:02

Ensure OperationTracer is not null before calling it

ca9d725

Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu>

shemnon suggested changes Aug 9, 2023

View reviewed changes

Move back hook calls into process

848165c

Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu>

shemnon reviewed Aug 10, 2023

View reviewed changes

Fix @shemnon comments

41c75f0

Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu>

delehef requested a review from shemnon August 10, 2023 15:10

daniellehrner and others added 12 commits August 21, 2023 16:28

Merge branch 'main' into feat/issue-5728/trace-context-changes

3d8f3df

added test for context enter and context exit

7d466bb

Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net>

spotless

21edd27

Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net>

Trigger OperationTracer on contexts enter & exit

836907c

Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu>

Update CHANGELOG.md

01f5d56

Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu>

Spotless

86e2cc1

Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu>

Daniel's comments

b9837f8

Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu>

Ensure OperationTracer is not null before calling it

a8029fa

Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu>

Move back hook calls into process

bc073d4

Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu>

Fix @shemnon comments

b5c206a

Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu>

added test for context enter and context exit

208cd34

Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net>

spotless

106f2fd

Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net>

daniellehrner approved these changes Aug 21, 2023

View reviewed changes

github-advanced-security bot found potential problems Aug 21, 2023

View reviewed changes

evm/src/main/java/org/hyperledger/besu/evm/tracing/OperationTracer.java Fixed Show fixed Hide fixed

evm/src/main/java/org/hyperledger/besu/evm/tracing/OperationTracer.java Fixed Show fixed Hide fixed

added a test without mocking

277ba8b

Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net>

daniellehrner added 3 commits August 23, 2023 11:33

Merge branch 'feat/issue-5728/trace-context-changes' of github.com:da…

f7e0255

…niellehrner/besu into feat/issue-5728/trace-context-changes

Merge branch 'main' into feat/issue-5728/trace-context-changes

764bf5d

fixed unit tests

3481bd8

Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net>

shemnon approved these changes Aug 23, 2023

View reviewed changes

daniellehrner enabled auto-merge (squash) August 23, 2023 21:24

Merge branch 'main' into feat/issue-5728/trace-context-changes

5303abb

daniellehrner merged commit 4ecfbb2 into hyperledger:main Aug 23, 2023

github-advanced-security bot found potential problems Aug 23, 2023

View reviewed changes

delehef deleted the feat/issue-5728/trace-context-changes branch September 1, 2023 09:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Trigger `OperationTracer` on contexts enter & exit #5756

Trigger `OperationTracer` on contexts enter & exit #5756

delehef commented Aug 7, 2023 •

edited

Loading

github-actions bot commented Aug 7, 2023

shemnon left a comment •

edited

Loading

shemnon commented Aug 21, 2023

daniellehrner commented Aug 23, 2023

Trigger OperationTracer on contexts enter & exit #5756

Trigger OperationTracer on contexts enter & exit #5756

Conversation

delehef commented Aug 7, 2023 • edited Loading

PR description

Fixed Issue(s)

github-actions bot commented Aug 7, 2023

shemnon left a comment • edited Loading

Choose a reason for hiding this comment

shemnon commented Aug 21, 2023

daniellehrner commented Aug 23, 2023

Trigger `OperationTracer` on contexts enter & exit #5756

Trigger `OperationTracer` on contexts enter & exit #5756

delehef commented Aug 7, 2023 •

edited

Loading

shemnon left a comment •

edited

Loading