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

Normalize default metrics #190

Merged
merged 26 commits into from
Aug 21, 2022
Merged

Normalize default metrics #190

merged 26 commits into from
Aug 21, 2022

Conversation

phnx47
Copy link
Member

@phnx47 phnx47 commented Feb 20, 2022

Rename metrics:

process_virtual_bytes -> process_virtual_memory_bytes
process_private_bytes -> process_private_memory_bytes
process_working_set -> process_working_set_bytes
dotnet_totalmemory -> dotnet_total_memory_bytes

Add metrics:

process_open_handles

Should be compatible with https://grafana.com/grafana/dashboards/10427

Rename metric from `process_virtual_bytes` to `process_virtual_memory_bytes`
Rename metric from `process_private_bytes` to `process_private_memory_bytes`
Rename metric from `process_working_set` to `process_working_set_bytes`
Add metric `process_open_handles`
Rename metric from `dotnet_totalmemory` to `dotnet_total_memory_bytes`
@phnx47 phnx47 requested a review from a team February 20, 2022 15:13
@phnx47 phnx47 linked an issue Feb 20, 2022 that may be closed by this pull request
@phnx47 phnx47 marked this pull request as draft February 20, 2022 15:25
@codecov
Copy link

codecov bot commented Feb 20, 2022

Codecov Report

Merging #190 (1861869) into master (0212d93) will increase coverage by 0.57%.
The diff coverage is 87.50%.

❗ Current head 1861869 differs from pull request most recent head fce4699. Consider uploading reports for the commit fce4699 to get more accurate results

@@            Coverage Diff             @@
##           master     #190      +/-   ##
==========================================
+ Coverage   87.34%   87.91%   +0.57%     
==========================================
  Files          48       48              
  Lines        1691     1746      +55     
  Branches      228      234       +6     
==========================================
+ Hits         1477     1535      +58     
+ Misses        164      161       -3     
  Partials       50       50              
Impacted Files Coverage Δ
.../Prometheus.Client/Collectors/DefaultCollectors.cs 50.00% <20.00%> (+50.00%) ⬆️
...lectors/DotNetStats/CollectorRegistryExtensions.cs 50.00% <20.00%> (+50.00%) ⬆️
...ectors/ProcessStats/CollectorRegistryExtensions.cs 50.00% <25.00%> (+50.00%) ⬆️
...llectors/DotNetStats/GCCollectionCountCollector.cs 100.00% <100.00%> (+4.54%) ⬆️
...t/Collectors/DotNetStats/GCTotalMemoryCollector.cs 100.00% <100.00%> (+7.69%) ⬆️
...Client/Collectors/ProcessStats/ProcessCollector.cs 100.00% <100.00%> (+2.50%) ⬆️
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@phnx47
Copy link
Member Author

phnx47 commented Feb 21, 2022

@sanych-sun Hi, any ideas on how we can collect these metrics?

process_resident_memory_bytes
process_open_fds
process_max_fds

@phnx47
Copy link
Member Author

phnx47 commented Feb 21, 2022

I'm not sure about process_processid, maybe better rename it to process_id?

@phnx47 phnx47 marked this pull request as ready for review February 22, 2022 03:20
@sanych-sun
Copy link
Member

sanych-sun commented Feb 23, 2022

"fds" - is it count of open file descriptors? If so, not sure if it's possible without pinvoke. We might want to report Process.HandleCount, but this count includes any kind of handle, not just open files.

@phnx47
Copy link
Member Author

phnx47 commented Feb 24, 2022

process_open_fds - Number of open file descriptors
process_max_fds - Maximum number of open file descriptors
process_resident_memory_bytes - Resident memory size in bytes

I found these metrics in Go and Java client

@phnx47
Copy link
Member Author

phnx47 commented Apr 6, 2022

I will come back to this task soon

@phnx47 phnx47 self-assigned this Jul 14, 2022
@phnx47 phnx47 added this to the 5.0 milestone Jul 14, 2022
@phnx47 phnx47 enabled auto-merge August 21, 2022 12:43
@phnx47 phnx47 merged commit c3ddece into master Aug 21, 2022
@phnx47 phnx47 deleted the feat/normalize-metrics branch August 21, 2022 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Normalize default metrics
2 participants