This PR wires Mastra tracing to persistent PostgreSQL storage for local development #73
This PR wires Mastra tracing to persistent PostgreSQL storage for local development #73MabudAlam wants to merge 2 commits into
Conversation
|
Caution Review failedFailed to post review comments 📝 WalkthroughWalkthroughAdds Mastra runtime integration: new runtime dependencies, MASTRA_DATABASE_URL in docker-compose, a PostgresStore using that URL, and an Observability instance wired into the exported Mastra singleton. The backend index imports mastra and calls mastra.observability.getDefaultInstance()?.flush() in the /populate route error path. Sequence Diagram(s)sequenceDiagram
participant Client
participant Backend
participant MastraRuntime
participant PostgresDB
participant MastraObservability
Client->>Backend: POST /populate
Backend->>MastraRuntime: invoke populate workflow
MastraRuntime->>PostgresDB: read/write workflow state
MastraRuntime->>MastraObservability: emit spans/metrics
alt error path
Backend->>MastraObservability: getDefaultInstance()?.flush()
Backend->>Client: return error response
else success
Backend->>Client: return success response
end
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@backend/src/mastra/index.ts`:
- Around line 12-15: The code uses a non-null assertion for
process.env.MASTRA_DATABASE_URL when constructing PostgresStore (storage), which
can pass undefined and produce cryptic runtime errors; replace that with an
explicit validation: read and validate MASTRA_DATABASE_URL (either in env.ts
alongside other vars or at the top of this module), throw a clear startup Error
if it's missing, and then pass the validated string into new PostgresStore({ id:
'mastra-pg-storage', connectionString: validatedMastraDatabaseUrl }); ensure you
remove the trailing "!" and reference the validated variable when creating
storage.
In `@docker-compose.dev.yml`:
- Around line 58-61: The mastra service is missing a dependency on the db
service even though MASTRA_DATABASE_URL points at PostgreSQL; update the mastra
service's depends_on block to include db with condition: service_healthy (in
addition to the existing convex dependency) so Docker Compose waits for the db
to be healthy before starting mastra.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 837453a3-38ae-4990-b738-49527951570d
📒 Files selected for processing (4)
backend/package.jsonbackend/src/index.tsbackend/src/mastra/index.tsdocker-compose.dev.yml
| const storage = new PostgresStore({ | ||
| id: 'mastra-pg-storage', | ||
| connectionString: process.env.MASTRA_DATABASE_URL!, | ||
| }) |
There was a problem hiding this comment.
Missing validation for MASTRA_DATABASE_URL environment variable.
The non-null assertion (!) on process.env.MASTRA_DATABASE_URL will pass undefined to PostgresStore if the variable is missing, likely causing a cryptic connection error at runtime rather than a clear startup failure.
Consider validating this in env.ts alongside other required variables, or add a guard here:
Proposed fix
+const mastraDbUrl = process.env.MASTRA_DATABASE_URL;
+if (!mastraDbUrl) {
+ throw new Error('MASTRA_DATABASE_URL environment variable is required');
+}
+
const storage = new PostgresStore({
id: 'mastra-pg-storage',
- connectionString: process.env.MASTRA_DATABASE_URL!,
+ connectionString: mastraDbUrl,
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const storage = new PostgresStore({ | |
| id: 'mastra-pg-storage', | |
| connectionString: process.env.MASTRA_DATABASE_URL!, | |
| }) | |
| const mastraDbUrl = process.env.MASTRA_DATABASE_URL; | |
| if (!mastraDbUrl) { | |
| throw new Error('MASTRA_DATABASE_URL environment variable is required'); | |
| } | |
| const storage = new PostgresStore({ | |
| id: 'mastra-pg-storage', | |
| connectionString: mastraDbUrl, | |
| }) |
🤖 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 `@backend/src/mastra/index.ts` around lines 12 - 15, The code uses a non-null
assertion for process.env.MASTRA_DATABASE_URL when constructing PostgresStore
(storage), which can pass undefined and produce cryptic runtime errors; replace
that with an explicit validation: read and validate MASTRA_DATABASE_URL (either
in env.ts alongside other vars or at the top of this module), throw a clear
startup Error if it's missing, and then pass the validated string into new
PostgresStore({ id: 'mastra-pg-storage', connectionString:
validatedMastraDatabaseUrl }); ensure you remove the trailing "!" and reference
the validated variable when creating storage.
| MASTRA_DATABASE_URL: postgresql://bigset:bigset@db:5432/bigset_internal | ||
| depends_on: | ||
| convex: | ||
| condition: service_healthy |
There was a problem hiding this comment.
mastra service should depend on db for PostgreSQL connectivity.
The mastra service uses MASTRA_DATABASE_URL pointing to the db service, but only declares a dependency on convex. If mastra starts before db is healthy, the PostgreSQL connection will fail.
Proposed fix
depends_on:
+ db:
+ condition: service_healthy
convex:
condition: service_healthy📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| MASTRA_DATABASE_URL: postgresql://bigset:bigset@db:5432/bigset_internal | |
| depends_on: | |
| convex: | |
| condition: service_healthy | |
| MASTRA_DATABASE_URL: postgresql://bigset:bigset@db:5432/bigset_internal | |
| depends_on: | |
| db: | |
| condition: service_healthy | |
| convex: | |
| condition: service_healthy |
🤖 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 `@docker-compose.dev.yml` around lines 58 - 61, The mastra service is missing a
dependency on the db service even though MASTRA_DATABASE_URL points at
PostgreSQL; update the mastra service's depends_on block to include db with
condition: service_healthy (in addition to the existing convex dependency) so
Docker Compose waits for the db to be healthy before starting mastra.
giaphutran12
left a comment
giaphutran12
left a comment
There was a problem hiding this comment.
Holding for launch safety. This branch is stale and merge-conflicting with latest main, and only CodeRabbit is reported. Please update it onto current main and rerun the normal checks before review.
2 participants
Issue : #71
What functionality was added
Why this is helpful