-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Improve collection of Docker memory metrics in Windows #11971
Comments
pinging @narph, I think you wanted to this module a pass on Windows |
Looks like this has been affected by #11676 - Windows memory stats don't include limit, so that change will prevent memory stat collection altogether on Windows. |
good catch! thank you for sharing. @fearful-symmetry we may need to rethink your fix? |
@exekias Yah. I tried to find a more elegant way of fixing the |
Still chewing over how to do this elegantly. After poking around the docker API for a bit, I figure there's two other checks we can get from the
Does any of this sound reasonable @chrisvanderpennen ? |
In case it's useful, here's a gist with json from the stats API on Windows for a started and stopped container. |
@chrisvanderpennen Thanks a ton! I'm gonna put in a PR for this, so we can at least get this part of the issue squared away. |
That PR has been merged, so we should be good to move on to adding the actual windows memory stats. |
That's been merged! I'm gonna close this for now, since it seems like we've squared away the problems in the original issue. |
Describe the enhancement:
module/docker/memory/helper.go
to report Windows container memory stats, if present.myRawStat.Stats.MemoryStats.Limit
is non-zero and only then assign toTotalRssP
andUsageP
, instead of dividing by zero and assigning NaN. This should avoid breaking JSON outputs.Describe a specific use case for the enhancement or feature:
Monitoring Windows container memory usage
The text was updated successfully, but these errors were encountered: