[v2] Add a Log execution event, created with the log syscall #1396
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## release/v2 #1396 +/- ##
==============================================
- Coverage 51.18% 50.25% -0.93%
==============================================
Files 126 126
Lines 10267 10458 +191
==============================================
+ Hits 5255 5256 +1
- Misses 5012 5202 +190
|
Stebalien
approved these changes
Jan 5, 2023
fvm/src/call_manager/mod.rs
Outdated
| @@ -121,6 +121,12 @@ pub trait CallManager: 'static { | |||
| self.gas_tracker_mut().apply_charge(charge)?; | |||
| Ok(()) | |||
| } | |||
|
|
|||
| /// Whether tracing is requested. | |||
| fn tracing(&self) -> bool; | |||
There was a problem hiding this comment.
Any reason not to just call call_manager.machine().context().tracing? It's long, but we only call this once.
Otherwise, I'd provide a default implementation that delegates to self.machine().context().tracing.
There was a problem hiding this comment.
Added the default implementation, thanks. There are additionally 3 internal calls to this, and the many hops to get there adds coupling through those objects at more places.
Stebalien
approved these changes
Jan 6, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I am attempting to improve instrumentation to support analysis of gas charges across spans of code. See #1397.
This PR adds a
Logexecution event to the trace format. A log event is recorded when thelogsyscall is invoked. The log event carries a singleStringmessage.A string is the most basic payload to carry here, so I think the right place to start. Some structure may be encoded into strings (for subsequent offline analysis). We could consider adding some kind of key/value pair metadata, but I suggest we defer until observed usage justifies it.
Note that the GasCharge event's
nameis aCow<'static, str>. I presume this is because the same literal name is expected to be used frequently and this type will lead to meaningful optimisation. I'm not sure that that's appropriate for theLogevent, if it would rule out dynamically constructed messages.This PR is against the
release/v2branch, because I am analysing the built-in actors that are not yet using fvm v3. I intend to forward-port it when stable.