-
Notifications
You must be signed in to change notification settings - Fork 0
feat: refactor endpoint errors metric #14
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
Conversation
Summary of ChangesHello @auricom, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the observability of RPC JSON endpoint health by integrating new Prometheus metrics. It allows for real-time monitoring of whether endpoints are reachable and provides detailed counts of various connection error types, which is crucial for diagnosing and responding to service disruptions. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
b4f85cf to
639b6a3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces new Prometheus metrics to track endpoint availability and connection errors. While the PR title mentions 'latency', the changes focus on availability and errors, which is a valuable addition for monitoring. The implementation within pkg/metrics/metrics.go correctly defines and initializes EndpointAvailability and EndpointErrors metrics. However, the new functions for recording these metrics, RecordEndpointAvailability and RecordEndpointError, are not called anywhere within the provided changes. This means the new metrics will be exposed but never updated, rendering the feature incomplete. It's important to ensure these functions are called from the relevant parts of the application to make this feature functional.
| // RecordEndpointAvailability records whether an endpoint is reachable | ||
| // available should be true if endpoint is reachable, false otherwise | ||
| func (m *Metrics) RecordEndpointAvailability(chainID, endpoint string, available bool) { | ||
| value := 0.0 | ||
| if available { | ||
| value = 1.0 | ||
| } | ||
| m.EndpointAvailability.WithLabelValues(chainID, endpoint).Set(value) | ||
| } | ||
|
|
||
| // RecordEndpointError records an endpoint connection error with its type | ||
| func (m *Metrics) RecordEndpointError(chainID, endpoint, errorType string) { | ||
| m.EndpointErrors.WithLabelValues(chainID, endpoint, errorType).Inc() | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new functions RecordEndpointAvailability and RecordEndpointError are defined but are not called anywhere in the pull request's changes. This means the new metrics (endpoint_availability and endpoint_errors_total) will be registered but will never be populated with data. This appears to be an incomplete implementation. Was the code that uses these functions intended to be part of this PR?
Overview