feat(collector): minimal FastAPI trace collector#4
Conversation
POST /v1/trace + /v1/trace/batch (validated by the TraceEvent contract), GET /v1/trace, /healthz; async SQLAlchemy store (sqlite for tests, Postgres via HYRULE_COLLECTOR_DATABASE_URL in prod). Optional 'collector' extra so core contracts stay dependency-light; run with 'python -m agent_core.collector'. ruff + mypy clean; 36 tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2535bf41dc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| [project.optional-dependencies] | ||
| dev = ["pytest>=8", "pyyaml>=6", "ruff>=0.5", "mypy>=1.8", "types-PyYAML"] | ||
| collector = ["fastapi>=0.110", "uvicorn>=0.29", "sqlalchemy>=2", "asyncpg>=0.29"] |
There was a problem hiding this comment.
When a user installs agent-core[collector] and runs the documented default collector without HYRULE_COLLECTOR_DATABASE_URL, the app falls back to sqlite+aiosqlite:///./collector.db in collector/db.py, but this extra installs asyncpg and not aiosqlite. In that default path SQLAlchemy cannot load the SQLite async driver, so python -m agent_core.collector fails before serving; include aiosqlite in the collector extra or avoid a SQLite default.
Useful? React with 👍 / 👎.
|
|
||
| @app.get("/v1/trace") | ||
| async def recent(run_id: str | None = None, limit: int = 50) -> list[dict[str, Any]]: | ||
| stmt = select(TraceEventRow).order_by(TraceEventRow.id.desc()).limit(min(limit, 500)) |
There was a problem hiding this comment.
When clients call /v1/trace?limit=-1 against the default SQLite store, min(limit, 500) passes the negative value through; SQLite treats a negative LIMIT as unbounded, so this bypasses the apparent 500-row cap and can load every stored trace JSON into memory. Add lower-bound validation or clamp the value before applying it.
Useful? React with 👍 / 👎.
1 participant
The C2 piece of the trace pipeline: a minimal service that ingests agent-core
TraceEvents.agent_core/collector/app.py— FastAPI:POST /v1/trace,POST /v1/trace/batch(both validated by the shared
TraceEventcontract → 422 on bad input),GET /v1/trace(recent / by run_id),
GET /healthz.agent_core/collector/db.py— async SQLAlchemytrace_events(sqlite for tests, Postgresvia
HYRULE_COLLECTOR_DATABASE_URL); flattens cost/labels into columns + keeps the fullevent JSON.
python -m agent_core.collectorto run. Lives behind the optionalcollectorextra,so
import agent_core.contractsstays dependency-light (no-heavy-imports test still green).ruff + mypy clean; 36 tests pass (+4 collector). Next: C3 deploy (Postgres + ansible),
C4 point producers at it.
🤖 Generated with Claude Code