Skip to content

Do not fail the process collector when the limits file has no Max open file line - #1231

Open
PerryLink wants to merge 1 commit into
prometheus:masterfrom
PerryLink:fix/process-collector-max-fds-unbound
Open

PerryLink wants to merge 1 commit into
prometheus:masterfrom
PerryLink:fix/process-collector-max-fds-unbound

Conversation

@PerryLink

Copy link
Copy Markdown

What

ProcessCollector.collect() reads /proc/<pid>/limits and only assigns max_fds when it finds the
Max open file line, but then uses it unconditionally:

for line in limits:
    if line.startswith(b'Max open file'):
        max_fds = GaugeMetricFamily(...)
        break
open_fds = GaugeMetricFamily(...)
result.extend([open_fds, max_fds])     # UnboundLocalError if the line was absent

If the file opens but the line is not there — seen in the wild with a corrupted Max open fides
header (home-assistant/core#140045, traceback at process_collector.py:93) — the collector raises
UnboundLocalError, and because PROCESS_COLLECTOR is registered in the default REGISTRY with
auto_describe=True, /metrics itself fails.

The change

Bind max_fds to None before the loop, append it only when it was found, and keep the existing
OSError guard as-is. No new validation: the metric is simply omitted when the kernel does not
report it, which is what happens on every platform without /proc already.

Adds a regression test using the repository's existing fixture convention, with a limits file that
has no Max open file line.

Verification

  • before: python -m unittest tests.test_process_collector → UnboundLocalError: cannot access local variable 'max_fds' at process_collector.py:93 (reproduced on Windows/Python 3.12.14 and on
    WSL2/Linux/Python 3.12.3);
  • after: 5 tests OK on both;
  • full suite: 428 passed before, 429 passed after; flake8 and isort clean; mypy reports only the
    pre-existing Windows-only warnings that the unmodified file reports too.

The adjacent float('unlimited') issue (#80/#383) is deliberately not bundled here.

DCO: commit is signed off.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant