feat(analytics): Platform metric - #165
Conversation
…unnel, and admin metrics dashboard This commit introduces the Startup Metrics Stack to monitor platform health and track developer activation. Key Changes: - Core Data: Added PlatformEvent and DeveloperActivity models. - BullMQ Jobs: Implemented daily activityRollupQueue and 5-min reliabilityAlertQueue to track error rate spikes. - Activation Funnel: Instrumented auth, project creation, and API usage to emit pipeline events. - Admin Dashboard: Added 7 new protected metrics APIs and built the AdminMetrics UI for operators. - Developer UI: Added DeveloperMetrics component to show personal activation status and 30-day activity.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis PR introduces a comprehensive event tracking and analytics platform. It adds ChangesPlatform Event Tracking and Analytics Infrastructure
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/dashboard-api/src/controllers/auth.controller.js (1)
161-189:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing
email_verifiedemission on GitHub email-merge path.When an existing local account is reconciled by email and we force
isVerified = true(Lines 161–169), noemail_verifiedevent is emitted. If that account was previously unverified, this branch flips it to verified without funneling the transition into the activation/funnel pipeline — analytics will under-count verifications. Consider emittingemail_verifiedhere when the prior state wasfalse.🛡️ Proposed fix
developer = await Developer.findOne({ email: profile.email }).select('+password +refreshToken'); if (developer) { + const wasVerifiedBefore = developer.isVerified === true; developer.githubId = profile.githubId; developer.githubUsername = profile.githubUsername; developer.avatarUrl = profile.avatarUrl; developer.isVerified = true; await developer.save(); + if (!wasVerifiedBefore) { + emitEvent(developer._id, 'email_verified', { method: 'github' }); + } return developer; }🤖 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 `@apps/dashboard-api/src/controllers/auth.controller.js` around lines 161 - 189, When reconciling an existing Developer in the GitHub flow (the branch that finds Developer via Developer.findOne and sets developer.githubId/githubUsername/avatarUrl/isVerified), check the previous isVerified value and, if it was false, call emitEvent(developer._id, 'email_verified', { method: 'github' }) after saving (or immediately before returning) so the verification funnel is recorded; update the block that assigns developer.isVerified = true and saves in the function handling the GitHub profile to conditionally emit this event when transitioning from unverified to verified.apps/dashboard-api/src/controllers/analytics.controller.js (3)
115-115:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix response format to match API standard.
The response does not follow the required
{ success: bool, data: {}, message: "" }format.🔧 Proposed fix
- res.json(formattedLogs); + res.json({ success: true, data: formattedLogs, message: '' });As per coding guidelines: "All API endpoints return:
{ success: bool, data: {}, message: "" }."🤖 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 `@apps/dashboard-api/src/controllers/analytics.controller.js` at line 115, The controller currently returns raw data via res.json(formattedLogs); update the handler (the function that sends formattedLogs in apps/dashboard-api/src/controllers/analytics.controller.js) to wrap the payload in the standard envelope by returning res.json({ success: true, data: formattedLogs, message: "" }) for successful responses (and similarly use { success:false, data:{}, message: "..." } for error paths) so all endpoints conform to the `{ success: bool, data: {}, message: "" }` API contract.
86-88:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winReplace raw error exposure with AppError.
The catch block directly exposes
err.messageto the client, which could leak internal MongoDB error details. As per coding guidelines, use the AppError class for errors and never expose MongoDB errors to the client.🛡️ Proposed fix
} catch (err) { - res.status(500).json({ success: false, data: {}, message: err.message }); + console.error('getGlobalStats error:', err); + throw new AppError('Failed to retrieve global statistics', 500); }As per coding guidelines: "Use AppError class for errors — never raw throw, never expose MongoDB errors to client."
🤖 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 `@apps/dashboard-api/src/controllers/analytics.controller.js` around lines 86 - 88, The catch currently returns err.message to the client; instead log the original err internally (e.g., console.error(err)) and replace the response with an AppError instance: create and pass new AppError('Internal server error', 500) to the Express error handler via next(new AppError(...)) (ensure the controller signature includes next), removing any use of err.message in res.status(...).json and keeping only a generic message to the client.
116-118:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winReplace raw error exposure with AppError and fix response format.
The catch block uses the wrong response format and directly exposes
err.message, which could leak internal MongoDB error details.🛡️ Proposed fix
} catch (err) { - res.status(500).json({ error: err.message }); + console.error('getRecentActivity error:', err); + throw new AppError('Failed to retrieve recent activity', 500); }As per coding guidelines: "All API endpoints return:
{ success: bool, data: {}, message: "" }. Use AppError class for errors — never raw throw, never expose MongoDB errors to client."🤖 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 `@apps/dashboard-api/src/controllers/analytics.controller.js` around lines 116 - 118, In the catch block of the analytics controller replace the direct response that exposes err.message with error-handling that uses the AppError class and the route's next() so the centralized error middleware formats the response as { success:false, data:{}, message:"" }; specifically, remove res.status(500).json({ error: err.message }) and instead log the original err (e.g., using console.error or processLogger.error) and call next(new AppError(500, "Internal Server Error")) so no MongoDB/internal messages are sent to the client and the global error handler returns the standardized payload.
🧹 Nitpick comments (10)
packages/common/src/models/DeveloperActivity.js (2)
27-30: ⚡ Quick winAdd validation to prevent negative activity counters.
The activity counters (
apiCallCount,mailSentCount, etc.) default to 0 but have no minimum constraint. If rollup logic uses$incoperations without validation, bugs could introduce negative values. Consider addingmin: 0validators or ensure rollup code validates increments.🛡️ Add minimum validators
- apiCallCount: { type: Number, default: 0 }, - mailSentCount: { type: Number, default: 0 }, - storageUploadsCount: { type: Number, default: 0 }, - webhookTriggeredCount: { type: Number, default: 0 }, + apiCallCount: { type: Number, default: 0, min: 0 }, + mailSentCount: { type: Number, default: 0, min: 0 }, + storageUploadsCount: { type: Number, default: 0, min: 0 }, + webhookTriggeredCount: { type: Number, default: 0, min: 0 },🤖 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 `@packages/common/src/models/DeveloperActivity.js` around lines 27 - 30, The numeric activity fields in DeveloperActivity (apiCallCount, mailSentCount, storageUploadsCount, webhookTriggeredCount) need non-negative validation; update the Mongoose schema for those fields to include a min: 0 validator (or equivalent validation) so attempts to set negative values are rejected, and keep the existing default: 0; ensure any rollup/`$inc` paths that update these fields still rely on schema validation or add runtime checks to prevent negative results when applying decrements.
23-26: ⚖️ Poor tradeoffConsider size limits for
activeProjectIdsarray.The
activeProjectIdsarray is unbounded and could grow large for highly active developers with many projects. While unlikely to hit MongoDB's 16MB document limit in practice, consider documenting expected max size or adding application-level limits if a developer can create hundreds of projects.🤖 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 `@packages/common/src/models/DeveloperActivity.js` around lines 23 - 26, The activeProjectIds array in the DeveloperActivity mongoose schema is unbounded and could grow large; add an application-level limit and validation to prevent excessive growth by updating the activeProjectIds path in DeveloperActivity.js to enforce a maximum array length (e.g., via Mongoose's validate or maxlength option) and document the expected max entries in the model comment/README; alternatively, if many project refs are expected, move these IDs to a separate collection or paginated subdocument store and update any functions that push/pop project IDs to respect the new limit and surface a clear error when exceeded.packages/common/src/models/PlatformEvent.js (3)
38-41: ⚖️ Poor tradeoffConsider size limits and validation for the
propertiesfield.The
propertiesfield usesMixedtype with no size constraints. Unbounded Mixed fields can lead to document bloat, slow queries, and eventual MongoDB document size limit (16MB) issues. Consider adding application-level validation or documentation specifying max size and allowed keys.🤖 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 `@packages/common/src/models/PlatformEvent.js` around lines 38 - 41, The PlatformEvent model's properties field is currently mongoose.Schema.Types.Mixed with no constraints; add validation to prevent oversized or unexpected keys by: implement a custom validator on the properties field in the PlatformEvent schema (or replace Mixed with a stricter subdocument/schema) that enforces a max serialized size (e.g., JSON.stringify(properties).length <= X bytes) and optionally restricts allowed top-level keys (whitelist) or depth, and update any create/update paths that set properties to ensure they respect this validation and return clear errors; reference the properties field in the PlatformEvent schema and the model construction to locate where to add the validator or nested schema.
24-24: ⚡ Quick winRemove redundant individual index on
developerId.The individual index
{ developerId: 1 }at line 24 is redundant because the compound index{ developerId: 1, event: 1, timestamp: -1 }at line 51 can serve queries ondeveloperIdalone via index prefix scanning. Keeping both wastes storage and slows down writes.♻️ Remove the redundant index
developerId: { type: mongoose.Schema.Types.ObjectId, ref: 'Developer', required: true, - index: true, },🤖 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 `@packages/common/src/models/PlatformEvent.js` at line 24, Remove the redundant single-field index on developerId: the model currently defines an individual index { developerId: 1 } and also a compound index { developerId: 1, event: 1, timestamp: -1 } (in PlatformEvent.js); drop the single-field index declaration for developerId so queries can use the compound index prefix and avoid extra storage and write overhead.
35-35: ⚡ Quick winRemove redundant individual index on
event.The individual index
{ event: 1 }at line 35 is redundant because the compound index{ event: 1, timestamp: -1 }at line 52 already provides efficient lookups oneventalone via index prefix. Duplicate indexes increase write overhead and storage costs.♻️ Remove the redundant index
event: { type: String, required: true, - index: true, },🤖 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 `@packages/common/src/models/PlatformEvent.js` at line 35, Remove the redundant single-field index on event in the PlatformEvent model: locate where the schema/indexes are defined (the entries `{ event: 1 }` and the compound `{ event: 1, timestamp: -1 }`) and delete the individual `{ event: 1 }` index so only the compound `{ event: 1, timestamp: -1 }` remains; this reduces duplicate index overhead while preserving query performance via the compound index prefix.apps/public-api/src/middlewares/api_usage.js (1)
83-84: 💤 Low valueHoist the
@urbackend/commonrequire to the top of the file.Calling
require('@urbackend/common')inside thesetImmediatecallback works because Node caches the resolved module, but it's the only inlinerequirein this file and it makes the dependency graph harder to read at a glance. BothProjectandPlatformEventare already exported by the package that's imported at the top of this file.♻️ Suggested change
-const { Log, redis, ApiAnalytics } = require('@urbackend/common'); +const { Log, redis, ApiAnalytics, Project, PlatformEvent } = require('@urbackend/common'); ... - const { Project, PlatformEvent } = require('@urbackend/common'); - const proj = await Project.findById(req.project._id).select('owner').lean(); + const proj = await Project.findById(req.project._id).select('owner').lean();🤖 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 `@apps/public-api/src/middlewares/api_usage.js` around lines 83 - 84, Remove the inline require inside the setImmediate callback and hoist the module import to the top of the file by adding a top-level const { Project, PlatformEvent } = require('@urbackend/common');; then update the setImmediate callback to use the already-imported Project and PlatformEvent (the occurrences around the setImmediate where proj is fetched with Project.findById and any PlatformEvent usage). Ensure there are no other inline requires for '@urbackend/common' left in this file.apps/dashboard-api/src/routes/admin.metrics.js (1)
14-22: ⚡ Quick winDefense-in-depth: enforce
isAdminat the router level too.Right now every route is
authMiddleware-only and relies on each controller callingrequireAdmin()internally. That's correct today, but the contract is invisible from the router and a new admin endpoint added later (or a refactor that drops the in-controller guard) will silently expose admin-only data to any logged-in developer. Adding the admin check once at the router is cheap and removes the coupling.♻️ Suggested change
const authMiddleware = require('../middlewares/authMiddleware'); +const requireAdmin = (req, res, next) => + req.user?.isAdmin ? next() : res.status(403).json({ success: false, data: {}, message: 'Admin access required.' }); + +router.use(authMiddleware, requireAdmin); ... -router.get('/overview', authMiddleware, getOverview); -router.get('/activation-funnel', authMiddleware, getActivationFunnel); -router.get('/cohorts', authMiddleware, getCohorts); -router.get('/feature-usage', authMiddleware, getFeatureUsage); -router.get('/reliability', authMiddleware, getReliability); -router.get('/top-projects', authMiddleware, getTopProjects); -router.get('/churn-signals', authMiddleware, getChurnSignals); +router.get('/overview', getOverview); +router.get('/activation-funnel', getActivationFunnel); +router.get('/cohorts', getCohorts); +router.get('/feature-usage', getFeatureUsage); +router.get('/reliability', getReliability); +router.get('/top-projects', getTopProjects); +router.get('/churn-signals', getChurnSignals);(Adjust the
requireAdminbody to match the existing project convention —AppError+ the{ success, data, message }envelope from your controller guidelines.)🤖 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 `@apps/dashboard-api/src/routes/admin.metrics.js` around lines 14 - 22, Add an explicit admin-check middleware to the router so admin-only routes use both authMiddleware and requireAdmin at the route level (e.g., change router.get('/overview', authMiddleware, getOverview) to router.get('/overview', authMiddleware, requireAdmin, getOverview) for all listed routes like '/overview', '/activation-funnel', '/cohorts', etc. Also update the requireAdmin implementation to follow project conventions by throwing an AppError (or passing an AppError to next) and returning the controller response envelope { success, data, message } on denial so the router-level guard matches existing error/response handling.apps/public-api/src/utils/emitEvent.js (1)
1-26: ⚡ Quick winDuplicated
emitEventhelper across services — consolidate into@urbackend/common.This file is functionally identical to
apps/dashboard-api/src/utils/emitEvent.js(same signature, samesetImmediate+PlatformEvent.createbody, same logging). Keeping two copies means future changes (tracing, sampling, batching, retry) will have to be applied in both places and will inevitably drift. SincePlatformEventis already exported from@urbackend/common, the helper belongs there too.Also note
Projectis imported but never used here.♻️ Suggested direction
Move the helper to
packages/common/src/utils/emitEvent.js, re-export frompackages/common/src/index.js, and replace both app-level copies with:-const { PlatformEvent, Project } = require('@urbackend/common'); - -function emitEvent(developerId, event, properties = {}, projectId = null) { - setImmediate(async () => { - try { - await PlatformEvent.create({ ... }); - } catch (err) { - console.error(`[emitEvent] Failed to write "${event}":`, err.message); - } - }); -} - -module.exports = { emitEvent }; +const { emitEvent } = require('@urbackend/common'); +module.exports = { emitEvent };🤖 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 `@apps/public-api/src/utils/emitEvent.js` around lines 1 - 26, The emitEvent helper is duplicated across services; move the function (signature emitEvent(developerId, event, properties = {}, projectId = null) that uses setImmediate and PlatformEvent.create) into the shared `@urbackend/common` utils, re-export it from the common package's public index, then update both app-level copies to import { emitEvent } from '@urbackend/common' instead of declaring it locally; also remove the unused Project import from the original file and ensure behavior (fire-and-forget, never throwing, same log message) remains identical after relocation.apps/web-dashboard/src/pages/AdminMetrics.jsx (1)
105-106: 💤 Low valueRemove unnecessary
queueMicrotaskwrapper from these useEffect hooks.The pattern of deferring callback execution with
queueMicrotaskis unusual and lacks a documented reason. Both callbacks can be called directly without the microtask deferral—the effect already runs at the appropriate time after render. This adds unnecessary complexity and makes debugging harder:Suggested change
- useEffect(() => { queueMicrotask(() => load()); }, [load]); - useEffect(() => { queueMicrotask(() => loadCohort()); }, [loadCohort]); + useEffect(() => { load(); }, [load]); + useEffect(() => { loadCohort(); }, [loadCohort]);🤖 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 `@apps/web-dashboard/src/pages/AdminMetrics.jsx` around lines 105 - 106, Remove the unnecessary queueMicrotask wrappers inside the two useEffect hooks: call load and loadCohort directly from their respective useEffect callbacks instead of wrapping them in queueMicrotask; update the effect bodies that reference load and loadCohort so they simply invoke load() and loadCohort() (preserving the dependency arrays) to simplify execution and make behavior easier to debug.apps/dashboard-api/src/controllers/analytics.controller.js (1)
244-244: ⚡ Quick winConsider optimizing unique project ID collection in aggregation.
The current approach uses
$push: '$activeProjectIds'which creates nested arrays (sinceactiveProjectIdsis already an array), then flattens them on line 258. For developers with many active days or projects, this could be memory-intensive.♻️ Proposed optimization
Add an
$unwindstage before$groupto flattenactiveProjectIdsupfront:const agg = await DeveloperActivity.aggregate([ { $match: { developerId: new mongoose.Types.ObjectId(developerId), date: { $gte: thirtyDaysAgo }, }, }, + { $unwind: { path: '$activeProjectIds', preserveNullAndEmptyArrays: true } }, { $group: { _id: null, totalApiCalls: { $sum: '$apiCallCount' }, totalMailSent: { $sum: '$mailSentCount' }, totalStorageUploads: { $sum: '$storageUploadsCount' }, totalWebhooksFired: { $sum: '$webhookTriggeredCount' }, activeDays: { $sum: 1 }, - allProjectIds: { $push: '$activeProjectIds' }, + allProjectIds: { $addToSet: '$activeProjectIds' }, }, }, ]);Then simplify line 259:
- const flatProjectIds = (result.allProjectIds || []).flat(); - const uniqueActiveProjects = new Set(flatProjectIds.map(String)).size; + const uniqueActiveProjects = (result.allProjectIds || []).filter(Boolean).length;Note: The
activeDayscount would need adjustment if you want distinct days (currently it counts per unwound document).🤖 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 `@apps/dashboard-api/src/controllers/analytics.controller.js` at line 244, The aggregation currently uses a $push of the array field activeProjectIds which creates nested arrays and is memory-inefficient; update the pipeline used in the analytics aggregation (the variable/array building the Mongo pipeline in the analytics controller) to $unwind the activeProjectIds field before the $group stage so each project id is emitted as a single value, then replace the group accumulator allProjectIds: { $push: '$activeProjectIds' } with allProjectIds: { $addToSet: '$activeProjectIds' } to collect unique project IDs without nested arrays; if you require distinct activeDays instead of counting unwound documents, adjust the activeDays calculation accordingly (e.g., use a separate $addToSet on the day field before counting).
🤖 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 `@apps/dashboard-api/src/controllers/admin.metrics.controller.js`:
- Around line 55-57: The catch blocks in admin.metrics.controller.js currently
return raw err.message to clients (e.g., the catch that sends
res.status(500).json({ success: false, data: {}, message: err.message }));
change each catch to log the full error internally (use the existing logger or
console.error) and return a generic client-facing message like "Internal server
error" or "An unexpected error occurred" while preserving success:false and
data:{}; apply this pattern to every catch in this file (the ones referenced in
the review) so controllers such as the admin metrics handlers never expose
MongoDB/internal error details to clients.
- Around line 147-166: checkDayRetention currently issues a
DeveloperActivity.findOne per signup causing N+1 queries; instead, for a given
daysAfter compute the per-cohort target and next UTC date boundaries, collect
all developerIds from signups, run a single DeveloperActivity.find (or
aggregate) with { developerId: { $in: developerIds }, date: { $gte: target, $lt:
next } } and count distinct developerId results to produce retained; update
checkDayRetention to accept signups and daysAfter and return the deduplicated
count (use DeveloperActivity.distinct or an aggregation pipeline) so you perform
one DB query per retention day rather than one per signup.
In `@apps/dashboard-api/src/controllers/analytics.controller.js`:
- Around line 318-320: Replace the direct exposure of err.message in the catch
block inside the analytics controller (the catch after the function that sends
analytics response) by wrapping the original error in the AppError class and
delegating to the global error handler instead of sending raw error text;
specifically, construct a new AppError with a safe client-facing message (e.g.,
"Failed to fetch analytics") and an appropriate status code, attach the original
error as metadata if needed, and call next(new AppError(...)) (or pass it into
the centralized error handler) rather than doing res.status(500).json({...,
message: err.message}).
- Around line 274-276: In the analytics controller catch block that currently
does res.status(500).json({ success: false, data: {}, message: err.message }),
stop exposing err.message; import/use the AppError class and replace that
response with forwarding a sanitized AppError to Express (e.g. next(new
AppError('Internal server error', 500))) and log the original err to server logs
(console.error or the existing logger) so internal MongoDB details aren't
returned to clients.
- Around line 159-161: The catch block in analytics.controller is exposing raw
err.message to clients; instead import and use the AppError class and the
Express error flow: replace res.status(500).json({...err.message}) with logging
the original error (e.g., logger.error(err) or console.error(err)) and call
next(new AppError('Internal server error', 500)); ensure the controller function
signature accepts next and add the AppError import (AppError) so no
MongoDB/internal messages are returned to clients.
- Around line 213-215: The catch block in the analytics controller currently
sends err.message to the client (res.status(500).json(...)), exposing internal
errors; replace this with creation/forwarding of an AppError so clients only
receive a generic message. Inside the catch, log the original err for server
diagnostics, then call next(new AppError('Internal server error', 500)) (or
construct an AppError and pass to next) instead of using err.message; reference
the catch's err variable, res usage and the AppError class to implement this
change in the analytics controller method.
In `@apps/dashboard-api/src/controllers/events.controller.js`:
- Around line 25-27: The 400 response in the events controller returns {
success, message } but must include a data field per guidelines; update the
handler that checks the event variable (the block validating "event" and calling
res.status(400).json(...)) to return { success: false, data: {}, message: 'event
name is required' } instead of the current shape so all responses include the
required data key.
- Around line 31-36: The JSON error response returned when
ALLOWED_FRONTEND_EVENTS does not contain normalizedEvent must follow the `{
success: bool, data: {}, message: "" }` order; update the response in the
controller (the block that checks `ALLOWED_FRONTEND_EVENTS.has(normalizedEvent)`
and calls `res.status(400).json(...)`) to return `success: false`, `data: {}`
and `message: "Unknown event: \"...\". Allowed: ..."` in that exact order while
keeping the 400 status and the same message content.
- Around line 47-50: The catch block in the events controller currently logs the
raw error and sends a raw 500 response; replace this with the AppError pattern:
import AppError from '@urbackend/common' and in the catch handler do not expose
err to the client—log it internally if needed but call next(new
AppError('Internal server error', 500)) (or throw new AppError(...)) instead of
res.status(500).json(...); ensure the unique catch block in events.controller
(the handler that currently does console.error('[events.controller] track
error:', err)) uses AppError and does not include err details in the
client-facing message.
In `@apps/public-api/src/middlewares/api_usage.js`:
- Around line 77-103: The NX Redis set for the activation flag (flagKey / the
redis.set call inside the setImmediate block) currently writes a permanent key;
change it to include a 2-year TTL so the key is set NX with expiry (e.g., use
redis.set(flagKey, '1', 'NX', 'EX', 63072000) or equivalent setnx + expire
sequence if your Redis client requires different args) to prevent re-emission
after a Redis flush or eviction.
In `@apps/web-dashboard/src/components/Dashboard/DeveloperMetrics.jsx`:
- Around line 21-31: The component currently swallows fetch errors and returns
null; add an error state (e.g., error, setError) in the DeveloperMetrics
component, set setError(err) in the catch of fetchMetrics (and clear error
before retries), and update the render logic so that when error is truthy you
render a user-facing error message and a retry control that calls fetchMetrics
again; keep using the existing loading, metrics, setLoading semantics so loading
still shows while fetching and metrics renders when successful.
In `@apps/web-dashboard/src/pages/AdminMetrics.jsx`:
- Around line 88-90: The catch block in AdminMetrics.jsx uses brittle string
matching on e.message to decide to call navigate('/dashboard') or setError;
change it to check a structured error property (e.g., e.code ===
'ADMIN_REQUIRED') first and fall back to the existing message check, so replace
the current string-only test with a check for a standardized error code (and
only then fallback to message includes), updating the handling around navigate
and setError accordingly; coordinate with the backend to return an error object
with a code (e.g., 'ADMIN_REQUIRED') so the front-end can reliably branch on
e.code while preserving the current fallback behavior.
In `@packages/common/src/models/PlatformEvent.js`:
- Around line 55-58: The TTL index on platformEventSchema is correct but we
should add a clarifying comment and safeguard to prevent accidental expiry
surprises: update the PlatformEvent.js near platformEventSchema.index to
document that expireAfterSeconds is calculated from the document's timestamp
field value (which defaults to Date.now) and warn that any explicit timestamp
overrides (e.g., in emitEvent.js and reliabilityAlertQueue.js where timestamp:
new Date() is set) will affect expiry; also ensure the schema keeps default:
Date.now and consider adding a unit test or runtime assertion in
emitEvent.js/reliabilityAlertQueue.js that timestamp is not set to a past date
before insert.
In `@packages/common/src/queues/activityRollupQueue.js`:
- Around line 64-78: The aggregation built by Log.aggregate currently computes
apiCallCount, mailCount, and storageCount but omits webhooks; add a
webhookTriggeredCount field to the pipeline (similar to mailCount/storageCount)
using a $sum with a $cond and $regexMatch on the request path (e.g., match
"/api/webhook" or "/api/webhooks" as your routes use) so webhook calls are
counted between dayStart and dayEnd, then update the downstream mapping logic
that reads logAgg results into the developer map to use this new
webhookTriggeredCount value instead of leaving webhookTriggeredCount initialized
to 0.
---
Outside diff comments:
In `@apps/dashboard-api/src/controllers/analytics.controller.js`:
- Line 115: The controller currently returns raw data via
res.json(formattedLogs); update the handler (the function that sends
formattedLogs in apps/dashboard-api/src/controllers/analytics.controller.js) to
wrap the payload in the standard envelope by returning res.json({ success: true,
data: formattedLogs, message: "" }) for successful responses (and similarly use
{ success:false, data:{}, message: "..." } for error paths) so all endpoints
conform to the `{ success: bool, data: {}, message: "" }` API contract.
- Around line 86-88: The catch currently returns err.message to the client;
instead log the original err internally (e.g., console.error(err)) and replace
the response with an AppError instance: create and pass new AppError('Internal
server error', 500) to the Express error handler via next(new AppError(...))
(ensure the controller signature includes next), removing any use of err.message
in res.status(...).json and keeping only a generic message to the client.
- Around line 116-118: In the catch block of the analytics controller replace
the direct response that exposes err.message with error-handling that uses the
AppError class and the route's next() so the centralized error middleware
formats the response as { success:false, data:{}, message:"" }; specifically,
remove res.status(500).json({ error: err.message }) and instead log the original
err (e.g., using console.error or processLogger.error) and call next(new
AppError(500, "Internal Server Error")) so no MongoDB/internal messages are sent
to the client and the global error handler returns the standardized payload.
In `@apps/dashboard-api/src/controllers/auth.controller.js`:
- Around line 161-189: When reconciling an existing Developer in the GitHub flow
(the branch that finds Developer via Developer.findOne and sets
developer.githubId/githubUsername/avatarUrl/isVerified), check the previous
isVerified value and, if it was false, call emitEvent(developer._id,
'email_verified', { method: 'github' }) after saving (or immediately before
returning) so the verification funnel is recorded; update the block that assigns
developer.isVerified = true and saves in the function handling the GitHub
profile to conditionally emit this event when transitioning from unverified to
verified.
---
Nitpick comments:
In `@apps/dashboard-api/src/controllers/analytics.controller.js`:
- Line 244: The aggregation currently uses a $push of the array field
activeProjectIds which creates nested arrays and is memory-inefficient; update
the pipeline used in the analytics aggregation (the variable/array building the
Mongo pipeline in the analytics controller) to $unwind the activeProjectIds
field before the $group stage so each project id is emitted as a single value,
then replace the group accumulator allProjectIds: { $push: '$activeProjectIds' }
with allProjectIds: { $addToSet: '$activeProjectIds' } to collect unique project
IDs without nested arrays; if you require distinct activeDays instead of
counting unwound documents, adjust the activeDays calculation accordingly (e.g.,
use a separate $addToSet on the day field before counting).
In `@apps/dashboard-api/src/routes/admin.metrics.js`:
- Around line 14-22: Add an explicit admin-check middleware to the router so
admin-only routes use both authMiddleware and requireAdmin at the route level
(e.g., change router.get('/overview', authMiddleware, getOverview) to
router.get('/overview', authMiddleware, requireAdmin, getOverview) for all
listed routes like '/overview', '/activation-funnel', '/cohorts', etc. Also
update the requireAdmin implementation to follow project conventions by throwing
an AppError (or passing an AppError to next) and returning the controller
response envelope { success, data, message } on denial so the router-level guard
matches existing error/response handling.
In `@apps/public-api/src/middlewares/api_usage.js`:
- Around line 83-84: Remove the inline require inside the setImmediate callback
and hoist the module import to the top of the file by adding a top-level const {
Project, PlatformEvent } = require('@urbackend/common');; then update the
setImmediate callback to use the already-imported Project and PlatformEvent (the
occurrences around the setImmediate where proj is fetched with Project.findById
and any PlatformEvent usage). Ensure there are no other inline requires for
'@urbackend/common' left in this file.
In `@apps/public-api/src/utils/emitEvent.js`:
- Around line 1-26: The emitEvent helper is duplicated across services; move the
function (signature emitEvent(developerId, event, properties = {}, projectId =
null) that uses setImmediate and PlatformEvent.create) into the shared
`@urbackend/common` utils, re-export it from the common package's public index,
then update both app-level copies to import { emitEvent } from
'@urbackend/common' instead of declaring it locally; also remove the unused
Project import from the original file and ensure behavior (fire-and-forget,
never throwing, same log message) remains identical after relocation.
In `@apps/web-dashboard/src/pages/AdminMetrics.jsx`:
- Around line 105-106: Remove the unnecessary queueMicrotask wrappers inside the
two useEffect hooks: call load and loadCohort directly from their respective
useEffect callbacks instead of wrapping them in queueMicrotask; update the
effect bodies that reference load and loadCohort so they simply invoke load()
and loadCohort() (preserving the dependency arrays) to simplify execution and
make behavior easier to debug.
In `@packages/common/src/models/DeveloperActivity.js`:
- Around line 27-30: The numeric activity fields in DeveloperActivity
(apiCallCount, mailSentCount, storageUploadsCount, webhookTriggeredCount) need
non-negative validation; update the Mongoose schema for those fields to include
a min: 0 validator (or equivalent validation) so attempts to set negative values
are rejected, and keep the existing default: 0; ensure any rollup/`$inc` paths
that update these fields still rely on schema validation or add runtime checks
to prevent negative results when applying decrements.
- Around line 23-26: The activeProjectIds array in the DeveloperActivity
mongoose schema is unbounded and could grow large; add an application-level
limit and validation to prevent excessive growth by updating the
activeProjectIds path in DeveloperActivity.js to enforce a maximum array length
(e.g., via Mongoose's validate or maxlength option) and document the expected
max entries in the model comment/README; alternatively, if many project refs are
expected, move these IDs to a separate collection or paginated subdocument store
and update any functions that push/pop project IDs to respect the new limit and
surface a clear error when exceeded.
In `@packages/common/src/models/PlatformEvent.js`:
- Around line 38-41: The PlatformEvent model's properties field is currently
mongoose.Schema.Types.Mixed with no constraints; add validation to prevent
oversized or unexpected keys by: implement a custom validator on the properties
field in the PlatformEvent schema (or replace Mixed with a stricter
subdocument/schema) that enforces a max serialized size (e.g.,
JSON.stringify(properties).length <= X bytes) and optionally restricts allowed
top-level keys (whitelist) or depth, and update any create/update paths that set
properties to ensure they respect this validation and return clear errors;
reference the properties field in the PlatformEvent schema and the model
construction to locate where to add the validator or nested schema.
- Line 24: Remove the redundant single-field index on developerId: the model
currently defines an individual index { developerId: 1 } and also a compound
index { developerId: 1, event: 1, timestamp: -1 } (in PlatformEvent.js); drop
the single-field index declaration for developerId so queries can use the
compound index prefix and avoid extra storage and write overhead.
- Line 35: Remove the redundant single-field index on event in the PlatformEvent
model: locate where the schema/indexes are defined (the entries `{ event: 1 }`
and the compound `{ event: 1, timestamp: -1 }`) and delete the individual `{
event: 1 }` index so only the compound `{ event: 1, timestamp: -1 }` remains;
this reduces duplicate index overhead while preserving query performance via the
compound index prefix.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 096de59e-93a2-49bb-98f2-755eeea33f30
📒 Files selected for processing (25)
apps/dashboard-api/src/__tests__/auth.controller.test.jsapps/dashboard-api/src/app.jsapps/dashboard-api/src/controllers/admin.metrics.controller.jsapps/dashboard-api/src/controllers/analytics.controller.jsapps/dashboard-api/src/controllers/auth.controller.jsapps/dashboard-api/src/controllers/events.controller.jsapps/dashboard-api/src/controllers/project.controller.jsapps/dashboard-api/src/routes/admin.metrics.jsapps/dashboard-api/src/routes/analytics.jsapps/dashboard-api/src/routes/events.jsapps/dashboard-api/src/utils/emitEvent.jsapps/public-api/src/app.jsapps/public-api/src/middlewares/api_usage.jsapps/public-api/src/utils/emitEvent.jsapps/web-dashboard/src/App.jsxapps/web-dashboard/src/components/Dashboard/DeveloperMetrics.jsxapps/web-dashboard/src/index.cssapps/web-dashboard/src/pages/AdminMetrics.jsxapps/web-dashboard/src/pages/Dashboard.jsxpackages/common/src/index.jspackages/common/src/models/DeveloperActivity.jspackages/common/src/models/PlatformEvent.jspackages/common/src/models/index.jspackages/common/src/queues/activityRollupQueue.jspackages/common/src/queues/reliabilityAlertQueue.js
| } catch (err) { | ||
| res.status(500).json({ success: false, data: {}, message: err.message }); | ||
| } |
There was a problem hiding this comment.
Replace raw error exposure with AppError.
The catch block directly exposes err.message to the client, which could leak internal MongoDB error details.
🛡️ Proposed fix
} catch (err) {
- res.status(500).json({ success: false, data: {}, message: err.message });
+ console.error('getActivationFunnel error:', err);
+ throw new AppError('Failed to retrieve activation funnel', 500);
}As per coding guidelines: "Use AppError class for errors — never raw throw, never expose MongoDB errors to client."
🤖 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 `@apps/dashboard-api/src/controllers/analytics.controller.js` around lines 159
- 161, The catch block in analytics.controller is exposing raw err.message to
clients; instead import and use the AppError class and the Express error flow:
replace res.status(500).json({...err.message}) with logging the original error
(e.g., logger.error(err) or console.error(err)) and call next(new
AppError('Internal server error', 500)); ensure the controller function
signature accepts next and add the AppError import (AppError) so no
MongoDB/internal messages are returned to clients.
| } catch (err) { | ||
| res.status(500).json({ success: false, data: {}, message: err.message }); | ||
| } |
There was a problem hiding this comment.
Replace raw error exposure with AppError.
The catch block directly exposes err.message to the client, which could leak internal MongoDB error details.
🛡️ Proposed fix
} catch (err) {
- res.status(500).json({ success: false, data: {}, message: err.message });
+ console.error('getRetention error:', err);
+ throw new AppError('Failed to retrieve retention data', 500);
}As per coding guidelines: "Use AppError class for errors — never raw throw, never expose MongoDB errors to client."
📝 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.
| } catch (err) { | |
| res.status(500).json({ success: false, data: {}, message: err.message }); | |
| } | |
| } catch (err) { | |
| console.error('getRetention error:', err); | |
| throw new AppError('Failed to retrieve retention data', 500); | |
| } |
🤖 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 `@apps/dashboard-api/src/controllers/analytics.controller.js` around lines 213
- 215, The catch block in the analytics controller currently sends err.message
to the client (res.status(500).json(...)), exposing internal errors; replace
this with creation/forwarding of an AppError so clients only receive a generic
message. Inside the catch, log the original err for server diagnostics, then
call next(new AppError('Internal server error', 500)) (or construct an AppError
and pass to next) instead of using err.message; reference the catch's err
variable, res usage and the AppError class to implement this change in the
analytics controller method.
| } catch (err) { | ||
| res.status(500).json({ success: false, data: {}, message: err.message }); | ||
| } |
There was a problem hiding this comment.
Replace raw error exposure with AppError.
The catch block directly exposes err.message to the client, which could leak internal MongoDB error details.
🛡️ Proposed fix
} catch (err) {
- res.status(500).json({ success: false, data: {}, message: err.message });
+ console.error('getEngagement error:', err);
+ throw new AppError('Failed to retrieve engagement data', 500);
}As per coding guidelines: "Use AppError class for errors — never raw throw, never expose MongoDB errors to client."
🤖 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 `@apps/dashboard-api/src/controllers/analytics.controller.js` around lines 274
- 276, In the analytics controller catch block that currently does
res.status(500).json({ success: false, data: {}, message: err.message }), stop
exposing err.message; import/use the AppError class and replace that response
with forwarding a sanitized AppError to Express (e.g. next(new
AppError('Internal server error', 500))) and log the original err to server logs
(console.error or the existing logger) so internal MongoDB details aren't
returned to clients.
| } catch (err) { | ||
| console.error('Failed to load personal metrics', err); | ||
| } finally { | ||
| setLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| fetchMetrics(); | ||
| }, []); | ||
|
|
||
| if (loading || !metrics) return null; |
There was a problem hiding this comment.
Consider showing error state to users.
The component logs fetch errors to the console but renders nothing on failure. Users won't know if metrics failed to load versus still loading. Consider adding an error state and displaying a message or retry option.
💡 Suggested improvement
export default function DeveloperMetrics() {
const [metrics, setMetrics] = useState(null);
const [loading, setLoading] = useState(true);
+ const [error, setError] = useState(null);
useEffect(() => {
const fetchMetrics = async () => {
try {
const [funnelRes, engRes] = await Promise.all([
api.get('/api/analytics/funnel'),
api.get('/api/analytics/engagement')
]);
setMetrics({
funnel: funnelRes.data?.data,
engagement: engRes.data?.data
});
} catch (err) {
console.error('Failed to load personal metrics', err);
+ setError('Failed to load metrics');
} finally {
setLoading(false);
}
};
fetchMetrics();
}, []);
- if (loading || !metrics) return null;
+ if (loading) return null;
+ if (error) return <div style={{color: 'var(--color-error)'}}>{error}</div>;
+ if (!metrics) return null;🤖 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 `@apps/web-dashboard/src/components/Dashboard/DeveloperMetrics.jsx` around
lines 21 - 31, The component currently swallows fetch errors and returns null;
add an error state (e.g., error, setError) in the DeveloperMetrics component,
set setError(err) in the catch of fetchMetrics (and clear error before retries),
and update the render logic so that when error is truthy you render a
user-facing error message and a retry control that calls fetchMetrics again;
keep using the existing loading, metrics, setLoading semantics so loading still
shows while fetching and metrics renders when successful.
| } catch (e) { | ||
| if (e.message?.includes('Admin')) navigate('/dashboard'); | ||
| else setError(e.message); |
There was a problem hiding this comment.
String-based error detection is fragile.
Checking if e.message?.includes('Admin') to determine redirect behavior is brittle. If the backend error message changes or is localized, this logic will break. Consider using error codes or a structured error response.
💡 Suggested improvement
Coordinate with the backend to return structured errors:
// Backend: throw new AppError('Admin access required', 403, 'ADMIN_REQUIRED');
// Frontend:
} catch (e) {
if (e.code === 'ADMIN_REQUIRED' || e.message?.includes('Admin')) {
navigate('/dashboard');
} else {
setError(e.message);
}
}🤖 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 `@apps/web-dashboard/src/pages/AdminMetrics.jsx` around lines 88 - 90, The
catch block in AdminMetrics.jsx uses brittle string matching on e.message to
decide to call navigate('/dashboard') or setError; change it to check a
structured error property (e.g., e.code === 'ADMIN_REQUIRED') first and fall
back to the existing message check, so replace the current string-only test with
a check for a standardized error code (and only then fallback to message
includes), updating the handling around navigate and setError accordingly;
coordinate with the backend to return an error object with a code (e.g.,
'ADMIN_REQUIRED') so the front-end can reliably branch on e.code while
preserving the current fallback behavior.
| platformEventSchema.index( | ||
| { timestamp: 1 }, | ||
| { expireAfterSeconds: 730 * 24 * 60 * 60 }, | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Search for PlatformEvent.create calls with timestamp field
rg -n -A5 -B2 --type=js --type=ts 'PlatformEvent\.create' | rg -C3 'timestamp'Repository: geturbackend/urBackend
Length of output: 876
🏁 Script executed:
# Find the PlatformEvent model definition
find . -name "PlatformEvent*" -type f | head -20Repository: geturbackend/urBackend
Length of output: 112
🏁 Script executed:
# Search for the model file location and examine timestamp field definition
rg -l "platformEventSchema" --type=js --type=tsRepository: geturbackend/urBackend
Length of output: 110
🏁 Script executed:
# Check for all PlatformEvent instantiation patterns beyond .create()
rg -n "new PlatformEvent|PlatformEvent\.save|PlatformEvent\.insert" --type=js --type=ts | head -20Repository: geturbackend/urBackend
Length of output: 174
🏁 Script executed:
cat packages/common/src/models/PlatformEvent.jsRepository: geturbackend/urBackend
Length of output: 1932
🏁 Script executed:
# Check reliabilityAlertQueue.js around line 109 to see how events are structured
sed -n '100,120p' packages/common/src/queues/reliabilityAlertQueue.jsRepository: geturbackend/urBackend
Length of output: 560
🏁 Script executed:
# Check both emitEvent.js files to see full context
cat apps/public-api/src/utils/emitEvent.jsRepository: geturbackend/urBackend
Length of output: 748
TTL index correctly expires documents 730 days from the timestamp field.
The schema has default: Date.now for timestamp, and current code (in emitEvent.js and reliabilityAlertQueue.js) explicitly sets timestamp: new Date(), ensuring documents expire based on creation time. The TTL behavior is working as intended. Note that if timestamp is ever explicitly set to a past or future date in the future, expiry would be calculated from that value, not insertion time—so be cautious when overriding the timestamp field.
🤖 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 `@packages/common/src/models/PlatformEvent.js` around lines 55 - 58, The TTL
index on platformEventSchema is correct but we should add a clarifying comment
and safeguard to prevent accidental expiry surprises: update the
PlatformEvent.js near platformEventSchema.index to document that
expireAfterSeconds is calculated from the document's timestamp field value
(which defaults to Date.now) and warn that any explicit timestamp overrides
(e.g., in emitEvent.js and reliabilityAlertQueue.js where timestamp: new Date()
is set) will affect expiry; also ensure the schema keeps default: Date.now and
consider adding a unit test or runtime assertion in
emitEvent.js/reliabilityAlertQueue.js that timestamp is not set to a past date
before insert.
There was a problem hiding this comment.
Pull request overview
This PR adds a first-party analytics/metrics stack (event logging + rollups) to track activation, engagement/retention, and platform reliability, and surfaces the results in both the developer dashboard and an admin/operator metrics page.
Changes:
- Introduces new Mongo models (
PlatformEvent,DeveloperActivity) plus BullMQ queues for daily activity rollups and frequent reliability spike detection. - Adds instrumentation + new analytics/admin endpoints in
dashboard-api, and emitsfirst_api_successfrompublic-apimiddleware. - Adds new React UI for per-developer metrics and an admin “Platform Metrics” page.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/common/src/queues/reliabilityAlertQueue.js | Adds a BullMQ repeatable job + worker to detect error-rate spikes and write reliability_spike events. |
| packages/common/src/queues/activityRollupQueue.js | Adds a daily BullMQ rollup job to aggregate Log data into DeveloperActivity. |
| packages/common/src/models/PlatformEvent.js | Introduces a TTL’d event collection with indexes for funnel queries. |
| packages/common/src/models/index.js | Expands model exports for queue modules to import from a single index. |
| packages/common/src/models/DeveloperActivity.js | Introduces a per-developer per-day rollup schema (unique index on developerId+date). |
| packages/common/src/index.js | Exports the new models and queues from @urbackend/common. |
| apps/web-dashboard/src/pages/Dashboard.jsx | Adds the new per-developer metrics card to the main dashboard page. |
| apps/web-dashboard/src/pages/AdminMetrics.jsx | Adds a new operator/admin metrics page with overview, funnel, cohorts, reliability, top projects, and churn. |
| apps/web-dashboard/src/index.css | Adds styling for the new Admin Metrics page. |
| apps/web-dashboard/src/components/Dashboard/DeveloperMetrics.jsx | Adds a “My Performance” component that calls new analytics endpoints and renders activation + 30-day engagement. |
| apps/web-dashboard/src/App.jsx | Registers a new /admin/metrics route in the web dashboard. |
| apps/public-api/src/utils/emitEvent.js | Adds a fire-and-forget helper for writing PlatformEvent from public-api. |
| apps/public-api/src/middlewares/api_usage.js | Emits first_api_success once per project using a Redis NX flag. |
| apps/public-api/src/app.js | Initializes and schedules the new BullMQ workers/cron jobs in public-api startup. |
| apps/dashboard-api/src/utils/emitEvent.js | Adds a fire-and-forget helper for writing PlatformEvent from dashboard-api. |
| apps/dashboard-api/src/routes/events.js | Adds an endpoint for dashboard UI → backend event tracking. |
| apps/dashboard-api/src/routes/analytics.js | Adds developer-facing metrics endpoints (funnel/retention/engagement/north-star). |
| apps/dashboard-api/src/routes/admin.metrics.js | Adds admin-only metrics endpoints under /api/admin/metrics/*. |
| apps/dashboard-api/src/controllers/project.controller.js | Emits project_created / collection_created activation events on successful commit. |
| apps/dashboard-api/src/controllers/events.controller.js | Implements frontend event allowlist + normalization and forwards to emitEvent. |
| apps/dashboard-api/src/controllers/auth.controller.js | Emits signup_completed and email_verified activation events. |
| apps/dashboard-api/src/controllers/analytics.controller.js | Implements new funnel/retention/engagement/north-star endpoints. |
| apps/dashboard-api/src/controllers/admin.metrics.controller.js | Implements admin overview/funnel/cohorts/usage/reliability/top-projects/churn queries. |
| apps/dashboard-api/src/app.js | Wires the new events and admin metrics routes. |
| apps/dashboard-api/src/tests/auth.controller.test.js | Updates mocks to accommodate new PlatformEvent.create() usage in auth flows. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| await activityRollupQueue.add( | ||
| 'daily-rollup', | ||
| {}, | ||
| { | ||
| repeat: { cron: '5 0 * * *' }, // 00:05 UTC daily | ||
| removeOnComplete: true, | ||
| removeOnFail: { count: 10 }, | ||
| }, | ||
| ); |
| @@ -0,0 +1,161 @@ | |||
| const { Queue, Worker } = require('bullmq'); | |||
| const connection = require('../config/redis'); | |||
| const mongoose = require('mongoose'); | |||
| const ownerId = ownerMap[row._id.toString()]; | ||
| if (!ownerId) continue; | ||
| const key = ownerId.toString(); | ||
| if (!devMap[key]) { | ||
| devMap[key] = { | ||
| developerId: ownerId, | ||
| activeProjectIds: [], | ||
| apiCallCount: 0, | ||
| mailSentCount: 0, | ||
| storageUploadsCount: 0, | ||
| webhookTriggeredCount: 0, | ||
| }; | ||
| } | ||
| devMap[key].activeProjectIds.push(row._id); | ||
| devMap[key].apiCallCount += row.apiCallCount; | ||
| devMap[key].mailSentCount += row.mailCount; | ||
| devMap[key].storageUploadsCount += row.storageCount; | ||
| } |
| // 1. Aggregate logs by project for the day | ||
| const logAgg = await Log.aggregate([ | ||
| { $match: { timestamp: { $gte: dayStart, $lt: dayEnd } } }, | ||
| { |
| /** | ||
| * Run the reliability check. | ||
| * Looks at the last 15 minutes of ApiAnalytics. | ||
| * If a project has >50 total requests and >5% error rate (5xx or 4xx depending on preference, we'll use >= 500 for true platform errors), |
| return res.json({ | ||
| success: true, | ||
| data: { | ||
| window: '30d', | ||
| totalApiCalls: result.totalApiCalls, | ||
| totalMailSent: result.totalMailSent, | ||
| totalStorageUploads: result.totalStorageUploads, | ||
| totalWebhooksFired: result.totalWebhooksFired, | ||
| activeDays: result.activeDays, | ||
| uniqueActiveProjects, | ||
| }, | ||
| message: '', |
| // --- Activation funnel: first_api_success --- | ||
| // Fires only once per project lifetime, on the very first 2xx response. | ||
| // Uses a permanent Redis NX flag so we don't hit MongoDB on every request. | ||
| if (req.project && res.statusCode >= 200 && res.statusCode < 300) { | ||
| setImmediate(async () => { | ||
| try { | ||
| const flagKey = `project:activation:first_api_success:${req.project._id}`; | ||
| const isFirst = await redis.set(flagKey, '1', 'NX'); | ||
| if (isFirst) { | ||
| const { Project, PlatformEvent } = require('@urbackend/common'); |
| @@ -0,0 +1,25 @@ | |||
| const { PlatformEvent, Project } = require('@urbackend/common'); | |||
| const {initActivityRollupWorker, scheduleActivityRollup} = require('@urbackend/common'); | ||
| const {initReliabilityAlertWorker, scheduleReliabilityAlert} = require('@urbackend/common'); | ||
|
|
||
| // Initialize webhook worker | ||
| if (process.env.NODE_ENV !== 'test') { | ||
| initWebhookWorker(); | ||
| initAuthEmailWorker(); | ||
| initPublicEmailWorker(); | ||
| initActivityRollupWorker(); | ||
| scheduleActivityRollup().catch((err) => | ||
| console.error('[ActivityRollup] Failed to schedule cron:', err.message) | ||
| ); | ||
| initReliabilityAlertWorker(); | ||
| scheduleReliabilityAlert().catch((err) => | ||
| console.error('[ReliabilityAlert] Failed to schedule cron:', err.message) | ||
| ); |
| ] = await Promise.all([ | ||
| Developer.countDocuments(), | ||
| Developer.countDocuments({ isVerified: true }), | ||
| Project.countDocuments(), | ||
| Log.countDocuments(), | ||
| Log.distinct('projectId', { | ||
| status: { $gte: 200, $lt: 300 }, | ||
| timestamp: { $gte: sevenDaysAgo }, | ||
| }), | ||
| ]); |
Agent-Logs-Url: https://github.com/geturbackend/urBackend/sessions/97646711-efe2-4fe4-a00f-1a81c8dfd698 Co-authored-by: yash-pouranik <172860064+yash-pouranik@users.noreply.github.com>
Agent-Logs-Url: https://github.com/geturbackend/urBackend/sessions/97646711-efe2-4fe4-a00f-1a81c8dfd698 Co-authored-by: yash-pouranik <172860064+yash-pouranik@users.noreply.github.com>
|
Implemented the requested PR feedback in commit UI screenshot (DeveloperMetrics field alignment update): |
|
ok to merge?? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 204939
🔴 Blocking Issues
🟡 Should-Fix Before Merge
ℹ️ Pre-Merge Check Failures
✅ What's Already Good
Once items 1–5 are addressed (especially the auth gap on |
|
@copilot |
Agent-Logs-Url: https://github.com/geturbackend/urBackend/sessions/9df9708d-0389-45cb-97ef-0f88fa9147fc Co-authored-by: yash-pouranik <172860064+yash-pouranik@users.noreply.github.com>
Done in |
|
please check now |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 16674 🏁 Script executed: Length of output: 23547 🏁 Script executed: Length of output: 6358
✅ All 5 Blocking Issues Fixed
✅ Other Previously Flagged Items Also Fixed
🟡 Remaining Minor Open Items (Non-Blocking)These won't break functionality but are worth a follow-up PR:
This PR is good to merge. All CI checks pass and all blocking issues are resolved. 🚀 [approve] |
|
please create one follow up issue @coderabbitai |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainSure! Creating the follow-up issue now. 🏁 Script executed: Length of output: 53
It covers the three remaining items:
|
Overview
This PR introduces a comprehensive, self-hosted Startup Metrics Stack to monitor platform health, track developer activation, and measure product retention without relying on external third-party dependencies.
The system is built on asynchronous event logging (
emitEvent) and daily BullMQ rollups to ensure absolute zero performance penalty on critical path API requests.Key Changes
1. Core Data Models & Queues (
packages/common)PlatformEventmodel (2-yr TTL) for discrete actions (e.g., signup, API success).DeveloperActivitymodel for daily time-series feature usage aggregation.activityRollupQueue(cron job running daily at00:05 UTC) to compress raw logs into cohort/retention data.reliabilityAlertQueue(cron job running every 5 minutes) to proactively detect project error rate spikes (>5% in 15 min window) and logreliability_spikeevents.2. Activation Funnel Instrumentation
auth.controller(signup_completed,email_verified).project.controller(project_created,collection_created).api_usagemiddleware using a Redis atomic flag (NX) to guaranteefirst_api_successis emitted exactly once per project without DB overhead.3. Operator Admin Dashboard
dashboard-api(/api/admin/metrics/*) protected by JWT +isAdminguards.AdminMetrics.jsxoperator page in the React dashboard featuring:4. Per-Developer UX
/api/analytics/funnel,/api/analytics/engagement).DeveloperMetricscomponent to the main developer dashboard to show their personal Activation Status and 30-Day Activity at a glance.Testing & Verification
apps/public-apiandapps/dashboard-apitest suites fully passing (100% success rate on existing auth/RLS tests).npm run lintpassing).```Ran command:
clearSummary by CodeRabbit