Skip to content
This repository has been archived by the owner on Jun 9, 2024. It is now read-only.

feat(telemetry): add timing operations #1343

Merged
merged 10 commits into from
Dec 11, 2023
Merged

feat(telemetry): add timing operations #1343

merged 10 commits into from
Dec 11, 2023

Conversation

itsdevbear
Copy link

@itsdevbear itsdevbear commented Dec 7, 2023

Summary by CodeRabbit

  • New Features

    • Integrated telemetry across various modules to enhance performance monitoring.
    • Added new search and state retrieval functionalities to the EVM backend.
    • Improved transaction broadcasting with metric tracking for mempool fullness and broadcast failures.
  • Bug Fixes

    • Corrected a typographical error in the entrypoint.sh script argument.
  • Documentation

    • Updated licensing information and added new telemetry-related constants in multiple files.
  • Refactor

    • Enhanced existing methods with telemetry for better insight into block building and gas usage.
    • Modified the initAppConfig function to enable telemetry by default.
  • Style

    • Standardized telemetry metric keys across different modules for consistency.
  • Tests

    • No visible changes to end-users regarding tests.
  • Chores

    • No visible changes to end-users regarding chores.
  • Revert

    • No reverts in this release.

Copy link

coderabbitai bot commented Dec 7, 2023

Walkthrough

The changes across the codebase introduce telemetry to various components, enhancing monitoring and performance metrics. New constants and telemetry-related code snippets are added to track execution times and gauge values, reflecting a focus on observability and optimization.

Changes

File Path Change Summary
cosmos/runtime/miner/..., cosmos/runtime/txpool/..., cosmos/store/snapmulti/..., cosmos/x/evm/... Added telemetry imports, constants, and measurements for performance tracking.
e2e/testapp/... Enabled telemetry in configuration and fixed a typographical error in a shell script.
eth/polar/... Added new functions and types to enhance the backend's capabilities.

🐇✨
In the cosmos where code does weave,
A rabbit hops with metrics to perceive.
With telemetry in hand, it leaps and bounds,
Through lines of code, where performance abounds. 🌟🚀

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 X ?


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • If you reply to a review comment from CodeRabbit, the bot will automatically respond.
  • To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
  • Note: Review comments are made on code diffs or files, not on the PR overview.
  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 1

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 3294140 and 93bd12b.
Files ignored due to filter (3)
  • e2e/localnet/go.mod
  • e2e/localnet/go.sum
  • e2e/precompile/go.mod
Files selected for processing (11)
  • cosmos/runtime/miner/abci.go (1 hunks)
  • cosmos/runtime/miner/miner.go (3 hunks)
  • cosmos/runtime/txpool/handler.go (3 hunks)
  • cosmos/store/snapmulti/store.go (4 hunks)
  • cosmos/x/evm/keeper/processor.go (2 hunks)
  • cosmos/x/evm/plugins/precompile/plugin.go (4 hunks)
  • cosmos/x/evm/types/keys.go (1 hunks)
  • e2e/testapp/entrypoint.sh (1 hunks)
  • e2e/testapp/polard/cmd/commands.go (1 hunks)
  • eth/polar/api_backend.go (3 hunks)
  • eth/polar/backend.go (3 hunks)
Files skipped from review due to trivial changes (1)
  • e2e/testapp/entrypoint.sh
Additional comments: 22
cosmos/runtime/miner/abci.go (1)
  • 42-44: The addition of the payloadEnvelopeBz and err variables within the PrepareProposal method is noted. Ensure that the usage of these variables is consistent with the intended telemetry feature implementation and that they are handled correctly throughout the method.
cosmos/runtime/miner/miner.go (3)
  • 41-44: The addition of constants for tracking mining metrics is correctly implemented.

  • 76-87: The modification to the buildBlock method to include telemetry for measuring block building time is correctly implemented.

  • 122-135: The modification to the resolveEnvelope method to include telemetry for block gas usage and transaction count is correctly implemented.

