Skip to content

feat: Phase 12 - Application Performance Monitoring and Security Governance - #224

Merged
seonghobae merged 5 commits into
masterfrom
feature/phase12-apms-and-security
May 24, 2026
Merged

seonghobae merged 5 commits into
masterfrom
feature/phase12-apms-and-security

Conversation

@seonghobae

@seonghobae seonghobae commented May 24, 2026 •

Copy link
Copy Markdown
Contributor

Description

Implements Phase 12 of the North Star Architecture.

Changes:

  • OpenTelemetry Instrumentation in FastAPI
  • Grafana, Prometheus, Loki, Tempo observability stack via docker-compose
  • Role-Based Access Control (RBAC) enforced on specific tenant_config endpoints
  • Strict handling for APM and RBAC tests

Summary by CodeRabbit

  • New Features

    • Added a global configuration API endpoint protected by admin-role checks.
  • Chores

    • Added distributed tracing service to observability stack and updated Prometheus/Grafana provisioning.
    • Minor development environment and ignore-file refinement.
    • Updated CI model identifier used by security gating.
  • Tests

    • Added tests ensuring admin-only access to the new endpoint and adjusted observability-related test expectations.
  • Documentation

    • Added Phase 11 and Phase 12 implementation plans and updated agent guidance.

Review Change Stack

Implement OpenTelemetry APM infrastructure and enforce RBAC for global tenant configuration as defined in phase 12 plan.
@coderabbitai

coderabbitai Bot commented May 24, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@seonghobae, we couldn't start this review because you've used your available PR reviews for now.

Your plan includes 5 reviews of capacity. Refill in 15 minutes and 33 seconds.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more review capacity refills, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a39bc924-9b38-4697-8cb0-e6c580688cba

📥 Commits

Reviewing files that changed from the base of the PR and between cb56ce8 and d189195.

