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

chore(metrics): renamed hb_request_duration to hb_request_duration_se… #538

Closed

Conversation

thoven87
Copy link

@thoven87 thoven87 commented Sep 5, 2024

Renamed hb_request_duration to hb_request_duration_seconds

I think it would be nice to expose the response status code if possible. This would afford us to compute metrics such as error status per method (GET, PUT, POST, DELETE) e.t.c

Copy link

codecov bot commented Sep 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.21%. Comparing base (e78cde7) to head (dfd6e5d).
Report is 121 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #538      +/-   ##
==========================================
- Coverage   84.86%   83.21%   -1.65%     
==========================================
  Files          98       99       +1     
  Lines        5320     4410     -910     
==========================================
- Hits         4515     3670     -845     
+ Misses        805      740      -65     

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

@thoven87
Copy link
Author

thoven87 commented Sep 5, 2024

hmm, I am not sure why the validity check if failing. It passes on my local machine

➜ hummingbird git:(stevenson/metrics) git push origin stevenson/metrics
Enumerating objects: 35, done.
Counting objects: 100% (35/35), done.
Delta compression using up to 12 threads
Compressing objects: 100% (17/17), done.
Writing objects: 100% (22/22), 3.67 KiB | 3.67 MiB/s, done.
Total 22 (delta 19), reused 7 (delta 5), pack-reused 0
remote: Resolving deltas: 100% (19/19), completed with 13 local objects.
remote:
remote: Create a pull request for 'stevenson/metrics' on GitHub by visiting:
remote:      https://github.com/thoven87/hummingbird/pull/new/stevenson/metrics
remote:
To github.com:thoven87/hummingbird.git
 * [new branch]      stevenson/metrics -> stevenson/metrics
➜ hummingbird git:(stevenson/metrics) ./scripts/validate.sh
=> Checking format... (using v0.54.3) okay.
➜ hummingbird git:(stevenson/metrics) git status
On branch stevenson/metrics
nothing to commit, working tree clean
➜ hummingbird git:(stevenson/metrics) git diff
➜ hummingbird git:(stevenson/metrics)
➜ hummingbird git:(stevenson/metrics) ./scripts/validate.sh
=> Checking format... (using v0.54.3) okay.
➜ hummingbird git:(stevenson/metrics) git status
On branch stevenson/metrics
nothing to commit, working tree clean
➜ hummingbird git:(stevenson/metrics)

Copy link
Member

@Joannis Joannis left a comment

Choose a reason for hiding this comment

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

This is a breaking change, let's carefully review all our label names if we need to ever change those.

@adam-fowler
Copy link
Member

Instead of doing this piecemeal we should do a review of all the metrics and logging metadata together. I intend to add an issue with all of these collated and we can then look to improve and ensure we have consistency across all libraries. We already have some inconsistency in usage of snake vs camel case.

@thoven87
Copy link
Author

thoven87 commented Sep 5, 2024

Sounds good, I will close this PR and follow up on the issue @adam-fowler plans to create.

@thoven87 thoven87 closed this Sep 5, 2024
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.

None yet

3 participants