cosmos/store/snapmulti/store.go (4)
  • 41-48: The addition of telemetry constants is correctly implemented and follows the naming convention established in the PR objectives.

  • 124-130: The implementation of telemetry in the Snapshot method is correct, using defer to ensure measurements are taken even if the method exits early.

  • 145-150: The implementation of telemetry in the RevertToSnapshot method is correct, using defer to ensure measurements are taken even if the method exits early.

  • 157-163: The implementation of telemetry in the Finalize method is correct, using defer to ensure measurements are taken even if the method exits early.

cosmos/x/evm/keeper/processor.go (3)
  • 23-37: The import of the telemetry package and the related changes in the ProcessPayloadEnvelope function align with the PR's objective to add telemetry features.

  • 39-39: The empty constant declaration const () is present. If this was intended to be populated with telemetry-related constants, it should be completed or removed if not needed.

  • 70-77: The defer statement for telemetry is correctly placed to measure the duration of the InsertBlockAndSetHead operation, aligning with the PR's objective.

cosmos/x/evm/plugins/precompile/plugin.go (4)
  • 23-26: The import of the time package is correctly added to support the new telemetry feature.

  • 47-50: The addition of constants MetricKeyBase and MetricKeyTime aligns with the telemetry feature implementation.

  • 178-191: The Run function correctly implements a deferred telemetry measurement to track execution time.

  • 36-54: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [23-191]

Overall, the changes in cosmos/x/evm/plugins/precompile/plugin.go are consistent with the PR objectives and summaries provided, and they correctly implement the telemetry feature without apparent issues.

cosmos/x/evm/types/keys.go (1)
  • 23-27: The addition of the MetricKeyInsertBlockAndSetHead constant is correctly implemented and follows the existing naming conventions in the file.
e2e/testapp/polard/cmd/commands.go (1)
  • 98-105: The changes to the initAppConfig function to enable telemetry are consistent with the PR objectives and the AI-generated summaries. This will allow the application to collect telemetry data as intended.
eth/polar/backend.go (5)
  • 23-47: The addition of imports for context, errors, fmt, and additional packages from the github.com/ethereum/go-ethereum and github.com/berachain/polaris repositories is correctly reflected in the code.

  • 187-193: The addition of a new service in the Polaris struct's APIs method is correctly implemented and aligns with the PR objectives to enhance telemetry features.

  • 232-258: The stateAtBlock method has been added to the Polaris struct. This method is designed to retrieve the state database associated with a certain block, which is in line with the PR objectives to add telemetry features.

  • 261-308: The stateAtTransaction method has been added to the Polaris struct. This method provides the execution environment of a certain transaction, which is consistent with the PR objectives.

  • 310-324: The pathState method has been added to the Polaris struct. This method returns the state at a specific block, which is consistent with the PR objectives and the summary provided.

cosmos/x/evm/keeper/processor.go Show resolved Hide resolved
@itsdevbear
Copy link
Author

Uploading image.png…

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 93bd12b and f738878.
Files ignored due to filter (6)
  • cosmos/go.mod
  • cosmos/go.sum
  • e2e/precompile/go.mod
  • e2e/precompile/go.sum
  • e2e/testapp/go.mod
  • e2e/testapp/go.sum
Files selected for processing (3)
  • cosmos/store/snapmulti/store.go (4 hunks)
  • cosmos/x/evm/keeper/processor.go (2 hunks)
  • cosmos/x/evm/plugins/precompile/plugin.go (3 hunks)
Files skipped from review due to trivial changes (1)
  • cosmos/x/evm/keeper/processor.go
Additional comments: 6
cosmos/store/snapmulti/store.go (4)
  • 43-50: The addition of telemetry constants is consistent with the PR's objective to add timing operations for telemetry.

  • 125-127: The integration of telemetry measurements in the Snapshot method is correctly implemented using defer to ensure measurements are taken.

  • 146-149: The integration of telemetry measurements in the RevertToSnapshot method is correctly implemented using defer to ensure measurements are taken.

  • 158-160: The integration of telemetry measurements in the Finalize method is correctly implemented using defer to ensure measurements are taken.

