feat: add taxonomy lifecycle observability

[BhagyaAmarasinghe]

Summary

• add bounded taxonomy lifecycle, duration, dispatch-failure, and reaper metrics
• emit correlated structured lifecycle logs with optional JSON formatting
• accept optional typed failure diagnostics and persist them in the existing run metrics JSON
• keep the public API and database schema unchanged

Verification

• golangci-lint run
• go test ./internal/observability ./internal/service ./internal/api/handlers ./internal/repository -count=1
• taxonomy persistence integration tests against a temporary Postgres instance

Rollout

This is PR 1 of 5 for ENG-1200. Merge and deploy this compatibility contract before the Taxonomy and GitOps changes.

[coderabbitai]

Walkthrough

The change adds configurable text or JSON logging for API and worker processes. It introduces taxonomy lifecycle metrics with bounded attributes. Taxonomy failure requests can include diagnostics that are serialized and persisted. Repository reaping returns run metadata instead of a count. The API passes taxonomy metrics to the service and reaper, which log reaped runs and record outcome and duration metrics. Tests cover logging, metric attributes, diagnostic persistence, and updated persistence contracts.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 36.84% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title uses Conventional Commits syntax and clearly identifies taxonomy lifecycle observability as the main change.
Description check	✅ Passed	The description covers the change, testing, rollout context, and linked issue, although it omits the template checklist.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

tests/taxonomy_persistence_test.go (1)

105-120: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Exercise the new metrics JSONB contract.

Lines 105-120 only call MarkRunFailed with nil, and the service test uses a mock repository. Add an integration case with known JSON. Assert the default {} and the non-empty value after MarkRunFailed and after GetRunForInternalService. This covers the new $4 binding and JSONB round trip.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/taxonomy_persistence_test.go` around lines 105 - 120, Extend the
integration coverage around MarkRunFailed to pass known metrics JSON instead of
only nil, then assert the persisted default {} for nil metrics and the exact
non-empty JSON for supplied metrics. Also call GetRunForInternalService and
verify the metrics value round-trips correctly, covering both MarkRunFailed’s $4
binding and JSONB persistence.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cmd/api/app.go`:
- Around line 953-977: Update FailStuckRuns and its reaped-run result to return
a per-run finished_at timestamp when each stale run is successfully
transitioned. In the reaper metrics loop around RecordRunDuration, calculate
duration from StartedAt or CreatedAt to that run’s finished_at, and clamp
negative durations to zero instead of using time.Now/time.Since after the batch
completes.

In `@internal/models/taxonomy.go`:
- Around line 259-273: Add validation for
TaxonomyRunFailureDiagnostics.PhaseDurations before the FailRun payload is
persisted to taxonomy_runs.metrics: permit only the approved taxonomy phase
keys, enforce a maximum number of entries, and reject negative duration values.
Reuse the repository’s validator configuration and add handler tests covering
valid keys, disallowed keys, excessive entries, and negative durations.

In `@tests/taxonomy_persistence_test.go`:
- Around line 168-170: Extend the assertions after repo.FailStuckRuns in the
taxonomy persistence test to locate the returned ReapedTaxonomyRun entries for
stuck.ID and pending.ID, then verify each entry’s tenant, scope, source, field,
StartedAt, and CreatedAt values against the corresponding fixtures. Keep the
existing length and error assertions.

---

Outside diff comments:
In `@tests/taxonomy_persistence_test.go`:
- Around line 105-120: Extend the integration coverage around MarkRunFailed to
pass known metrics JSON instead of only nil, then assert the persisted default
{} for nil metrics and the exact non-empty JSON for supplied metrics. Also call
GetRunForInternalService and verify the metrics value round-trips correctly,
covering both MarkRunFailed’s $4 binding and JSONB persistence.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: be9354a5-e8e6-47dd-86d4-915ae3118db3

📥 Commits

Reviewing files that changed from the base of the PR and between eec1a0b and 304e44f.

📒 Files selected for processing (17)

• .env.example
• cmd/api/app.go
• cmd/api/main.go
• cmd/worker/main.go
• internal/api/handlers/taxonomy_internal_handler.go
• internal/config/config.go
• internal/models/taxonomy.go
• internal/observability/aggregate.go
• internal/observability/logging.go
• internal/observability/logging_test.go
• internal/observability/names.go
• internal/observability/taxonomy.go
• internal/observability/taxonomy_test.go
• internal/repository/taxonomy_repository.go
• internal/service/taxonomy_service.go
• internal/service/taxonomy_service_test.go
• tests/taxonomy_persistence_test.go

[BhagyaAmarasinghe]

Addressed the outside-diff CodeRabbit request in 15f43df: the Postgres integration suite now verifies that nil failure metrics persist and round-trip as {}, and that a known non-empty diagnostics JSON value round-trips through both MarkRunFailed and GetRunForInternalService. The focused taxonomy API and persistence integration tests pass against pgvector/Postgres 18.

[xernobyl]

All three points are addressed, and the fixes look right to me:

• the second TraceContextHandler wrap is gone from NewApp, so SetupLogging is the single wrap point for both binaries now — and the count assertion added to logging_test.go keeps it from creeping back
• phase and phase_durations_seconds share one vocabulary
• hub_taxonomy_outcomes_total lines up with the other _outcomes_total counters

Checked locally at affdd30: build, vet and go test ./internal/... all green, and request_id now appears exactly once in the JSON output. The per-run finished_at for the reaper duration is a nicer fix than what I'd have suggested.

Approving 👍