Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ABCI method timing histogram metric #1366

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

aWN4Y25pa2EK
Copy link

This PR introduces a new histogram metric, abci_connection_method_timing_seconds, to measure the timings for each of the ABCI methods. This metric will help in tracking and analysing the performance of various ABCI methods by providing detailed timing information. The metric includes labels for method and type to allow for granular analysis of the data.

Changes Introduced

  • New Metric: The abci_connection_method_timing_seconds histogram metric is added to capture the duration of ABCI method calls.
  • Metrics Struct Update: The Metrics struct is updated to include the new histogram metric.
  • Prometheus Metrics Initialization: The PrometheusMetrics function is updated to initialize the new histogram metric with appropriate labels.
  • No-Op Metrics Update: The NopMetrics function is updated to include a no-op version of the new histogram metric.

Files to Review

metrics.go: This file contains the primary changes for adding the new metric. It includes updates to the Metrics struct, the PrometheusMetrics function, and the NopMetrics function.

Importance

Tracking the performance of ABCI methods is crucial for understanding and optimizing the behavior of the system. By adding this metric, we provide operators and developers with a tool to gain insights into the timing of different ABCI method executions, which can help in identifying bottlenecks and improving the overall efficiency of the system.

@aWN4Y25pa2EK aWN4Y25pa2EK requested a review from a team as a code owner May 27, 2024 10:46
@aWN4Y25pa2EK aWN4Y25pa2EK requested review from cmwaters and rach-id and removed request for a team May 27, 2024 10:46
Copy link
Member

@rach-id rach-id left a comment

Choose a reason for hiding this comment

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

I see the ABCIMethodTimingSeconds added, but I don't see it being used anywhere. How does this new metric capture the values?

@aWN4Y25pa2EK aWN4Y25pa2EK marked this pull request as draft May 27, 2024 17:21
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