cosmos/x/evm/plugins/precompile/plugin.go (2)
  • 47-49: The constants MetricKeyBase and MetricKeyTime are added for telemetry. Ensure these metric keys are consistent with the telemetry strategy used throughout the codebase.

The constants MetricKeyBase and MetricKeyTime are consistent with the telemetry strategy used throughout the codebase.

  • 173-190: The Run function now includes telemetry measurement. Verify that the addition of telemetry does not alter the behavior of the function, especially with regard to error handling and gas metering.

Copy link

codecov bot commented Dec 7, 2023

Codecov Report

Merging #1343 (d301b86) into main (5c432b9) will increase coverage by 0.05%.
The diff coverage is 80.76%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1343      +/-   ##
==========================================
+ Coverage   48.58%   48.63%   +0.05%     
==========================================
  Files          84       84              
  Lines        4899     4914      +15     
==========================================
+ Hits         2380     2390      +10     
- Misses       2342     2347       +5     
  Partials      177      177              
Files Coverage Δ
cosmos/store/snapmulti/store.go 96.66% <100.00%> (+0.51%) ⬆️
cosmos/x/evm/plugins/precompile/plugin.go 66.07% <100.00%> (+0.61%) ⬆️
cosmos/runtime/txpool/handler.go 63.00% <0.00%> (-1.29%) ⬇️
cosmos/x/evm/keeper/processor.go 0.00% <0.00%> (ø)

cosmos/runtime/miner/miner.go Outdated Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between f738878 and 6a5c7b5.
Files selected for processing (8)
  • cosmos/runtime/miner/miner.go (3 hunks)
  • cosmos/runtime/miner/telemetry.go (1 hunks)
  • cosmos/runtime/txpool/handler.go (2 hunks)
  • cosmos/runtime/txpool/telemetry.go (1 hunks)
  • cosmos/store/snapmulti/store.go (4 hunks)
  • cosmos/store/snapmulti/telemetry.go (1 hunks)
  • cosmos/x/evm/plugins/precompile/plugin.go (3 hunks)
  • cosmos/x/evm/plugins/precompile/telemetry.go (1 hunks)
Files skipped from review due to trivial changes (3)
  • cosmos/runtime/txpool/telemetry.go
  • cosmos/store/snapmulti/telemetry.go
  • cosmos/x/evm/plugins/precompile/telemetry.go
Additional comments: 13
cosmos/runtime/miner/miner.go (1)
  • 116-129: The summary does not mention the addition of telemetry data recording in the resolveEnvelope method, but the code shows that telemetry has been added. This should be reflected in the summary.
cosmos/runtime/miner/telemetry.go (1)
  • 23-27: The addition of telemetry metric constants is consistent and follows a clear naming convention.
cosmos/runtime/txpool/handler.go (2)
  • 230-243: The telemetry counters are incremented correctly in response to specific error codes. However, ensure that MetricKeyMempoolFull and MetricKeyBroadcastFailure are properly defined and that their values are consistent with the rest of the telemetry implementation.

The telemetry counters MetricKeyMempoolFull and MetricKeyBroadcastFailure are defined in cosmos/runtime/txpool/telemetry.go and used correctly in cosmos/runtime/txpool/handler.go as indicated by the new context provided.

  • 243-243: Ensure that the retry logic for failed transactions does not lead to an infinite loop in case of persistent errors that are not recoverable.
cosmos/store/snapmulti/store.go (5)
  • 114-117: The addition of telemetry measurements using defer statements in the Snapshot method is correctly implemented. The use of defer ensures that the telemetry measurements are recorded after the method's logic is executed, regardless of where the method exits.

  • 138-139: The addition of telemetry measurements using defer statements in the RevertToSnapshot method is correctly implemented. The use of defer ensures that the telemetry measurements are recorded after the method's logic is executed, regardless of where the method exits.

  • 149-150: The addition of telemetry measurements using defer statements in the Finalize method is correctly implemented. The use of defer ensures that the telemetry measurements are recorded after the method's logic is executed, regardless of where the method exits.

  • 116-116: Verify that the constants MetricKeySnapshot, MetricKeyRevertToSnapshot, and MetricKeyFinalize are defined elsewhere in the codebase and are accessible within this package.

