build: Convert to module by Mantisus · Pull Request #9 · apify/cgroups-sensor

Mantisus · 2026-08-20T13:47:17Z

Closes: #1
Closes: #2

Pijukatel

This is really great. It is a big change, and I want to review it carefully, so I will do it in steps over several days. Some comments are more like open questions, not necessarily a change request.

This is the first-step review:
Public documentation + test result glance
(without looking at internal implementation)

Follow-up reviews will be:

Interface and usage
Implementation internals
Test internals
CI

Pijukatel · 2026-08-21T05:47:15Z

+    return controllers.exists() and bool(controllers.read_text().strip())
+
+
+def machine_memory_bytes() -> int:


For this and some other reference values, why not use psutils where we know it correctly estimates?

Ideally, I'd like to keep this package zero-dependency. That guarantees it runs anywhere Python does. psutil ships a C extension, so it needs a prebuilt wheel matching the platform, or a compiler on the machine to build one.

Pijukatel · 2026-08-21T05:53:46Z

+"""
+
+
+def unavailable(capability: str, reason: str) -> None:


Have you considered using explicit Pytest markers for tests?
https://docs.pytest.org/en/stable/example/markers.html#marking-test-functions-and-selecting-them-for-a-run

The reader of the test suite can be surprised by entries like this and wonder whether it is missing some coverage due to runners not being set up correctly or if this is the desired behavior:

SKIPPED [1] tests/e2e/test_docker.py:174: docker uses the 'cgroupfs' cgroup driver; this needs systemd SKIPPED [1] tests/e2e/test_kubernetes.py:182: kind, kubectl and docker are needed to run a cluster SKIPPED [1] tests/e2e/test_kubernetes.py:202: kind, kubectl and docker are needed to run a cluster SKIPPED [1] tests/e2e/test_machine.py:50: the controllers are not on the unified hierarchy

If it is known in advance which environment has which capability, then we should be able to call just the right subset of the tests through markers.

Pijukatel · 2026-08-21T06:00:17Z

-python3 report.py results/ --check   # same, exit 1 if a probe failed
+This package reads the files, walks the levels, and reports only what restricts the process. `None` means nothing restricts it - the machine is then the honest answer, and `get_machine_cpu_count()` and `get_machine_memory_bytes()` below give it.
+
+Linux only, Python 3.10 or newer, and no dependencies. Off Linux every limit reads as `None`. Some examples below pair it with `psutil`, which this package does not require - it is there to answer what the machine is using, which is not a question about limits.


Off Linux every limit reads as None

Should we rather raise when not running in the expected environment for now?

Could you also elaborate on the environment, like a Linux container that is running through different mechanisms on MacOS or Windows?

hould we rather raise when not running in the expected environment for now?

None says there are no limits, or that they could not be determined.On an unsupported environment that is the correct answer.

We can add an is_supported() function as a simple way to check whether the current environment supports cgroups.

Could you also elaborate on the environment, like a Linux container that is running through different mechanisms on MacOS or Windows?

This is about macOS and Windows themselves. A Linux container should work correctly wherever it runs. With one correction: the machine-wide values come from the virtual machine, not from the physical host.

Pijukatel · 2026-08-21T06:10:43Z

+    """The total and the used memory a budget should be derived from, in bytes."""
+    budget = cgroups_sensor.get_memory_budget()
+    if budget is not None:
+        return budget.limit, budget.working_set


What is the reasoning behind memory.total, memory.total - memory.available not being the default(when there are no tighther restrictions) of budget.limit, budget.working_set? Is it to avoid dependencies?

or if we keep it this way, could we turn def memory_budget() into a utility function closed behind psutils as an optional dependency?

I can imagine this or a similar utility function would be repeated by a huge number of users.

(Ok, here I am going wild and just throwing ideas. We could also keep this package completely clear of dependencies and later add a second package like "psutils_cgropups_aware" which would be thin wrapper around the original psutils, delegating almost everything back to psutils and patching just some return values using new functionality you have introduced here. The motivation would be to have a "drop-in replacement" like import psutils_cgropups_aware as psutils)

Pijukatel · 2026-08-21T06:23:28Z

+    time.sleep(5)
+```
+
+The loop has to pace itself, as the `sleep` above does: every call here returns at once where there is nothing to measure, and `sample()` never waits at all. For a single measurement there is `get_cpu_used_ratio(interval)`, which waits out the window itself, and `get_cpu_used_ratio_async(interval)` for asyncio. Keep the interval generous - the kernel updates the counter in coarse steps, so a tenth of a second can report a busy process as idle, and anything below 0.01 seconds is refused outright.


From just this description, I could not understand whether the load.sample() is measuring against its previous call or what it is actually measuring. I need to go inside the function to understand it. Could you please try to explain it more in this readme so that the readers do not need to check the internals for top level description. (Maybe it is just me being slow though :D )

Pijukatel · 2026-08-21T07:15:32Z

+
+## Diagnostics
+
+`describe()` explains a reading that looks wrong. It carries the readings themselves, so one dump answers what was reported as well as why, and around them a `Source` per metric - the mechanism it was read through and the levels searched - the raw values before filtering, the machine it compared against, the levels the memory and the CPU limit actually came from, and a notice for every reading that is not there. A reading of `None` with no notice about it means the mechanism was there and nothing limited this process in a way that kills it - only hard limits are read, and `memory.high` throttles reclaim instead. The levels searched are not the levels a reading came from: a level carries no files until a limit is written there, and it is kept in the chain regardless.


The levels searched are not the levels a reading came from This sounds strange in isolation, could you please further explain or rephrase it.

My dumb interpretation without knowing the internals yet. "We searched some levels, but they are unrelated to the reading :-)"

Pijukatel · 2026-08-21T07:18:45Z

-page cache — what `docker stats` shows. The second table is the same run seen through `get_memory_info()` and
-`get_cpu_info()`, where a limit either reaches the caller or falls back to host values. When a reading looks
-wrong, `evidence` says who is at fault: the limit is in the control files, or it never got there.
+| `MEMORY_LIMIT_COVERS_MACHINE` | the limit is at least the memory of the machine |


"at least the memory of the machine" , does it mean that memory limit can be higher than that?

Maybe something like "the limit is limited(or saturated) by the memory of the machine" ?

Pijukatel · 2026-08-21T07:20:53Z

+
+`Source.interface` is an `Interface` member, and a notice carries a `NoticeCode`. Branch on those rather than on the strings they print as:
+
+| `NoticeCode` | Meaning |


It would be great if each notice carried at least one real-world example of when such a notice can arrive

Pijukatel · 2026-08-21T07:22:02Z

+| `CPU_USAGE_SCOPE_MISMATCH` | the level the CPU limit applies to counts no CPU time, so no rate can be measured there |
+| `MEMORY_METRICS_UNAVAILABLE` | nothing here carries a memory limit at all, which is what a machine without cgroups looks like |
+| `CPU_METRICS_UNAVAILABLE` | nothing here carries a CPU limit at all, for the same reasons |
+| `MEMORY_LIMIT_UNREADABLE` | a level holds a memory limit that says nothing usable, so what it enforces is unknown |


Haha, I am so curious about an example of when this happens. I want to know what the unknown enforcement is :-)

printf 'not a number\n' > /tmp/fake-memory-max && docker run --rm --memory 512m \ -v "$PWD/src:/sensor:ro" -e PYTHONPATH=/sensor \ -v /tmp/fake-memory-max:/sys/fs/cgroup/memory.max:ro \ python:3.13-alpine python -c ' import dataclasses, json, cgroups_sensor print(json.dumps(dataclasses.asdict(cgroups_sensor.describe()), indent=2, default=str))'

Pijukatel · 2026-08-21T07:24:05Z

+build-backend = "uv_build"
+
+[project]
+name = "cgroups-sensor"


Let's keep it as a working placeholder name for now, but have an internal debate with the rest of the team before publication
(I am also bad with names)

Unless we find something better, my suggestion is synomys or prairidog (уeah, that's the best I could come up with).

clean analysis data

a53354a

Mantisus self-assigned this Aug 20, 2026

Mantisus requested a review from Pijukatel August 20, 2026 14:08

convert to package

076040e

Mantisus force-pushed the convert-to-module branch from faec224 to 076040e Compare August 20, 2026 21:12

Pijukatel reviewed Aug 21, 2026

View reviewed changes

		return controllers.exists() and bool(controllers.read_text().strip())


		def machine_memory_bytes() -> int:


		## Diagnostics

		`describe()` explains a reading that looks wrong. It carries the readings themselves, so one dump answers what was reported as well as why, and around them a `Source` per metric - the mechanism it was read through and the levels searched - the raw values before filtering, the machine it compared against, the levels the memory and the CPU limit actually came from, and a notice for every reading that is not there. A reading of `None` with no notice about it means the mechanism was there and nothing limited this process in a way that kills it - only hard limits are read, and `memory.high` throttles reclaim instead. The levels searched are not the levels a reading came from: a level carries no files until a limit is written there, and it is kept in the chain regardless.


		`Source.interface` is an `Interface` member, and a notice carries a `NoticeCode`. Branch on those rather than on the strings they print as:

		\| `NoticeCode` \| Meaning \|

Uh oh!

Conversation

Mantisus commented Aug 20, 2026

Uh oh!

Pijukatel left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants