sneak/upaas
1
0
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

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

Closed
clawbot wants to merge 2 commits from feature/observability-improvements into main
pull from: feature/observability-improvements
merge into: sneak:main
Conversation 5 Commits 2 Files Changed 22 +1446 -55
clawbot commented 2026-03-17 10:24:18 +01:00
Collaborator
Copy Link

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)
All checks were successful
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
Author
Collaborator
Copy Link

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
## 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
All checks were successful
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
Author
Collaborator
Copy Link

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
Author
Collaborator
Copy Link

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
Owner
Copy Link

please fix conflict

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
Author
Collaborator
Copy Link

Rebase completed — conflicts resolved

Rebased feature/observability-improvements onto main to resolve conflicts introduced by PR #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)
## 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
All checks were successful
Check / check (pull_request) Successful in 3m10s
Required
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
bug
Something is not working
 duplicate
This issue or pull request already exists
 enhancement
New feature
 help wanted
Need some help
 invalid
Something is wrong
 merge-ready
All checks pass, reviewed, rebased, ready for merge
 needs-checks
make check does not pass cleanly
 needs-rebase
PR has merge conflicts or is behind main
 needs-review
Code review not yet done
 needs-rework
Code review found issues to fix
 notplanned
question
More information is needed
 wontfix
This won't be fixed
No Label merge-ready
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees sneak
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/upaas#169
Delete Branch "feature/observability-improvements"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?