⛔ Files ignored due to path filters (89)
  • backend/__pycache__/import_fixtures.cpython-310.pyc is excluded by !**/*.pyc
  • backend/__pycache__/main.cpython-310.pyc is excluded by !**/*.pyc
  • backend/api/__pycache__/__init__.cpython-310.pyc is excluded by !**/*.pyc
  • backend/api/__pycache__/accounts.cpython-310.pyc is excluded by !**/*.pyc
  • backend/api/__pycache__/auth.cpython-310.pyc is excluded by !**/*.pyc
  • backend/api/__pycache__/calendar.cpython-310.pyc is excluded by !**/*.pyc
  • backend/api/__pycache__/dav.cpython-310.pyc is excluded by !**/*.pyc
  • backend/api/__pycache__/emails.cpython-310.pyc is excluded by !**/*.pyc
  • backend/api/__pycache__/llm.cpython-310.pyc is excluded by !**/*.pyc
  • backend/api/__pycache__/llm_providers.cpython-310.pyc is excluded by !**/*.pyc
  • backend/api/__pycache__/network.cpython-310.pyc is excluded by !**/*.pyc
  • backend/api/__pycache__/ontology.cpython-310.pyc is excluded by !**/*.pyc
  • backend/api/__pycache__/prompts.cpython-310.pyc is excluded by !**/*.pyc
  • backend/api/__pycache__/runner_config.cpython-310.pyc is excluded by !**/*.pyc
  • backend/api/__pycache__/runner_ws.cpython-310.pyc is excluded by !**/*.pyc
  • backend/api/__pycache__/runtime_config.cpython-310.pyc is excluded by !**/*.pyc
  • backend/api/__pycache__/search.cpython-310.pyc is excluded by !**/*.pyc
  • backend/api/__pycache__/tasks.cpython-310.pyc is excluded by !**/*.pyc
  • backend/api/__pycache__/tenant_config.cpython-310.pyc is excluded by !**/*.pyc
  • backend/core/__pycache__/__init__.cpython-310.pyc is excluded by !**/*.pyc
  • backend/core/__pycache__/__init__.cpython-311.pyc is excluded by !**/*.pyc
  • backend/core/__pycache__/config.cpython-310.pyc is excluded by !**/*.pyc
  • backend/core/__pycache__/config.cpython-311.pyc is excluded by !**/*.pyc
  • backend/core/__pycache__/exceptions.cpython-310.pyc is excluded by !**/*.pyc
  • backend/db/__pycache__/__init__.cpython-310.pyc is excluded by !**/*.pyc
  • backend/db/__pycache__/__init__.cpython-311.pyc is excluded by !**/*.pyc
  • backend/db/__pycache__/models.cpython-310.pyc is excluded by !**/*.pyc
  • backend/db/__pycache__/models.cpython-311.pyc is excluded by !**/*.pyc
  • backend/db/__pycache__/session.cpython-310.pyc is excluded by !**/*.pyc
  • backend/scripts/__pycache__/bootstrap_db.cpython-310.pyc is excluded by !**/*.pyc
  • backend/scripts/__pycache__/import_fixtures.cpython-310.pyc is excluded by !**/*.pyc
  • backend/services/__pycache__/__init__.cpython-310.pyc is excluded by !**/*.pyc
  • backend/services/__pycache__/access_policy.cpython-310.pyc is excluded by !**/*.pyc
  • backend/services/__pycache__/archive.cpython-310.pyc is excluded by !**/*.pyc
  • backend/services/__pycache__/calendar_service.cpython-310.pyc is excluded by !**/*.pyc
  • backend/services/__pycache__/calendar_sync.cpython-310.pyc is excluded by !**/*.pyc
  • backend/services/__pycache__/email_client.cpython-310.pyc is excluded by !**/*.pyc
  • backend/services/__pycache__/email_parser.cpython-310.pyc is excluded by !**/*.pyc
  • backend/services/__pycache__/embedding.cpython-310.pyc is excluded by !**/*.pyc
  • backend/services/__pycache__/exceptions.cpython-310.pyc is excluded by !**/*.pyc
  • backend/services/__pycache__/imap_worker.cpython-310.pyc is excluded by !**/*.pyc
  • backend/services/__pycache__/knowledge_extractor.cpython-310.pyc is excluded by !**/*.pyc
  • backend/services/__pycache__/llm_provider_urls.cpython-310.pyc is excluded by !**/*.pyc
  • backend/services/__pycache__/llm_service.cpython-310.pyc is excluded by !**/*.pyc
  • backend/services/__pycache__/text_safety.cpython-310.pyc is excluded by !**/*.pyc
  • backend/services/__pycache__/threading_service.cpython-310.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/__init__.cpython-310.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/__init__.cpython-311.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/conftest.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_access_policy.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_accounts_api.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_apm_observability.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_archive.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_auth_real.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_bootstrap_db.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_calendar_api.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_calendar_service.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_calendar_sync.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_config.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_config.cpython-311-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_dav_api.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_db.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_email_client.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_email_client_smtp.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_email_parser.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_emails_api.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_embedding.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_imap_worker_sync.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_import_fixtures.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_infra_evaluations.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_knowledge_extractor.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_llm_api.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_llm_providers_api.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_llm_service.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_main.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_network_api.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_ontology_api.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_prompts_api.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_release_governance.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_repo_hygiene.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_runner_config_api.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_runtime_config_api.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_search.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_tasks_api.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_tenant_config_api.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_tenant_config_model.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_tenant_config_model.cpython-311-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_text_safety.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
  • backend/tests/__pycache__/test_threading_service.cpython-310-pytest-9.0.3.pyc is excluded by !**/*.pyc
📒 Files selected for processing (2)
  • docs/plans/2026-05-24-phase11-implementation.md
  • docs/plans/2026-05-24-phase12-apms-and-security.md

Note

.coderabbit.yaml has unrecognized properties

CodeRabbit is using all valid settings from your configuration. Unrecognized properties (listed below) have been ignored and may indicate typos or deprecated fields that can be removed.

⚠️ Parsing warnings (1)
Validation error: Unrecognized key: "version"
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
📝 Walkthrough

Walkthrough

