feat: add observability improvements (metrics, audit log, structured logging) #169

2026-03-17T10:24:18+01:00

clawbot commented

2026-03-17 10:24:18 +01:00

closes #84

Summary

Adds comprehensive observability to upaas: Prometheus metrics, an audit log table, and enhanced structured logging.

Prometheus Metrics (`internal/metrics`)

Deployment metrics: upaas_deployments_total (counter by app/status), upaas_deployments_duration_seconds (histogram), upaas_deployments_in_flight (gauge)
Container health: upaas_container_healthy (gauge, 1=healthy/0=unhealthy)
Webhook metrics: upaas_webhook_events_total (counter by app/event_type/matched)
HTTP metrics: upaas_http_requests_total, upaas_http_request_duration_seconds, upaas_http_response_size_bytes
Audit metrics: upaas_audit_events_total (counter by action)
Test-safe: NewForTest() uses a custom registry to avoid cross-test collisions
/metrics endpoint is now always available (optionally auth-protected)

Audit Log

New audit_log SQLite table via migration 007_add_audit_log.sql with indexes on created_at, action, and (resource_type, resource_id)
AuditEntry model (internal/models/audit_log.go) with Save, FindAuditEntries, FindAuditEntriesByResource, CountAuditEntries
Audit service (internal/service/audit) with Log, LogFromRequest, Recent, ForResource methods
Audited actions: login/logout, app create/update/delete, deploy, cancel, rollback, restart/stop/start, webhook receive, initial setup
IP extraction from X-Real-IP → X-Forwarded-For → RemoteAddr
New API endpoint: GET /api/audit?limit=N (default 50, max 500)

Structured Logging Enhancements

HTTP middleware now logs response size (bytes field) alongside existing request metadata
All metrics are recorded in the logging middleware defer block for consistency

Integration Points

Deploy service: tracks in-flight gauge, records duration/count on completion, sets container health gauge after health check
Webhook service: increments event counter with app/event_type/matched labels
HTTP middleware: records request count, duration histogram, and response size histogram
All handler operations: call h.auditLog() helper which resolves the current user and delegates to the audit service

Tests

internal/metrics/metrics_test.go: constructor, all metric families registered, label operations
internal/models/models_test.go: 7 new audit entry tests (CRUD, find by resource, count, limit, ordering)
internal/service/audit/audit_test.go: 6 tests covering Log, LogFromRequest, IP extraction, Recent, ForResource, optional fields
Updated existing test infrastructure (handlers, webhook) to supply metrics and audit dependencies

README

Added Observability section documenting all metrics, audit log, and structured logging.

closes #84 ## Summary Adds comprehensive observability to upaas: Prometheus metrics, an audit log table, and enhanced structured logging. ### Prometheus Metrics (`internal/metrics`) - **Deployment metrics**: `upaas_deployments_total` (counter by app/status), `upaas_deployments_duration_seconds` (histogram), `upaas_deployments_in_flight` (gauge) - **Container health**: `upaas_container_healthy` (gauge, 1=healthy/0=unhealthy) - **Webhook metrics**: `upaas_webhook_events_total` (counter by app/event_type/matched) - **HTTP metrics**: `upaas_http_requests_total`, `upaas_http_request_duration_seconds`, `upaas_http_response_size_bytes` - **Audit metrics**: `upaas_audit_events_total` (counter by action) - Test-safe: `NewForTest()` uses a custom registry to avoid cross-test collisions - `/metrics` endpoint is now always available (optionally auth-protected) ### Audit Log - New `audit_log` SQLite table via migration `007_add_audit_log.sql` with indexes on `created_at`, `action`, and `(resource_type, resource_id)` - `AuditEntry` model (`internal/models/audit_log.go`) with `Save`, `FindAuditEntries`, `FindAuditEntriesByResource`, `CountAuditEntries` - Audit service (`internal/service/audit`) with `Log`, `LogFromRequest`, `Recent`, `ForResource` methods - Audited actions: login/logout, app create/update/delete, deploy, cancel, rollback, restart/stop/start, webhook receive, initial setup - IP extraction from X-Real-IP → X-Forwarded-For → RemoteAddr - New API endpoint: `GET /api/audit?limit=N` (default 50, max 500) ### Structured Logging Enhancements - HTTP middleware now logs response size (`bytes` field) alongside existing request metadata - All metrics are recorded in the logging middleware defer block for consistency ### Integration Points - Deploy service: tracks in-flight gauge, records duration/count on completion, sets container health gauge after health check - Webhook service: increments event counter with app/event_type/matched labels - HTTP middleware: records request count, duration histogram, and response size histogram - All handler operations: call `h.auditLog()` helper which resolves the current user and delegates to the audit service ### Tests - `internal/metrics/metrics_test.go`: constructor, all metric families registered, label operations - `internal/models/models_test.go`: 7 new audit entry tests (CRUD, find by resource, count, limit, ordering) - `internal/service/audit/audit_test.go`: 6 tests covering Log, LogFromRequest, IP extraction, Recent, ForResource, optional fields - Updated existing test infrastructure (handlers, webhook) to supply metrics and audit dependencies ### README Added Observability section documenting all metrics, audit log, and structured logging.

