search

appsignal / probes-rs

Rust library to read out system stats from a machine running Unix
MIT License
15 stars 2 forks source link

Use runtime metrics for container CPU metrics #28

Closed tombruijn closed 6 years ago

tombruijn commented 6 years ago

Most "files" in the virtual file system /proc/ return metrics for the host system rather than the container in which it is fetched. To get the values for the host metrics in containers we need to read the /sys/fs/ virtual file system instead.

This commit fixes the CPU metrics reporting for containers that expose the /sys/fs/ virtual file system.

Since only user, system and total are exposed in these runtime metrics a lot of values default to 0. It might be interesting to avoid reporting these as they don't have actual values. The value None, which would mean wrapping it in an Option, would maybe work better here.

Based on the docs as described here: https://docs.docker.com/config/containers/runmetrics/

Tracking issue: https://github.com/appsignal/appsignal-agent/issues/329 Related issue: #25 Based on #27 Extracted from #26 PR is very WIP. Reporting of CPU metrics is still inaccurate.

tombruijn commented 6 years ago

Fairly certain this wasn't supposed to be merged yet. Can we revert?

thijsc commented 6 years ago

It's already been released as 0.2.0, so no. What do you want to change?

tombruijn commented 6 years ago

PR is very WIP. Reporting of CPU metrics is still inaccurate.

The PR wasn't done yet. It was in review while we would check if it was accurate enough. It isn't last I checked, reporting 100000% CPU usage and such. So let's revert and plan work on this later.

tombruijn commented 6 years ago

Oh good, just saw this wasn't actually merged on the branch from PR #27 before that was merged. It's not included in the 0.2.0 release.