Adds a role-protected GET /api/config/global endpoint with tests, updates observability docker-compose to use ./observability and add Tempo, updates Strix model identifiers in CI/docs, adds Phase 11/12 planning docs, and adds a duplicate .worktrees/ entry to .gitignore.

Changes

Phase 12 APM and Security Implementation

Layer / File(s) Summary
Global Config RBAC Endpoint
backend/api/tenant_config.py, backend/tests/test_tenant_config_api.py
Adds GET /api/config/global secured via get_current_user_role; returns 403 for non-admin roles and 200 with {"status": "ok", "global_settings": {}} for authorized roles. Tests cover both denial and success.
Observability Infrastructure Updates
docker-compose.infra.yml, backend/tests/test_apm_observability.py
Switches Prometheus and Grafana mounts to ./observability/, adds a tempo service (config ./observability/tempo.yaml, OTLP gRPC 4317, HTTP 3200), and updates the test to assert docker-compose.infra.yml exists.
Phase 11 and Phase 12 Planning Documentation
docs/plans/2026-05-24-phase11-implementation.md, docs/plans/2026-05-24-phase12-apms-and-security.md
Adds Phase 11 implementation plan and Phase 12 APM/security plan listing instrumentation, observability stack, and RBAC enforcement task breakdowns.
Strix model identifier update
.github/workflows/strix.yml, AGENTS.md
Replaces github_models/gpt-4o with github/gpt-4o in CI workflow steps and AGENTS.md.
Misc: .gitignore
.gitignore
Adds a duplicate .worktrees/ ignore entry near the end of the file.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 A global config endpoint springs to life,
Role-checked gates keep access free of strife,
Tempo and traces join the observability song,
Plans penned for phases to keep the work strong,
A tiny gitignore tweak hops along. 🥕✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and accurately summarizes the main changes: implementing Phase 12 with APM instrumentation and security governance (RBAC enforcement) across multiple files and services.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/phase12-apms-and-security

Comment @coderabbitai help to get the list of available commands and usage tips.

@seonghobae
seonghobae enabled auto-merge May 24, 2026 23:20
@github-actions

github-actions Bot commented May 24, 2026 •

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for d189195f83f9903b3c1f7f3e8eb33b0e2c6aa7ac:

  • Review decision is CHANGES_REQUESTED; address requested changes before merge.
  • Current-head CodeRabbit issue comment has blocking warning/failure evidence on d189195.
  • Current-head CodeRabbit review comment has blocking warning/failure evidence on d189195.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (2)
docs/plans/2026-05-24-phase11-implementation.md (1)

13-13: 💤 Low value

Optional: Fix markdown heading level increment.

The markdown linter flags that heading levels should only increment by one level at a time (jumping from h1 to h3 skips h2).

📝 Suggested fix
-### Task 1: Email Threading and Deduplication Pipeline
+## Task 1: Email Threading and Deduplication Pipeline

Apply the same change to all task headings (lines 13, 25, 37, 48).

