fix(nautobot): enable prometheus multiprocess mode in the image - #181
fix(nautobot): enable prometheus multiprocess mode in the image#181dmathur0 wants to merge 3 commits into
Conversation
📝 WalkthroughWalkthroughNautobot’s image and Helm workloads now provide shared, size-limited Prometheus multiprocess storage. Both Prometheus environment-variable forms point to ChangesPrometheus multiprocess metrics
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Helm
participant NautobotPod
participant Prometheus
participant CeleryProbe
Helm->>NautobotPod: Mount 256Mi /prom_cache emptyDir
NautobotPod->>NautobotPod: Aggregate worker metrics from /prom_cache
Prometheus->>NautobotPod: Scrape /metrics/
NautobotPod->>Prometheus: Return aggregated metrics
CeleryProbe->>NautobotPod: Unset multiprocess variables and run celery inspect ping
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@deploy/helm/templates/_nautobot-configmap-data.tpl`:
- Around line 197-199: The _install_metric_reaper hook must also bound the
multiprocess metrics directory during worker shutdown, not only mark the current
process dead. Update the cleanup behavior in
deploy/helm/templates/_nautobot-configmap-data.tpl:197-199 to remove stale
counter, summary, and histogram files while preserving live-gauge reaping;
deploy/helm/values.yaml:2771-2773 and
deploy/helm/templates/nautobot.yaml:792-798 require no direct changes and are
affected configuration/context sites.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7cc822f9-fd8f-489d-b1c3-6a4196c70c2a
📒 Files selected for processing (3)
deploy/helm/templates/_nautobot-configmap-data.tpldeploy/helm/templates/nautobot.yamldeploy/helm/values.yaml
| @uwsgidecorators.postfork | ||
| def _install_metric_reaper() -> None: | ||
| atexit.register(lambda: multiprocess.mark_process_dead(os.getpid())) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
helm template multiproc-on deploy/helm \
-f deploy/helm/values-ci.yaml \
--set nautobot.enabled=true \
--set nautobot.metrics.multiproc.enabled=true >/tmp/multiproc-on.yaml
helm template multiproc-off deploy/helm \
-f deploy/helm/values-ci.yaml \
--set nautobot.enabled=true \
--set nautobot.metrics.multiproc.enabled=false >/tmp/multiproc-off.yaml
rg -n -C2 'metrics_bootstrap|PROMETHEUS_MULTIPROC_DIR|prometheus-multiproc' \
/tmp/multiproc-on.yaml /tmp/multiproc-off.yamlRepository: NVIDIA/nv-config-manager
Length of output: 204
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## _nautobot-configmap-data.tpl\n'
sed -n '180,220p' deploy/helm/templates/_nautobot-configmap-data.tpl | cat -n
printf '\n## values.yaml\n'
sed -n '2758,2785p' deploy/helm/values.yaml | cat -n
printf '\n## nautobot.yaml\n'
sed -n '780,810p' deploy/helm/templates/nautobot.yaml | cat -n
printf '\n## search for multiprocess cleanup/config\n'
rg -n 'mark_process_dead|PROMETHEUS_MULTIPROC_DIR|multiproc|startup-clear|worker reaping' deploy/helm -SRepository: NVIDIA/nv-config-manager
Length of output: 8338
🌐 Web query:
prometheus_client multiprocess.mark_process_dead removes only gauge_live files counters histograms cleanup semantics
💡 Result:
The function prometheus_client.multiprocess.mark_process_dead specifically targets and removes only the memory-mapped database files associated with Gauges configured with "live" multiprocess modes (e.g., livesum, liveall, livemin, livemax, livemostrecent) for a given process ID [1][2][3]. It does not remove files for other metric types such as Counters, Summaries, or Histograms, nor does it remove files for non-live Gauges [1][4][5]. The rationale for this behavior is that "live" Gauges represent transient state—such as active connections or current queue depth—that becomes stale once the process that generated those metrics exits [1][4][6]. Removing these files prevents stale values from contributing to aggregated metrics [1]. Conversely, other metric types (like Counters or Histograms) track cumulative data, where the final value recorded by a process remains meaningful even after the process has terminated [1][4]. These non-live metric files are intended to persist until the entire multiprocess directory is cleared, which is typically handled by the user or process manager upon application restart or deployment [1][4][6].
Citations:
- 1: https://prometheus.github.io/client_python/multiprocess/
- 2: https://github.com/prometheus/client_python/blob/master/prometheus_client/multiprocess.py
- 3: https://github.com/prometheus/client_python/blob/18017c62a282bf3fb275aaee6564fb2248c85a8d/prometheus_client/multiprocess.py
- 4: https://github.com/prometheus/client_python/blob/master/docs/content/multiprocess/_index.md
- 5: Improve multiprocessing documentation prometheus/client_python#524
- 6: Tmp mm files haven't been clear while process exit prometheus/client_python#236
🌐 Web query:
prometheus_client multiprocess.mark_process_dead only removes gauge_live files documentation source
💡 Result:
The prometheus_client library function mark_process_dead is designed specifically to manage multiprocess gauge files and does not clean up metrics for other types [1][2]. As documented in the official Prometheus client_python documentation, mark_process_dead only removes files corresponding to live gauge modes (such as livesum or liveall) for a given process ID [1][3]. These files are created to ensure that metrics reflecting the current state of living processes do not accumulate stale data after a process exits [1][4]. Files for other metric types (such as Counter, Summary, or Histogram) or non-live Gauge modes are intentionally left in place by mark_process_dead [1][5]. These non-live metrics are designed to persist their last values until the directory is cleared, typically upon application or worker restart [1][5]. The accumulation of these other files is expected behavior in the current design of the multiprocess collector, as they track cumulative data that remains relevant even if a specific process has terminated [1][6].
Citations:
- 1: https://prometheus.github.io/client_python/multiprocess/
- 2: Improve multiprocessing documentation prometheus/client_python#524
- 3: https://github.com/prometheus/client_python/blob/master/prometheus_client/multiprocess.py
- 4: https://github.com/prometheus/client_python/blob/master/docs/content/multiprocess/_index.md
- 5: https://github.com/prometheus/client_python/blob/v0.23.1/prometheus_client/multiprocess.py
- 6: Tmp mm files haven't been clear while process exit prometheus/client_python#236
🌐 Web query:
site:github.com prometheus_client multiprocess.py mark_process_dead gauge_live remove files
💡 Result:
In the prometheus_client Python library, the function mark_process_dead(pid, path=None) is specifically designed to perform cleanup for multiprocess applications [1][2]. It operates by removing only the temporary.db files associated with "live" Gauge metrics for a given process ID (PID) [1][2]. These "live" gauge modes (e.g., liveall, livesum, livemin, livemax, livemostrecent) are designed to reflect the state of currently living processes only; therefore, removing their files upon process termination prevents stale data from being included in aggregate calculations [2][3][4]. Key technical details regarding this mechanism include: 1. Scope of Cleanup: The function does not remove files for other metric types, such as Counters, Summaries, or Histograms [5][3]. Deleting those files would lead to the loss of accumulated data, causing metrics to incorrectly reset or go backwards [5]. 2. Usage: It is standard practice to call this function from a process manager's child exit hook (e.g., in Gunicorn) to manage the lifecycle of "live" gauge metrics [2][3]. 3. Persistent Files: Files for non-live metrics remain on disk even after a process dies [2][4]. The library expects the multiprocess directory to be wiped entirely when the application (or server) is restarted to ensure a clean state [2][6][4]. 4. Path Handling: The function looks for the directory defined by the PROMETHEUS_MULTIPROC_DIR environment variable (or the older prometheus_multiproc_dir) [1][7][8]. If the path is not provided as an argument, it automatically detects this variable to locate the files [2][9]. Because mark_process_dead does not perform a full cleanup of all metric types, environments with high process churn (such as frequently rotating workers) may see a accumulation of files over time, which is a known limitation that typically requires a manual cleanup strategy during service restarts or deployments [3][6][4].
Citations:
- 1: https://github.com/prometheus/client_python/blob/master/prometheus_client/multiprocess.py
- 2: https://github.com/prometheus/client_python/blob/master/docs/content/multiprocess/_index.md
- 3: High cpu when using Gunicorn with multiprocess prometheus/client_python#568
- 4: Tmp mm files haven't been clear while process exit prometheus/client_python#236
- 5: Multiprocess: Only gauge files deleted on child exit? prometheus/client_python#566
- 6: Gunicorn with multiprocess tmp folder cleanup prometheus/client_python#1100
- 7: Issue with prometheus_multiproc_dir deprecation prometheus/client_python#643
- 8: https://github.com/prometheus/client_python/blob/18017c62a282bf3fb275aaee6564fb2248c85a8d/prometheus_client/multiprocess.py
- 9: https://github.com/prometheus/client_python/blob/v0.23.1/prometheus_client/multiprocess.py
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## multiproc bootstrap context\n'
sed -n '120,170p' deploy/helm/templates/_nautobot-configmap-data.tpl | cat -n
printf '\n## Prometheus metric definitions in repo\n'
rg -n 'from prometheus_client import|prometheus_client\.(Counter|Histogram|Summary|Gauge)|Gauge\(' . -SRepository: NVIDIA/nv-config-manager
Length of output: 3909
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '170,220p' deploy/helm/templates/_nautobot-configmap-data.tpl | cat -nRepository: NVIDIA/nv-config-manager
Length of output: 2267
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '170,210p' deploy/helm/templates/_nautobot-configmap-data.tpl | cat -nRepository: NVIDIA/nv-config-manager
Length of output: 1663
mark_process_dead() doesn’t bound the multiprocess directory. Startup clears old *.db files, but this hook only reaps live-gauge files; counter/summary/histogram files can still accumulate across worker churn until the next pod restart. The 64Mi emptyDir cap is still the only backstop during a long-lived pod.
📍 Affects 3 files
deploy/helm/templates/_nautobot-configmap-data.tpl#L197-L199(this comment)deploy/helm/values.yaml#L2771-L2773deploy/helm/templates/nautobot.yaml#L792-L798
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deploy/helm/templates/_nautobot-configmap-data.tpl` around lines 197 - 199,
The _install_metric_reaper hook must also bound the multiprocess metrics
directory during worker shutdown, not only mark the current process dead. Update
the cleanup behavior in
deploy/helm/templates/_nautobot-configmap-data.tpl:197-199 to remove stale
counter, summary, and histogram files while preserving live-gauge reaping;
deploy/helm/values.yaml:2771-2773 and
deploy/helm/templates/nautobot.yaml:792-798 require no direct changes and are
affected configuration/context sites.
Source: Coding guidelines
00091b7 to
32a039a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@build/nautobot.Dockerfile`:
- Around line 84-92: Update the Prometheus environment-variable comment in the
Dockerfile to state that these variables only enable multiprocess mode and
configure the shared metrics directory; clarify that the /metrics handler must
still create an isolated CollectorRegistry and register MultiProcessCollector.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 1d4316bd-b2ba-4845-bc26-2b9ae80f60f6
📒 Files selected for processing (1)
build/nautobot.Dockerfile
dmathur0
changed the title
The Nautobot web server runs multiple uWSGI workers. Without PROMETHEUS_MULTIPROC_DIR set, prometheus_client uses the global in-process registry (auto_describe=True), where the first /metrics scrape self-deadlocks: the collector registers while holding Prometheus's non-reentrant registry lock, then touches Django's cache, which lazily imports django_prometheus.cache.metrics and re-acquires the same lock. (PR #174 mitigated this via a uwsgi preload.) Match the stock Nautobot image (which bakes prometheus_multiproc_dir) instead of adding Helm knobs: set prometheus_multiproc_dir=/prom_cache (+ upper-case alias) in the image and create the dir (writable by uid 1000, via the builder + COPY, since the distroless runtime has no shell). Every Nautobot worker then uses a fresh CollectorRegistry(auto_describe=False): the /metrics self-deadlock becomes structurally impossible and per-worker metrics aggregate correctly. Signed-off-by: Davesh Mathur <daveshm@nvidia.com>
32a039a to
19059c0
Compare
With prometheus_multiproc_dir baked into the image, Nautobot's /metrics handler builds a per-request CollectorRegistry() instead of using the global REGISTRY. CollectorRegistry defaults to auto_describe=False, and register() only substitutes collector.collect() for a missing describe() when auto_describe is set, so collect() is never invoked while the registry lock is held. The lazy django_prometheus.cache.metrics import therefore can no longer re-enter that lock, which makes the uwsgi.ini preload added in #174 dead weight. Verified against nautobot 2.4.36 (core/views/__init__.py:489-501) and prometheus_client registry.py (__init__ default, _get_names line 66). Signed-off-by: Davesh Mathur <daveshm@nvidia.com>
| # global REGISTRY, and metrics aggregate across all uWSGI workers. Nautobot accepts | ||
| # either spelling; the stock Nautobot image bakes the lower-case one, so both are set. | ||
| ENV prometheus_multiproc_dir=/prom_cache | ||
| ENV PROMETHEUS_MULTIPROC_DIR=/prom_cache |
There was a problem hiding this comment.
this will enable MP mode for every process using this image (including liveness probes that run nautobot-server every few minutes) - is that intended? if so we might need to check how many files this will generate and if it could exchaust container resources
Enabling multiprocess mode makes prometheus_client write one set of *.db files per PID under /prom_cache, and it only reaps live-gauge files -- so the directory grows for as long as a pod lives and every /metrics scrape has to read all of it. Cerebro, which has run in multiprocess mode all along, reached 49,636 files (778 MB) after 22 days and its scrapes started timing out. Two changes keep that from happening here: - /prom_cache is now an emptyDir with a 256Mi limit on the web and celery pods, so it starts empty on every pod start and cannot grow unbounded. The image still creates the directory; the volume only adds lifecycle. - The celery probes drop $prometheus_multiproc_dir from the environment of the nautobot-server process they fork. Celery has no HTTP listener, so these probes cannot become httpGet the way the web pod's already are; without the variable the forked process keeps prometheus_client in single-process mode and writes no files at all. `celery inspect ping` never touches /metrics, so no metrics are lost. Celery beat is left alone: its probe is a file-mtime check that never forks nautobot-server, so nothing accumulates there. Signed-off-by: Davesh Mathur <daveshm@nvidia.com>
2 participants
Summary
Enable prometheus_client multiprocess mode for Nautobot by baking
prometheus_multiproc_dir=/prom_cacheinto the image — exactly how the stockNautobot image does it. This makes the
/metricsself-deadlock structurallyimpossible (not just mitigated) and produces metrics aggregated across all
uWSGI workers instead of whichever single worker answers the scrape.
Multiprocess mode brings one liability with it:
prometheus_clientkeys itsmetric files by PID and never reaps them, so
/prom_cachegrows for as long asa pod lives. This PR also bounds that directory and stops the celery probes
from feeding it, so the mode is enabled and contained in a single change.
Supersedes the mitigation in #174, whose
uwsgi.inipreload is removed here aspart of the same change.
No new Helm values or knobs.
Root cause (condensed)
The web server runs multiple uWSGI workers. With no
PROMETHEUS_MULTIPROC_DIR,prometheus_clientuses the global in-process registry created withauto_describe=True:REGISTRY.register(NautobotAppMetricsCollector()).describe(), so withauto_describe=True,register()calls
collect()while holding the non-reentrantREGISTRY._lock.collect()touches Django's cache → lazily importsdjango_prometheus.cache.backends.redis→django_prometheus.cache.metrics,which instantiates
Counter()s at import time, each re-acquiring the lockthe same thread already holds → self-deadlock.
Setting
prometheus_multiproc_dirswitches every worker to a freshCollectorRegistry(auto_describe=False), soregister()never callscollect()under the lock. Full reproduction/proof is in #174.
The liability that comes with multiprocess mode
prometheus_clientwrites one set of*.dbfiles per PID and only reapslive-gauge files — counters and histograms are cumulative, so deleting them
would make metrics go backwards. Anything that spawns short-lived
nautobot-serverprocesses therefore leaves orphans behind permanently, andevery
/metricsscrape has to read the whole directory.This is not theoretical. A sibling deployment that has run in multiprocess mode
all along accumulated 49,636 files (778 MB) in
/prom_cacheafter 22 days,at which point concurrent scrapes became GIL-bound on file I/O and timed out. A
uwsgi --connect-and-readtracebacker dump on that live pod showed every workerthread parked in
prometheus_client/mmap_dict.py:read_all_values_from_file. Itpresents like a deadlock but is a throughput collapse — no lock involved. Its
cause was an
exec: nautobot-server health_checkliveness probe forking a newPID every 120s.
Change
build/nautobot.DockerfileENV prometheus_multiproc_dir=/prom_cache(+ upper-casePROMETHEUS_MULTIPROC_DIRalias for cross-version compatibility).
/prom_cachein the builder stage andCOPY --chown=1000:1000it intothe runtime image (the distroless runtime has no shell, so it can't
mkdiratbuild time; this mirrors the existing
media/.cachedirs and makes itwritable by the runtime uid).
prometheus_clientrequires the dir to exist.deploy/helm/templates/_nautobot-configmap-data.tplimport = django_prometheus.cache.metrics. With multiprocess mode on,the
/metricshandler builds a per-requestCollectorRegistry();CollectorRegistrydefaults toauto_describe=False, andregister()onlysubstitutes
collector.collect()for a missingdescribe()whenauto_describeis set.collect()is therefore never invoked while theregistry lock is held, the lazy
django_prometheus.cache.metricsimport can nolonger re-enter it, and the preload is dead weight. The template is now
byte-identical to its pre-fix(nautobot): prevent uWSGI /metrics self-deadlock and fix scrape path #174 content.
deploy/helm/templates/nautobot.yaml/prom_cachebecomes anemptyDirwith a 256MisizeLimiton the web andcelery pods. The image still creates the directory; the volume only adds
lifecycle management — it starts empty on every pod start and cannot grow
without bound. Steady state is a handful of files per uWSGI worker (tens of
KB each), so the limit is a regression signal, not something normal operation
approaches.
fsGroup: 1000is already set on these pods and the image chownsthe directory to 1000:1000, so the mount is writable.
$prometheus_multiproc_dirand$PROMETHEUS_MULTIPROC_DIRfrom the environment of thenautobot-serverprocess they fork. A celery worker has no HTTP listener, so these probes can't
become
httpGetthe way the web pod's already are. Without the variable theforked process keeps
prometheus_clientin single-process mode and writes nofiles at all.
celery inspect pingnever touches/metrics, so nothing islost.
The env is image-wide (matching the stock Nautobot image), so it applies to the
web server as well as celery/beat — consistent with the stock image's behavior.
Validation
Confirmed against the pinned Nautobot 2.4.36 that
NautobotMetricsView.get()selects
CollectorRegistry()+MultiProcessCollectorwhen either env spellingis present (
nautobot/core/views/__init__.py:489-501), and againstprometheus_client/registry.pythatCollectorRegistry.__init__defaults toauto_describe=Falseand_get_names()only falls back tocollector.collectwhen
_auto_describeis truthy. That is what makes the preload removal safe.helm template ... --values values-ci.yamlrenders; the emitteduwsgi.inicontains no
importdirective and is otherwise unchanged. Also renders cleanwith
values-observability.yamllayered, andhelm lintpasses (only thepre-existing temporal name-length warnings).
Audited every rendered Nautobot pod for probe handler and mount:
Re-rendered with
nautobot.customJobs.enabled=trueto cover the restructuredcelery
volumes/volumeMountsblocks, which were previously wrapped entirelyin that conditional; both
prom-cacheandcustom-jobsappear.Executed the new probe one-liner with both env spellings set and confirmed the
forked child sees neither, still sees
HOSTNAME, and that the exit codepropagates (so probe semantics are unchanged).
Checked the runtime image is actually able to use the directory: the distroless
base runs as
nvs= uid/gid 1000 (matching the--chown), and the chart'scontainer security context does not set
readOnlyRootFilesystem, so/prom_cacheis writable.Diffed our image against the upstream
nautobot:2.4.36finalstage: itscomplete production
ENVset isPYTHONUNBUFFERED,NAUTOBOT_ROOTandprometheus_multiproc_dir, so this was the only missing environmentvariable. Everything else stock provides is either already covered
(
docker-entrypoint.sh'spost_upgrade/superuser work is done bymigration_runner.py;uwsgi.inicomes from the chart ConfigMap;/etc/mime.typesis already in the distroless base;INSTALLATION_METRICS_ENABLED=Falseis set innautobot_config.py) ordeliberately unused (self-signed certs +
https/8443,libmariadb3,libxmlsec1).Notes
metrics.multiproc.*values / toggle (removed from an earlier revision ofthis PR at review request — matches the stock image exactly).
already
httpGet /health/(fix(helm): use /health endpoint for nautobot readiness #173), served by the existing uWSGI workers, so itcreates no new PIDs. Before that fix it would have been the worst offender —
readiness every 10s plus liveness every 120s.
/tmpfile-mtimecheck that never forks
nautobot-server, and the beat process itself is asingle long-lived PID, so nothing accumulates.
respawned (harakiri, OOM, crash) gets a new PID and orphans its predecessor's
files for the life of the pod.
harakiriis 1210s and none ofmax-requests/max-worker-lifetime/reload-on-rss/cheaperare set,so worker churn is rare, and the
emptyDircaps the worst case regardless.Summary by CodeRabbit
Bug Fixes
/metrics/404 errors for NVIDIA Config Manager monitoring.Operations