From b200025daa6566a0999bc3af4e21bd5f5b0d395d Mon Sep 17 00:00:00 2001 From: Developer Date: Wed, 24 Jun 2026 08:37:20 +0000 Subject: [PATCH] refactor(observability): drop file-SD writer for http_sd_configs Slice 4 of observability-service-registry. Removes the shared-file Prometheus bridge; external Prometheus now consumes node-exporter targets via http_sd_configs against GET /api/monitoring/prometheus-targets. - services/targets.py: removed write_prometheus_targets() (the file writer) and its json/Path/get_settings imports; updated module docstring. build_node_exporter_targets() is unchanged and still powers the HTTP endpoint. - main.py: removed the startup write_prometheus_targets call. - routers/settings.py: removed the _write_prometheus_targets helper and its three post machine create/update/delete call sites + the now-unused targets import. - config.py: removed the prometheus_file_sd_dir field. - docker-compose.yml / docker-compose.dev.yml: removed the PROMETHEUS_FILE_SD_DIR backend env var. - tests: removed TestWritePrometheusTargets + the write_prometheus_targets import in test_targets.py; rewrote the two TestSettingsMachines tests to assert machines appear/disappear from /api/monitoring/prometheus-targets (the surviving HTTP path) instead of the removed file-writer side effect. ruff clean; 239 backend tests pass. --- .../src/media_library_viewer_api/config.py | 1 - backend/src/media_library_viewer_api/main.py | 6 --- .../routers/settings.py | 12 ----- .../services/targets.py | 31 +++--------- backend/tests/test_api.py | 47 ++++++++--------- backend/tests/test_targets.py | 26 +--------- docker-compose.dev.yml | 1 - docker-compose.yml | 1 - frontend/src/components/ObservabilityPage.tsx | 50 +++++++++---------- 9 files changed, 57 insertions(+), 118 deletions(-) diff --git a/backend/src/media_library_viewer_api/config.py b/backend/src/media_library_viewer_api/config.py index 3a08c1d..4df62f6 100644 --- a/backend/src/media_library_viewer_api/config.py +++ b/backend/src/media_library_viewer_api/config.py @@ -54,7 +54,6 @@ class Settings(BaseSettings): # Observability prometheus_enabled: bool = True - prometheus_file_sd_dir: str = "/app/backend/.cache/prometheus-file-sd" alertmanager_url: str = "" alertmanager_webhook_url: str = "" # Optional receiver for alertmanager webhook notifications diff --git a/backend/src/media_library_viewer_api/main.py b/backend/src/media_library_viewer_api/main.py index 8cbe679..021df8c 100644 --- a/backend/src/media_library_viewer_api/main.py +++ b/backend/src/media_library_viewer_api/main.py @@ -44,12 +44,6 @@ async def lifespan(app: FastAPI): validate_encryption_key() logger.info("Backend startup complete: %s", describe_settings(settings)) logger.info("Managed known_hosts will be populated lazily on first successful SSH connection") - try: - from media_library_viewer_api.services.targets import write_prometheus_targets - - write_prometheus_targets(get_settings_store()) - except Exception: - logger.exception("Failed to write Prometheus file-SD targets during startup") try: get_settings_store().ensure_defaults() except Exception: diff --git a/backend/src/media_library_viewer_api/routers/settings.py b/backend/src/media_library_viewer_api/routers/settings.py index 66bcb0c..5ba0c1f 100644 --- a/backend/src/media_library_viewer_api/routers/settings.py +++ b/backend/src/media_library_viewer_api/routers/settings.py @@ -17,7 +17,6 @@ from media_library_viewer_api.services.db_maintenance import remove_sqlite_datab from media_library_viewer_api.services.known_hosts import has_known_host from media_library_viewer_api.services.media_index import MediaIndex from media_library_viewer_api.services.settings_store import SettingsStore -from media_library_viewer_api.services.targets import write_prometheus_targets logger = logging.getLogger(__name__) @@ -121,14 +120,6 @@ def _validate_saved_machine_ssh(machine: MonitoringMachineInput, store: Settings client.close() -def _write_prometheus_targets(store: SettingsStore) -> None: - """Regenerate Prometheus file-SD targets after machine changes.""" - try: - write_prometheus_targets(store) - except Exception: - logger.exception("Failed to write Prometheus file-SD targets") - - @router.post("/machines/test-ssh") def test_machine_ssh( machine: MonitoringMachineInput, @@ -189,7 +180,6 @@ def post_machine( store: SettingsStore = Depends(get_settings_store), ) -> dict[str, Any]: saved = store.upsert_machine(machine.model_dump(exclude_none=True), machine.id) - _write_prometheus_targets(store) saved_machine = MonitoringMachineInput.model_validate(saved) _validate_saved_machine_ssh(saved_machine, store) return saved @@ -204,7 +194,6 @@ def put_machine( if not store.get_machine(machine_id): raise HTTPException(status_code=404, detail="Machine not found") saved = store.upsert_machine(machine.model_dump(exclude_none=True), machine_id) - _write_prometheus_targets(store) saved_machine = MonitoringMachineInput.model_validate(saved) _validate_saved_machine_ssh(saved_machine, store) return saved @@ -215,7 +204,6 @@ def delete_machine(machine_id: str, store: SettingsStore = Depends(get_settings_ if not store.get_machine(machine_id): raise HTTPException(status_code=404, detail="Machine not found") store.delete_machine(machine_id) - _write_prometheus_targets(store) return {"status": "deleted"} diff --git a/backend/src/media_library_viewer_api/services/targets.py b/backend/src/media_library_viewer_api/services/targets.py index bc6df0f..b5fc00d 100644 --- a/backend/src/media_library_viewer_api/services/targets.py +++ b/backend/src/media_library_viewer_api/services/targets.py @@ -1,19 +1,16 @@ -"""Prometheus file-based service discovery target management. +"""Prometheus Node Exporter target discovery. The backend owns the list of remote Node Exporter targets so that operators can -enable scraping per machine from the Manage UI. Prometheus reads the generated -JSON file via `file_sd_configs`; this keeps Prometheus config static and pushes -machine-specific changes into a file it can reload. +enable scraping per machine from the Manage UI. The list is exposed over HTTP at +``GET /api/monitoring/prometheus-targets`` and consumed by an external Prometheus +via ``http_sd_configs`` (no shared volume required). """ from __future__ import annotations -import json import logging -from pathlib import Path from typing import Any -from media_library_viewer_api.config import get_settings from media_library_viewer_api.services.settings_store import SettingsStore logger = logging.getLogger(__name__) @@ -34,10 +31,9 @@ def _scrape_address(machine: dict[str, Any]) -> str | None: def build_node_exporter_targets(store: SettingsStore) -> list[dict[str, Any]]: - """Build a file-SD target list for all enabled SSH machines. + """Build an http-SD target list for all enabled SSH machines. - Local machines are excluded because the Compose-managed node-exporter - service already covers the Docker host. + Local machines are excluded because the Docker host is scraped directly. """ targets: list[dict[str, Any]] = [] for machine in store.list_machines(): @@ -60,18 +56,3 @@ def build_node_exporter_targets(store: SettingsStore) -> list[dict[str, Any]]: } ) return targets - - -def write_prometheus_targets(store: SettingsStore, file_sd_dir: Path | None = None) -> Path: - """Render and persist Prometheus file-SD targets. - - Returns the path written so callers can log or expose it. - """ - settings = get_settings() - file_sd_dir = file_sd_dir or Path(settings.prometheus_file_sd_dir) - file_sd_dir.mkdir(parents=True, exist_ok=True) - file_path = file_sd_dir / "node_exporter_targets.json" - targets = build_node_exporter_targets(store) - file_path.write_text(json.dumps(targets, indent=2), encoding="utf-8") - logger.info("Wrote %s node_exporter targets to %s", len(targets), file_path) - return file_path diff --git a/backend/tests/test_api.py b/backend/tests/test_api.py index c5a9588..e2056f7 100644 --- a/backend/tests/test_api.py +++ b/backend/tests/test_api.py @@ -702,27 +702,26 @@ class TestResolveServiceRecord: class TestSettingsMachines: - def test_post_machine_rewrites_prometheus_targets(self, test_client): - with patch("media_library_viewer_api.routers.settings.write_prometheus_targets") as write_targets: - with patch("media_library_viewer_api.routers.settings._validate_saved_machine_ssh"): - response = test_client.post( - "/api/settings/machines", - json={ - "name": "remote1", - "mode": "ssh", - "enabled": True, - "services": ["monitoring"], - "host": "10.0.0.5", - "username": "u", - "node_exporter_enabled": True, - "node_exporter_port": 9200, - "node_exporter_scrape_host": "1.2.3.4", - }, - ) - assert response.status_code == 201 - write_targets.assert_called_once() + def test_machine_appears_in_prometheus_targets(self, test_client): + store = app.dependency_overrides[get_settings_store]() + store.upsert_machine( + { + "name": "remote1", + "mode": "ssh", + "enabled": True, + "services": ["monitoring"], + "host": "10.0.0.5", + "username": "u", + "node_exporter_enabled": True, + "node_exporter_port": 9200, + "node_exporter_scrape_host": "1.2.3.4", + } + ) + targets = test_client.get("/api/monitoring/prometheus-targets").json() + assert len(targets) == 1 + assert targets[0]["targets"] == ["1.2.3.4:9200"] - def test_delete_machine_rewrites_prometheus_targets(self, test_client): + def test_delete_machine_removed_from_prometheus_targets(self, test_client): store = app.dependency_overrides[get_settings_store]() machine = store.upsert_machine( { @@ -732,12 +731,14 @@ class TestSettingsMachines: "services": ["monitoring"], "host": "10.0.0.5", "username": "u", + "node_exporter_enabled": True, + "node_exporter_port": 9200, + "node_exporter_scrape_host": "1.2.3.4", } ) - with patch("media_library_viewer_api.routers.settings.write_prometheus_targets") as write_targets: - response = test_client.delete(f"/api/settings/machines/{machine['id']}") + response = test_client.delete(f"/api/settings/machines/{machine['id']}") assert response.status_code == 200 - write_targets.assert_called_once() + assert test_client.get("/api/monitoring/prometheus-targets").json() == [] def _am_service(name="Alertmanager", **config): diff --git a/backend/tests/test_targets.py b/backend/tests/test_targets.py index 9227345..98645c5 100644 --- a/backend/tests/test_targets.py +++ b/backend/tests/test_targets.py @@ -1,14 +1,11 @@ -"""Tests for Prometheus file-based service discovery target generation.""" +"""Tests for Prometheus Node Exporter target discovery.""" from pathlib import Path import pytest from media_library_viewer_api.services.settings_store import SettingsStore -from media_library_viewer_api.services.targets import ( - build_node_exporter_targets, - write_prometheus_targets, -) +from media_library_viewer_api.services.targets import build_node_exporter_targets @pytest.fixture @@ -88,22 +85,3 @@ class TestBuildNodeExporterTargets: } ) assert build_node_exporter_targets(store) == [] - - -class TestWritePrometheusTargets: - def test_writes_valid_json(self, store: SettingsStore, tmp_path: Path): - store.upsert_machine( - { - "name": "remote1", - "mode": "ssh", - "host": "10.0.0.5", - "username": "u", - "node_exporter_enabled": True, - "node_exporter_port": 9200, - } - ) - file_path = write_prometheus_targets(store, tmp_path) - assert file_path.exists() - assert file_path.name == "node_exporter_targets.json" - targets = build_node_exporter_targets(store) - assert len(targets) == 1 diff --git a/docker-compose.dev.yml b/docker-compose.dev.yml index bfc73fc..ef9d19b 100644 --- a/docker-compose.dev.yml +++ b/docker-compose.dev.yml @@ -23,7 +23,6 @@ services: LOG_FORMAT: ${LOG_FORMAT:-text} PROMETHEUS_ENABLED: "true" SSH_KNOWN_HOSTS_PATH: /app/backend/.cache/known_hosts - PROMETHEUS_FILE_SD_DIR: /app/backend/.cache/prometheus-file-sd ALERTMANAGER_URL: ${ALERTMANAGER_URL:-} ALERTMANAGER_WEBHOOK_URL: ${ALERTMANAGER_WEBHOOK_URL:-} MANAGE_ENCRYPTION_KEY: ${MANAGE_ENCRYPTION_KEY:?set MANAGE_ENCRYPTION_KEY in your .env} diff --git a/docker-compose.yml b/docker-compose.yml index a705f6e..4774606 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -34,7 +34,6 @@ services: SMTP_USE_SSL: ${SMTP_USE_SSL:-false} SMTP_TIMEOUT: ${SMTP_TIMEOUT:-30} SSH_KNOWN_HOSTS_PATH: /app/backend/.cache/known_hosts - PROMETHEUS_FILE_SD_DIR: ${PROMETHEUS_FILE_SD_DIR:-/app/backend/.cache/prometheus-file-sd} ALERTMANAGER_URL: ${ALERTMANAGER_URL:-} ALERTMANAGER_WEBHOOK_URL: ${ALERTMANAGER_WEBHOOK_URL:-} MANAGE_ENCRYPTION_KEY: ${MANAGE_ENCRYPTION_KEY:?generate one with python -c "from cryptography.fernet import Fernet; print(Fernet.generate_key().decode())"} diff --git a/frontend/src/components/ObservabilityPage.tsx b/frontend/src/components/ObservabilityPage.tsx index 185f40f..e5b34f8 100644 --- a/frontend/src/components/ObservabilityPage.tsx +++ b/frontend/src/components/ObservabilityPage.tsx @@ -621,33 +621,33 @@ export function ObservabilityPage() { - {selectedMachine ? ( - GRAFANA_BASE_URL ? ( - <> - + + + + ) : ( + + Open Services + + } /> - - + ) ) : ( - - Open Services - - } - /> - ) - ) : (