Also applies to: 138-138, 149-149


The constants MetricKeySnapshot, MetricKeyRevertToSnapshot, and MetricKeyFinalize are defined in the cosmos/store/snapmulti/telemetry.go file and are used appropriately in the store.go file within the same package. The verification confirms that these constants are accessible within the package.

  • 117-117: Verify that the constants MetricKeySnapshotSize, MetricKeyRevertToSnapshotSize, and MetricKeyFinalizeSize are defined elsewhere in the codebase and are accessible within this package.

Also applies to: 139-139, 150-150


The constants MetricKeySnapshotSize, MetricKeyRevertToSnapshotSize, and MetricKeyFinalizeSize are defined in cosmos/store/snapmulti/telemetry.go and are used within cosmos/store/snapmulti/store.go as shown in the output. This confirms that the constants are defined and accessible within the package.

Final review comment:

The constants `MetricKeySnapshotSize`, `MetricKeyRevertToSnapshotSize`, and `MetricKeyFinalizeSize` are properly defined and used within the package as intended.

cosmos/x/evm/plugins/precompile/plugin.go (4)
  • 23-26: The addition of the time package import is appropriate for the telemetry feature being implemented.

  • 171-182: The implementation of telemetry using telemetry.MeasureSince to measure the execution time of the pc.Run function is correct and follows best practices for performance monitoring.

  • 171-182: The use of a block to scope the defer statement for telemetry measurement is a good practice to ensure accurate timing of the pc.Run function call.

  • 168-186: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [23-185]

Overall, the changes made to the cosmos/x/evm/plugins/precompile/plugin.go file align with the PR's objective to add telemetry for timing operations and are implemented correctly.

@itsdevbear itsdevbear added merge me daddy Trigger Beradozer to bulldoze the PR backport/v0.0.2-alpha labels Dec 11, 2023
@mergify mergify bot merged commit a701588 into main Dec 11, 2023
8 checks passed
@mergify mergify bot deleted the telemetry branch December 11, 2023 17:24
mergify bot pushed a commit that referenced this pull request Dec 11, 2023
## Summary by CodeRabbit

- **New Features**
  - Integrated telemetry across various modules to enhance performance monitoring.
  - Added new search and state retrieval functionalities to the EVM backend.
  - Improved transaction broadcasting with metric tracking for mempool fullness and broadcast failures.

- **Bug Fixes**
  - Corrected a typographical error in the `entrypoint.sh` script argument.

- **Documentation**
  - Updated licensing information and added new telemetry-related constants in multiple files.

- **Refactor**
  - Enhanced existing methods with telemetry for better insight into block building and gas usage.
  - Modified the `initAppConfig` function to enable telemetry by default.

- **Style**
  - Standardized telemetry metric keys across different modules for consistency.

- **Tests**
  - No visible changes to end-users regarding tests.

- **Chores**
  - No visible changes to end-users regarding chores.

- **Revert**
  - No reverts in this release.

(cherry picked from commit a701588)
itsdevbear pushed a commit that referenced this pull request Dec 11, 2023
This is an automatic backport of pull request #1343 done by
[Mergify](https://mergify.com).


---


<details>
<summary>Mergify commands and options</summary>

<br />

More conditions and actions can be found in the
[documentation](https://docs.mergify.com/).

You can also trigger Mergify actions by commenting on this pull request:

- `@Mergifyio refresh` will re-evaluate the rules
- `@Mergifyio rebase` will rebase this PR on its base branch
- `@Mergifyio update` will merge the base branch into this PR
- `@Mergifyio backport <destination>` will backport this PR on
`<destination>` branch

Additionally, on Mergify [dashboard](https://dashboard.mergify.com) you
can:

- look at your merge queues
- generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.com
</details>

Co-authored-by: Devon Bear <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/v0.0.2-alpha merge me daddy Trigger Beradozer to bulldoze the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants