Skip to content

Conversation

@tnkshuuhei
Copy link
Member

  • Introduced registerOTel for OpenTelemetry integration, enabling observability features.
  • Added LibSQLStore for persistent storage with a specified database file.
  • Updated telemetry settings to enable observability by default, enhancing monitoring capabilities.

- Introduced `registerOTel` for OpenTelemetry integration, enabling observability features.
- Added `LibSQLStore` for persistent storage with a specified database file.
- Updated telemetry settings to enable observability by default, enhancing monitoring capabilities.
@vercel
Copy link

vercel bot commented Nov 21, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
muse Ready Ready Preview, Comment Dec 23, 2025 2:59am

@tnkshuuhei tnkshuuhei self-assigned this Nov 21, 2025
@tnkshuuhei tnkshuuhei linked an issue Nov 21, 2025 that may be closed by this pull request
@tnkshuuhei tnkshuuhei changed the base branch from dev-agent to dev December 22, 2025 08:57
@tnkshuuhei tnkshuuhei marked this pull request as draft December 23, 2025 02:54
@coderabbitai
Copy link

coderabbitai bot commented Dec 23, 2025

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/observability

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

@claude
Copy link

claude bot commented Dec 23, 2025

PR Review: Observability Integration

Overview

This PR introduces OpenTelemetry observability to the Muse application, adding monitoring capabilities through Vercel's OTel integration and Mastra's observability features.


✅ Positive Aspects

  1. Clean Implementation: The observability setup is straightforward and follows Next.js conventions with the instrumentation.ts file
  2. Consistent Service Naming: Both OpenTelemetry and Mastra telemetry use "Muse" as the service name
  3. Production-Ready Dependencies: Uses stable, well-maintained packages (@vercel/otel, @mastra/libsql, @mastra/loggers)
  4. Storage Layer: Adds LibSQLStore for persistent storage, which is essential for production observability data

⚠️ Issues & Recommendations

1. Database File Management (Important)

Issue: The database file mastra.db is hardcoded but not in .gitignore

// mastra/index.ts:27
storage: new LibSQLStore({
  url: "file:./mastra.db",
}),

Recommendation: Add mastra.db to .gitignore to prevent committing local database files:

# .gitignore
.serena
.mastra
+mastra.db
+mastra.db-*
# Vector store for RAG evidence search
vector.*

2. Environment-Based Configuration (High Priority)

Issue: Observability is always enabled (enabled: true) regardless of environment

// mastra/index.ts:23-25
observability: {
  default: { enabled: true },
},
// ...
telemetry: {
  serviceName: "Muse",
  enabled: true,  // Always on
},

Recommendation: Make observability configurable based on environment:

const isProduction = process.env.NEXT_PUBLIC_ENV === 'production';
const isDevelopment = process.env.NODE_ENV === 'development';

export const mastra = new Mastra({
  // ...
  observability: {
    default: { enabled: isProduction },
  },
  telemetry: {
    serviceName: "Muse",
    enabled: isProduction || (isDevelopment && process.env.ENABLE_TELEMETRY === 'true'),
  },
});

This allows:

  • Production: Always enabled
  • Development: Opt-in via environment variable
  • Reduced noise during local development

3. Storage Configuration (Medium Priority)

Issue: Database path should be configurable for different environments

Recommendation: Use environment variables for database location:

storage: new LibSQLStore({
  url: process.env.MASTRA_DB_URL || "file:./mastra.db",
}),

Add to .env.example:

# Mastra observability database
# MASTRA_DB_URL="file:./mastra.db"  # local
# MASTRA_DB_URL="libsql://[your-turso-db].turso.io"  # production

4. Missing Next.js Configuration (Important)

Issue: The instrumentation.ts file won't be loaded automatically without proper Next.js config

Current next.config.ts doesn't enable instrumentation hook:

const nextConfig: NextConfig = {
  pageExtensions: ["js", "jsx", "ts", "tsx", "md", "mdx"],
  serverExternalPackages: ["@mastra/*"],
  experimental: {
    mdxRs: false,
+   instrumentationHook: true,  // Required for instrumentation.ts
  },
};

Reference: Next.js Instrumentation Docs


5. Logger Configuration (Low Priority)

Issue: PinoLogger is instantiated without configuration

logger: new PinoLogger(),  // Uses defaults

Recommendation: Consider configuring log levels:

logger: new PinoLogger({
  level: process.env.LOG_LEVEL || (isProduction ? 'info' : 'debug'),
}),

6. Commented Code Cleanup (Minor)

The commented TODO about CONNECTION_URL references file:./mastra.db but the actual code now uses this path. Consider updating or removing the comment:

// Lines 8-11 in mastra/index.ts
// TODO: validate CONNECTION_URL for production
// const connectionUrl = process.env.CONNECTION_URL
//   ? `file:${process.env.CONNECTION_URL}/vector.db`
//   : "file:./mastra.db",  // This is now the actual implementation

🔒 Security Considerations

Good: No sensitive data exposed in configuration
Good: Uses local file storage by default (safe for development)
⚠️ Consider: For production, ensure mastra.db has proper file permissions and is not web-accessible


📊 Performance Considerations

  • LibSQL Performance: File-based storage is appropriate for observability data; consider migration to Turso (remote LibSQL) for production scale
  • OpenTelemetry Overhead: Vercel's OTel integration is optimized, but ensure you monitor the overhead in production
  • Batch Processing: Consider if Mastra's observability data should be batched for high-traffic scenarios

✅ Test Coverage

Note: According to CLAUDE.md, there are no test scripts configured in this project. However, for observability features, consider:

  1. Integration Testing: Verify instrumentation hook is called
  2. Manual Testing:
    • Check that mastra.db is created on first run
    • Verify telemetry data is collected (if using Vercel's dashboard)
    • Test with pnpm dev:mastra to ensure observability doesn't break Mastra workflows

📝 Documentation Needs

Consider adding to docs/:

  • How to view observability data
  • What metrics are being collected
  • How to configure OpenTelemetry exporters for different environments
  • Database maintenance/cleanup procedures for mastra.db

Summary

Overall: This is a solid foundation for observability! The implementation is clean and uses industry-standard tools.

Must-Fix Before Merge:

  1. Add mastra.db to .gitignore
  2. Enable instrumentationHook: true in next.config.ts

Should Consider:
3. Environment-based configuration for telemetry
4. Configurable database path via environment variables

Nice to Have:
5. Logger configuration
6. Cleanup commented code
7. Documentation for observability features


Recommendation: ✅ Approve with requested changes (items 1-2)

Great work on adding observability! This will be valuable for monitoring the Mastra agent workflows in production. 🚀

@tnkshuuhei
Copy link
Member Author

tnkshuuhei commented Dec 23, 2025

FYI @Naokiakazawa, this branch has updated due to current changes on dev branch.
and turned this branch to draft PR

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

introduce mastra observability

2 participants