Improve Akka.Cluster.Metrics collected values#6203
Conversation
Aaronontheweb
left a comment
Aaronontheweb
left a comment
There was a problem hiding this comment.
LGTM but needs API approvals
| // VirtualMemorySize64 is not best idea here... | ||
| NodeMetrics.Types.Metric.Create(StandardMetrics.MemoryAvailable, process.VirtualMemorySize64).Value, | ||
| NodeMetrics.Types.Metric.Create(StandardMetrics.MemoryUsed, GC.GetTotalMemory(false)).Value, | ||
| NodeMetrics.Types.Metric.Create(StandardMetrics.MemoryAvailable, process.WorkingSet64).Value, |
|
We will also need to backport this to v1.5 @Arkatufus |
|
@Arkatufus looks like some of the MNTR specs for Akka.Cluster.Metrics are not passing. Could you investigate? |
|
Need a re-review. |
|
@Arkatufus DocFx warning check here |
| NodeMetrics.Types.Metric.Create(StandardMetrics.MemoryUsed, GC.GetTotalMemory(true)).Value, | ||
| // VirtualMemorySize64 is not best idea here... | ||
| NodeMetrics.Types.Metric.Create(StandardMetrics.MemoryUsed, process.WorkingSet64).Value, | ||
| NodeMetrics.Types.Metric.Create(StandardMetrics.MemoryAvailable, process.VirtualMemorySize64).Value, |
There was a problem hiding this comment.
this should be working set
| { | ||
| // Memory | ||
| // Forcing garbage collection to keep metrics more resilent to occasional allocations | ||
| NodeMetrics.Types.Metric.Create(StandardMetrics.MemoryUsed, GC.GetTotalMemory(true)).Value, |
| @@ -55,12 +55,11 @@ public NodeMetrics Sample() | |||
| { | |||
| using (var process = Process.GetCurrentProcess()) | |||
There was a problem hiding this comment.
I wonder if this is wrong - shouldn't we just cache this value somewhere?
|
The right algorithm, which we can't implement on .NET Standard 2.0 without a lot of hacks: used memory = gc.totalsize OR process.workingset64 We should be measuring how close we're getting to hitting the total amount of available memory on the system as a percentage. In the meantime, we can do the following on .NET Standard 2.0: used memory = gc.totalsize This will tell us how much memory is being consumed by the .NET application itself - it won't be able to factor in issues like system memory and some unmanaged memory without the user manually calling |
|
Testing out the latter now - we'll go back to the drawing board if we keep running into problems getting the MNTR to pass. |
|
@Arkatufus needs a port to v1.5 |
* Improve collected values * Update API Verify list * Fix metrics collection * Fix documentation * Update API Verify list * Update DefaultCollector.cs * Update DefaultCollector.cs Co-authored-by: Aaron Stannard <aaron@petabridge.com> (cherry picked from commit dca908b)
* Improve collected values * Update API Verify list * Fix metrics collection * Fix documentation * Update API Verify list * Update DefaultCollector.cs * Update DefaultCollector.cs Co-authored-by: Aaron Stannard <aaron@petabridge.com> (cherry picked from commit dca908b)
2 participants
close #4142
This improves the values collected by the metrics plugin, it does not fix the heap selector since it is still missing informations.
Heap metric based routing can only be implemented after we implement the new
EventCountersAPI.