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

update grafana dashboard and exported metrics doc for watchable_panics_recovered_total #5036

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

DeeBi9
Copy link

@DeeBi9 DeeBi9 commented Jan 11, 2025

What type of PR is this?

Update Grafana dashboard and exported metrics doc for watchable_panics_recovered_total

What this PR does / why we need it:

This PR updates the Grafana Dashboard to include the newly introduced metric watchable_panics_recovered_total, which tracks the total number of panics recovered in the Envoy Gateway system. It also updates the Exported Metrics Documentation to reflect the new metric and provide users with guidance on its use.

Which issue(s) this PR fixes:

#4728

@DeeBi9 DeeBi9 requested a review from a team as a code owner January 11, 2025 19:34
Copy link

codecov bot commented Jan 12, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.81%. Comparing base (00ed0b8) to head (cdf548d).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5036      +/-   ##
==========================================
+ Coverage   66.75%   66.81%   +0.05%     
==========================================
  Files         209      209              
  Lines       32409    32409              
==========================================
+ Hits        21636    21655      +19     
+ Misses       9468     9455      -13     
+ Partials     1305     1299       -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@arkodg arkodg requested a review from shawnh2 January 13, 2025 19:40
Copy link
Contributor

@shawnh2 shawnh2 left a comment

Choose a reason for hiding this comment

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

Code overall looks good!

Can you run make -k gen-check to make the gencheck pass? Also can you add a screenshot image of this metric in grafana in the PR description ?

@DeeBi9
Copy link
Author

DeeBi9 commented Jan 14, 2025

This is the screenshot for the metric watchable_panics_recovered_total in the grafana dashboard , but the metrics appear on the dashboard does not display any data.

Screenshot from 2025-01-14 16-41-40

I have verified that the other metrics (like watchable_depth) are showing data, so the scraping is working.

Screenshot from 2025-01-14 16-44-45

I have checked the prometheus directly for the metrics watchable_panics_recovered_total but it doesn't appear there either.
I have followed the official documentation for deploying the Envoy Gateway and add-ons for observability. Is there a specific configuration or condition required for this metric to work ?

Thanks

@DeeBi9
Copy link
Author

DeeBi9 commented Jan 14, 2025

I also ran the make -k gen-check command locally as requested. Initially, I encountered an error similar to the one the codecov bot reported. This led to the addition of some test files, these are :
Screenshot from 2025-01-14 09-46-06

which I committed after seeing that two directories were modified. Upon committing the changes, I re-ran the command make -k gen-check, and it successfully completed without issues. However, I noticed that the section describing the watchable_panics_recovered_total metric in the docs was removed during this process, but the test ran smoothly afterward. Could you please clarify why this section was removed during the test execution? Let me know if there’s something specific I missed or if this is expected behavior.

Thanks

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