Add per-unit resource usage to pod status replies #58
Conversation
There was a problem hiding this comment.
I'm mostly concerned with full-on buying of float64s as our storage mechanism since we're only storing ints. That said, I don't know exactly what the best thing to do is. We could either store ints and hope that we never need to add floats or we could store floats and have to deal with those. From googling around a bit, it doesn't seem like we'll loose too much from storing floats until we get up to 2**53 of anything...
| } | ||
| defer control.Delete() | ||
| pid := os.Getpid() | ||
| err = control.Add(cgroups.Process{Pid: pid}) |
There was a problem hiding this comment.
So it's the default behavior that child processes get put into the parent's the cgroup?
pkg/metrics/metrics.go
Outdated
| return metrics | ||
| } | ||
| if cm.CPU != nil && cm.CPU.Usage != nil { | ||
| metrics["cpuUsage"] = float64(cm.CPU.Usage.Total) |
There was a problem hiding this comment.
Should these values continue to be float64s? It seems that all our metrics are ints.
There was a problem hiding this comment.
I think we'll need to circle back to metrics as we try more use cases (k8s pod autoscaler, etc). I'd prefer not to do much more until we have a better idea how metrics will be used.
| @@ -108,6 +108,15 @@ func (s *Server) statusHandler(w http.ResponseWriter, r *http.Request) { | |||
| var resourceUsage api.ResourceMetrics | |||
| if time.Since(s.lastMetricTime) > minMetricPeriod { | |||
| resourceUsage = s.metrics.GetSystemMetrics() | |||
| for _, us := range append(status, initStatus...) { | |||
| unitResourceUsage := s.metrics.GetUnitMetrics(us.Name) | |||
| for k, v := range unitResourceUsage { | |||
There was a problem hiding this comment.
instead of updating the map, would it make sense to name these correctly (with the unit name) in metrics.go?
ldx
force-pushed
the
vilmos-unit-resourceusage
branch
from
12fb454
to
f0b2962
Compare
No description provided.