docs: add metrics configuration #468
Conversation
WalkthroughThe changes enhance the documentation in Changes
Assessment against linked issues
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 using PR comments)
Other keywords and placeholders
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: 1
🧹 Outside diff range and nitpick comments (3)
guides/metrics.md (3)
1-9: LGTM! Consider adding more context.The introduction provides a clear and concise overview of Rollkit's Prometheus metrics functionality. To enhance user understanding, consider adding a brief explanation of why metrics are useful and how they can benefit users of Rollkit.
10-14: Remove unnecessary backslashes in config parameter.The instructions for enabling metrics are clear. However, there's a minor formatting issue in the config parameter name.
Apply this change to line 13:
-`instrumentation.prometheus\_listen\_addr`). +`instrumentation.prometheus_listen_addr`).This removes the unnecessary backslashes, as Markdown doesn't require escaping underscores within backticks.
🧰 Tools
🪛 LanguageTool
[grammar] ~11-~11: It looks like there is a word missing here. Did you mean “Listen to address”?
Context: ...er/metricson 26660 port by default. Listen address can be changed in the config file (see ...(LISTEN_TO_ME)
1-25: Expand documentation to fully address PR objectives.While this document provides a good starting point for Rollkit metrics configuration, it could be expanded to fully meet the objectives outlined in issue #431. Consider adding the following sections to align more closely with the CometBFT metrics documentation:
- A brief introduction explaining the importance of metrics in Rollkit.
- More detailed configuration options, if available (e.g., setting custom ports, adjusting reporting intervals).
- Examples of how to query these metrics using Prometheus.
- Best practices for monitoring Rollkit using these metrics.
- Any known limitations or considerations when using metrics in Rollkit.
Adding these sections would provide a more comprehensive guide for users looking to leverage Rollkit metrics effectively.
🧰 Tools
🪛 LanguageTool
[grammar] ~11-~11: It looks like there is a word missing here. Did you mean “Listen to address”?
Context: ...er/metricson 26660 port by default. Listen address can be changed in the config file (see ...(LISTEN_TO_ME)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- guides/metrics.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
guides/metrics.md
[grammar] ~11-~11: It looks like there is a word missing here. Did you mean “Listen to address”?
Context: ...er/metricson 26660 port by default. Listen address can be changed in the config file (see ...(LISTEN_TO_ME)
🔇 Additional comments (1)
guides/metrics.md (1)
15-20: LGTM! Clear and well-structured table header.The table header provides a clear structure for presenting the metrics information. It includes all necessary columns: Name, Type, Tags, and Description.
guides/metrics.md
Outdated
| | gas_price | Gauge | | Gas Price of the DA transaction | | ||
| | last_blob_size | Gauge | | Last blob size submitted to the DA | | ||
| | transaction_status | Gauge | | Tranasction status of the DA transaction | | ||
| | num_pending_blocks | Gauge | | Number of blocks pending DA submission | | ||
| | included_block_height | Gauge | | Block height of the last DA transaction | |
There was a problem hiding this comment.
Correct typo and improve metric descriptions.
There's a typo in the description of the "transaction_status" metric, and some descriptions could be more informative.
- Correct the typo in line 23:
-| transaction_status | Gauge | | Tranasction status of the DA transaction |
+| transaction_status | Gauge | | Transaction status of the DA transaction |- Consider improving the following descriptions for clarity:
- gas_price: "Current gas price for DA transactions"
- last_blob_size: "Size of the most recent blob submitted to the DA layer"
- num_pending_blocks: "Number of blocks waiting for DA submission"
- included_block_height: "Height of the most recent block included in a DA transaction"
These changes provide more specific information about what each metric represents.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | gas_price | Gauge | | Gas Price of the DA transaction | | |
| | last_blob_size | Gauge | | Last blob size submitted to the DA | | |
| | transaction_status | Gauge | | Tranasction status of the DA transaction | | |
| | num_pending_blocks | Gauge | | Number of blocks pending DA submission | | |
| | included_block_height | Gauge | | Block height of the last DA transaction | | |
| | gas_price | Gauge | | Current gas price for DA transactions | | |
| | last_blob_size | Gauge | | Size of the most recent blob submitted to the DA layer | | |
| | transaction_status | Gauge | | Transaction status of the DA transaction | | |
| | num_pending_blocks | Gauge | | Number of blocks waiting for DA submission | | |
| | included_block_height | Gauge | | Height of the most recent block included in a DA transaction | |
|
docs preview is failing: https://github.com/rollkit/docs/actions/runs/11072258066/job/30766061486?pr=468 but it looks like something broke it previously https://github.com/rollkit/docs/actions/workflows/preview.yml Let's fix this so we can see how this update renders. |
|
Tracking build failure here: #469 |
|
| This functionality is disabled by default. | ||
|
|
||
| To enable the Prometheus metrics, set `instrumentation.prometheus=true` in your | ||
| config file. Metrics will be served under `/metrics` on 26660 port by default. |
There was a problem hiding this comment.
Maybe a dumb question - but which file exactly?
There was a problem hiding this comment.
The page isn't linked to the site yet https://rollkit.github.io/docs-preview/pr-468/guides/overview
You need to add it to the menu.
tuxcanfly
force-pushed
the
tux/metrics-docs
branch
from
194a8be
to
5230d42
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (5)
learn/centralized-sequencer-node.md (3)
1-18: Enhance readability with minor formatting adjustmentsThe introduction and configuration table are informative and well-structured. Consider the following suggestions to improve clarity:
- Add a brief description of what a centralized sequencer is and its role in the system.
- Consider adding a blank line between the table header and the first row for better readability.
- Align the default values in the table for consistency.
Here's a suggested improvement for the table formatting:
| Flag | Usage | Default | |------------------|-------------------------------------------------------------|--------------------------| | `batch-time` | Time in seconds to wait before generating a new batch | 2 seconds | | `da_address` | DA address | `http://localhost:26658` | | `da_auth_token` | Auth token for the DA | `""` | | `da_namespace` | DA namespace where the sequencer submits transactions | `""` | | `host` | Centralized sequencer host | localhost | | `port` | Centralized sequencer port | 50051 | | `listen-all` | Listen on all network interfaces instead of just localhost | disabled | | `metrics` | Enable Prometheus metrics | disabled | | `metrics-address`| Address to expose Prometheus metrics | `":8080"` |
20-21: Improve metrics introduction formatting and grammarThe metrics introduction provides valuable information but could be improved for clarity:
- Consider using a bullet point list for better readability.
- There's a minor grammatical issue in the second sentence.
Here's a suggested improvement:
The `centralized-sequencer` node reports Prometheus metrics when the `-metrics` flag is enabled: - By default, metrics are exported to `http://localhost:8080/metrics`. - The listen address and port can be configured with the `-metrics-address` flag.🧰 Tools
🪛 LanguageTool
[uncategorized] ~20-~20: Did you mean: “By default,”?
Context: ...cs when the-metricsflag is enabled. By default metrics are exported to `http://localho...(BY_DEFAULT_COMMA)
[grammar] ~21-~21: It looks like there is a word missing here. Did you mean “listen to address”?
Context: ...tohttp://localhost:8080/metrics, the listen address and port can be configured with `-metri...(LISTEN_TO_ME)
1-31: Enhance document with additional context and usage informationThe document provides valuable information about the Centralized Sequencer Node, its configuration, and metrics. To make it even more useful for users, consider adding the following:
- A brief conclusion or summary that ties together the configuration options and metrics.
- Information on how to interpret and use these metrics for monitoring and troubleshooting.
- Examples of common use cases or scenarios where these metrics would be particularly useful.
- Links to related documentation or resources for further reading.
Here's a suggested addition to the end of the document:
### Interpreting and Using Metrics The provided metrics offer insights into the performance and status of your Centralized Sequencer Node: - Use `gas_price` and `last_blob_size` to monitor resource usage and optimize costs. - Track `transaction_status` and `num_pending_blocks` to identify potential bottlenecks or issues in the sequencing process. - Monitor `included_block_height` to ensure your node is keeping up with the network. For more information on optimizing your Centralized Sequencer Node and interpreting these metrics, refer to our [Advanced Monitoring Guide](link-to-guide).🧰 Tools
🪛 LanguageTool
[uncategorized] ~20-~20: Did you mean: “By default,”?
Context: ...cs when the-metricsflag is enabled. By default metrics are exported to `http://localho...(BY_DEFAULT_COMMA)
[grammar] ~21-~21: It looks like there is a word missing here. Did you mean “listen to address”?
Context: ...tohttp://localhost:8080/metrics, the listen address and port can be configured with `-metri...(LISTEN_TO_ME)
guides/metrics.md (2)
1-13: LGTM! Consider adding a configuration file example.The "How to configure metrics" section provides clear and concise instructions for enabling and configuring Prometheus metrics in Rollkit. The information is accurate and helpful for users.
To further enhance this section, consider adding a small example snippet of the configuration file showing how to enable metrics and set the listen address. This would provide a concrete reference for users.
🧰 Tools
🪛 LanguageTool
[grammar] ~11-~11: It looks like there is a word missing here. Did you mean “Listen to address”?
Context: ...er/metricson 26660 port by default. Listen address can be changed in the config file (see ...(LISTEN_TO_ME)
15-54: LGTM! Consider minor formatting improvements for consistency.The "List of available metrics" section provides a comprehensive and well-structured table of available metrics. Each metric is clearly described with its name, type, tags, and description, which is very helpful for users.
To improve consistency and readability:
Consider using consistent capitalization in the "Description" column. For example:
- "Height of the chain" -> "Height of the chain"
- "Number of validators" -> "Number of validators"
- "Total voting power of all validators" -> "Total voting power of all validators"
Ensure consistent use of periods at the end of descriptions. Currently, some descriptions end with periods while others don't.
These minor adjustments will enhance the overall presentation of the metrics table.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~26-~26: ‘Amount of’ should usually only be used with uncountable or mass nouns. Consider using “number” if this is not the case.
Context: ... | Gauge | | Total amount of blocks missed for the node, if the n...(AMOUNTOF_TO_NUMBEROF)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- guides/metrics.md (1 hunks)
- learn/centralized-sequencer-node.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
guides/metrics.md
[grammar] ~11-~11: It looks like there is a word missing here. Did you mean “Listen to address”?
Context: ...er/metricson 26660 port by default. Listen address can be changed in the config file (see ...(LISTEN_TO_ME)
[uncategorized] ~26-~26: ‘Amount of’ should usually only be used with uncountable or mass nouns. Consider using “number” if this is not the case.
Context: ... | Gauge | | Total amount of blocks missed for the node, if the n...(AMOUNTOF_TO_NUMBEROF)
learn/centralized-sequencer-node.md
[uncategorized] ~20-~20: Did you mean: “By default,”?
Context: ...cs when the-metricsflag is enabled. By default metrics are exported to `http://localho...(BY_DEFAULT_COMMA)
[grammar] ~21-~21: It looks like there is a word missing here. Did you mean “listen to address”?
Context: ...tohttp://localhost:8080/metrics, the listen address and port can be configured with `-metri...(LISTEN_TO_ME)
🔇 Additional comments (1)
guides/metrics.md (1)
1-54: Great job on the metrics documentation!This new file provides excellent documentation on configuring and using Prometheus metrics in Rollkit. The content is well-structured, informative, and easy to follow. It covers both the configuration process and a comprehensive list of available metrics, which will be very helpful for users implementing monitoring solutions.
The minor suggestions provided in the previous comments (adding a configuration example and improving formatting consistency) would further enhance the already high-quality document. Overall, this is a valuable addition to the Rollkit documentation.
🧰 Tools
🪛 LanguageTool
[grammar] ~11-~11: It looks like there is a word missing here. Did you mean “Listen to address”?
Context: ...er/metricson 26660 port by default. Listen address can be changed in the config file (see ...(LISTEN_TO_ME)
[uncategorized] ~26-~26: ‘Amount of’ should usually only be used with uncountable or mass nouns. Consider using “number” if this is not the case.
Context: ... | Gauge | | Total amount of blocks missed for the node, if the n...(AMOUNTOF_TO_NUMBEROF)
| The following metrics are available: | ||
|
|
||
| | **Name** | **Type** | **Tags** | **Description** | | ||
| |--------------------------------------------|-----------|------------------|------------------------------------------------------------------------| | ||
| | gas_price | Gauge | | Gas Price of the DA transaction | | ||
| | last_blob_size | Gauge | | Last blob size submitted to the DA | | ||
| | transaction_status | Gauge | | Tranasction status of the DA transaction | | ||
| | num_pending_blocks | Gauge | | Number of blocks pending DA submission | | ||
| | included_block_height | Gauge | | Block height of the last DA transaction | |
There was a problem hiding this comment.
Fix typo and enhance metrics table
The metrics table is informative and well-structured. However, there's a typo in one of the descriptions:
- In the "transaction_status" row, "Tranasction" should be "Transaction".
Also, consider adding units to the "last_blob_size" metric for clarity.
Here's the corrected and improved version of the affected row:
| transaction_status | Gauge | | Transaction status of the DA transaction |
| last_blob_size | Gauge | | Last blob size submitted to the DA (in bytes) |
Overview
Fixes #431
Summary by CodeRabbit
New Features
/metricsendpoint.Documentation