Skip to content

Conversation

@marioevz
Copy link
Member

Work in progress

Introduces a custom tracer that brings most of the functionality of the json logger, and it also logs the call frame enter and exit of each call.

return &opcodeTracer{traces: make([]json.RawMessage, 0), callStack: make([]common.Address, 0), cfg: cfg}, nil
}

func (t *opcodeTracer) appendTrace(item interface{}) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This can blow up the node quickly specially with memory enabled. We should rather stream out the result.

@s1na
Copy link
Contributor

s1na commented Mar 19, 2024

I must admit I liked the first PR more. I think this is just duplicating the JSON logger and adding stuff on top. What I didn't like there was having to add EnableCallFrame to the logger.Config object. Because that config object is shared with StructLogger which is not adding this feature and is overloaded anyway.

But I just had an idea: one of the nice things about the new tracing structure in #29189 is that we can enable/disable methods during the runtime. So we could implement OnEnter and OnExit logic methods on JSONLogger but not set the function pointers on the Hooks object until a method EnableCallFrames() is called explicitly.

Otherwise we can have a new tracer that embeds the JSONLogger for most of logic and adds call frames on top.

@s1na
Copy link
Contributor

s1na commented Mar 20, 2024

But I just had an idea: one of the nice things about the new tracing structure in #29189 is that we can enable/disable methods during the runtime. So we could implement OnEnter and OnExit logic methods on JSONLogger but not set the function pointers on the Hooks object until a method EnableCallFrames() is called explicitly.

I implemented this idea here: https://github.com/s1na/go-ethereum/tree/extended-tracer-json-call

I'd still like to use the json config flag also for the json loggers options as in other tracers. But I'll leave that for another day.

@s1na
Copy link
Contributor

s1na commented Mar 27, 2024

#29353 was merged instead.

@s1na s1na closed this Mar 27, 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.

2 participants