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

feat(prometheus): add bytes metrics as counter #3246

Merged
merged 2 commits into from
Oct 9, 2023

Conversation

flaneur2020
Copy link
Contributor

@flaneur2020 flaneur2020 commented Oct 9, 2023

in the previous PR I converted the metrics in PrometheusLayer as it into PrometheusClientLayer, but I missed an important metric not included in the PrometheusLayer before is the bytes total metrics.

we already have a histogram about the bytes which is useful for us to find out the distribution of the size on different IO requests. however, a bytes metric in counter is more useful for us to get the IO bandwidth infomation, which is important for us to diagnose the issues on IO.

this PR also fixed a naming issue about counters in PrometheusClientLayer, the prometheus-client tend to append a _total suffix in a dump way for every counters, if we name a counter as opendal_errors_total what we finally get is opendal_errors_total_total, which is bad :(

@flaneur2020 flaneur2020 force-pushed the add-metrics-about-bytes-counter branch from 66bf78e to 4ef8b0b Compare October 9, 2023 11:11
@Xuanwo Xuanwo changed the title fead(prometheus): add bytes metrics as counter feat(prometheus): add bytes metrics as counter Oct 9, 2023
Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

Thanks!

@Xuanwo Xuanwo merged commit 560f8a0 into apache:main Oct 9, 2023
104 checks passed
Zheaoli pushed a commit to Zheaoli/opendal that referenced this pull request Oct 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants