Skip to content

feat(health): expose pg/es/s3 used size on /api/health (#7356) - #7357

Draft
Sébastien Bocahu (sbocahu) wants to merge 4 commits into
mainfrom
issue/7356
Draft

feat(health): expose pg/es/s3 used size on /api/health (#7356)#7357
Sébastien Bocahu (sbocahu) wants to merge 4 commits into
mainfrom
issue/7356

Conversation

@sbocahu

Copy link
Copy Markdown
Contributor

Closes #7356

What

Adds opt-in storage metrics to GET /api/health, behind a new details=true request parameter:

{ "status": "success", "pg_used_size": 104857600, "es_used_size": 524288000, "s3_used_size": 20971520 }

Without details (or with details=false) the response is byte-for-byte unchanged (plain success), so existing load-balancer probes keep working and pay no extra cost.

Sizes are in bytes. A null value means the metric could not be retrieved from that dependency.

How each size is obtained

Metric Source Note
pg_used_size pg_database_size(current_database()) Cheap, single scalar query
es_used_size Engine _stats restricted to the store metric on the {prefix}_* index pattern Primaries only — replicas hold a copy of the very same documents, so counting them would report the cluster filesystem footprint (× replication factor) instead of the real data volume. Implemented for both Elasticsearch and OpenSearch
s3_used_size Sum of the object sizes in the bucket Object storage exposes no aggregate, so the whole listing must be walked — this is the expensive one

Why the values are cached

The health endpoint is polled very frequently (LB probes), while computing these metrics walks the entire object storage listing and queries the engine cluster. So HealthCheckService#getStorageUsage() caches the three values for openaev.healthcheck.storage-usage-cache-duration (default PT4H, configurable, documented in docs/deployment/configuration.md).

The refresh is single-flight: only one caller recomputes, concurrent callers keep being served the previous (stale) value instead of piling up on the engine and the bucket. Only the very first call — when there is nothing cached yet — blocks.

A metric that fails to compute is logged and reported as null; it never turns a healthy platform into a 503, since the health status keeps reflecting connectivity only.

⚠️ Note on what /api/health actually checks

Worth stating explicitly, as it is commonly misunderstood (a javadoc note was added on runHealthCheck() to that effect):

  • PostgreSQLselect 1
  • RabbitMQ — connection check
  • MinIO / S3statObject with 2s timeouts
  • Elasticsearch / OpenSearch — NOT checked. There is no engine connectivity check at all, today or in this PR. The engine is only contacted for the (cached) storage metric, and a failure there yields null rather than a 503. So an engine outage is still invisible to this probe.
  • Redis — no such check, and no Redis dependency on this platform. (Mentioned because it is often assumed to be part of the probe.)

Adding a real engine connectivity check is a deliberate follow-up decision, not part of this PR: it would change the failure semantics of the probe (an engine outage would start taking instances out of the LB pool), which deserves its own discussion.

The endpoint also remains @Transactional(propagation = NOT_SUPPORTED) — see #6837, it must not pin a Hikari connection while doing external network I/O.

Tests

  • HealthCheckApiTest: details=false does not compute the usage at all; details=true returns the three sizes.
  • HealthCheckServiceTest: per-metric values, a degraded dependency reported as null without failing, and cache hit on a second call.

Checklist

  • mvn spotless:apply
  • mvn compile (openaev-model + openaev-api)
  • mvn test-compile
  • Documentation updated (docs/deployment/configuration.md)
  • Full test suite run against live services

@github-actions

Copy link
Copy Markdown
Contributor

📖 Documentation check — ✅ Passed

9 functional file(s), 1 doc file(s) changed.

Documentation-worthy changes detected and documentation was updated. 👏

Detected changes (covered by doc updates)
  • 🟡 New configuration propertyopenaev-api/src/main/java/io/openaev/service/HealthCheckService.java

@Filigran-Automation Filigran Automation (Filigran-Automation) added the filigran team Item from the Filigran team. label Aug 11, 2026
@github-actions

github-actions Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Container vulnerability scan — Passed

Previously reported findings are no longer present.

Image Critical High Total Status
Standard 0 0 0 ✅ Clear
UBI9 0 0 0 ✅ Clear

View workflow run · Standard JSON report · UBI9 JSON report

Updated from CI run attempt 1.

@sbocahu

Copy link
Copy Markdown
Contributor Author

Two fixes pushed after testing on a deployed environment, where /api/health returned 503 on every call with no log output at all.

Root cause

MinioService.isTenantPathExists() probed the object storage with:

client.statObject(... .object(getTenantPath("")) ...);   // key = "<tenant-uuid>/"

A tenant path is a key prefix, not an object. statObject only succeeds if a zero-byte directory marker happens to exist at that exact key — never the case on a freshly provisioned bucket, and never on plain S3, which has no directory markers at all. Result:

io.openaev.service.exception.HealthCheckFailureException: FileStorage check failure
  caused by io.minio.errors.ErrorResponseException: Object does not exist

The probe failed permanently and the load balancer pulled the instance out of rotation.

This only stayed hidden because MinIO tolerates the call as soon as any file exists under the prefix — which is true in dev and CI, but not in a new environment.

Fix 1 — probe with a listing (df88107)

Replaced with a one-key listing on the tenant prefix. It still proves connectivity, credential validity and bucket access, but succeeds when the tenant has no files yet. Renamed to checkTenantPathAccessible, since a method named isTenantPathExists that intentionally passes when the path does not exist would invite someone to "fix" it straight back.

Added a regression test in MinioServiceTest that points TenantContext at a random tenant UUID (guaranteed empty) and asserts the check does not throw — it fails against the previous implementation.

Fix 2 — make the failure observable (5384f07)

The failure was completely silent, which is why it took a deployment to find. Two things combined:

  • logging.level.root=fatal and logging.level.io.openaev=error — nothing below ERROR is emitted.
  • The 503 is a ResponseStatusException, which Spring resolves via ResponseStatusExceptionResolver at DEBUG. RestBehavior has ~25 exception handlers but none for ResponseStatusException, so nothing else logged it.

A health probe that can fail forever without emitting a single line is undebuggable, so the failing dependency is now logged at ERROR with its cause.

Note for reviewers

Fix 1 changes probe semantics: the file-storage check no longer requires the tenant path to contain anything. That is intentional — the previous behaviour made "tenant has never stored a file" indistinguishable from "object storage is down". Happy to split these two commits into their own PR if you would rather keep this one purely additive.

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

Labels

filigran team Item from the Filigran team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(health): expose pg/es/s3 used size on /api/health

2 participants