clawbot added 1 commit 2026-03-17 10:24:18 +01:00

feat: add observability improvements (metrics, audit log, structured logging)

Check / check (pull_request) Successful in 1m45s

Details

f558e2cdd8

- Add Prometheus metrics package (internal/metrics) with deployment,
  container health, webhook, HTTP request, and audit counters/histograms
- Add audit_log SQLite table via migration 007
- Add AuditEntry model with CRUD operations and query methods
- Add audit service (internal/service/audit) for recording user actions
- Instrument deploy service with deployment duration, count, and
  in-flight metrics; container health gauge updates on deploy completion
- Instrument webhook service with event counters by app/type/matched
- Instrument HTTP middleware with request count, duration, and response
  size metrics; also log response bytes in structured request logs
- Add audit logging to all key handler operations: login/logout, app
  CRUD, deploy, cancel, rollback, restart/stop/start, webhook receipt,
  and initial setup
- Add GET /api/audit endpoint for querying recent audit entries
- Make /metrics endpoint always available (optionally auth-protected)
- Add comprehensive tests for metrics, audit model, and audit service
- Update existing test infrastructure with metrics and audit dependencies
- Update README with Observability section documenting all metrics,
  audit log, and structured logging

clawbot commented

2026-03-17 10:44:37 +01:00

Code Review: PR #169 - Observability Improvements

1. Policy Divergences

BLOCKING: Security - IP spoofing in audit logs

extractRemoteIP() in internal/service/audit/audit.go unconditionally trusts X-Real-IP and X-Forwarded-For headers from any client without checking if the request comes from a trusted proxy.

The existing realIP() function in internal/middleware/middleware.go correctly validates that the connecting IP is from a trusted proxy (RFC1918/loopback) before trusting these headers. The audit service reimplements IP extraction without this security check.

This allows any direct client to forge their IP address in audit records by setting X-Real-IP: 1.2.3.4, undermining the integrity of the audit log. The audit service should reuse the middleware's trusted-proxy-aware logic (export realIP or pass the already-resolved IP from the handler layer).

BLOCKING: Incomplete audit coverage - 9 dead audit action constants

The following audit action constants are defined in internal/models/audit_log.go but never referenced anywhere in the codebase:

AuditActionEnvVarSave
AuditActionLabelAdd, AuditActionLabelEdit, AuditActionLabelDelete
AuditActionVolumeAdd, AuditActionVolumeEdit, AuditActionVolumeDelete
AuditActionPortAdd, AuditActionPortDelete

The corresponding handler operations (HandleEnvVarSave, HandleLabelAdd, HandleLabelEdit, HandleLabelDelete, HandleVolumeAdd, HandleVolumeEdit, HandleVolumeDelete, HandlePortAdd, HandlePortDelete) exist in the routes but are not instrumented with audit logging. These are user-facing actions. The issue scope says "Audit log table for user actions" - these qualify. Either instrument them or remove the dead constants.

BLOCKING: README inconsistency - Wrong API endpoint path

README states the audit log is available at GET /api/audit but the actual route registered in internal/server/routes.go is under the /api/v1 group, making the real path GET /api/v1/audit.

Minor: README - Duplicate numbering in DI order

The Dependency Injection order section has duplicate numbers - items 10 and 11 appear twice (notify/audit then deploy/webhook).

