2 Commits

Author	SHA1	Message	Date
Andrey Antukh	452f38cf5d	✨ Add Prometheus metrics for storage operations (#11700) ... * ✨ Add storage operation metrics for S3 and buckets Expose Prometheus metrics for the object storage subsystem. The S3 backend now attaches an AWS SDK MetricPublisher that counts API calls, retries and latency per operation and target. The storage layer counts logical operations and deduplication outcomes per Penpot bucket, and the assets handlers count served requests per route. Closes #11676 AI-assisted-by: muse-spark-1.3-contributor * ✨ Fix storage metrics labels, errors and test gaps Address the review findings on the storage metrics commit. Label reads with the object's own backend, count failed asset serving as errors without swallowing them, and cover the failed S3 call, S3 asset path and permission-denied branches with tests. Also share the label helper and reuse the metrics test helper. Closes #11676 AI-assisted-by: muse-spark-1.3-contributor * ✨ Harden storage metrics and fill test gaps Address the second-round review findings on storage metrics. Unknown backends now fail explicitly and count as errors, exists stays paired with its dedup outcome, and the thumbnail, missing storage, expired reads, unknown buckets and write failure paths are covered by tests. Label coercion goes through the shared metrics helper. Closes #11676 AI-assisted-by: muse-spark-1.3-contributor * ✨ Harden storage metrics accuracy and coverage Address the full-branch review findings on storage metrics. Touch and delete emit only on changed rows, reads emit after the backend fetch, unknown backends fail explicitly, and tempfile mismatches count as unauthorized. Publisher nil policy, pairing rules and attempt semantics are documented and covered by tests. Closes #11676 AI-assisted-by: muse-spark-1.3-contributor * ✨ Address full-branch review findings on storage metrics Touch and delete resolve labels from the row, reads stay paired, failures are covered by tests, and logging, ranges and docs are tightened. Includes the label helper unit tests and the retries wording clarification. Closes #11676 AI-assisted-by: muse-spark-1.3-contributor * ⚡ Label touch and del metrics from UPDATE RETURNING The storage metrics change resolved metric labels for touch-object! and del-object! with an extra SELECT per id-based call. Since app.main instruments storage unconditionally, every GC collector and binfile import paid that extra round trip: deleting a team with 10k media objects doubled the storage_object statements exactly on the paths that already process the most rows. touch-object! and del-object! now take only the object id (UUID) and read the labels from the updated row itself via RETURNING id, backend, metadata: one statement, no pre-read, and labels that always match the row actually mutated. del-object! additionally guards on deleted_at IS NULL, so a repeated delete returns false and emits no metric. Also from the review of the full branch: extract the duplicated serve/emit/rethrow block in app.http.assets into one helper; give penpot_storage_s3_timing explicit histogram buckets up to 60s (the default cap at 7.5s hid the slow S3 calls the metric exists for); drop the unused ::target-id config key from the S3 backend and hardcode the :default target label until per-bucket routing lands. AI-assisted-by: glm-5.3-flash * ✨ Harden storage metric recording and definitions The metric definition schema is now closed and declares every key the collectors read: buckets, quantiles, max-age and reg. A typo such as a misspelled ::mdef/buckets used to compile and silently fall back to the default histogram buckets; it now fails at startup. The asset result-label fallback coerced an absent status to 500, so a future serve path without a status would have counted successes as errors. The mapping is now explicit and documented: served below 400, unauthorized for 401/403, not-found for 404, and error for everything else, including an absent status. The never-fail try/catch around metric recording existed four times with drift. One app.metrics/run-safe! helper replaces them: it no-ops on a nil metrics instance and logs the first failure per hint at warn level, then at debug, so a broken setup surfaces once without flooding the log. The S3 publisher keeps its outer try/catch: it is the SDK MetricPublisher contract boundary. AI-assisted-by: glm-5.3-flash * ✨ Make metrics mandatory and run! safe by default Recording a metric must never change the behavior of the operation being measured, so `run!` now catches recording failures itself: the first failure per metric id logs at warn, later ones at debug. This replaces the `run-safe!` helper, whose four copies had drifted, and applies the guarantee to every emit site instead of only storage. The metrics instance precondition is a plain assert, and the collector lookup stays outside the recording guard, so a missing instance fails hard even when asserts are disabled. Metrics is therefore no longer optional: the storage, s3-backend and db-pool schemas require `::mtx/metrics`, and the assets handler cfg always carries it. `wrap-publisher` no longer returns nil for a nil instance, and the db pool wires the prometheus tracker unconditionally. AI-assisted-by: deepseek-v4.1-flash	2026-09-23 15:25:21 +02:00
Andrey Antukh	89e91ba372	✨ Add observability improvements (#11854) ... * 🐳 Add upstream diagnostics to nginx access log Enrich every access-log line with the internal journey of the request: the status the backend answered (us), the time spent connecting to it (uct), the time spent waiting for its answer (urt) and the internal address that served the request (ua). A plain 502 line used to say nothing about where the request died. With this format, the tail of the line classifies the failure: connection rejected, backend accepted and hung (uct + urt under 1s), or backend stuck until read timeout. This was the missing witness in the Sep 20 incident, where nginx received connection resets with zero timeouts and zero rejections. Applied both to the production image template and the devenv config. With proxy_pass on variables there is no upstream keepalive, so uct measures one real TCP connection per request. Parsing the new fields (us, uct, urt, ua) on the log shipper is left to ops, so they can be filtered in Loki. AI-assisted-by: glm-5.3-flash * 🐳 Add stub_status endpoint for nginx metrics Add a dedicated localhost-only server (listen 127.0.0.1:8082) exposing /stub_status next to every other location of the public server. Ops can run the official nginx-prometheus-exporter as a sidecar against http://127.0.0.1:8082/stub_status and get nginx_connections_active, accepted vs handled, reading/writing/waiting and request rates in Prometheus. Binding it to localhost and its own server keeps it unreachable from outside the host and out of the public surface, and access_log off avoids polluting Loki with one line per Prometheus scrape. The base image already ships stub_status compiled in, so no image rebuild is needed. Applied both to the production image template and the devenv config. AI-assisted-by: glm-5.3-flash * ✨ Expose http server gate metrics (worker and connector) The backend already measured dispatch latency but nothing reported the state of the "house door": the xnio worker queue and threads, and the monitor-level listener counters. This was the exact blind spot of the Sep 20 incident, where the server kept answering health checks while it accepted connections and dropped them without response. Add a periodic metrics sampler that lives and dies with the http server (single daemon thread, 15s interval, each sample guarded so an unexpected error does not cancel subsequent runs) and publishes: - worker (xnio MXBean gauges): penpot_http_worker_queue_size, busy_threads, pool_size and max_pool_size. Negative samples are discarded: the MXBean transiently reports -1 on the busy thread count (verified live), and a stale negative would read as zero. - listener (Undertow connector statistics, enabled via the new :server/statistics yetti option): penpot_http_connector_active* _connections gauge and requests_total / errors_total counters. Undertow exposes absolute totals, so the sampler keeps a watermark atom and publishes deltas, skipping (and moving forward past) a counter reset. The connector-level part depends on yetti v11.11, which now accepts a :server/statistics server option (patch authored and released upstream; before it, ListenerInfo#getConnectorStatistics always returned nil). New tests cover the samplers with fake MXBean/collector statistics against real prometheus collectors, including the negative-sample filter, the delta/watermark logic and the sampler lifecycle. AI-assisted-by: glm-5.3-flash * 🐛 Include jdk.management in the backend runtime JRE The production image builds a trimmed JRE with jlink and omitted jdk.management. Without that module the OS MXBean is sun.management.BaseOperatingSystemImpl, which has no getProcessCpuTime, getOpenFileDescriptorCount nor getMaxFileDescriptorCount. The prometheus client StandardExports reads those getters reflectively and collect() swallows the NoSuchMethodException, so process_open_fds, process_max_fds and process_cpu_seconds_total silently disappeared from /metrics while the other process_* families kept flowing. Verified against Prometheus: the app job only ever exposed process_start_time_seconds, process_virtual_memory_bytes and process_resident_memory_bytes; the fd and cpu families were absent. Reproduced locally by running the backend metrics registry on a JRE built with the same jlink module list (false/false/false) and on one with jdk.management added (true/true/true). Add the module to --add-modules and pin the metric contract with backend-tests.metrics-test. AI-assisted-by: deepseek-v4.1-flash * ♻️ Build the http metrics sampler on promesa.exec Replace the hand-rolled ScheduledThreadPoolExecutor and ThreadFactory with promesa.exec primitives: px/scheduled-executor with a daemon thread factory, and a px/schedule chain that reschedules the next sample when the current one finishes. Beyond fitting the existing periodic-task pattern (worker/cron, rpc/rlimit), the chained schedule makes the docstring promise real: with scheduleAtFixedRate an exception escaping the runnable cancelled the following executions, while the reschedule now happens in a finally block. The sampler shutdown uses px/shutdown-now (shutdown! is deprecated in promesa 12.0.0) to cancel the pending sample, keeping the previous halt semantics. The lifecycle test moves to the promesa predicates and a new test covers the error-resilience promise: the first sample runs, throws, and the next one is still scheduled. AI-assisted-by: deepseek-v4.1-flash * ♻️ Tighten the http metrics samplers The samplers are leaf functions: they receive what they need and publish it. Drop the internal nil guards (if there is no metrics instance or no mxbean there is nothing to call them for) and move the checks to the boundary, where the optional data is resolved: sample-http-metrics now short-circuits with some-> and when-let. Write the four worker gauges as four static operations instead of a vector of pairs walked by doseq: the set is fixed, so the collection only adds an allocation and hides each operation. Drop the ! suffix from the sample-*-metrics family: ! marks a function whose contract is to mutate state, while these report, and the mutation happens in the mtx/run! they call. The constant true return, which only existed so the removed guard tests could assert it, goes away too. Tests follow the move: the internal-guard tests are replaced by one boundary test (a nil server publishes nothing). AI-assisted-by: deepseek-v4.1-flash * 📚 Add the function design rules memory Document the rules that came out of the http metrics sampler review: preconditions are checked at the boundary instead of re-checked in the core, optional-by-design data is guarded where the optionality is born, a fixed set of operations is written statically, ! marks mutation and not reporting, and production code is not shaped for tests. Also state in the memory maintenance guide that memories must not use manual line wrapping. Linked from critical-info so it is read when designing a solution or an API, not only when touching the samplers. AI-assisted-by: deepseek-v4.1-flash * 📚 Unwrap the critical-info memory lines The memory maintenance guide forbids manual line wrapping, so rewrite critical-info with one line per bullet and paragraph. A stray `*` at the start of one continuation line is dropped. AI-assisted-by: deepseek-v4.1-flash * ♻️ Drop the redundant guard in the http server halt create-metrics-sampler always returns the scheduler, so the sampler is always present when integrant calls halt-key!; the nil check was dead code, same as the yt/stop! call next to it. AI-assisted-by: deepseek-v4.1-flash * ✨ Add srepl helper to delete profiles by email Add `delete-profiles-by-email!` to app.srepl.main. It accepts a single email, a comma separated list of emails or a coll of emails, resolves each profile, logs it to audit and enqueues the delete-object task. The deleted-at is backdated with the configured deletion-delay so profiles and their owned teams are purged on the next gc pass. Extract the per-email deletion logic into a private fn and reuse it from `delete-profiles-in-bulk!`. Add tests for the new `parse-emails` helper. AI-assisted-by: glm-5.3-flash	2026-09-23 12:50:18 +02:00