* ✨ 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
12 KiB
Backend Storage
Abstraction
app.storagestores binary objects.- Each object has a
storage_objectdatabase row. - The row stores the UUID, size, backend, timestamps, and Transit metadata.
- The backend stores the binary content.
- Supported backends are
:fsand:s3. - FS uses one root directory and a UUID-derived path.
- S3 uses one configured bucket and an optional prefix.
- A Penpot bucket is metadata. It is not an S3 bucket or a filesystem directory.
- FS and S3 use the same UUID-derived object path. The bucket does not change the path.
PENPOT_OBJECTS_STORAGE_*configures the current object backend.- Deprecated asset-storage config keys remain supported for migration.
- Database rows keep the backend name. Keep the legacy
:assets-fsand:assets-s3aliases.
Object Lifecycle
put-object!creates the database row before it writes backend content.- Backend content is written only when the row is new.
- A failed backend write can leave an unreferenced database row.
- Callers often set
:touched-atso garbage collection can remove such rows. get-objectexcludes rows withdeleted_at.- Existing object values can remain readable until physical deletion.
:expired-atblocks reads after the expiration time.del-object!setsdeleted_aton live rows only (deleted_at IS NULL): a repeated call returnsfalse. It does not remove backend content.storage-gc-deletedremoves the database row and backend content after the deletion delay.storage-gc-touchedfinds references before it setsdeleted_at.objects-gcremoves deleted domain rows and touches their storage object IDs.- Use
::db/reuse-conn truewithsto/resolveinside a database transaction.
Connection Reuse Details
app.storage/resolve patterns:
1. Pool mode (default) - (sto/resolve cfg)
- Returns storage abstraction from config
- Uses whatever database pool is available
- Safe to call outside transaction context
- Used in:
rpc/commands/media.clj:363,rpc/commands/auth.clj:327,rpc/commands/profile.clj:362
2. Connection reuse mode - (sto/resolve cfg ::db/reuse-conn true)
- Internally calls
db/get-connection cfgto obtain connectable - Configures storage with the specific connection from config
- Must be paired with transaction that owns this connection
- Used in:
features/fdata.clj:100,rpc/commands/media.clj:425,rpc/commands/files_thumbnails.clj:307,319,binfile/v3.clj:722
3. Explicit configuration - (sto/configure storage conn)
- Sets
::db/connon storage map directly - Asserts
db/conn? connection(storage.clj:349) - Used inside
db/tx-run!blocks whereconnis already available - Used in:
tasks/file_gc.clj:256,rpc/commands/files_thumbnails.clj:347,371
Key Warning (from function notes):
The improved note in import-storage-objects and handle-persistence warns:
Do not reuse the main database connection for storage operations within a transaction. The storage upload process can fail mid-operation, leaving orphaned objects on the backend. If the outer transaction aborts, pending storage objects become unreconciliable because the storage subsystem registers its pending state in separate transactions.
Rule of Thumb for sto/put-object!:
Since put-object! uses backend-specific operations (impl/resolve-backend + impl/put-object) and does not directly use ::db/conn or ::db/pool, all usage of put-object! will never run inside a common transaction (if configured at all). The storage backend operations are independent of the database transaction boundary.
Deduplication
- Deduplication requires
::sto/deduplicate?, a content hash, and bucket metadata. - The lookup matches hash, bucket, backend, and
deleted_at IS NULL. - The lookup only considers rows with
status='valid'; pending rows are invisible. - A hit whose blob is missing is repaired in place: the same row/id is kept,
and
put-object!rewrites the blob under that id. This heals all existing references to the object. If the rewrite fails, the row is left live and valid for a later retry. - The lookup does not include file ID, profile ID, team ID, or organization ID.
- Objects can therefore share content across users and files within one bucket.
- Deleted objects are not reused.
tempfileobjects never use deduplication, even when the caller requests it.- Use
sto/wrap-with-hashwhen the caller already calculated the content hash.
Bucket Rules
| Bucket | Content and references | Dedup | Direct /assets/by-id access |
Cleanup |
|---|---|---|---|---|
file-media-object |
Original file images and generated media thumbnails. References: file_media_object.media_id and thumbnail_id. |
Yes | Public | Reference scan. |
team-font-variant |
Font variants in team_font_variant. References: woff1_file_id, woff2_file_id, otf_file_id, and ttf_file_id. |
Yes | Public | Reference scan. |
file-object-thumbnail |
Frame and component thumbnails in file_tagged_object_thumbnail.media_id. |
Yes | Public | Reference scan. |
file-thumbnail |
File grid thumbnails in file_thumbnail.media_id. |
Yes | Authentication required | Reference scan. |
profile |
User and team profile photos. References: profile.photo_id and team.photo_id. |
Yes | Authentication required | Reference scan. |
organization |
Organization logos uploaded by the Nitrate management API. | Yes | Public | No reference scan. A touched object is deleted. |
tempfile |
Export files and temporary font downloads. | No | Authentication required | No reference scan. A touched object uses a two-hour deletion delay. |
upload-session |
Chunked-upload chunks. References: upload_session_chunk.object_id and upload_session_chunk.session_id (both NO ACTION DEFERRABLE: restrict semantics, procedural deletion). |
No | Authentication required | No reference scan. A touched object is deleted after the delay; gc-deleted removes mappings before rows. |
file-data |
Encoded file data when file-data-backend is storage. Reference metadata has storage-ref-id, file-id, and the file_data row ID. |
Yes | Authentication required | Reference scan. |
file-data-fragment |
Compatibility value for file-data fragments. The current backend has no dedicated producer for this bucket. | No current write semantics | Public | No touched-object collector case. |
file-change |
Compatibility value for file changes. Current snapshots store data in file_data, not this bucket. |
No current write semantics | Authentication required | No touched-object collector case. |
- The valid bucket set lives in
app.storage/valid-buckets. file-media-objectis the default bucket for old rows without bucket metadata.- Do not assign a new bucket without adding its access and cleanup behavior.
- The touched-object collector raises an internal error for an unknown bucket.
- It supports
file-media-object,team-font-variant,file-object-thumbnail,file-thumbnail,profile,file-data,tempfile,upload-session, andorganization. - It does not support
file-data-fragmentorfile-change.
Access Rules
app.http.assetsdecides direct object authentication from the bucket.- Public buckets are
file-media-object,file-object-thumbnail,team-font-variant,file-data-fragment, andorganization. - Other valid buckets require a session or access-token profile ID.
- File-media routes also require file read permission.
- Non-public direct responses set
content-disposition: attachment. - FS responses use
x-accel-redirectfor the configured asset path. - S3 responses use a presigned URL and an HTTP redirect.
File Data
file-data-backendacceptslegacy-db,db, orstorage.legacy-dbstores main data infile.dataand snapshots infile_change.data.dbstores encoded data infile_data.data.storagestores encoded data in storage subsystem withfile-databucket and keepsdatanil infile_datatable.- The
file_data.metadata.storage-ref-idvalue points to the storage object. fdata/upsert!touches a storage object from incoming metadata before it stores the new row.- File snapshots use
file_datafor snapshot data andfile_changefor snapshot metadata.
Metrics
bucketis always the Penpot logical bucket (object metadata), never an S3 bucket. Unknown/absent buckets are labeled"unknown".targetis the physical S3 destination id. Today it is always"default"(hardcoded inapp.storage.s3/build-s3-client; the::target-idconfig key was removed as unused until the per-bucket routing plan lands).- Physical S3 API calls (AWS SDK
MetricPublisher,app.storage.s3.metrics):penpot_storage_s3_requests_total{operation,target,result}— one count per logical SDK call (the publishedApiCallcollection, not per attempt); retries are counted apart inretries_total, so total attempts =requests + retries.resultis"ok"only when the SDK reports success as exactlytrue; a missing success flag counts aserror.penpot_storage_s3_retries_total{operation,target}— SDK retry count.penpot_storage_s3_timing{operation,target}— call latency histogram (ms); explicit buckets up to 60000 ms (S3 slow calls exceed the default 7500 ms cap).
- Logical storage operations (
app.storage,::mtx/metricsrequired by the schema):penpot_storage_operations_total{op,bucket,backend}—put,repair,get-data,get-bytes,del,touch,exists. All ops are success-only:put/repairemit after the backend write,get-*after the backend fetch opens,touch/delonly when a row actually changed.touch-object!/del-object!take the object id (UUID) only — no object overload. Labels come from the updated row itself viaUPDATE ... RETURNING id, backend, metadata(no extraSELECT); with no row matched they emit nothing.del-object!only matches live rows (deleted_at IS NULL): a repeated del returnsfalseand emits nothing. Post-open stream read errors stay counted as attempts.delonly marksdeleted_at; physical deletion is a GC concern.existsis emitted per deduplication-hit probe, always paired with ahit/repairoutcome (never on probe failure), not per user-facing existence check.penpot_storage_dedup_total{result,bucket}—hit,miss,repair,skip.
- Asset serving (
app.http.assets,::mtx/metricsrequired in the handler cfg):penpot_storage_asset_requests_total{route,backend,bucket,result}—routeisby-id,by-file-media-id, orthumbnail;resultisserved(<400),not-found(404),unauthorized(401/403), orerror(everything else, including a nil/non-number status: every serve path must set::yres/status). Serve-path exceptions are counted byserve-object-measuredand then rethrown. Permission-denied file-media requests and tempfile ownership mismatches both answer HTTP 404 (to avoid leaking existence) but are counted asunauthorized. Malformed UUIDs raise before any emission point and are never counted. Counts backend requests that trigger a browser GET to the object store (one per cache miss), so it is a proxy for object GETs, not an exact count.
- The physical and logical counters intentionally overlap in coverage but differ in meaning; do not sum them.
- Metrics is not optional:
::mtx/metricsis required by the storage and s3-backend schemas, and the assets handler cfg always carries it. Wiring a component without metrics is a bug, not a supported mode. - Recording never fails:
app.metrics/run!is safe by default at every emit point (emit-op!,emit-dedup!,emit-asset!, and the three S3 publisher emissions). The first recording failure per metric id logs atwarn, later ones atdebug(no log flood). Theinstanceprecondition is a plain assert and the collector lookup is outside the recording guard, so a missing instance fails hard (seemem:backend/subtleties). Thepublishouter try/catch stays: it is an SDKMetricPublishercontract boundary, not a metrics guard.