2. Requirements Checklist (Issue #84)

Structured logging for all operations: PASS - HTTP middleware now logs response size; existing slog usage maintained
Prometheus metrics deployment count/duration: PASS - upaas_deployments_total, upaas_deployments_duration_seconds
Prometheus metrics container health: PASS - upaas_container_healthy gauge
Prometheus metrics webhook events: PASS - upaas_webhook_events_total
Audit log table for user actions: PARTIAL FAIL - Core actions audited but env var, label, volume, port CRUD not audited despite constants defined

3. Build Result

docker build . passes - lint, fmt-check, test, build all green.

4. Additional Observations

No linter/CI/test config changes - clean.
Migration 007_add_audit_log.sql - correct for post-1.0.0 per policy.
No new dependencies - prometheus/client_golang was already in go.mod.
Test coverage for new code is thorough.
NewForTest() pattern for metrics isolation is well-designed.

5. Verdict: FAIL

Blocking issues:

Security: extractRemoteIP() trusts spoofable proxy headers from any client, allowing IP forgery in audit records
Incomplete implementation: 9 audit action constants defined but unused - corresponding user-facing operations lack audit instrumentation
README: Wrong API endpoint path (/api/audit vs actual /api/v1/audit)
README: Duplicate numbering in DI order section

## Code Review: PR #169 - Observability Improvements ### 1. Policy Divergences #### BLOCKING: Security - IP spoofing in audit logs extractRemoteIP() in internal/service/audit/audit.go unconditionally trusts X-Real-IP and X-Forwarded-For headers from any client without checking if the request comes from a trusted proxy. The existing realIP() function in internal/middleware/middleware.go correctly validates that the connecting IP is from a trusted proxy (RFC1918/loopback) before trusting these headers. The audit service reimplements IP extraction without this security check. This allows any direct client to forge their IP address in audit records by setting X-Real-IP: 1.2.3.4, undermining the integrity of the audit log. The audit service should reuse the middleware's trusted-proxy-aware logic (export realIP or pass the already-resolved IP from the handler layer). #### BLOCKING: Incomplete audit coverage - 9 dead audit action constants The following audit action constants are defined in internal/models/audit_log.go but never referenced anywhere in the codebase: - AuditActionEnvVarSave - AuditActionLabelAdd, AuditActionLabelEdit, AuditActionLabelDelete - AuditActionVolumeAdd, AuditActionVolumeEdit, AuditActionVolumeDelete - AuditActionPortAdd, AuditActionPortDelete The corresponding handler operations (HandleEnvVarSave, HandleLabelAdd, HandleLabelEdit, HandleLabelDelete, HandleVolumeAdd, HandleVolumeEdit, HandleVolumeDelete, HandlePortAdd, HandlePortDelete) exist in the routes but are not instrumented with audit logging. These are user-facing actions. The issue scope says "Audit log table for user actions" - these qualify. Either instrument them or remove the dead constants. #### BLOCKING: README inconsistency - Wrong API endpoint path README states the audit log is available at GET /api/audit but the actual route registered in internal/server/routes.go is under the /api/v1 group, making the real path GET /api/v1/audit. #### Minor: README - Duplicate numbering in DI order The Dependency Injection order section has duplicate numbers - items 10 and 11 appear twice (notify/audit then deploy/webhook). ### 2. Requirements Checklist (Issue #84) - Structured logging for all operations: PASS - HTTP middleware now logs response size; existing slog usage maintained - Prometheus metrics deployment count/duration: PASS - upaas_deployments_total, upaas_deployments_duration_seconds - Prometheus metrics container health: PASS - upaas_container_healthy gauge - Prometheus metrics webhook events: PASS - upaas_webhook_events_total - Audit log table for user actions: PARTIAL FAIL - Core actions audited but env var, label, volume, port CRUD not audited despite constants defined ### 3. Build Result docker build . passes - lint, fmt-check, test, build all green. ### 4. Additional Observations - No linter/CI/test config changes - clean. - Migration 007_add_audit_log.sql - correct for post-1.0.0 per policy. - No new dependencies - prometheus/client_golang was already in go.mod. - Test coverage for new code is thorough. - NewForTest() pattern for metrics isolation is well-designed. ### 5. Verdict: FAIL Blocking issues: 1. Security: extractRemoteIP() trusts spoofable proxy headers from any client, allowing IP forgery in audit records 2. Incomplete implementation: 9 audit action constants defined but unused - corresponding user-facing operations lack audit instrumentation 3. README: Wrong API endpoint path (/api/audit vs actual /api/v1/audit) 4. README: Duplicate numbering in DI order section

clawbot added 1 commit 2026-03-17 10:54:33 +01:00

fix: address review findings for observability PR

Check / check (pull_request) Successful in 1m42s

Details

ef109b9513

1. Security: Replace insecure extractRemoteIP() in audit service with
   middleware.RealIP() which validates trusted proxies before trusting
   X-Real-IP/X-Forwarded-For headers. Export RealIP from middleware.
   Update audit tests to verify anti-spoofing behavior.

2. Audit coverage: Add audit instrumentation to all 9 handlers that
   had dead action constants: HandleEnvVarSave, HandleLabelAdd,
   HandleLabelEdit, HandleLabelDelete, HandleVolumeAdd, HandleVolumeEdit,
   HandleVolumeDelete, HandlePortAdd, HandlePortDelete.

3. README: Fix API path from /api/audit to /api/v1/audit.

4. README: Fix duplicate numbering in DI order section (items 10-11
   were listed twice, now correctly numbered 10-16).

clawbot commented

2026-03-17 10:54:54 +01:00

Rework Summary

All 4 review findings have been fixed:

1. Security: IP spoofing in audit logs ✅

Exported RealIP() from internal/middleware/middleware.go (was realIP()).
Replaced the insecure extractRemoteIP() in internal/service/audit/audit.go with middleware.RealIP(), which validates trusted proxies (RFC1918/loopback) before trusting X-Real-IP/X-Forwarded-For headers.
Removed the vulnerable extractRemoteIP() function and its unused imports (net, strings).
Updated realip_test.go to reference the now-exported RealIP().
Replaced the original audit test (TestAuditServiceLogFromRequestWithXRealIP) with two new tests:
- TestAuditServiceLogFromRequestWithXRealIPTrustedProxy — verifies X-Real-IP is honoured when request comes from RFC1918 proxy.
- TestAuditServiceLogFromRequestWithXRealIPUntrustedProxy — verifies X-Real-IP is ignored when request comes from a public IP (anti-spoof).

2. Incomplete audit coverage — 9 dead action constants ✅

Added audit instrumentation to all 9 handlers:

HandleEnvVarSave → AuditActionEnvVarSave
HandleLabelAdd → AuditActionLabelAdd
HandleLabelEdit → AuditActionLabelEdit
HandleLabelDelete → AuditActionLabelDelete
HandleVolumeAdd → AuditActionVolumeAdd
HandleVolumeEdit → AuditActionVolumeEdit
HandleVolumeDelete → AuditActionVolumeDelete
HandlePortAdd → AuditActionPortAdd
HandlePortDelete → AuditActionPortDelete

All audit calls are placed after successful operations only (not on error paths), with descriptive detail strings.

3. README wrong API path ✅

Fixed GET /api/audit?limit=N → GET /api/v1/audit?limit=N.

4. README duplicate numbering ✅

Fixed DI order section: items 10-16 now numbered correctly (was 10, 11, 10, 11, 12, 13, 14).

make fmt ✅ | make test ✅ | docker build . ✅

## Rework Summary All 4 review findings have been fixed: ### 1. Security: IP spoofing in audit logs ✅ - Exported `RealIP()` from `internal/middleware/middleware.go` (was `realIP()`). - Replaced the insecure `extractRemoteIP()` in `internal/service/audit/audit.go` with `middleware.RealIP()`, which validates trusted proxies (RFC1918/loopback) before trusting `X-Real-IP`/`X-Forwarded-For` headers. - Removed the vulnerable `extractRemoteIP()` function and its unused imports (`net`, `strings`). - Updated `realip_test.go` to reference the now-exported `RealIP()`. - Replaced the original audit test (`TestAuditServiceLogFromRequestWithXRealIP`) with two new tests: - `TestAuditServiceLogFromRequestWithXRealIPTrustedProxy` — verifies X-Real-IP is honoured when request comes from RFC1918 proxy. - `TestAuditServiceLogFromRequestWithXRealIPUntrustedProxy` — verifies X-Real-IP is **ignored** when request comes from a public IP (anti-spoof). ### 2. Incomplete audit coverage — 9 dead action constants ✅ Added audit instrumentation to all 9 handlers: - `HandleEnvVarSave` → `AuditActionEnvVarSave` - `HandleLabelAdd` → `AuditActionLabelAdd` - `HandleLabelEdit` → `AuditActionLabelEdit` - `HandleLabelDelete` → `AuditActionLabelDelete` - `HandleVolumeAdd` → `AuditActionVolumeAdd` - `HandleVolumeEdit` → `AuditActionVolumeEdit` - `HandleVolumeDelete` → `AuditActionVolumeDelete` - `HandlePortAdd` → `AuditActionPortAdd` - `HandlePortDelete` → `AuditActionPortDelete` All audit calls are placed after successful operations only (not on error paths), with descriptive detail strings. ### 3. README wrong API path ✅ Fixed `GET /api/audit?limit=N` → `GET /api/v1/audit?limit=N`. ### 4. README duplicate numbering ✅ Fixed DI order section: items 10-16 now numbered correctly (was 10, 11, 10, 11, 12, 13, 14). --- `make fmt` ✅ | `make test` ✅ | `docker build .` ✅

clawbot commented

2026-03-17 10:59:26 +01:00

Code Review: PR #169 (Re-review) — Observability Improvements

1. Policy Compliance

No violations found.

Docker base images pinned by @sha256: ✅
Migration 007_add_audit_log.sql correctly numbered for post-1.0.0 ✅
.golangci.yml, Makefile, Dockerfile, CI config: untouched ✅
No secrets in code ✅
3 new nolint annotations are all justified (funlen on webhook handler expanded by audit calls, revive for metrics package naming vs runtime/metrics, mnd for Prometheus bucket boundaries) ✅
go.mod/go.sum properly maintained ✅

2. Previous Review Findings — Verification

#	Finding	Status	Evidence
1	IP spoofing via `extractRemoteIP()`	FIXED ✅	`extractRemoteIP()` deleted entirely. `audit.LogFromRequest()` now calls `middleware.RealIP()` which validates trusted proxies (RFC1918/loopback) before trusting `X-Real-IP`/`X-Forwarded-For`. Two new tests verify: trusted proxy honours header, untrusted proxy ignores it.
2	9 dead audit action constants	FIXED ✅	All 9 handlers instrumented: `HandleEnvVarSave`, `HandleLabelAdd/Edit/Delete`, `HandleVolumeAdd/Edit/Delete`, `HandlePortAdd/Delete`. All audit calls placed after successful operations only.
3	README wrong API path (`/api/audit`)	FIXED ✅	README now says `GET /api/v1/audit?limit=N`. Route registered at `r.Get("/audit", ...)` inside the `/api/v1` group in `routes.go`.
4	README duplicate DI numbering	FIXED ✅	DI order now numbered 1–16 sequentially, matching actual `fx.Provide` order in `cmd/upaasd/main.go`.

3. Requirements Checklist (Issue #84)

Requirement	Status	Implementation
Structured logging for all operations	✅ PASS	HTTP middleware logs response size (`bytes` field). All existing `slog` usage maintained. Audit events logged to stdout.
Prometheus metrics: deployment count/duration	✅ PASS	`upaas_deployments_total` (counter), `upaas_deployments_duration_seconds` (histogram), `upaas_deployments_in_flight` (gauge)
Prometheus metrics: container health	✅ PASS	`upaas_container_healthy` gauge set after health check in deploy service
Prometheus metrics: webhook events	✅ PASS	`upaas_webhook_events_total` counter with `app`/`event_type`/`matched` labels
Audit log table for user actions	✅ PASS	`audit_log` SQLite table via migration 007. All user-facing actions instrumented including login/logout, app CRUD, deploy/cancel/rollback, container start/stop/restart, env var/label/volume/port CRUD, webhook receive, initial setup.

4. Test Coverage

internal/metrics/metrics_test.go: 6 tests — constructor, all 9 metric families verified via registry gather ✅
internal/models/models_test.go: 7 new audit tests — create/find, all fields, find-by-resource, count, limit, ordering ✅
internal/service/audit/audit_test.go: 7 tests — Log, LogFromRequest, trusted proxy IP, untrusted proxy IP (anti-spoof), Recent, ForResource, optional fields ✅
Updated test infrastructure (handlers_test.go, webhook_test.go) correctly supplies metrics and audit dependencies ✅
internal/middleware/realip_test.go: updated to reference exported RealIP() ✅

5. Build Result

$ docker build .
✅ make fmt-check — PASS
✅ make lint — PASS  
✅ make test — PASS
✅ make build — PASS
Build completed successfully.

6. Additional Observations

NewForTest() pattern for metrics isolation prevents cross-test collisions — well designed
MetricsAuth() correctly passes through when no credentials configured, making /metrics always available
containerActionToAuditAction() helper with sensible fallback default case
HandleAPIAuditLog handler properly validates limit param (bounds checking, default fallback)
nullStringValue helper keeps JSON serialisation clean
Webhook handler audits directly via audit.LogFromRequest (no authenticated user context) — correct
No changes to linter config, CI, or test infrastructure integrity

7. Verdict: PASS ✅

All 4 previous findings are fixed correctly. All requirements from issue #84 are fully implemented. Code is clean, well-tested, and follows all repository policies. Build succeeds.

## Code Review: PR #169 (Re-review) — Observability Improvements ### 1. Policy Compliance **No violations found.** - Docker base images pinned by `@sha256:` ✅ - Migration `007_add_audit_log.sql` correctly numbered for post-1.0.0 ✅ - `.golangci.yml`, `Makefile`, `Dockerfile`, CI config: untouched ✅ - No secrets in code ✅ - 3 new `nolint` annotations are all justified (`funlen` on webhook handler expanded by audit calls, `revive` for metrics package naming vs `runtime/metrics`, `mnd` for Prometheus bucket boundaries) ✅ - `go.mod`/`go.sum` properly maintained ✅ ### 2. Previous Review Findings — Verification | # | Finding | Status | Evidence | |---|---------|--------|----------| | 1 | IP spoofing via `extractRemoteIP()` | **FIXED** ✅ | `extractRemoteIP()` deleted entirely. `audit.LogFromRequest()` now calls `middleware.RealIP()` which validates trusted proxies (RFC1918/loopback) before trusting `X-Real-IP`/`X-Forwarded-For`. Two new tests verify: trusted proxy honours header, untrusted proxy ignores it. | | 2 | 9 dead audit action constants | **FIXED** ✅ | All 9 handlers instrumented: `HandleEnvVarSave`, `HandleLabelAdd/Edit/Delete`, `HandleVolumeAdd/Edit/Delete`, `HandlePortAdd/Delete`. All audit calls placed after successful operations only. | | 3 | README wrong API path (`/api/audit`) | **FIXED** ✅ | README now says `GET /api/v1/audit?limit=N`. Route registered at `r.Get("/audit", ...)` inside the `/api/v1` group in `routes.go`. | | 4 | README duplicate DI numbering | **FIXED** ✅ | DI order now numbered 1–16 sequentially, matching actual `fx.Provide` order in `cmd/upaasd/main.go`. | ### 3. Requirements Checklist ([Issue #84](https://git.eeqj.de/sneak/upaas/issues/84)) | Requirement | Status | Implementation | |-------------|--------|----------------| | Structured logging for all operations | ✅ PASS | HTTP middleware logs response size (`bytes` field). All existing `slog` usage maintained. Audit events logged to stdout. | | Prometheus metrics: deployment count/duration | ✅ PASS | `upaas_deployments_total` (counter), `upaas_deployments_duration_seconds` (histogram), `upaas_deployments_in_flight` (gauge) | | Prometheus metrics: container health | ✅ PASS | `upaas_container_healthy` gauge set after health check in deploy service | | Prometheus metrics: webhook events | ✅ PASS | `upaas_webhook_events_total` counter with `app`/`event_type`/`matched` labels | | Audit log table for user actions | ✅ PASS | `audit_log` SQLite table via migration 007. All user-facing actions instrumented including login/logout, app CRUD, deploy/cancel/rollback, container start/stop/restart, env var/label/volume/port CRUD, webhook receive, initial setup. | ### 4. Test Coverage - `internal/metrics/metrics_test.go`: 6 tests — constructor, all 9 metric families verified via registry gather ✅ - `internal/models/models_test.go`: 7 new audit tests — create/find, all fields, find-by-resource, count, limit, ordering ✅ - `internal/service/audit/audit_test.go`: 7 tests — Log, LogFromRequest, trusted proxy IP, untrusted proxy IP (anti-spoof), Recent, ForResource, optional fields ✅ - Updated test infrastructure (`handlers_test.go`, `webhook_test.go`) correctly supplies metrics and audit dependencies ✅ - `internal/middleware/realip_test.go`: updated to reference exported `RealIP()` ✅ ### 5. Build Result ``` $ docker build . ✅ make fmt-check — PASS ✅ make lint — PASS ✅ make test — PASS ✅ make build — PASS Build completed successfully. ``` ### 6. Additional Observations - `NewForTest()` pattern for metrics isolation prevents cross-test collisions — well designed - `MetricsAuth()` correctly passes through when no credentials configured, making `/metrics` always available - `containerActionToAuditAction()` helper with sensible fallback default case - `HandleAPIAuditLog` handler properly validates limit param (bounds checking, default fallback) - `nullStringValue` helper keeps JSON serialisation clean - Webhook handler audits directly via `audit.LogFromRequest` (no authenticated user context) — correct - No changes to linter config, CI, or test infrastructure integrity ### 7. Verdict: **PASS** ✅ All 4 previous findings are fixed correctly. All requirements from [issue #84](https://git.eeqj.de/sneak/upaas/issues/84) are fully implemented. Code is clean, well-tested, and follows all repository policies. Build succeeds.

clawbot added the merge-ready label 2026-03-17 11:00:06 +01:00

sneak was assigned by clawbot

2026-03-17 11:00:06 +01:00

sneak commented

2026-03-20 06:46:05 +01:00

please fix conflict

clawbot force-pushed feature/observability-improvements from ef109b9513 to b593f544d2

2026-03-20 06:50:51 +01:00

Compare

clawbot commented

2026-03-20 06:51:11 +01:00

Rebase completed — conflicts resolved

Rebased feature/observability-improvements onto main to resolve conflicts introduced by PR #165 (CPU/memory resource limits).

Conflicts resolved

internal/models/models_test.go — Both branches added new test functions at the end of the file. Kept both: resource limits tests (from main/PR #165) and audit entry tests (from this PR). Added missing closing brace for TestAppResourceLimits that was lost in the conflict boundary.
internal/database/migrations/007_add_audit_log.sql → 008_add_audit_log.sql — PR #165 merged with migration 007_add_resource_limits.sql. Renumbered this PR's audit log migration from 007 to 008. The migration runner auto-discovers via embed.FS sorted by filename, so no code references needed updating.

Auto-merged (no conflicts)

README.md
internal/handlers/app.go
internal/service/deploy/deploy.go

Verification

make fmt ✅
docker build . ✅ (lint, all tests pass, binary builds)

## Rebase completed — conflicts resolved Rebased `feature/observability-improvements` onto `main` to resolve conflicts introduced by [PR #165](https://git.eeqj.de/sneak/upaas/pulls/165) (CPU/memory resource limits). ### Conflicts resolved 1. **`internal/models/models_test.go`** — Both branches added new test functions at the end of the file. Kept both: resource limits tests (from main/PR #165) and audit entry tests (from this PR). Added missing closing brace for `TestAppResourceLimits` that was lost in the conflict boundary. 2. **`internal/database/migrations/007_add_audit_log.sql` → `008_add_audit_log.sql`** — PR #165 merged with migration `007_add_resource_limits.sql`. Renumbered this PR's audit log migration from 007 to 008. The migration runner auto-discovers via `embed.FS` sorted by filename, so no code references needed updating. ### Auto-merged (no conflicts) - `README.md` - `internal/handlers/app.go` - `internal/service/deploy/deploy.go` ### Verification - `make fmt` ✅ - `docker build .` ✅ (lint, all tests pass, binary builds)

sneak closed this pull request

2026-03-22 00:48:50 +01:00

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/upaas#169

feat: add observability improvements (metrics, audit log, structured logging) #169

Summary

Prometheus Metrics (internal/metrics)

Audit Log

Structured Logging Enhancements

Integration Points

Tests

README

Code Review: PR #169 - Observability Improvements

1. Policy Divergences

BLOCKING: Security - IP spoofing in audit logs

BLOCKING: Incomplete audit coverage - 9 dead audit action constants

BLOCKING: README inconsistency - Wrong API endpoint path

Minor: README - Duplicate numbering in DI order

2. Requirements Checklist (Issue #84)

3. Build Result

4. Additional Observations

5. Verdict: FAIL

Rework Summary

1. Security: IP spoofing in audit logs ✅

2. Incomplete audit coverage — 9 dead action constants ✅

3. README wrong API path ✅

4. README duplicate numbering ✅

Code Review: PR #169 (Re-review) — Observability Improvements

1. Policy Compliance

2. Previous Review Findings — Verification

3. Requirements Checklist (Issue #84)

4. Test Coverage

5. Build Result

6. Additional Observations

7. Verdict: PASS ✅

Rebase completed — conflicts resolved

Conflicts resolved

Auto-merged (no conflicts)

Verification

Pull request closed

Prometheus Metrics (`internal/metrics`)