🤖 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 `@docs/plans/2026-05-24-phase11-implementation.md` at line 13, The task
headings jump levels (skipping h2), so update each "### Task ..." heading to the
next correct level (e.g., change "### Task 1: Email Threading and Deduplication
Pipeline" and the other task headings to "## Task ...") so headings increment by
only one level at a time; apply the same change to all task headings referenced
(the other Task headings in the file).
docs/plans/2026-05-24-phase12-apms-and-security.md (1)

17-17: 💤 Low value

Optional: Fix markdown heading level increment.

The markdown linter flags that heading levels should only increment by one level at a time (jumping from h1 to h3 skips h2).

📝 Suggested fix
-### Task 1: OpenTelemetry Instrumentation
+## Task 1: OpenTelemetry Instrumentation

Apply the same change to all task headings (lines 17, 29, 42).

🤖 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 `@docs/plans/2026-05-24-phase12-apms-and-security.md` at line 17, The Task
headings currently jump levels (e.g., "### Task 1: OpenTelemetry
Instrumentation") and should increment by one level from the document root;
change each task heading (the lines showing "Task 1: OpenTelemetry
Instrumentation" and the other task headings referenced) from h3 (###) to h2
(##) so headings progress correctly (h1 → h2 → h3) and apply the same adjustment
to the other task headings in the file.
🤖 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 `@docs/plans/2026-05-24-phase11-implementation.md`:
- Around line 26-36: The task omits enforcing tenant and source linkage when
persisting email-derived ontology; update backend/services/ontology_service.py
(the function that saves the SenderRelationship returned by
analyze_sender_relationship and any code paths that read/write
SenderRelationship) to accept the DB session and persist the relationship
including explicit tenant_id and source_id/thread_id fields, ensure
process_self_to_self handling (the code that triggers the knowledge extraction
task) passes tenant and source context to the extractor, and update or implement
the API handler for /api/ontology/relationships to filter DAG results by
tenant_id and source_id so data remains scoped to the originating email/thread
and tenant owner.
- Line 35: Update Step 4 to explicitly require the default signed-session
authentication by stating that the new/updated /api/ontology/relationships
router must be registered with the get_auth_context dependency (signed-session)
so it uses the private backend auth pattern; mention the exact dependency name
get_auth_context and the endpoint /api/ontology/relationships in the plan text
so implementers know to apply the signed-session auth when registering the
router.
- Line 46: Update Step 3 to include verification that the /api/emails endpoint
remains protected by the default signed-session authentication: when exposing
requires_reply and schedule_conflict ensure the router handling /api/emails is
registered with the get_auth_context dependency (or equivalent signed-session
dependency) so the private backend rule is enforced; mention verifying
get_auth_context is present in the router registration and that any
controller/handler which returns requires_reply and schedule_conflict uses the
authenticated context.

---

Nitpick comments:
In `@docs/plans/2026-05-24-phase11-implementation.md`:
- Line 13: The task headings jump levels (skipping h2), so update each "### Task
..." heading to the next correct level (e.g., change "### Task 1: Email
Threading and Deduplication Pipeline" and the other task headings to "## Task
...") so headings increment by only one level at a time; apply the same change
to all task headings referenced (the other Task headings in the file).

In `@docs/plans/2026-05-24-phase12-apms-and-security.md`:
- Line 17: The Task headings currently jump levels (e.g., "### Task 1:
OpenTelemetry Instrumentation") and should increment by one level from the
document root; change each task heading (the lines showing "Task 1:
OpenTelemetry Instrumentation" and the other task headings referenced) from h3
(###) to h2 (##) so headings progress correctly (h1 → h2 → h3) and apply the
same adjustment to the other task headings in the file.
🪄 Autofix (Beta)

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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 622ae39e-2aab-4bab-bd1b-1fc9a41b066c

📥 Commits

Reviewing files that changed from the base of the PR and between 8a2e162 and a01760d.

📒 Files selected for processing (7)
  • .gitignore
  • backend/api/tenant_config.py
  • backend/tests/test_apm_observability.py
  • backend/tests/test_tenant_config_api.py
  • docker-compose.infra.yml
  • docs/plans/2026-05-24-phase11-implementation.md
  • docs/plans/2026-05-24-phase12-apms-and-security.md

Comment thread docs/plans/2026-05-24-phase11-implementation.md
Comment thread docs/plans/2026-05-24-phase11-implementation.md Outdated
Comment thread docs/plans/2026-05-24-phase11-implementation.md Outdated
…s-and-security

# Conflicts:
#	backend/__pycache__/main.cpython-310.pyc
#	backend/api/__pycache__/tenant_config.cpython-310.pyc
#	backend/db/__pycache__/models.cpython-310.pyc
#	backend/tests/__pycache__/test_apm_observability.cpython-310-pytest-9.0.3.pyc
#	backend/tests/__pycache__/test_tenant_config_api.cpython-310-pytest-9.0.3.pyc
@seonghobae

Copy link
Copy Markdown
Contributor Author

@coderabbitai resolve

@seonghobae
seonghobae merged commit ebf2608 into master May 24, 2026
14 of 16 checks passed
@coderabbitai

coderabbitai Bot commented May 24, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Comments resolved and changes approved.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant