Add evm and flow fees metric #428
Conversation
illia-malachyn
requested review from
sideninja,
peterargue and
m-Peter
as code owners
|
Warning Rate limit exceeded@illia-malachyn has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 27 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe recent changes enhance the functionality and observability of the system, particularly in transaction processing and metrics collection. Key modifications include the introduction of a new Changes
Sequence Diagram(s)sequenceDiagram
participant Engine
participant Collector
participant TxPool
participant Transaction
Engine->>Transaction: Process Events
Transaction->>Engine: From() (Sender Address)
Engine->>Collector: EVMFeesCollected(Sender, GasUsed, GasPrice)
Engine->>TxPool: Send(Transaction)
TxPool->>Collector: FlowFeesCollected(Payer, Events)
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
metrics/collector.go (1)
24-25: Implement Missing Methods in Collector Interface ImplementationsThe
EVMFeesCollectedandFlowFeesCollectedmethods have not been implemented in any of the structs that use theCollectorinterface. This oversight could lead to runtime errors if these methods are invoked. Please ensure that these methods are implemented in all relevant structs.
- Check and update the following files where the
Collectorinterface is used:
metrics/nop.gometrics/collector.goapi/server.gometrics/handler.goapi/debug.goservices/requester/pool.goservices/traces/engine.goservices/ingestion/engine.goservices/requester/requester.goapi/api.goAnalysis chain
Interface Update: Ensure Consistency with Implementations.
The
Collectorinterface now includesEVMFeesCollectedandFlowFeesCollected. Ensure all implementations of this interface are updated accordingly.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all implementations of the `Collector` interface are updated with the new methods. # Test: Search for implementations of the `Collector` interface. Expect: Implementations include the new methods. ast-grep --lang go --pattern 'type $_ struct { $$$ }' | rg 'Collector'Length of output: 661
Script:
#!/bin/bash # Check for the implementation of the new methods EVMFeesCollected and FlowFeesCollected in the identified files. # Look for the implementation of EVMFeesCollected method in relevant files. ast-grep --lang go --pattern 'func ($_).EVMFeesCollected($_, $_, $_) { $$$ }' metrics/nop.go metrics/collector.go # Look for the implementation of FlowFeesCollected method in relevant files. ast-grep --lang go --pattern 'func ($_).FlowFeesCollected($_, $_) { $$$ }' metrics/nop.go metrics/collector.goLength of output: 228
Script:
#!/bin/bash # Search for the implementation of the new methods EVMFeesCollected and FlowFeesCollected across all relevant files. # Look for the implementation of EVMFeesCollected method in all files where Collector is used. ast-grep --lang go --pattern 'func ($_).EVMFeesCollected($_, $_, $_) { $$$ }' api/server.go metrics/handler.go api/debug.go services/requester/pool.go services/traces/engine.go services/ingestion/engine.go services/requester/requester.go api/api.go # Look for the implementation of FlowFeesCollected method in all files where Collector is used. ast-grep --lang go --pattern 'func ($_).FlowFeesCollected($_, $_) { $$$ }' api/server.go metrics/handler.go api/debug.go services/requester/pool.go services/traces/engine.go services/ingestion/engine.go services/requester/requester.go api/api.goLength of output: 498
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
Files selected for processing (8)
- bootstrap/bootstrap.go (1 hunks)
- config/config.go (1 hunks)
- go.mod (3 hunks)
- metrics/collector.go (4 hunks)
- metrics/nop.go (2 hunks)
- services/ingestion/engine.go (1 hunks)
- services/requester/pool.go (4 hunks)
- services/requester/remote_state.go (1 hunks)
Files skipped from review due to trivial changes (1)
- config/config.go
Additional comments not posted (18)
metrics/nop.go (1)
23-24: Methods added tonopCollectorare correctly defined.The
EVMFeesCollectedandFlowFeesCollectedmethods are added as no-operations, consistent with the purpose of thenopCollector. Ensure these methods align with theCollectorinterface requirements.services/requester/remote_state.go (1)
83-83: Method name and return type change are consistent with functionality.The renaming from
AllocateStorageIndextoAllocateSlabIndexand the change in return type reflect a shift in the storage mechanism. The panic behavior remains appropriate for this read-only function.services/requester/pool.go (3)
34-34: Collector field correctly added toTxPool.The addition of the
collectorfield toTxPoolis consistent with the goal of enhancing metrics collection. Ensure that this field is properly utilized throughout theTxPoolmethods.
42-49: Constructor updated to include collector.The
NewTxPoolconstructor now requires acollectorparameter, aligning with the changes to theTxPoolstruct. Ensure that all instances ofNewTxPoolare updated accordingly in the codebase.
103-104: Flow fees collection integrated intoSendmethod.The
Sendmethod now collects flow fees using thecollector, enhancing the observability of transaction costs. This change is consistent with the PR's objectives.metrics/collector.go (4)
75-87: Prometheus GaugeVec Initialization: Ensure Proper Labeling.The
evmFeesandflowFeesmetrics are initialized with appropriate labels. Ensure that the labels used are consistent with the rest of the metrics in the system.
156-168: EVM Fees Collection: Check for Precision Loss Handling.The
EVMFeesCollectedmethod calculates gas fees and logs a warning if precision is lost. Ensure that the precision loss handling is adequate and that logging is at the appropriate level.
170-186: Flow Fees Collection: Verify Event Decoding Logic.The
FlowFeesCollectedmethod decodes theFlowFeesDeductedevent. Ensure that the event decoding logic is robust and handles all possible errors.
38-39: New Fields inDefaultCollector: Verify Metric Registration.The addition of
evmFeesandflowFeesasprometheus.GaugeVectypes is appropriate for tracking fees. Ensure these metrics are correctly registered and used.services/ingestion/engine.go (1)
162-165: EVM Fees Collection in Event Processing: Verify Integration.The addition of EVM fee collection in the
processEventsmethod integrates fee tracking before transaction indexing. Ensure this step does not disrupt the existing processing flow and handles errors appropriately.bootstrap/bootstrap.go (1)
298-298: Transaction Pool Initialization: Verify Collector Integration.The addition of
collectorto theNewTxPoolfunction call suggests enhanced functionality. Ensure that thecollectoris utilized effectively within the transaction pool and that this change is compatible with existing logic.go.mod (7)
11-11: Verify compatibility with the updatedatreeversion.The update to
github.com/onflow/atree v0.8.0-rc.5may introduce changes. Ensure compatibility and check for any breaking changes or new features that need attention.
14-14: Verify compatibility with the updatedflow-go-sdkversion.The update to
github.com/onflow/flow-go-sdk v1.0.0-preview.47may introduce changes. Ensure compatibility and check for any breaking changes or new features that need attention.
7-7: Verify the necessity of the AWS SDK dependency.The addition of
github.com/aws/aws-sdk-go-v2 v1.27.0suggests new functionality related to AWS services. Ensure that this dependency is necessary and correctly integrated within the codebase.
41-41: Verify the necessity of the Smithy Go dependency.The addition of
github.com/aws/smithy-go v1.20.2as an indirect dependency is likely related to the AWS SDK. Ensure that this dependency is necessary and correctly integrated within the codebase.Verification successful
Smithy Go Dependency is Justified and Correctly Integrated
The
github.com/aws/smithy-go v1.20.2dependency is necessary and correctly integrated as an indirect dependency. It appears ingo.sum, indicating it is used by other dependencies in the project. No further action is required.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the Smithy Go dependency in the codebase. # Test: Search for usage of Smithy Go. Expect: Relevant code using Smithy Go. rg --type go $'smithy'Length of output: 21
Script:
#!/bin/bash # Description: Check for dependencies that might rely on `smithy-go` in go.mod and go.sum. # List all dependencies in go.mod that might indirectly require smithy-go. rg 'require' go.mod # Check go.sum for any references to smithy-go to see if it is used indirectly. rg 'smithy-go' go.sumLength of output: 493
213-213: Verify compatibility with the updatedblake3version.The update to
lukechampine.com/blake3 v1.3.0may include performance improvements or bug fixes. Ensure compatibility and check for any breaking changes.Verification successful
No impact from
blake3update found.The
lukechampine.com/blake3 v1.3.0dependency is not directly or indirectly used in the codebase, indicating that the update does not affect current functionality. No further action is needed.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify compatibility with the updated `blake3` version. # Test: Search for `blake3` usage. Expect: No breaking changes or necessary updates. rg --type go $'blake3'Length of output: 21
Script:
#!/bin/bash # Search for import statements of `blake3` to verify if it is used in the codebase. rg --type go 'import.*blake3' -A 5 # Additionally, search for any indirect usage through other packages. rg --type go 'blake3' -A 5Length of output: 63
217-217: Verify the necessity of theflow-goreplacement directive.The replacement of
github.com/onflow/flow-gosuggests custom modifications or fixes. Ensure that these modifications are necessary and correctly integrated within the codebase.Verification successful
The
flow-goreplacement directive is justified.The replacement of
github.com/onflow/flow-gowith a custom version is necessary to address specific issues, such as hash calculation fixes and pending upstream issues (e.g., onflow/flow-go#5481). These modifications are crucial for the current functionality of the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage and necessity of the `flow-go` replacement directive in the codebase. # Test: Search for `flow-go` usage. Expect: Justification for the replacement. rg --type go $'flow-go'Length of output: 7235
Script:
#!/bin/bash # Search for comments or documentation that justify the custom `flow-go` version. rg "flow-go" --type md --type go -A 3 -B 3 | grep -i "custom\|modification\|fix\|feature"Length of output: 321
12-12: Verify compatibility with the updatedcadenceversion.The update to
github.com/onflow/cadence v1.0.0-preview.44may introduce changes. Ensure compatibility and check for any breaking changes or new features that need attention.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
go.sumis excluded by!**/*.sumtests/go.sumis excluded by!**/*.sum
Files selected for processing (3)
- bootstrap/bootstrap.go (1 hunks)
- go.mod (1 hunks)
- tests/go.mod (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- bootstrap/bootstrap.go
Additional comments not posted (2)
go.mod (1)
215-215: Verify the use of a forked version offlow-go.The replacement of
github.com/onflow/flow-gowith a forked version suggests custom modifications. Ensure these changes are documented and necessary for your project.tests/go.mod (1)
246-246: Verify the use of a forked version offlow-goin tests.The replacement of
github.com/onflow/flow-gowith a forked version in the test environment suggests custom modifications. Ensure these changes are necessary and consistent with the main project.
This covers
Report fees paid on Flow and EVM side as a metric.EVM fees (gas) are gathered in the event ingestion.
Flow fees are collected in the requester package only, therefore we will miss some fees paid on the Flow side. This is done intentionally and is a topic to discuss
Depends on onflow/flow-go#6304
Summary by CodeRabbit
New Features
Bug Fixes
8080to8081.Chores