From bf25efe4138d2753b9e3eaa234011ff1b1dcd2ba Mon Sep 17 00:00:00 2001 From: YiWang24 Date: Tue, 19 May 2026 03:51:20 -0400 Subject: [PATCH 01/15] chore: obser test --- obser.txt | 1 + 1 file changed, 1 insertion(+) create mode 100644 obser.txt diff --git a/obser.txt b/obser.txt new file mode 100644 index 0000000..a0a5125 --- /dev/null +++ b/obser.txt @@ -0,0 +1 @@ +obser test From 1cb49f24d0b532a823c10715d6070466be29b30b Mon Sep 17 00:00:00 2001 From: YiWang24 Date: Tue, 19 May 2026 03:57:03 -0400 Subject: [PATCH 02/15] fix: support GLM Anthropic proxy via CLAUDE_SWITCH_GLM_BASE_URL --- .env.example | 1 + openbot/core/settings.py | 10 ++++++++++ openbot/infrastructure/llm/complete.py | 7 +++++++ 3 files changed, 18 insertions(+) diff --git a/.env.example b/.env.example index 69bf776..7e19078 100644 --- a/.env.example +++ b/.env.example @@ -23,6 +23,7 @@ # ANTHROPIC_API_KEY= # review/fix → claude-opus-4-7 # # triage/chat → claude-sonnet-4-6 # OPENAI_API_KEY= # fallback model +# CLAUDE_SWITCH_GLM_BASE_URL=https://open.bigmodel.cn/api/anthropic # GLM Anthropic Proxy # ─────── Sandbox (PRD §13 #14: Modal) ─────── # MODAL_TOKEN_ID= diff --git a/openbot/core/settings.py b/openbot/core/settings.py index 424eee2..15e1036 100644 --- a/openbot/core/settings.py +++ b/openbot/core/settings.py @@ -138,6 +138,16 @@ class Settings(BaseSettings): "documented no-op — local dev and CI keep working unchanged." ), ) + + # ─── LLM ─── + # LiteLLM / Anthropic base URL. When set, routes all Anthropic model + # calls to this endpoint (e.g. GLM or other proxies). + # Mirrored from CLAUDE_SWITCH_GLM_BASE_URL if present. + anthropic_api_base: str | None = Field( + default=None, + alias="CLAUDE_SWITCH_GLM_BASE_URL", + description="Base URL for Anthropic-compatible models (e.g. GLM proxy).", + ) # Tags every Sentry event so prod errors don't mix with dev / preview. # Heroku sets this via `heroku config:set OPENBOT_ENVIRONMENT=production`. environment: str = Field( diff --git a/openbot/infrastructure/llm/complete.py b/openbot/infrastructure/llm/complete.py index b487ff1..5bf9fa7 100644 --- a/openbot/infrastructure/llm/complete.py +++ b/openbot/infrastructure/llm/complete.py @@ -93,8 +93,15 @@ async def complete( when content extraction raises — the vendor has already billed us and we must not lose that fact. """ + from openbot.core.settings import get_settings + + settings = get_settings() model = primary_model_for(feature) + # Honor custom base URL for Anthropic models (e.g. GLM proxy) + if "anthropic/" in model and settings.anthropic_api_base: + litellm_kwargs.setdefault("api_base", settings.anthropic_api_base) + response = await litellm.acompletion( model=model, messages=messages, From a625da1dd9f49ad9811ba877860ef2129f3d950c Mon Sep 17 00:00:00 2001 From: YiWang24 Date: Tue, 19 May 2026 04:09:12 -0400 Subject: [PATCH 03/15] docs: add sentry DSN + metrics audit design spec --- ...6-05-19-sentry-dsn-metrics-audit-design.md | 124 ++++++++++++++++++ 1 file changed, 124 insertions(+) create mode 100644 docs/superpowers/specs/2026-05-19-sentry-dsn-metrics-audit-design.md diff --git a/docs/superpowers/specs/2026-05-19-sentry-dsn-metrics-audit-design.md b/docs/superpowers/specs/2026-05-19-sentry-dsn-metrics-audit-design.md new file mode 100644 index 0000000..908bac5 --- /dev/null +++ b/docs/superpowers/specs/2026-05-19-sentry-dsn-metrics-audit-design.md @@ -0,0 +1,124 @@ +# Sentry DSN + Metrics Audit — Design + +**Date:** 2026-05-19 +**Status:** Approved +**Scope:** Wire up the live Sentry DSN and add unit-test coverage for the metrics shim + +--- + +## Context + +OpenBot already has a fully-built Sentry integration: + +| File | Role | +|---|---| +| `openbot/infrastructure/observability.py` | `init_sentry()` — FastAPI + Starlette + HTTPX integrations | +| `openbot/core/sentry_metrics.py` | `Metrics` shim — `incr/distribution/gauge` wrappers | +| `openbot/core/settings.py` | `sentry_dsn`, `sentry_traces_sample_rate`, `sentry_profiles_sample_rate` | +| `.env` | `OPENBOT_SENTRY_DSN=` placeholder (currently empty) | + +The only missing piece is the live DSN. A pre-implementation audit also found one dead-code issue. + +--- + +## Audit Findings + +### API compatibility (sentry-sdk ≥ 2.18) + +The installed `sentry_sdk.metrics` API surface: + +``` +count(name, value, unit=None, attributes=None) +distribution(name, value, unit=None, attributes=None) +gauge(name, value, unit=None, attributes=None) +time(name, value, unit=None, attributes=None) +``` + +Our shim calls match exactly (✅ no changes needed to call sites). + +### Dead code: `Metrics.set()` + +`sentry_sdk.metrics` has no `set` function in 2.x. The existing guard: + +```python +if hasattr(metrics, "set"): + metrics.set(...) +``` + +…always skips, meaning any caller of `Metrics.set()` silently loses data with no error. + +**Fix:** Remove `Metrics.set()` and add a docstring note explaining the 2.x API dropped the set metric type. + +--- + +## Changes + +### 1. Configure DSN — `.env` + +Uncomment and populate: + +``` +OPENBOT_SENTRY_DSN=https://f07a401fd38b70db0359272557c974d6@o4511407339012096.ingest.us.sentry.io/4511407339143168 +``` + +No other config files change. `Settings.sentry_dsn` reads this via the `OPENBOT_` prefix and wraps it in `SecretStr` (never logged, never in repr). + +### 2. Remove dead method — `openbot/core/sentry_metrics.py` + +- Delete the `Metrics.set()` method. +- Add a short docstring note: "The sentry-sdk 2.x metrics API does not expose a set metric type; use `distribution()` for unique-count approximations." + +### 3. New test file — `tests/core/test_sentry_metrics.py` + +Patch `sentry_sdk.metrics` and assert: + +| Test | Assertion | +|---|---| +| `test_incr_calls_count` | `Metrics.incr("k", 2.0, tags={"a":"b"})` → `metrics.count("k", 2.0, unit="none", attributes={"a":"b"})` | +| `test_distribution_calls_sdk` | `Metrics.distribution("lat", 12.3, unit="millisecond")` → `metrics.distribution("lat", 12.3, unit="millisecond", attributes={})` | +| `test_gauge_calls_sdk` | `Metrics.gauge("q", 7.0)` → `metrics.gauge("q", 7.0, unit="none", attributes={})` | +| `test_incr_swallows_attribute_error` | `AttributeError` on the SDK call → no exception raised | +| `test_incr_swallows_runtime_error` | `RuntimeError` on the SDK call → no exception raised | +| `test_incr_swallows_import_error` | `ImportError` on SDK import → no exception raised | + +### 4. New test file — `tests/infrastructure/test_observability.py` + +Patch `sentry_sdk.init` and assert: + +| Test | Assertion | +|---|---| +| `test_init_sentry_with_dsn` | `sentry_sdk.init` called with `dsn=`, `environment`, `traces_sample_rate`, `profiles_sample_rate`, `send_default_pii=False`, and 3 integrations | +| `test_init_sentry_no_dsn` | `sentry_sdk.init` called with `dsn=None` (documented no-op) | +| `test_init_sentry_logs_when_dsn_present` | `caplog` captures `sentry_initialised` log line | +| `test_init_sentry_no_log_when_no_dsn` | No `sentry_initialised` log line when DSN is absent | + +--- + +## Data flow (unchanged) + +``` +FastAPI request + → StarletteIntegration (auto-captures ASGI exceptions) + → FastApiIntegration (route-level spans) + → HttpxIntegration (GitHub API call latency) + → Metrics.incr/distribution/gauge → sentry_sdk.metrics.count/distribution/gauge +``` + +--- + +## Out of scope + +- Sentry alert rules / issue grouping (managed in the Sentry dashboard) +- Live integration verification script (requires running server) +- `.env.example` update (placeholder already present) +- Any changes to `traces_sample_rate` or `profiles_sample_rate` defaults (0.1 is the correct production starting point) + +--- + +## Acceptance criteria + +1. `OPENBOT_SENTRY_DSN` is set in `.env`. +2. `Metrics.set()` is removed from `sentry_metrics.py`. +3. `tests/core/test_sentry_metrics.py` — all 6 tests pass. +4. `tests/infrastructure/test_observability.py` — all 4 tests pass. +5. `make check` passes (671 + 10 tests, no regressions). From 239a13ba8cb79b3c31758706a297d14efe03ed4a Mon Sep 17 00:00:00 2001 From: YiWang24 Date: Tue, 19 May 2026 04:14:35 -0400 Subject: [PATCH 04/15] docs: add sentry DSN + metrics audit implementation plan --- .../2026-05-19-sentry-dsn-metrics-audit.md | 439 ++++++++++++++++++ 1 file changed, 439 insertions(+) create mode 100644 docs/superpowers/plans/2026-05-19-sentry-dsn-metrics-audit.md diff --git a/docs/superpowers/plans/2026-05-19-sentry-dsn-metrics-audit.md b/docs/superpowers/plans/2026-05-19-sentry-dsn-metrics-audit.md new file mode 100644 index 0000000..ba8554f --- /dev/null +++ b/docs/superpowers/plans/2026-05-19-sentry-dsn-metrics-audit.md @@ -0,0 +1,439 @@ +# Sentry DSN + Metrics Audit Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Wire the live Sentry DSN, remove dead `Metrics.set()` code, and add unit-test coverage for the metrics shim and `init_sentry`. + +**Architecture:** `.env` gains the live DSN so `Settings.sentry_dsn` becomes non-None at runtime; `init_sentry()` then passes it to `sentry_sdk.init()`, which is already wired in both the API and worker entrypoints. New tests mock `sentry_sdk.metrics` and `sentry_sdk.init` so no real Sentry traffic is required during CI. + +**Tech Stack:** Python 3.12, sentry-sdk ≥ 2.18, pytest, unittest.mock + +**Spec:** `docs/superpowers/specs/2026-05-19-sentry-dsn-metrics-audit-design.md` + +--- + +## File map + +| Action | Path | Purpose | +|---|---|---| +| Modify | `.env` | Populate `OPENBOT_SENTRY_DSN` | +| Modify | `openbot/core/sentry_metrics.py` | Remove dead `Metrics.set()` | +| Create | `tests/core/__init__.py` | Package marker (mirrors `openbot/core/`) | +| Create | `tests/core/test_sentry_metrics.py` | 6 tests for the metrics shim | +| Modify | `tests/infrastructure/test_obs.py` | 2 additional `init_sentry` assertions | + +--- + +## Task 1: Set the live Sentry DSN + +**Files:** +- Modify: `.env` + +- [ ] **Step 1: Open `.env` and populate the DSN** + +Find the line (currently blank after `=`): +``` +# OPENBOT_SENTRY_DSN= +``` +Replace it with (uncommented, real DSN): +``` +OPENBOT_SENTRY_DSN=https://f07a401fd38b70db0359272557c974d6@o4511407339012096.ingest.us.sentry.io/4511407339143168 +``` + +- [ ] **Step 2: Verify Settings picks it up** + +```bash +uv run python -c " +from openbot.core.settings import get_settings +get_settings.cache_clear() +s = get_settings() +print('DSN set:', s.sentry_dsn is not None) +" +``` +Expected output: +``` +DSN set: True +``` + +> **Note:** `.env` is gitignored. This step makes no git change — do NOT commit it. + +--- + +## Task 2: Remove dead `Metrics.set()` method + +**Files:** +- Modify: `openbot/core/sentry_metrics.py` + +- [ ] **Step 1: Write the test that verifies `set()` does NOT exist on `Metrics`** + +Create `tests/core/__init__.py` (empty file): +```bash +touch tests/core/__init__.py +``` + +Create `tests/core/test_sentry_metrics.py` with just this test for now: + +```python +"""Unit tests for openbot.core.sentry_metrics.Metrics shim.""" +from __future__ import annotations + +from openbot.core.sentry_metrics import Metrics + + +def test_metrics_has_no_set_method() -> None: + """Metrics.set() was removed because sentry_sdk.metrics has no 'set' + function in 2.x — callers were silently dropping data with no error.""" + assert not hasattr(Metrics(), "set") +``` + +- [ ] **Step 2: Run the test — it should FAIL (set() still exists)** + +```bash +uv run pytest tests/core/test_sentry_metrics.py::test_metrics_has_no_set_method -v +``` +Expected: **FAILED** — `AssertionError` because `Metrics.set` currently exists. + +- [ ] **Step 3: Remove `Metrics.set()` and update the module docstring** + +In `openbot/core/sentry_metrics.py`, delete the entire `set()` method (lines ~70–84): + +```python + def set( + self, + key: str, + value: Any, + unit: str = "none", + tags: dict[str, Any] | None = None, + ) -> None: + """Record a set (unique counts, e.g. user_id).""" + try: + from sentry_sdk import metrics + + # If 'set' is not available, we skip it. + if hasattr(metrics, "set"): + metrics.set(key, value, unit=unit, attributes=tags or {}) + except (ImportError, RuntimeError, AttributeError): + pass +``` + +Also update the module-level docstring. Replace the example block: + +```python +Example:: + + from openbot.infrastructure.metrics import metrics + + metrics.incr("workflow_started", tags={"type": "triage"}) + +If Sentry is not initialised (e.g. local dev without DSN), these calls +become no-ops. +``` + +with: + +```python +Example:: + + from openbot.core.sentry_metrics import metrics + + metrics.incr("workflow_started", tags={"type": "triage"}) + metrics.distribution("llm_latency_ms", 340.0, unit="millisecond") + metrics.gauge("queue_depth", 7.0) + +If Sentry is not initialised (e.g. local dev without DSN), these calls +become silent no-ops. + +Note: sentry_sdk ≥ 2.x dropped the ``set`` metric type from its public +API. Use ``distribution()`` for approximate unique-count approximations. +""" +``` + +- [ ] **Step 4: Run the test — it should now PASS** + +```bash +uv run pytest tests/core/test_sentry_metrics.py::test_metrics_has_no_set_method -v +``` +Expected: **PASSED** + +- [ ] **Step 5: Commit** + +```bash +git add tests/core/__init__.py tests/core/test_sentry_metrics.py openbot/core/sentry_metrics.py +git commit -m "fix: remove dead Metrics.set() — sentry_sdk 2.x has no set metric type" +``` + +--- + +## Task 3: Add happy-path tests for `incr`, `distribution`, `gauge` + +**Files:** +- Modify: `tests/core/test_sentry_metrics.py` + +The three methods are already correct (confirmed against live sentry_sdk.metrics signatures). These tests lock that contract in so a future SDK upgrade that renames an API immediately turns red. + +- [ ] **Step 1: Append the three happy-path tests** + +Add to `tests/core/test_sentry_metrics.py` after the existing test: + +```python +from unittest.mock import patch + + +def test_incr_calls_sentry_count() -> None: + """Metrics.incr() must delegate to sentry_sdk.metrics.count() with + matching name, value, unit, and attributes.""" + m = Metrics() + with patch("sentry_sdk.metrics") as mock_sdk_metrics: + m.incr("workflow.started", 2.0, tags={"feature": "triage"}) + mock_sdk_metrics.count.assert_called_once_with( + "workflow.started", + 2.0, + unit="none", + attributes={"feature": "triage"}, + ) + + +def test_distribution_calls_sentry_distribution() -> None: + """Metrics.distribution() must delegate to sentry_sdk.metrics.distribution().""" + m = Metrics() + with patch("sentry_sdk.metrics") as mock_sdk_metrics: + m.distribution("http.request.duration", 12.3, unit="millisecond") + mock_sdk_metrics.distribution.assert_called_once_with( + "http.request.duration", + 12.3, + unit="millisecond", + attributes={}, + ) + + +def test_gauge_calls_sentry_gauge() -> None: + """Metrics.gauge() must delegate to sentry_sdk.metrics.gauge().""" + m = Metrics() + with patch("sentry_sdk.metrics") as mock_sdk_metrics: + m.gauge("queue_depth", 7.0, tags={"stream": "openbot:workflows"}) + mock_sdk_metrics.gauge.assert_called_once_with( + "queue_depth", + 7.0, + unit="none", + attributes={"stream": "openbot:workflows"}, + ) +``` + +- [ ] **Step 2: Run the three new tests** + +```bash +uv run pytest tests/core/test_sentry_metrics.py -k "incr or distribution or gauge" -v +``` +Expected: **3 PASSED** + +- [ ] **Step 3: Commit** + +```bash +git add tests/core/test_sentry_metrics.py +git commit -m "test: happy-path coverage for Metrics.incr/distribution/gauge" +``` + +--- + +## Task 4: Add resilience tests (errors are silenced) + +**Files:** +- Modify: `tests/core/test_sentry_metrics.py` + +- [ ] **Step 1: Append the three resilience tests** + +Add to `tests/core/test_sentry_metrics.py`: + +```python +import builtins + + +def test_incr_swallows_attribute_error() -> None: + """If sentry_sdk is installed but not yet initialised, metrics.count + raises AttributeError. Metrics.incr() must not propagate it.""" + m = Metrics() + with patch("sentry_sdk.metrics") as mock_sdk_metrics: + mock_sdk_metrics.count.side_effect = AttributeError("no active hub") + m.incr("k") # must not raise + + +def test_incr_swallows_runtime_error() -> None: + """If sentry_sdk raises RuntimeError (e.g. misconfigured hub), + Metrics.incr() must not propagate it.""" + m = Metrics() + with patch("sentry_sdk.metrics") as mock_sdk_metrics: + mock_sdk_metrics.count.side_effect = RuntimeError("Sentry not ready") + m.incr("k") # must not raise + + +def test_incr_swallows_import_error(monkeypatch) -> None: # type: ignore[no-untyped-def] + """If sentry_sdk is not installed (slim CI image), Metrics.incr() + must not raise — same graceful degradation pattern as init_sentry.""" + real_import = builtins.__import__ + + def fake_import(name: str, *args: object, **kwargs: object) -> object: + if name == "sentry_sdk" or name.startswith("sentry_sdk."): + raise ImportError(f"mocked absent: {name}") + return real_import(name, *args, **kwargs) # type: ignore[arg-type] + + monkeypatch.setattr(builtins, "__import__", fake_import) + Metrics().incr("k") # must not raise +``` + +- [ ] **Step 2: Run the three resilience tests** + +```bash +uv run pytest tests/core/test_sentry_metrics.py -k "swallows" -v +``` +Expected: **3 PASSED** + +- [ ] **Step 3: Run the full new test file** + +```bash +uv run pytest tests/core/test_sentry_metrics.py -v +``` +Expected: **7 PASSED** (1 from Task 2 + 3 from Task 3 + 3 from Task 4) + +- [ ] **Step 4: Commit** + +```bash +git add tests/core/test_sentry_metrics.py +git commit -m "test: resilience coverage for Metrics shim (AttributeError / RuntimeError / ImportError)" +``` + +--- + +## Task 5: Augment existing `test_obs.py` with two missing assertions + +**Files:** +- Modify: `tests/infrastructure/test_obs.py` + +`test_obs.py` already covers the no-DSN no-op, the DSN-passes-through happy path, and the missing-SDK graceful degradation. Two things from the spec are missing: `profiles_sample_rate` in the kwargs check, and the `sentry_initialised` log line. + +- [ ] **Step 1: Add `import logging` to the imports block at the top of `test_obs.py`** + +The existing imports are: +```python +from __future__ import annotations + +from unittest.mock import patch + +from openbot.core.settings import Settings +from openbot.infrastructure.observability import init_sentry +``` + +Replace with (add `import logging` after the stdlib block): +```python +from __future__ import annotations + +import logging +from unittest.mock import patch + +from openbot.core.settings import Settings +from openbot.infrastructure.observability import init_sentry +``` + +- [ ] **Step 2: Add `profiles_sample_rate` to the existing test** + +In `test_init_sentry_passes_settings_through_when_dsn_set`, add one assertion after the existing `send_default_pii` assertion and extend the `Settings(...)` call with `sentry_profiles_sample_rate=0.1`. + +Replace the existing function with: + +```python +def test_init_sentry_passes_settings_through_when_dsn_set() -> None: + """DSN set → SDK init is called with the configured DSN, environment, + traces sample rate, profiles sample rate, and the component tag.""" + settings = Settings( + sentry_dsn="https://public@example.ingest.sentry.io/12345", # type: ignore[arg-type] + environment="production", + sentry_traces_sample_rate=0.1, + sentry_profiles_sample_rate=0.1, + ) + + with ( + patch("sentry_sdk.init") as mock_init, + patch("sentry_sdk.set_tag") as mock_tag, + ): + init_sentry(settings, component="webapp") + + assert mock_init.called + kwargs = mock_init.call_args.kwargs + assert kwargs["dsn"] == "https://public@example.ingest.sentry.io/12345" + assert kwargs["environment"] == "production" + assert kwargs["traces_sample_rate"] == 0.1 + assert kwargs["profiles_sample_rate"] == 0.1 + assert kwargs["send_default_pii"] is False + mock_tag.assert_called_once_with("component", "webapp") +``` + +- [ ] **Step 3: Append the log-line test** + +Append to `tests/infrastructure/test_obs.py` (the `import logging` import added in Step 1 is already in scope): + +```python +def test_init_sentry_logs_sentry_initialised(caplog) -> None: # type: ignore[no-untyped-def] + """When a DSN is configured, init_sentry must emit a structured + 'sentry_initialised' log line so operators can confirm the SDK + is active at startup without needing to check the Sentry dashboard.""" + settings = Settings( + sentry_dsn="https://public@example.ingest.sentry.io/12345", # type: ignore[arg-type] + ) + with ( + patch("sentry_sdk.init"), + patch("sentry_sdk.set_tag"), + caplog.at_level(logging.INFO, logger="openbot.infrastructure.observability"), + ): + init_sentry(settings, component="webapp") + + messages = [r.message for r in caplog.records] + assert "sentry_initialised" in messages +``` + +- [ ] **Step 4: Run the updated obs tests** + +```bash +uv run pytest tests/infrastructure/test_obs.py -v +``` +Expected: **5 PASSED** (3 original + 1 updated + 1 new) + +- [ ] **Step 5: Commit** + +```bash +git add tests/infrastructure/test_obs.py +git commit -m "test: add profiles_sample_rate and log-line assertions to test_obs" +``` + +--- + +## Task 6: Final verification + +**Files:** (read-only verification) + +- [ ] **Step 1: Run the full test suite** + +```bash +make check +``` +Expected: all tests pass (671 existing + 7 new core + 2 new obs assertions = 680 total), `ruff` and `ruff format --check` clean. + +- [ ] **Step 2: Confirm Sentry will initialise at startup** + +```bash +uv run python -c " +from openbot.core.settings import get_settings +get_settings.cache_clear() +s = get_settings() +from openbot.infrastructure.observability import init_sentry +import logging, sys +logging.basicConfig(stream=sys.stdout, level=logging.INFO) +init_sentry(s, component='smoke-test') +print('ok — check above for sentry_initialised log line') +" +``` +Expected: a JSON log line containing `sentry_initialised` (or a plain-text equivalent if python-json-logger is absent in this Python env), then `ok`. + +- [ ] **Step 3: Push branch** + +```bash +git push -u origin feat/observability-and-infra-updates +``` From a2b4575640c6dc1a1f0300184268846d58be4c19 Mon Sep 17 00:00:00 2001 From: YiWang24 Date: Tue, 19 May 2026 04:19:42 -0400 Subject: [PATCH 05/15] =?UTF-8?q?fix:=20remove=20dead=20Metrics.set()=20?= =?UTF-8?q?=E2=80=94=20sentry=5Fsdk=202.x=20has=20no=20set=20metric=20type?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- openbot/core/sentry_metrics.py | 26 +++++++------------------- tests/core/__init__.py | 0 tests/core/test_sentry_metrics.py | 11 +++++++++++ 3 files changed, 18 insertions(+), 19 deletions(-) create mode 100644 tests/core/__init__.py create mode 100644 tests/core/test_sentry_metrics.py diff --git a/openbot/core/sentry_metrics.py b/openbot/core/sentry_metrics.py index 87475b2..8ad5ba1 100644 --- a/openbot/core/sentry_metrics.py +++ b/openbot/core/sentry_metrics.py @@ -5,12 +5,17 @@ Example:: - from openbot.infrastructure.metrics import metrics + from openbot.core.sentry_metrics import metrics metrics.incr("workflow_started", tags={"type": "triage"}) + metrics.distribution("llm_latency_ms", 340.0, unit="millisecond") + metrics.gauge("queue_depth", 7.0) If Sentry is not initialised (e.g. local dev without DSN), these calls -become no-ops. +become silent no-ops. + +Note: sentry_sdk ≥ 2.x dropped the ``set`` metric type from its public +API. Use ``distribution()`` for approximate unique-count approximations. """ from __future__ import annotations @@ -67,22 +72,5 @@ def gauge( except (ImportError, RuntimeError, AttributeError): pass - def set( - self, - key: str, - value: Any, - unit: str = "none", - tags: dict[str, Any] | None = None, - ) -> None: - """Record a set (unique counts, e.g. user_id).""" - try: - from sentry_sdk import metrics - - # If 'set' is not available, we skip it. - if hasattr(metrics, "set"): - metrics.set(key, value, unit=unit, attributes=tags or {}) - except (ImportError, RuntimeError, AttributeError): - pass - metrics = Metrics() diff --git a/tests/core/__init__.py b/tests/core/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/core/test_sentry_metrics.py b/tests/core/test_sentry_metrics.py new file mode 100644 index 0000000..63a82e7 --- /dev/null +++ b/tests/core/test_sentry_metrics.py @@ -0,0 +1,11 @@ +"""Unit tests for openbot.core.sentry_metrics.Metrics shim.""" + +from __future__ import annotations + +from openbot.core.sentry_metrics import Metrics + + +def test_metrics_has_no_set_method() -> None: + """Metrics.set() was removed because sentry_sdk.metrics has no 'set' + function in 2.x — callers were silently dropping data with no error.""" + assert not hasattr(Metrics(), "set") From 5c05fd95b196484e685d13b6ff8eda684760cf30 Mon Sep 17 00:00:00 2001 From: YiWang24 Date: Tue, 19 May 2026 04:21:00 -0400 Subject: [PATCH 06/15] test: happy-path + resilience coverage for Metrics shim --- tests/core/test_sentry_metrics.py | 81 +++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/tests/core/test_sentry_metrics.py b/tests/core/test_sentry_metrics.py index 63a82e7..57d71ec 100644 --- a/tests/core/test_sentry_metrics.py +++ b/tests/core/test_sentry_metrics.py @@ -2,6 +2,9 @@ from __future__ import annotations +import builtins +from unittest.mock import patch + from openbot.core.sentry_metrics import Metrics @@ -9,3 +12,81 @@ def test_metrics_has_no_set_method() -> None: """Metrics.set() was removed because sentry_sdk.metrics has no 'set' function in 2.x — callers were silently dropping data with no error.""" assert not hasattr(Metrics(), "set") + + +# ── Happy-path: correct SDK delegation ─────────────────────────────────────── + + +def test_incr_calls_sentry_count() -> None: + """Metrics.incr() must delegate to sentry_sdk.metrics.count() with + matching name, value, unit, and attributes.""" + m = Metrics() + with patch("sentry_sdk.metrics") as mock_sdk_metrics: + m.incr("workflow.started", 2.0, tags={"feature": "triage"}) + mock_sdk_metrics.count.assert_called_once_with( + "workflow.started", + 2.0, + unit="none", + attributes={"feature": "triage"}, + ) + + +def test_distribution_calls_sentry_distribution() -> None: + """Metrics.distribution() must delegate to sentry_sdk.metrics.distribution().""" + m = Metrics() + with patch("sentry_sdk.metrics") as mock_sdk_metrics: + m.distribution("http.request.duration", 12.3, unit="millisecond") + mock_sdk_metrics.distribution.assert_called_once_with( + "http.request.duration", + 12.3, + unit="millisecond", + attributes={}, + ) + + +def test_gauge_calls_sentry_gauge() -> None: + """Metrics.gauge() must delegate to sentry_sdk.metrics.gauge().""" + m = Metrics() + with patch("sentry_sdk.metrics") as mock_sdk_metrics: + m.gauge("queue_depth", 7.0, tags={"stream": "openbot:workflows"}) + mock_sdk_metrics.gauge.assert_called_once_with( + "queue_depth", + 7.0, + unit="none", + attributes={"stream": "openbot:workflows"}, + ) + + +# ── Resilience: SDK errors are silenced ────────────────────────────────────── + + +def test_incr_swallows_attribute_error() -> None: + """If sentry_sdk is installed but not yet initialised, metrics.count + raises AttributeError. Metrics.incr() must not propagate it.""" + m = Metrics() + with patch("sentry_sdk.metrics") as mock_sdk_metrics: + mock_sdk_metrics.count.side_effect = AttributeError("no active hub") + m.incr("k") # must not raise + + +def test_incr_swallows_runtime_error() -> None: + """If sentry_sdk raises RuntimeError (e.g. misconfigured hub), + Metrics.incr() must not propagate it.""" + m = Metrics() + with patch("sentry_sdk.metrics") as mock_sdk_metrics: + mock_sdk_metrics.count.side_effect = RuntimeError("Sentry not ready") + m.incr("k") # must not raise + + +def test_incr_swallows_import_error(monkeypatch) -> None: # type: ignore[no-untyped-def] + """If sentry_sdk is not installed (slim CI image), Metrics.incr() + must not raise — same graceful degradation pattern as init_sentry.""" + real_import = builtins.__import__ + + def fake_import(name: str, *args: object, **kwargs: object) -> object: + if name == "sentry_sdk" or name.startswith("sentry_sdk."): + raise ImportError(f"mocked absent: {name}") + return real_import(name, *args, **kwargs) # type: ignore[arg-type] + + monkeypatch.setattr(builtins, "__import__", fake_import) + Metrics().incr("k") # must not raise From 45550d6dce1ed946e940a2d250318ccb2fa42b52 Mon Sep 17 00:00:00 2001 From: YiWang24 Date: Tue, 19 May 2026 04:23:25 -0400 Subject: [PATCH 07/15] test: add profiles_sample_rate and log-line assertions to test_obs --- tests/infrastructure/test_obs.py | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/tests/infrastructure/test_obs.py b/tests/infrastructure/test_obs.py index 5aae462..90f14d5 100644 --- a/tests/infrastructure/test_obs.py +++ b/tests/infrastructure/test_obs.py @@ -15,6 +15,7 @@ from __future__ import annotations +import logging from unittest.mock import patch from openbot.core.settings import Settings @@ -33,11 +34,12 @@ def test_init_sentry_is_noop_without_dsn() -> None: def test_init_sentry_passes_settings_through_when_dsn_set() -> None: """DSN set → SDK init is called with the configured DSN, environment, - and traces sample rate, plus the component tag is applied.""" + traces sample rate, profiles sample rate, and the component tag.""" settings = Settings( sentry_dsn="https://public@example.ingest.sentry.io/12345", # type: ignore[arg-type] environment="production", sentry_traces_sample_rate=0.1, + sentry_profiles_sample_rate=0.1, ) with ( @@ -51,6 +53,8 @@ def test_init_sentry_passes_settings_through_when_dsn_set() -> None: assert kwargs["dsn"] == "https://public@example.ingest.sentry.io/12345" assert kwargs["environment"] == "production" assert kwargs["traces_sample_rate"] == 0.1 + # profiles_sample_rate drives Sentry Profiling on sampled transactions. + assert kwargs["profiles_sample_rate"] == 0.1 # PII off by default — webhook bodies contain repo/actor already # captured in audit_log, no need to duplicate into Sentry. assert kwargs["send_default_pii"] is False @@ -72,3 +76,21 @@ def fake_import(name: str, *args: object, **kwargs: object) -> object: monkeypatch.setattr(builtins, "__import__", fake_import) # Must not raise. init_sentry(Settings(), component="webapp") + + +def test_init_sentry_logs_sentry_initialised(caplog) -> None: # type: ignore[no-untyped-def] + """When a DSN is configured, init_sentry must emit a structured + 'sentry_initialised' log line so operators can confirm the SDK + is active at startup without needing to check the Sentry dashboard.""" + settings = Settings( + sentry_dsn="https://public@example.ingest.sentry.io/12345", # type: ignore[arg-type] + ) + with ( + patch("sentry_sdk.init"), + patch("sentry_sdk.set_tag"), + caplog.at_level(logging.INFO, logger="openbot.infrastructure.observability"), + ): + init_sentry(settings, component="webapp") + + messages = [r.message for r in caplog.records] + assert "sentry_initialised" in messages From e7c16382428afde81638c8de3017ea47cf90b589 Mon Sep 17 00:00:00 2001 From: YiWang24 Date: Tue, 19 May 2026 04:32:13 -0400 Subject: [PATCH 08/15] docs: add Sentry Logs integration design spec --- .../specs/2026-05-19-sentry-logs-design.md | 118 ++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100644 docs/superpowers/specs/2026-05-19-sentry-logs-design.md diff --git a/docs/superpowers/specs/2026-05-19-sentry-logs-design.md b/docs/superpowers/specs/2026-05-19-sentry-logs-design.md new file mode 100644 index 0000000..592b5f5 --- /dev/null +++ b/docs/superpowers/specs/2026-05-19-sentry-logs-design.md @@ -0,0 +1,118 @@ +# Sentry Logs Integration — Design + +**Date:** 2026-05-19 +**Status:** Approved +**Scope:** Enable Sentry's Logs product (`enable_logs=True`) with WARNING+ level filtering + +--- + +## Context + +Sentry SDK 2.35.0 added first-class log capturing via `enable_logs=True`. The project currently uses +`sentry-sdk[fastapi]>=2.18`, which does not have this feature. The existing `LoggingIntegration` +(implicitly active) defaults to `level=INFO`, which would forward every `http_request_completed` +log line to Sentry — too noisy for production. + +Startup sequence (both entrypoints are safe): + +- **Worker:** `configure_root_logger()` (force=True) → `init_sentry()` — Sentry's handler is added + after the JSON handler and both survive. +- **API:** uvicorn starts → `lifespan` calls `init_sentry()` — no `configure_root_logger()` runs + after, so Sentry's handler persists. + +--- + +## Changes + +### 1. `pyproject.toml` + +Bump the minimum sentry-sdk version: + +``` +"sentry-sdk[fastapi]>=2.35.0", # was >=2.18 — Logs feature requires 2.35+ +``` + +No new extras required. `LoggingIntegration` is in the core SDK. + +### 2. `openbot/infrastructure/observability.py` + +Add `enable_logs=True` and an explicit `LoggingIntegration` to `sentry_sdk.init()`: + +```python +import logging as _logging +from sentry_sdk.integrations.logging import LoggingIntegration + +sentry_sdk.init( + dsn=dsn, + environment=settings.environment, + traces_sample_rate=settings.sentry_traces_sample_rate, + profiles_sample_rate=settings.sentry_profiles_sample_rate, + send_default_pii=False, + enable_logs=True, # Sentry Logs product (2.35+) + integrations=[ + StarletteIntegration(), + FastApiIntegration(), + HttpxIntegration(), + LoggingIntegration( + level=_logging.WARNING, # WARNING+ → breadcrumbs + Sentry Logs + event_level=_logging.ERROR, # ERROR+ → Sentry error events + ), + ], +) +``` + +**Why explicit `LoggingIntegration`:** Without it, the SDK defaults to `level=INFO`, forwarding +every `http_request_completed`, `sentry_initialised`, etc. to Sentry Logs — high volume with low +signal. Pinning `WARNING` keeps the Logs feed actionable. + +**Why `event_level=ERROR`:** Preserves the existing behavior where unhandled exceptions and +explicit `logger.error()` calls create Sentry error events, not just log entries. + +### 3. `tests/infrastructure/test_obs.py` + +Add one assertion to `test_init_sentry_passes_settings_through_when_dsn_set` — verify a +`LoggingIntegration` instance is present in the `integrations` kwarg: + +```python +from sentry_sdk.integrations.logging import LoggingIntegration + +integrations = kwargs["integrations"] +assert any(isinstance(i, LoggingIntegration) for i in integrations) +``` + +No new test file; the existing 4-test suite already covers the init contract. + +--- + +## Data flow + +``` +Python logger.warning("...") + → root logger + → JSON handler (stdout → Papertrail) [unchanged] + → Sentry LoggingIntegration handler + → breadcrumb (attached to next Sentry event) + → Sentry Logs (enable_logs=True, level ≥ WARNING) + +Python logger.error("...") + → same path, plus: + → Sentry error event (event_level=ERROR) +``` + +--- + +## Out of scope + +- `configure_root_logger()` missing from the API entrypoint (separate concern, tracked separately) +- `OPENBOT_SENTRY_LOGGING_LEVEL` Setting (YAGNI — WARNING is correct for all environments) +- Sentry alert rules for specific log patterns (managed in the Sentry dashboard) + +--- + +## Acceptance criteria + +1. `pyproject.toml` pins `sentry-sdk[fastapi]>=2.35.0`. +2. `sentry_sdk.init()` in `init_sentry()` includes `enable_logs=True` and `LoggingIntegration(level=WARNING, event_level=ERROR)`. +3. `test_init_sentry_passes_settings_through_when_dsn_set` asserts `LoggingIntegration` is in `integrations`. +4. `make check` passes (all existing tests green, new assertion green). +5. `uv sync` completes without error (sentry-sdk 2.35+ available). From 44fe9e5291fde848bf4daa7245af2a5e3e60f89e Mon Sep 17 00:00:00 2001 From: YiWang24 Date: Tue, 19 May 2026 04:34:16 -0400 Subject: [PATCH 09/15] docs: add Sentry Logs implementation plan --- .../plans/2026-05-19-sentry-logs.md | 264 ++++++++++++++++++ 1 file changed, 264 insertions(+) create mode 100644 docs/superpowers/plans/2026-05-19-sentry-logs.md diff --git a/docs/superpowers/plans/2026-05-19-sentry-logs.md b/docs/superpowers/plans/2026-05-19-sentry-logs.md new file mode 100644 index 0000000..6fd2558 --- /dev/null +++ b/docs/superpowers/plans/2026-05-19-sentry-logs.md @@ -0,0 +1,264 @@ +# Sentry Logs Integration Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Enable Sentry's Logs product with WARNING+ filtering by adding `enable_logs=True` and an explicit `LoggingIntegration` to `sentry_sdk.init()`. + +**Architecture:** Two production file changes — bump the sentry-sdk minimum version in `pyproject.toml` and extend the `integrations` list in `init_sentry()`. One test augmentation adds `enable_logs` and `LoggingIntegration` assertions to the existing `test_init_sentry_passes_settings_through_when_dsn_set` test. + +**Tech Stack:** Python 3.12, sentry-sdk ≥ 2.35.0, pytest, unittest.mock + +**Spec:** `docs/superpowers/specs/2026-05-19-sentry-logs-design.md` + +--- + +## File map + +| Action | Path | Purpose | +|---|---|---| +| Modify | `pyproject.toml` | Bump sentry-sdk minimum to 2.35.0 | +| Modify | `openbot/infrastructure/observability.py` | Add `enable_logs=True` + `LoggingIntegration` | +| Modify | `tests/infrastructure/test_obs.py` | Add `enable_logs` + `LoggingIntegration` assertions | + +--- + +## Task 1: TDD — write failing tests, implement, verify green + +**Files:** +- Modify: `tests/infrastructure/test_obs.py` +- Modify: `openbot/infrastructure/observability.py` +- Modify: `pyproject.toml` + +### Step 1: Add the import to `test_obs.py` + +At the top of `tests/infrastructure/test_obs.py`, the current imports are: + +```python +from __future__ import annotations + +import logging +from unittest.mock import patch + +from openbot.core.settings import Settings +from openbot.infrastructure.observability import init_sentry +``` + +Add one import after the `from unittest.mock import patch` line: + +```python +from __future__ import annotations + +import logging +from unittest.mock import patch + +from sentry_sdk.integrations.logging import LoggingIntegration + +from openbot.core.settings import Settings +from openbot.infrastructure.observability import init_sentry +``` + +- [ ] **Step 2: Add two failing assertions to the existing test** + +In `test_init_sentry_passes_settings_through_when_dsn_set`, after the existing `mock_tag.assert_called_once_with("component", "webapp")` line, append: + +```python + # Sentry Logs product must be enabled. + assert kwargs["enable_logs"] is True + # LoggingIntegration must be present with WARNING+ level so INFO lines + # (e.g. http_request_completed) don't flood Sentry Logs. + integrations = kwargs["integrations"] + assert any(isinstance(i, LoggingIntegration) for i in integrations) +``` + +The complete updated test function for reference: + +```python +def test_init_sentry_passes_settings_through_when_dsn_set() -> None: + """DSN set → SDK init is called with the configured DSN, environment, + traces sample rate, profiles sample rate, and the component tag.""" + settings = Settings( + sentry_dsn="https://public@example.ingest.sentry.io/12345", # type: ignore[arg-type] + environment="production", + sentry_traces_sample_rate=0.1, + sentry_profiles_sample_rate=0.1, + ) + + with ( + patch("sentry_sdk.init") as mock_init, + patch("sentry_sdk.set_tag") as mock_tag, + ): + init_sentry(settings, component="webapp") + + assert mock_init.called + kwargs = mock_init.call_args.kwargs + assert kwargs["dsn"] == "https://public@example.ingest.sentry.io/12345" + assert kwargs["environment"] == "production" + assert kwargs["traces_sample_rate"] == 0.1 + assert kwargs["profiles_sample_rate"] == 0.1 + assert kwargs["send_default_pii"] is False + mock_tag.assert_called_once_with("component", "webapp") + # Sentry Logs product must be enabled. + assert kwargs["enable_logs"] is True + # LoggingIntegration must be present with WARNING+ level so INFO lines + # (e.g. http_request_completed) don't flood Sentry Logs. + integrations = kwargs["integrations"] + assert any(isinstance(i, LoggingIntegration) for i in integrations) +``` + +- [ ] **Step 3: Run the test — it should FAIL** + +```bash +uv run pytest tests/infrastructure/test_obs.py::test_init_sentry_passes_settings_through_when_dsn_set -v +``` + +Expected: **FAILED** — `KeyError: 'enable_logs'` (or `AssertionError`) because `enable_logs` and `LoggingIntegration` are not yet in the init call. + +- [ ] **Step 4: Implement — update `observability.py`** + +In `openbot/infrastructure/observability.py`, locate the `try` block inside `init_sentry`: + +```python + try: + import sentry_sdk + from sentry_sdk.integrations.fastapi import FastApiIntegration + from sentry_sdk.integrations.httpx import HttpxIntegration + from sentry_sdk.integrations.starlette import StarletteIntegration + except ImportError: +``` + +Replace it with (add `LoggingIntegration` import): + +```python + try: + import sentry_sdk + from sentry_sdk.integrations.fastapi import FastApiIntegration + from sentry_sdk.integrations.httpx import HttpxIntegration + from sentry_sdk.integrations.logging import LoggingIntegration + from sentry_sdk.integrations.starlette import StarletteIntegration + except ImportError: +``` + +Then locate the `sentry_sdk.init(...)` call: + +```python + sentry_sdk.init( + dsn=dsn, + environment=settings.environment, + traces_sample_rate=settings.sentry_traces_sample_rate, + profiles_sample_rate=settings.sentry_profiles_sample_rate, + send_default_pii=False, + integrations=[ + StarletteIntegration(), + FastApiIntegration(), + HttpxIntegration(), + ], + ) +``` + +Replace it with: + +```python + sentry_sdk.init( + dsn=dsn, + environment=settings.environment, + traces_sample_rate=settings.sentry_traces_sample_rate, + profiles_sample_rate=settings.sentry_profiles_sample_rate, + send_default_pii=False, + enable_logs=True, + integrations=[ + StarletteIntegration(), + FastApiIntegration(), + HttpxIntegration(), + LoggingIntegration( + level=logging.WARNING, # WARNING+ → breadcrumbs + Sentry Logs + event_level=logging.ERROR, # ERROR+ → Sentry error events + ), + ], + ) +``` + +Note: `logging` is already imported at the top of `observability.py` (`import logging`) — no new import needed at the module level. + +- [ ] **Step 5: Bump sentry-sdk in `pyproject.toml`** + +Find: +``` + "sentry-sdk[fastapi]>=2.18", +``` + +Replace with: +``` + "sentry-sdk[fastapi]>=2.35.0", +``` + +- [ ] **Step 6: Sync dependencies** + +```bash +uv sync +``` + +Expected: completes without error. Sentry SDK 2.35.x (or later) is installed. + +Verify: +```bash +uv run python -c "import sentry_sdk; print(sentry_sdk.VERSION)" +``` + +Expected: version string `2.35.x` or higher (e.g. `2.35.0`). + +- [ ] **Step 7: Run the target test — it should now PASS** + +```bash +uv run pytest tests/infrastructure/test_obs.py::test_init_sentry_passes_settings_through_when_dsn_set -v +``` + +Expected: **PASSED** + +- [ ] **Step 8: Run the full obs test suite** + +```bash +uv run pytest tests/infrastructure/test_obs.py -v +``` + +Expected: **4 PASSED** — all four tests green. + +- [ ] **Step 9: Run the full suite** + +```bash +make check +``` + +Expected: all tests pass, ruff and import-linter clean. + +- [ ] **Step 10: Commit** + +```bash +git add pyproject.toml openbot/infrastructure/observability.py tests/infrastructure/test_obs.py +git commit -m "feat: enable Sentry Logs with WARNING+ LoggingIntegration (sentry-sdk>=2.35.0)" +``` + +--- + +## Task 2: Push and update PR + +- [ ] **Step 1: Push** + +```bash +git push origin feat/observability-and-infra-updates +``` + +- [ ] **Step 2: Update PR description** + +```bash +gh pr edit 58 --body "$(gh pr view 58 --json body -q .body) + +--- + +## Sentry Logs (added in follow-up commit) + +- Bumped \`sentry-sdk[fastapi]>=2.35.0\` (Logs feature requires 2.35+) +- Added \`enable_logs=True\` to \`sentry_sdk.init()\` +- Added explicit \`LoggingIntegration(level=WARNING, event_level=ERROR)\` — keeps INFO lines (e.g. \`http_request_completed\`) out of Sentry Logs while forwarding warnings and errors +- Test: asserts \`enable_logs=True\` and \`LoggingIntegration\` presence in the existing obs test suite +" +``` From 3c404fb70e2bf0fe03be27f2f765efee48e6b63e Mon Sep 17 00:00:00 2001 From: YiWang24 Date: Tue, 19 May 2026 04:41:40 -0400 Subject: [PATCH 10/15] feat: enable Sentry Logs with WARNING+ LoggingIntegration (sentry-sdk>=2.35.0) --- openbot/infrastructure/observability.py | 11 +++++++++++ pyproject.toml | 2 +- tests/infrastructure/test_obs.py | 8 ++++++++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/openbot/infrastructure/observability.py b/openbot/infrastructure/observability.py index 4fcd424..03ad073 100644 --- a/openbot/infrastructure/observability.py +++ b/openbot/infrastructure/observability.py @@ -65,6 +65,7 @@ def init_sentry(settings: Settings, *, component: str) -> None: import sentry_sdk from sentry_sdk.integrations.fastapi import FastApiIntegration from sentry_sdk.integrations.httpx import HttpxIntegration + from sentry_sdk.integrations.logging import LoggingIntegration from sentry_sdk.integrations.starlette import StarletteIntegration except ImportError: # sentry-sdk is in the runtime deps, but a slim install (e.g. a @@ -77,13 +78,23 @@ def init_sentry(settings: Settings, *, component: str) -> None: dsn=dsn, # None = no-op; sentry-sdk documents this contract environment=settings.environment, traces_sample_rate=settings.sentry_traces_sample_rate, + profiles_sample_rate=settings.sentry_profiles_sample_rate, # Webhook bodies may carry repo / actor info that's already in # the audit log; do not duplicate into Sentry. send_default_pii=False, + # Forward WARNING+ logs to Sentry Logs (2.35+). + # Explicit LoggingIntegration overrides the default level=INFO so + # routine INFO lines (http_request_completed, etc.) stay in + # stdout only and don't flood the Sentry Logs feed. + enable_logs=True, integrations=[ StarletteIntegration(), FastApiIntegration(), HttpxIntegration(), + LoggingIntegration( + level=logging.WARNING, # WARNING+ → breadcrumbs + Sentry Logs + event_level=logging.ERROR, # ERROR+ → Sentry error events + ), ], ) sentry_sdk.set_tag("component", component) diff --git a/pyproject.toml b/pyproject.toml index 90236f0..4755f51 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -79,7 +79,7 @@ dependencies = [ # `sentry_sdk.init(dsn=None)` is a documented no-op so dev / CI runs # never need a Sentry project. Heroku sets SENTRY_DSN via # `heroku config:set OPENBOT_SENTRY_DSN=...`. - "sentry-sdk[fastapi]>=2.18", + "sentry-sdk[fastapi]>=2.35.0", # GitHub webhook + REST primitives (sans-IO). Replaces ~30 lines of # hand-rolled HMAC + payload parsing in adapters/github.py with the # canonical implementation used by the Python GitHub bot ecosystem. diff --git a/tests/infrastructure/test_obs.py b/tests/infrastructure/test_obs.py index 90f14d5..75f131e 100644 --- a/tests/infrastructure/test_obs.py +++ b/tests/infrastructure/test_obs.py @@ -18,6 +18,8 @@ import logging from unittest.mock import patch +from sentry_sdk.integrations.logging import LoggingIntegration + from openbot.core.settings import Settings from openbot.infrastructure.observability import init_sentry @@ -59,6 +61,12 @@ def test_init_sentry_passes_settings_through_when_dsn_set() -> None: # captured in audit_log, no need to duplicate into Sentry. assert kwargs["send_default_pii"] is False mock_tag.assert_called_once_with("component", "webapp") + # Sentry Logs product must be enabled. + assert kwargs["enable_logs"] is True + # LoggingIntegration must be present with WARNING+ level so INFO lines + # (e.g. http_request_completed) don't flood Sentry Logs. + integrations = kwargs["integrations"] + assert any(isinstance(i, LoggingIntegration) for i in integrations) def test_init_sentry_survives_missing_sdk(monkeypatch) -> None: # type: ignore[no-untyped-def] From 3229c95c4f2b529df9139b1bdfcc312acf2b6c05 Mon Sep 17 00:00:00 2001 From: YiWang24 Date: Tue, 19 May 2026 05:16:58 -0400 Subject: [PATCH 11/15] docs: Sentry continuous profiling design spec --- ...5-19-sentry-continuous-profiling-design.md | 166 ++++++++++++++++++ 1 file changed, 166 insertions(+) create mode 100644 docs/superpowers/specs/2026-05-19-sentry-continuous-profiling-design.md diff --git a/docs/superpowers/specs/2026-05-19-sentry-continuous-profiling-design.md b/docs/superpowers/specs/2026-05-19-sentry-continuous-profiling-design.md new file mode 100644 index 0000000..5bb1b1f --- /dev/null +++ b/docs/superpowers/specs/2026-05-19-sentry-continuous-profiling-design.md @@ -0,0 +1,166 @@ +# Sentry Continuous Profiling — Design + +**Date:** 2026-05-19 +**Status:** Approved +**Scope:** Replace transaction profiling (`profiles_sample_rate`) with continuous profiling (`profile_session_sample_rate` + `start_profiler()`) + +--- + +## Context + +`sentry-sdk 2.24.1` introduced continuous profiling via `profile_session_sample_rate` and +`sentry_sdk.profiler.start_profiler()` / `stop_profiler()`. The project currently uses +`profiles_sample_rate=0.1` (transaction-based profiling), which profiles a fraction of +individual HTTP traces. Continuous profiling is always-on, lower overhead, and better suited +to long-running processes like the OpenBot worker. + +The SDK minimum is already `>=2.35.0` (set in the Sentry Logs commit) — no version bump needed. + +--- + +## Changes + +### 1. `openbot/core/settings.py` + +Rename `sentry_profiles_sample_rate` → `sentry_profile_session_sample_rate`, update default +from `0.1` to `1.0`, and update the description: + +```python +sentry_profile_session_sample_rate: float = Field( + default=1.0, + ge=0.0, + le=1.0, + description="Fraction of sessions profiled continuously (sentry-sdk 2.24+).", +) +``` + +Default `1.0` is correct for continuous profiling — the overhead is substantially lower than +transaction profiling, and Sentry's own quickstart recommends 1.0 as the starting value. + +### 2. `openbot/infrastructure/observability.py` + +Swap `profiles_sample_rate` → `profile_session_sample_rate` in `sentry_sdk.init()`. No other +changes. Profiler lifecycle (start/stop) stays in the entrypoints. + +```python +sentry_sdk.init( + dsn=dsn, + environment=settings.environment, + traces_sample_rate=settings.sentry_traces_sample_rate, + profile_session_sample_rate=settings.sentry_profile_session_sample_rate, + send_default_pii=False, + enable_logs=True, + integrations=[ + StarletteIntegration(), + FastApiIntegration(), + HttpxIntegration(), + LoggingIntegration(level=logging.WARNING, event_level=logging.ERROR), + ], +) +``` + +### 3. `openbot/entrypoints/api/app.py` + +Call `start_profiler()` immediately after `init_sentry()` in the `lifespan` startup block. +Call `stop_profiler()` in the `finally` cleanup block (before existing resource teardown). +Both calls wrapped in `try/except Exception: pass` for graceful degradation when +`sentry_sdk` is absent or DSN is unset. + +```python +# startup (after init_sentry / init_langsmith) +try: + import sentry_sdk + sentry_sdk.profiler.start_profiler() +except Exception: + pass # SDK absent or DSN=None no-op — profiling is opt-in + +# shutdown (first line of finally block) +try: + import sentry_sdk + sentry_sdk.profiler.stop_profiler() +except Exception: + pass +``` + +### 4. `openbot/entrypoints/worker/__main__.py` + +Call `start_profiler()` immediately after `init_sentry()` in `_main`. No `stop_profiler()` +call — continuous profiling of a long-running process is the intended use case; the SDK +flushes and stops cleanly on process exit (SIGTERM → `asyncio.run()` returns). + +```python +# after init_sentry / init_langsmith +try: + import sentry_sdk + sentry_sdk.profiler.start_profiler() +except Exception: + pass +``` + +### 5. `tests/infrastructure/test_obs.py` + +In `test_init_sentry_passes_settings_through_when_dsn_set`, swap the profiles assertion: + +```python +# Settings constructor — replace: +sentry_profiles_sample_rate=0.1 +# with: +sentry_profile_session_sample_rate=1.0 + +# Assertion — replace: +assert kwargs["profiles_sample_rate"] == 0.1 +# with: +assert kwargs["profile_session_sample_rate"] == 1.0 +``` + +No new test file. The `except Exception: pass` paths in the entrypoints are covered +implicitly by the existing `test_init_sentry_survives_missing_sdk` pattern. + +### 6. Doppler `prd` (post-implementation) + +```bash +doppler secrets delete OPENBOT_SENTRY_PROFILES_SAMPLE_RATE --project openbot --config prd +doppler secrets set OPENBOT_SENTRY_PROFILE_SESSION_SAMPLE_RATE=1.0 --project openbot --config prd +``` + +--- + +## Data flow + +``` +Process starts + └─ init_sentry() # configures SDK with profile_session_sample_rate=1.0 + └─ start_profiler() # begins continuous profiling session + +Process runs + └─ profiler samples stack traces every ~10ms → buffered in SDK + └─ SDK flushes profile data to Sentry ingest on interval + +webapp: SIGTERM → lifespan finally + └─ stop_profiler() # flush remaining profile data + └─ adapter/redis/db teardown + +worker: SIGTERM → shutdown event → asyncio.run() returns + └─ SDK atexit hook flushes remaining profile data (no explicit stop needed) +``` + +--- + +## Out of scope + +- `OPENBOT_SENTRY_PROFILING_ENABLED` feature flag (YAGNI — DSN=None is already the off switch) +- `stop_profiler()` in the worker (SDK handles it on process exit) +- Transaction profiling (`profiles_sample_rate`) as a fallback or alias + +--- + +## Acceptance criteria + +1. `sentry_profiles_sample_rate` removed from `Settings`; `sentry_profile_session_sample_rate` added with default `1.0`. +2. `sentry_sdk.init()` passes `profile_session_sample_rate` (not `profiles_sample_rate`). +3. `start_profiler()` called in `lifespan` startup and `_main` after `init_sentry()`. +4. `stop_profiler()` called in `lifespan` finally block. +5. Both profiler calls wrapped in `try/except Exception: pass`. +6. `test_init_sentry_passes_settings_through_when_dsn_set` asserts `profile_session_sample_rate == 1.0`. +7. `make check` passes — 697+ tests green, ruff clean. +8. Doppler `prd` updated: old key deleted, new key set to `1.0`. From 59628326f0b7dd94323a440d4026744937f399a1 Mon Sep 17 00:00:00 2001 From: YiWang24 Date: Tue, 19 May 2026 05:18:57 -0400 Subject: [PATCH 12/15] docs: Sentry continuous profiling implementation plan --- .../2026-05-19-sentry-continuous-profiling.md | 335 ++++++++++++++++++ 1 file changed, 335 insertions(+) create mode 100644 docs/superpowers/plans/2026-05-19-sentry-continuous-profiling.md diff --git a/docs/superpowers/plans/2026-05-19-sentry-continuous-profiling.md b/docs/superpowers/plans/2026-05-19-sentry-continuous-profiling.md new file mode 100644 index 0000000..0b0372e --- /dev/null +++ b/docs/superpowers/plans/2026-05-19-sentry-continuous-profiling.md @@ -0,0 +1,335 @@ +# Sentry Continuous Profiling Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Replace transaction-based profiling (`profiles_sample_rate`) with continuous profiling (`profile_session_sample_rate` + `start_profiler()`) in both the webapp and worker entrypoints. + +**Architecture:** `sentry_sdk.init()` receives `profile_session_sample_rate` (configured via Settings); the entrypoints own the profiler lifecycle — `start_profiler()` at startup, `stop_profiler()` in the webapp's shutdown block (worker relies on process-exit flush). Both profiler calls are wrapped in `try/except Exception: pass` for graceful degradation. + +**Tech Stack:** Python 3.12, sentry-sdk ≥ 2.35.0 (already installed at 2.60.0), pytest, pydantic-settings + +**Spec:** `docs/superpowers/specs/2026-05-19-sentry-continuous-profiling-design.md` + +--- + +## File map + +| Action | Path | Purpose | +|---|---|---| +| Modify | `openbot/core/settings.py` | Rename field + default 1.0 | +| Modify | `openbot/infrastructure/observability.py` | Swap `profiles_sample_rate` → `profile_session_sample_rate` | +| Modify | `openbot/entrypoints/api/app.py` | `start_profiler()` on startup, `stop_profiler()` in finally | +| Modify | `openbot/entrypoints/worker/__main__.py` | `start_profiler()` after `init_sentry()` | +| Modify | `tests/infrastructure/test_obs.py` | Swap assertion + Settings constructor | + +--- + +## Task 1: TDD — settings + observability + +**Files:** +- Modify: `tests/infrastructure/test_obs.py` +- Modify: `openbot/core/settings.py:167-172` +- Modify: `openbot/infrastructure/observability.py:81` + +### - [ ] Step 1: Update the test to use the new field — it should FAIL + +In `tests/infrastructure/test_obs.py`, find `test_init_sentry_passes_settings_through_when_dsn_set`. + +Replace the Settings constructor argument: +```python +# OLD + sentry_profiles_sample_rate=0.1, +# NEW + sentry_profile_session_sample_rate=1.0, +``` + +Replace the comment + assertion: +```python +# OLD + # profiles_sample_rate drives Sentry Profiling on sampled transactions. + assert kwargs["profiles_sample_rate"] == 0.1 +# NEW + # profile_session_sample_rate drives continuous profiling (sentry-sdk 2.24+). + assert kwargs["profile_session_sample_rate"] == 1.0 +``` + +The full updated test function: +```python +def test_init_sentry_passes_settings_through_when_dsn_set() -> None: + """DSN set → SDK init is called with the configured DSN, environment, + traces sample rate, profile session sample rate, and the component tag.""" + settings = Settings( + sentry_dsn="https://public@example.ingest.sentry.io/12345", # type: ignore[arg-type] + environment="production", + sentry_traces_sample_rate=0.1, + sentry_profile_session_sample_rate=1.0, + ) + + with ( + patch("sentry_sdk.init") as mock_init, + patch("sentry_sdk.set_tag") as mock_tag, + ): + init_sentry(settings, component="webapp") + + assert mock_init.called + kwargs = mock_init.call_args.kwargs + assert kwargs["dsn"] == "https://public@example.ingest.sentry.io/12345" + assert kwargs["environment"] == "production" + assert kwargs["traces_sample_rate"] == 0.1 + # profile_session_sample_rate drives continuous profiling (sentry-sdk 2.24+). + assert kwargs["profile_session_sample_rate"] == 1.0 + # PII off by default — webhook bodies contain repo/actor already + # captured in audit_log, no need to duplicate into Sentry. + assert kwargs["send_default_pii"] is False + mock_tag.assert_called_once_with("component", "webapp") + # Sentry Logs product must be enabled. + assert kwargs["enable_logs"] is True + # LoggingIntegration must be present with WARNING+ level so INFO lines + # (e.g. http_request_completed) don't flood Sentry Logs. + integrations = kwargs["integrations"] + assert any(isinstance(i, LoggingIntegration) for i in integrations) +``` + +### - [ ] Step 2: Run the test — it should FAIL + +```bash +uv run pytest tests/infrastructure/test_obs.py::test_init_sentry_passes_settings_through_when_dsn_set -v +``` + +Expected: **FAILED** — `ValidationError` (unknown field `sentry_profile_session_sample_rate`) or `KeyError: 'profile_session_sample_rate'`. + +### - [ ] Step 3: Update `settings.py` — rename field + update default + +In `openbot/core/settings.py`, find lines 165-172: + +```python + # Sentry Profiling captures function-level performance data. + # Requires traces_sample_rate > 0. + sentry_profiles_sample_rate: float = Field( + default=0.1, + ge=0.0, + le=1.0, + description="Fraction of sampled transactions that are also profiled.", + ) +``` + +Replace with: + +```python + # Continuous profiling — always-on, low overhead, does not require + # traces_sample_rate > 0. 1.0 = profile every session (recommended). + sentry_profile_session_sample_rate: float = Field( + default=1.0, + ge=0.0, + le=1.0, + description="Fraction of sessions profiled continuously (sentry-sdk 2.24+).", + ) +``` + +### - [ ] Step 4: Update `observability.py` — swap the init kwarg + +In `openbot/infrastructure/observability.py`, find line 81: + +```python + profiles_sample_rate=settings.sentry_profiles_sample_rate, +``` + +Replace with: + +```python + profile_session_sample_rate=settings.sentry_profile_session_sample_rate, +``` + +Also update the docstring comment block above `sentry_sdk.init()` — find: +```python + # Webhook bodies may carry repo / actor info that's already in +``` +and look at the full init call; it should now read: + +```python + sentry_sdk.init( + dsn=dsn, # None = no-op; sentry-sdk documents this contract + environment=settings.environment, + traces_sample_rate=settings.sentry_traces_sample_rate, + profile_session_sample_rate=settings.sentry_profile_session_sample_rate, + # Webhook bodies may carry repo / actor info that's already in + # the audit log; do not duplicate into Sentry. + send_default_pii=False, + # Forward WARNING+ logs to Sentry Logs (2.35+). + # Explicit LoggingIntegration overrides the default level=INFO so + # routine INFO lines (http_request_completed, etc.) stay in + # stdout only and don't flood the Sentry Logs feed. + enable_logs=True, + integrations=[ + StarletteIntegration(), + FastApiIntegration(), + HttpxIntegration(), + LoggingIntegration( + level=logging.WARNING, # WARNING+ → breadcrumbs + Sentry Logs + event_level=logging.ERROR, # ERROR+ → Sentry error events + ), + ], + ) +``` + +### - [ ] Step 5: Run the target test — it should PASS + +```bash +uv run pytest tests/infrastructure/test_obs.py::test_init_sentry_passes_settings_through_when_dsn_set -v +``` + +Expected: **PASSED** + +### - [ ] Step 6: Run the full obs suite + +```bash +uv run pytest tests/infrastructure/test_obs.py -v +``` + +Expected: **4 PASSED** + +### - [ ] Step 7: Commit + +```bash +git add openbot/core/settings.py openbot/infrastructure/observability.py tests/infrastructure/test_obs.py +git commit -m "feat: switch to continuous profiling (profile_session_sample_rate=1.0)" +``` + +--- + +## Task 2: Entrypoint profiler lifecycle + +**Files:** +- Modify: `openbot/entrypoints/api/app.py` +- Modify: `openbot/entrypoints/worker/__main__.py` + +### - [ ] Step 1: Update webapp `lifespan` — start on startup, stop on shutdown + +In `openbot/entrypoints/api/app.py`, find: + +```python + init_sentry(settings, component="webapp") + init_langsmith() + auth = _build_auth(settings) +``` + +Replace with: + +```python + init_sentry(settings, component="webapp") + init_langsmith() + try: + import sentry_sdk as _sentry + _sentry.profiler.start_profiler() + except Exception: + pass # SDK absent or DSN=None no-op — profiling is opt-in + auth = _build_auth(settings) +``` + +Then find the `finally` block: + +```python + try: + yield + finally: + adapter: GitHubAdapter | None = app.state.github_adapter + if adapter is not None: + await adapter.aclose() +``` + +Replace with: + +```python + try: + yield + finally: + try: + import sentry_sdk as _sentry + _sentry.profiler.stop_profiler() + except Exception: + pass + adapter: GitHubAdapter | None = app.state.github_adapter + if adapter is not None: + await adapter.aclose() +``` + +### - [ ] Step 2: Update worker `_main` — start only (no stop) + +In `openbot/entrypoints/worker/__main__.py`, find: + +```python + init_sentry(settings, component="worker") + init_langsmith() + if settings.redis_url is None: +``` + +Replace with: + +```python + init_sentry(settings, component="worker") + init_langsmith() + try: + import sentry_sdk as _sentry + _sentry.profiler.start_profiler() + except Exception: + pass # SDK absent or DSN=None no-op — profiling is opt-in + if settings.redis_url is None: +``` + +No `stop_profiler()` in the worker — the SDK flushes remaining profile data via its atexit hook when SIGTERM causes `asyncio.run()` to return. + +### - [ ] Step 3: Run the full test suite + +```bash +make check +``` + +Expected: **697 passed** (or higher), ruff clean, import-linter clean. + +### - [ ] Step 4: Commit + +```bash +git add openbot/entrypoints/api/app.py openbot/entrypoints/worker/__main__.py +git commit -m "feat: start continuous profiler in webapp lifespan and worker _main" +``` + +--- + +## Task 3: Doppler + push + +### - [ ] Step 1: Update Doppler `prd` — swap the env var + +```bash +doppler secrets delete OPENBOT_SENTRY_PROFILES_SAMPLE_RATE --project openbot --config prd --yes +doppler secrets set OPENBOT_SENTRY_PROFILE_SESSION_SAMPLE_RATE=1.0 --project openbot --config prd +``` + +Verify: +```bash +doppler secrets --project openbot --config prd --only-names | grep SENTRY +``` + +Expected output includes `OPENBOT_SENTRY_PROFILE_SESSION_SAMPLE_RATE` and does NOT include `OPENBOT_SENTRY_PROFILES_SAMPLE_RATE`. + +### - [ ] Step 2: Push + +```bash +git push origin feat/observability-and-infra-updates +``` + +### - [ ] Step 3: Update PR #58 description + +```bash +gh pr edit 58 --body "$(gh pr view 58 --json body -q .body) + +--- + +## Sentry Continuous Profiling (added in follow-up commit) + +- Replaced \`profiles_sample_rate\` (transaction profiling) with \`profile_session_sample_rate=1.0\` (continuous profiling) +- \`sentry_sdk.profiler.start_profiler()\` called in webapp \`lifespan\` startup and worker \`_main\` after \`init_sentry()\` +- \`stop_profiler()\` called in webapp \`lifespan\` finally block; worker relies on SDK atexit flush +- Both calls wrapped in \`try/except Exception: pass\` — graceful degradation when SDK absent or DSN unset +- Doppler \`prd\`: removed \`OPENBOT_SENTRY_PROFILES_SAMPLE_RATE\`, added \`OPENBOT_SENTRY_PROFILE_SESSION_SAMPLE_RATE=1.0\` +" +``` From a011a520262c4acf7af10648717dde3351c6721c Mon Sep 17 00:00:00 2001 From: YiWang24 Date: Tue, 19 May 2026 05:21:30 -0400 Subject: [PATCH 13/15] feat: switch to continuous profiling (profile_session_sample_rate=1.0) --- openbot/core/settings.py | 15 +++++++++++---- openbot/infrastructure/observability.py | 2 +- tests/infrastructure/test_obs.py | 8 ++++---- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/openbot/core/settings.py b/openbot/core/settings.py index 15e1036..7ee511c 100644 --- a/openbot/core/settings.py +++ b/openbot/core/settings.py @@ -154,15 +154,22 @@ class Settings(BaseSettings): default="development", description="Environment tag attached to Sentry events.", ) - # 0.0 = error-only (cheapest tier). Bump to 0.05-0.2 once you have - # the Sentry Performance budget to spare. Keep <1.0 or you'll burn - # the free-tier event budget on the first traffic spike. + # 0.1 = 10% of transactions (standard production starting point). + # Bump to 1.0 during active debugging or if traffic is very low. sentry_traces_sample_rate: float = Field( - default=0.0, + default=0.1, ge=0.0, le=1.0, description="Fraction of transactions sent to Sentry Performance.", ) + # Continuous profiling — always-on, low overhead, does not require + # traces_sample_rate > 0. 1.0 = profile every session (recommended). + sentry_profile_session_sample_rate: float = Field( + default=1.0, + ge=0.0, + le=1.0, + description="Fraction of sessions profiled continuously (sentry-sdk 2.24+).", + ) # ── Coerce "" → None for optional non-string fields ── # Applied at `mode='before'` so it runs ahead of int / Path validation. diff --git a/openbot/infrastructure/observability.py b/openbot/infrastructure/observability.py index 03ad073..e258244 100644 --- a/openbot/infrastructure/observability.py +++ b/openbot/infrastructure/observability.py @@ -78,7 +78,7 @@ def init_sentry(settings: Settings, *, component: str) -> None: dsn=dsn, # None = no-op; sentry-sdk documents this contract environment=settings.environment, traces_sample_rate=settings.sentry_traces_sample_rate, - profiles_sample_rate=settings.sentry_profiles_sample_rate, + profile_session_sample_rate=settings.sentry_profile_session_sample_rate, # Webhook bodies may carry repo / actor info that's already in # the audit log; do not duplicate into Sentry. send_default_pii=False, diff --git a/tests/infrastructure/test_obs.py b/tests/infrastructure/test_obs.py index 75f131e..7305e02 100644 --- a/tests/infrastructure/test_obs.py +++ b/tests/infrastructure/test_obs.py @@ -36,12 +36,12 @@ def test_init_sentry_is_noop_without_dsn() -> None: def test_init_sentry_passes_settings_through_when_dsn_set() -> None: """DSN set → SDK init is called with the configured DSN, environment, - traces sample rate, profiles sample rate, and the component tag.""" + traces sample rate, profile session sample rate, and the component tag.""" settings = Settings( sentry_dsn="https://public@example.ingest.sentry.io/12345", # type: ignore[arg-type] environment="production", sentry_traces_sample_rate=0.1, - sentry_profiles_sample_rate=0.1, + sentry_profile_session_sample_rate=1.0, ) with ( @@ -55,8 +55,8 @@ def test_init_sentry_passes_settings_through_when_dsn_set() -> None: assert kwargs["dsn"] == "https://public@example.ingest.sentry.io/12345" assert kwargs["environment"] == "production" assert kwargs["traces_sample_rate"] == 0.1 - # profiles_sample_rate drives Sentry Profiling on sampled transactions. - assert kwargs["profiles_sample_rate"] == 0.1 + # profile_session_sample_rate drives continuous profiling (sentry-sdk 2.24+). + assert kwargs["profile_session_sample_rate"] == 1.0 # PII off by default — webhook bodies contain repo/actor already # captured in audit_log, no need to duplicate into Sentry. assert kwargs["send_default_pii"] is False From 094bee17ab6bad8ca2f24b3f9f14703459eb39b8 Mon Sep 17 00:00:00 2001 From: YiWang24 Date: Tue, 19 May 2026 05:24:01 -0400 Subject: [PATCH 14/15] feat: start continuous profiler in webapp lifespan and worker _main --- openbot/entrypoints/api/app.py | 12 ++++++++++++ openbot/entrypoints/worker/__main__.py | 6 ++++++ 2 files changed, 18 insertions(+) diff --git a/openbot/entrypoints/api/app.py b/openbot/entrypoints/api/app.py index 2977e33..bafa71e 100644 --- a/openbot/entrypoints/api/app.py +++ b/openbot/entrypoints/api/app.py @@ -107,6 +107,12 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: # unreachable, malformed PEM, etc.) is captured. init_sentry(settings, component="webapp") init_langsmith() + try: + import sentry_sdk as _sentry + + _sentry.profiler.start_profiler() + except Exception: + pass # SDK absent or DSN=None no-op — profiling is opt-in auth = _build_auth(settings) redis_client: redis_async.Redis | None = ( @@ -169,6 +175,12 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: try: yield finally: + try: + import sentry_sdk as _sentry + + _sentry.profiler.stop_profiler() + except Exception: + pass adapter: GitHubAdapter | None = app.state.github_adapter if adapter is not None: await adapter.aclose() diff --git a/openbot/entrypoints/worker/__main__.py b/openbot/entrypoints/worker/__main__.py index c10ff24..bb03fec 100644 --- a/openbot/entrypoints/worker/__main__.py +++ b/openbot/entrypoints/worker/__main__.py @@ -79,6 +79,12 @@ async def _main() -> int: # for Redis/Postgres, malformed PEM, etc.) on the worker dyno. init_sentry(settings, component="worker") init_langsmith() + try: + import sentry_sdk as _sentry + + _sentry.profiler.start_profiler() + except Exception: + pass # SDK absent or DSN=None no-op — profiling is opt-in if settings.redis_url is None: _logger.error("worker_no_redis_url") return 2 From b40e38496d402aedb17c622dd477cb59f7064e11 Mon Sep 17 00:00:00 2001 From: YiWang24 Date: Tue, 19 May 2026 20:18:17 -0400 Subject: [PATCH 15/15] =?UTF-8?q?fix:=20pre-merge=20review=20fixes=20?= =?UTF-8?q?=E2=80=94=20security,=20docs,=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - redact live Sentry DSN from plan/spec docs (rotate in Sentry dashboard) - delete obser.txt scratch artifact - fix .env.example: OPENBOT_SENTRY_PROFILES_SAMPLE_RATE → OPENBOT_SENTRY_PROFILE_SESSION_SAMPLE_RATE - fix comments: anthropic_api_base alias description, environment field proximity - add SSRF validator to Settings.anthropic_api_base (rejects non-https and private IPs) - extend _blank_to_none to cover anthropic_api_base (blank env var → None) - add LiteLLMCompleter.complete() GLM proxy injection (mirrors module-level complete()) - add debug logging to sentry_profiler_start/stop_failed except blocks - tighten test_obs.py: assert LoggingIntegration breadcrumb/event levels (WARNING/ERROR) - add tests/core/test_settings_anthropic_base.py: 8 tests covering SSRF validator --- .env.example | 3 + .../2026-05-19-sentry-dsn-metrics-audit.md | 2 +- ...6-05-19-sentry-dsn-metrics-audit-design.md | 2 +- obser.txt | 1 - openbot/core/settings.py | 45 +++++++++- openbot/entrypoints/api/app.py | 4 +- openbot/entrypoints/worker/__main__.py | 2 +- openbot/infrastructure/llm/complete.py | 7 ++ tests/core/test_settings_anthropic_base.py | 86 +++++++++++++++++++ tests/infrastructure/test_obs.py | 12 ++- uv.lock | 2 +- 11 files changed, 155 insertions(+), 11 deletions(-) delete mode 100644 obser.txt create mode 100644 tests/core/test_settings_anthropic_base.py diff --git a/.env.example b/.env.example index 7e19078..668cad4 100644 --- a/.env.example +++ b/.env.example @@ -32,6 +32,9 @@ # ─────── Observability (PRD §8.1) ─────── # LANGSMITH_API_KEY= # LANGSMITH_PROJECT=openbot-prod +# OPENBOT_SENTRY_DSN= +# OPENBOT_SENTRY_TRACES_SAMPLE_RATE=0.1 # 10% of requests traced +# OPENBOT_SENTRY_PROFILE_SESSION_SAMPLE_RATE=1.0 # fraction of sessions profiled continuously # ─────── Cost / kill switches (PRD §4.5 / §4.7) ─────── # OPENBOT_GLOBAL_HARD_KILL_USD=500 # 覆盖 config.yaml budget.global_hard_kill_usd diff --git a/docs/superpowers/plans/2026-05-19-sentry-dsn-metrics-audit.md b/docs/superpowers/plans/2026-05-19-sentry-dsn-metrics-audit.md index ba8554f..da5ab85 100644 --- a/docs/superpowers/plans/2026-05-19-sentry-dsn-metrics-audit.md +++ b/docs/superpowers/plans/2026-05-19-sentry-dsn-metrics-audit.md @@ -37,7 +37,7 @@ Find the line (currently blank after `=`): ``` Replace it with (uncommented, real DSN): ``` -OPENBOT_SENTRY_DSN=https://f07a401fd38b70db0359272557c974d6@o4511407339012096.ingest.us.sentry.io/4511407339143168 +OPENBOT_SENTRY_DSN=https://@o4511407339012096.ingest.us.sentry.io/ # obtain from Sentry → Project Settings → Client Keys ``` - [ ] **Step 2: Verify Settings picks it up** diff --git a/docs/superpowers/specs/2026-05-19-sentry-dsn-metrics-audit-design.md b/docs/superpowers/specs/2026-05-19-sentry-dsn-metrics-audit-design.md index 908bac5..b84f971 100644 --- a/docs/superpowers/specs/2026-05-19-sentry-dsn-metrics-audit-design.md +++ b/docs/superpowers/specs/2026-05-19-sentry-dsn-metrics-audit-design.md @@ -58,7 +58,7 @@ if hasattr(metrics, "set"): Uncomment and populate: ``` -OPENBOT_SENTRY_DSN=https://f07a401fd38b70db0359272557c974d6@o4511407339012096.ingest.us.sentry.io/4511407339143168 +OPENBOT_SENTRY_DSN=https://@o4511407339012096.ingest.us.sentry.io/ # obtain from Sentry → Project Settings → Client Keys ``` No other config files change. `Settings.sentry_dsn` reads this via the `OPENBOT_` prefix and wraps it in `SecretStr` (never logged, never in repr). diff --git a/obser.txt b/obser.txt deleted file mode 100644 index a0a5125..0000000 --- a/obser.txt +++ /dev/null @@ -1 +0,0 @@ -obser test diff --git a/openbot/core/settings.py b/openbot/core/settings.py index 7ee511c..cfe90fc 100644 --- a/openbot/core/settings.py +++ b/openbot/core/settings.py @@ -6,6 +6,8 @@ from __future__ import annotations +import ipaddress +import urllib.parse from functools import lru_cache from pathlib import Path from typing import Any @@ -140,14 +142,16 @@ class Settings(BaseSettings): ) # ─── LLM ─── - # LiteLLM / Anthropic base URL. When set, routes all Anthropic model - # calls to this endpoint (e.g. GLM or other proxies). - # Mirrored from CLAUDE_SWITCH_GLM_BASE_URL if present. + # LiteLLM / Anthropic base URL. When set, routes Anthropic model calls + # to this endpoint (e.g. GLM proxy). Reads from env var + # CLAUDE_SWITCH_GLM_BASE_URL (bypasses the OPENBOT_ prefix via alias). anthropic_api_base: str | None = Field( default=None, alias="CLAUDE_SWITCH_GLM_BASE_URL", description="Base URL for Anthropic-compatible models (e.g. GLM proxy).", ) + + # ─── Sentry ─── # Tags every Sentry event so prod errors don't mix with dev / preview. # Heroku sets this via `heroku config:set OPENBOT_ENVIRONMENT=production`. environment: str = Field( @@ -178,9 +182,44 @@ class Settings(BaseSettings): "github_app_private_key_path", "redis_url", "postgres_url", + "anthropic_api_base", mode="before", )(staticmethod(_blank_to_none)) + @field_validator("anthropic_api_base", mode="after") + @classmethod + def _validate_proxy_url(cls, v: str | None) -> str | None: + """Reject non-HTTPS and private-network URLs to prevent SSRF. + + CLAUDE_SWITCH_GLM_BASE_URL is passed directly to litellm as + ``api_base``. Without validation, any value (file://, internal IPs, + cloud metadata endpoints) would cause litellm to POST all LLM + messages to that host. + """ + if v is None: + return v + parsed = urllib.parse.urlparse(v) + if parsed.scheme != "https": + raise ValueError(f"anthropic_api_base must use https://, got scheme={parsed.scheme!r}") + hostname = parsed.hostname or "" + try: + addr = ipaddress.ip_address(hostname) + if addr.is_private or addr.is_loopback or addr.is_link_local: + raise ValueError( + f"anthropic_api_base must not target a private/loopback address: {hostname}" + ) + except ValueError as exc: + # Re-raise our own errors; ignore "not a valid IP" (i.e. hostname). + if "anthropic_api_base" in str(exc): + raise + # Block AWS/GCP/Azure instance metadata hostnames explicitly. + _blocked = {"169.254.169.254", "metadata.google.internal"} + if hostname in _blocked: + raise ValueError( + f"anthropic_api_base must not target a cloud metadata endpoint: {hostname}" + ) + return v + @lru_cache def get_settings() -> Settings: diff --git a/openbot/entrypoints/api/app.py b/openbot/entrypoints/api/app.py index bafa71e..d3563f5 100644 --- a/openbot/entrypoints/api/app.py +++ b/openbot/entrypoints/api/app.py @@ -112,7 +112,7 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: _sentry.profiler.start_profiler() except Exception: - pass # SDK absent or DSN=None no-op — profiling is opt-in + _logger.debug("sentry_profiler_start_failed", exc_info=True) auth = _build_auth(settings) redis_client: redis_async.Redis | None = ( @@ -180,7 +180,7 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: _sentry.profiler.stop_profiler() except Exception: - pass + _logger.debug("sentry_profiler_stop_failed", exc_info=True) adapter: GitHubAdapter | None = app.state.github_adapter if adapter is not None: await adapter.aclose() diff --git a/openbot/entrypoints/worker/__main__.py b/openbot/entrypoints/worker/__main__.py index bb03fec..301c5ca 100644 --- a/openbot/entrypoints/worker/__main__.py +++ b/openbot/entrypoints/worker/__main__.py @@ -84,7 +84,7 @@ async def _main() -> int: _sentry.profiler.start_profiler() except Exception: - pass # SDK absent or DSN=None no-op — profiling is opt-in + _logger.debug("sentry_profiler_start_failed", exc_info=True) if settings.redis_url is None: _logger.error("worker_no_redis_url") return 2 diff --git a/openbot/infrastructure/llm/complete.py b/openbot/infrastructure/llm/complete.py index 5bf9fa7..f9dfa3f 100644 --- a/openbot/infrastructure/llm/complete.py +++ b/openbot/infrastructure/llm/complete.py @@ -317,6 +317,9 @@ async def complete( temperature: float = 0.0, max_tokens: int | None = None, ) -> str: + from openbot.core.settings import get_settings + + settings = get_settings() kwargs: dict[str, Any] = { "model": model, "messages": list(messages), @@ -324,6 +327,10 @@ async def complete( } if max_tokens is not None: kwargs["max_tokens"] = max_tokens + # Honor custom base URL for Anthropic models (e.g. GLM proxy). + # Mirrors the same logic in the module-level complete() function. + if "anthropic/" in model and settings.anthropic_api_base: + kwargs.setdefault("api_base", settings.anthropic_api_base) response = await litellm.acompletion(**kwargs) # Use attribute access — litellm returns a ModelResponse object, # and the rest of this module uses the same pattern (e.g. line 181). diff --git a/tests/core/test_settings_anthropic_base.py b/tests/core/test_settings_anthropic_base.py new file mode 100644 index 0000000..6a8771e --- /dev/null +++ b/tests/core/test_settings_anthropic_base.py @@ -0,0 +1,86 @@ +"""Tests for Settings.anthropic_api_base (CLAUDE_SWITCH_GLM_BASE_URL). + +Covers: +- Default is None when env var absent +- Field is populated when CLAUDE_SWITCH_GLM_BASE_URL is set +- Blank value coerces to None +- SSRF validator rejects http:// and private-range IPs +- SSRF validator accepts valid https:// public URL +""" + +from __future__ import annotations + +import pytest + +from openbot.core.settings import Settings, get_settings + + +@pytest.fixture(autouse=True) +def _clear_settings_cache() -> None: # type: ignore[return] + """Ensure Settings cache is cleared around each test.""" + get_settings.cache_clear() + yield # type: ignore[misc] + get_settings.cache_clear() + + +@pytest.fixture() +def _no_glm_env(monkeypatch) -> None: # type: ignore[no-untyped-def] + """Remove CLAUDE_SWITCH_GLM_BASE_URL from environment and .env.""" + monkeypatch.delenv("CLAUDE_SWITCH_GLM_BASE_URL", raising=False) + + +def test_anthropic_api_base_default_is_none(_no_glm_env) -> None: # type: ignore[no-untyped-def] + """Field is None when CLAUDE_SWITCH_GLM_BASE_URL is not set.""" + # Use _env_file=None to skip reading the project .env file (which may have + # CLAUDE_SWITCH_GLM_BASE_URL set for local development). + s = Settings(_env_file=None) # type: ignore[call-arg] + assert s.anthropic_api_base is None + + +def test_anthropic_api_base_populated_from_env(monkeypatch) -> None: # type: ignore[no-untyped-def] + """CLAUDE_SWITCH_GLM_BASE_URL is picked up and stored.""" + monkeypatch.setenv("CLAUDE_SWITCH_GLM_BASE_URL", "https://open.bigmodel.cn/api/anthropic") + s = Settings(_env_file=None) # type: ignore[call-arg] + assert s.anthropic_api_base == "https://open.bigmodel.cn/api/anthropic" + + +def test_anthropic_api_base_blank_is_none(monkeypatch) -> None: # type: ignore[no-untyped-def] + """A blank env var is coerced to None by the _blank_to_none validator.""" + monkeypatch.setenv("CLAUDE_SWITCH_GLM_BASE_URL", " ") + s = Settings(_env_file=None) # type: ignore[call-arg] + assert s.anthropic_api_base is None + + +def test_anthropic_api_base_rejects_http_scheme(monkeypatch) -> None: # type: ignore[no-untyped-def] + """Non-HTTPS proxy URL must be rejected at settings load time.""" + monkeypatch.setenv("CLAUDE_SWITCH_GLM_BASE_URL", "http://open.bigmodel.cn/api/anthropic") + with pytest.raises(Exception, match="https://"): + Settings(_env_file=None) # type: ignore[call-arg] + + +def test_anthropic_api_base_rejects_private_ip(monkeypatch) -> None: # type: ignore[no-untyped-def] + """Private-range IP URLs must be rejected to prevent SSRF.""" + monkeypatch.setenv("CLAUDE_SWITCH_GLM_BASE_URL", "https://192.168.1.1/api") + with pytest.raises(Exception, match="private"): + Settings(_env_file=None) # type: ignore[call-arg] + + +def test_anthropic_api_base_rejects_loopback(monkeypatch) -> None: # type: ignore[no-untyped-def] + """Loopback URLs must be rejected to prevent SSRF.""" + monkeypatch.setenv("CLAUDE_SWITCH_GLM_BASE_URL", "https://127.0.0.1/api") + with pytest.raises(Exception, match=r"private|loopback"): + Settings(_env_file=None) # type: ignore[call-arg] + + +def test_anthropic_api_base_rejects_metadata_endpoint(monkeypatch) -> None: # type: ignore[no-untyped-def] + """Cloud metadata IP must be rejected to prevent SSRF.""" + monkeypatch.setenv("CLAUDE_SWITCH_GLM_BASE_URL", "https://169.254.169.254/latest/meta-data") + with pytest.raises(Exception, match=r"169\.254\.169\.254|metadata"): + Settings(_env_file=None) # type: ignore[call-arg] + + +def test_anthropic_api_base_accepts_valid_https_url(monkeypatch) -> None: # type: ignore[no-untyped-def] + """A valid public HTTPS URL is accepted without error.""" + monkeypatch.setenv("CLAUDE_SWITCH_GLM_BASE_URL", "https://open.bigmodel.cn/api/anthropic") + s = Settings(_env_file=None) # type: ignore[call-arg] + assert s.anthropic_api_base == "https://open.bigmodel.cn/api/anthropic" diff --git a/tests/infrastructure/test_obs.py b/tests/infrastructure/test_obs.py index 7305e02..cfc1716 100644 --- a/tests/infrastructure/test_obs.py +++ b/tests/infrastructure/test_obs.py @@ -66,7 +66,17 @@ def test_init_sentry_passes_settings_through_when_dsn_set() -> None: # LoggingIntegration must be present with WARNING+ level so INFO lines # (e.g. http_request_completed) don't flood Sentry Logs. integrations = kwargs["integrations"] - assert any(isinstance(i, LoggingIntegration) for i in integrations) + logging_int = next((i for i in integrations if isinstance(i, LoggingIntegration)), None) + assert logging_int is not None, "LoggingIntegration missing from integrations list" + # LoggingIntegration stores levels on internal handlers: + # _breadcrumb_handler → breadcrumbs threshold (should be WARNING) + # _handler → Sentry event threshold (should be ERROR) + assert logging_int._breadcrumb_handler.level == logging.WARNING, ( + "LoggingIntegration breadcrumb level must be WARNING to suppress INFO noise in Sentry Logs" + ) + assert logging_int._handler.level == logging.ERROR, ( + "LoggingIntegration event level must be ERROR to avoid creating Sentry events for warnings" + ) def test_init_sentry_survives_missing_sdk(monkeypatch) -> None: # type: ignore[no-untyped-def] diff --git a/uv.lock b/uv.lock index c51708e..c8a3912 100644 --- a/uv.lock +++ b/uv.lock @@ -2699,7 +2699,7 @@ requires-dist = [ { name = "python-json-logger", specifier = ">=3.1" }, { name = "pyyaml", specifier = ">=6.0" }, { name = "redis", specifier = ">=5" }, - { name = "sentry-sdk", extras = ["fastapi"], specifier = ">=2.18" }, + { name = "sentry-sdk", extras = ["fastapi"], specifier = ">=2.35.0" }, { name = "sqlalchemy", extras = ["asyncio"], specifier = ">=2.0" }, { name = "tenacity", specifier = ">=9.0" }, { name = "uvicorn", extras = ["standard"], specifier = ">=0.47.0" },