fix: add statistics update in startup #38127
Conversation
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: 116369d The changes in this PR will be included in the next version bump. This PR includes changesets to release 40 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughA new method Changes
Sequence DiagramsequenceDiagram
participant Migration as Migration System
participant Stats as Statistics Service
participant DB as Database
Migration->>Migration: Detect server version change
activate Migration
Migration->>Stats: updateDeploymentData()
deactivate Migration
activate Stats
Stats->>DB: Load latest Statistics document
DB-->>Stats: Statistics data
Stats->>Stats: Gather deployment metadata<br/>(OS, process, deploy method, etc.)
Stats->>Stats: Merge with base statistics
Stats->>DB: Insert new Statistics document<br/>with fresh createdAt
DB-->>Stats: Success
deactivate Stats
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (2)
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: 1
🤖 Fix all issues with AI agents
In @apps/meteor/server/startup/serverRunning.ts:
- Around line 82-83: Replace the space-indented line calling statistics.save()
with a tab-indented one and wrap the await statistics.save() call in a try/catch
so DB errors don’t abort startup; specifically change to: await
statistics.save() inside a try block and in the catch log the error (e.g.
console.error or the file’s logger) and continue so the success banner still
displays. Ensure indentation matches surrounding tabs and preserve the await
call semantics.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/meteor/server/startup/serverRunning.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/server/startup/serverRunning.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (1)
apps/meteor/server/startup/serverRunning.ts (1)
9-9: LGTM!The import follows the existing pattern and correctly imports the statistics module needed for the startup save operation.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #38127 +/- ##
===========================================
+ Coverage 70.65% 70.67% +0.01%
===========================================
Files 3147 3147
Lines 108854 108855 +1
Branches 19586 19613 +27
===========================================
+ Hits 76916 76928 +12
+ Misses 29930 29925 -5
+ Partials 2008 2002 -6
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
📦 Docker Image Size Report➡️ Changes
📊 Historical Trend---
config:
theme: "dark"
xyChart:
width: 900
height: 400
---
xychart
title "Image Size Evolution by Service (Last 30 Days + This PR)"
x-axis ["11/18 22:53", "11/19 23:02", "11/21 16:49", "11/24 17:34", "11/27 22:32", "11/28 19:05", "12/01 23:01", "12/02 21:57", "12/03 21:00", "12/04 18:17", "12/05 21:56", "12/08 20:15", "12/09 22:17", "12/10 23:26", "12/11 21:56", "12/12 22:45", "12/13 01:34", "12/15 22:31", "12/16 22:18", "12/17 21:04", "12/18 23:12", "12/19 23:27", "12/20 21:03", "12/22 18:54", "12/23 16:16", "12/24 19:38", "12/25 17:51", "12/26 13:18", "12/29 19:01", "12/30 20:52", "01/14 14:51 (PR)"]
y-axis "Size (GB)" 0 --> 0.5
line "account-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.00]
line "authorization-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.00]
line "ddp-streamer-service" [0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.00]
line "omnichannel-transcript-service" [0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.00]
line "presence-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.00]
line "queue-worker-service" [0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.00]
line "rocketchat" [0.35, 0.35, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.00]
Statistics (last 30 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Wed, 14 Jan 2026 14:51:43 GMT |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @apps/meteor/app/statistics/server/lib/statistics.ts:
- Around line 602-651: Wrap the call to updateDeploymentInfo() in
apps/meteor/server/startup/serverRunning.ts inside a try-catch block (mirroring
the Node/MongoDB validation pattern used in the same function) so any errors
from getMongoInfo(), getControl(), or Statistics.insertOne() are caught and
logged instead of crashing startup; in the catch, use the same logging mechanism
as lines 61-80 to record the error and continue startup.
🧹 Nitpick comments (1)
apps/meteor/app/statistics/server/lib/statistics.ts (1)
611-636: Consider extracting shared deployment info collection logic.The deployment info construction (OS, process, deploy, mongo, migration) duplicates code from the
get()method (lines 325-387). Extracting this into a shared helper would improve maintainability and ensure consistency when either method is updated.♻️ Suggested refactor
// Extract to a helper function async function getDeploymentInfo(): Promise<Pick<IStats, 'os' | 'process' | 'deploy' | 'msEnabled' | 'mongoVersion' | 'mongoStorageEngine' | 'migration'>> { const { mongoVersion, mongoStorageEngine } = await getMongoInfo(); return { os: { type: os.type(), platform: os.platform(), arch: os.arch(), release: os.release(), uptime: os.uptime(), loadavg: os.loadavg(), totalmem: os.totalmem(), freemem: os.freemem(), cpus: os.cpus(), }, process: { nodeVersion: process.version, pid: process.pid, uptime: process.uptime(), }, deploy: { method: process.env.DEPLOY_METHOD || 'tar', platform: process.env.DEPLOY_PLATFORM || 'selfinstall', }, msEnabled: isRunningMs(), mongoVersion, mongoStorageEngine: mongoStorageEngine || '', migration: await getControl(), }; }Then use in both
get()andupdateDeploymentInfo().
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.changeset/tall-timers-swim.mdapps/meteor/app/statistics/server/lib/statistics.tsapps/meteor/server/startup/serverRunning.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/meteor/server/startup/serverRunning.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/app/statistics/server/lib/statistics.ts
🧬 Code graph analysis (1)
apps/meteor/app/statistics/server/lib/statistics.ts (2)
apps/meteor/app/utils/server/functions/getMongoInfo.ts (1)
getMongoInfo(41-61)packages/core-typings/src/IStats.ts (1)
IStats(20-275)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (2)
.changeset/tall-timers-swim.md (1)
1-5: LGTM!The changeset correctly describes the fix and uses the appropriate patch level for a bug fix.
apps/meteor/app/statistics/server/lib/statistics.ts (1)
642-650: LGTM!The merge logic correctly preserves all existing statistics fields while updating deployment-specific data with a fresh
createdAttimestamp. Excluding_idfrom the spread and letting MongoDB generate a new one is the right approach.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 3 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/meteor/server/startup/serverRunning.ts">
<violation number="1" location="apps/meteor/server/startup/serverRunning.ts:82">
P2: `statistics.updateDeploymentInfo()` no-ops when there is no existing statistics document, so replacing the startup `statistics.save()` call regresses fresh deployments—they will not have any statistics persisted until a manual refresh.</violation>
<violation number="2" location="apps/meteor/server/startup/serverRunning.ts:82">
P1: The `updateDeploymentInfo()` call lacks error handling. If `getMongoInfo()`, `getControl()`, or `Statistics.insertOne()` throws an error, it will crash the server startup process. Wrap this call in a try-catch block to log errors gracefully without blocking server startup, matching the error handling pattern used for Node and MongoDB validation earlier in this function.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.changeset/tall-timers-swim.mdapps/meteor/app/statistics/server/lib/statistics.tsapps/meteor/server/lib/migrations.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/meteor/app/statistics/server/lib/statistics.ts
- .changeset/tall-timers-swim.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/server/lib/migrations.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: 📦 Build Packages
- GitHub Check: cubic · AI code reviewer
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (1)
apps/meteor/server/lib/migrations.ts (1)
8-8: LGTM!Import follows the existing relative path patterns used in this file.
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/meteor/app/statistics/server/lib/statistics.ts">
<violation number="1" location="apps/meteor/app/statistics/server/lib/statistics.ts:654">
P2: The caught `error` is not included in the log output, losing valuable debugging information. Following the codebase pattern, pass the error object to the logger.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @apps/meteor/app/statistics/server/lib/statistics.ts:
- Around line 653-655: The catch block currently logs a generic message but
omits the caught error; update the catch in the statistics save flow so
SystemLogger.error includes the error details (e.g., SystemLogger.error('Error
saving statistics with new deployment data', error) or append
error.message/stack). Locate the catch that logs 'Error saving statistics with
new deployment data' and pass the caught error (or its message/stack) into
SystemLogger.error to preserve the failure information.
🧹 Nitpick comments (1)
apps/meteor/app/statistics/server/lib/statistics.ts (1)
613-638: Consider extracting shared deployment data collection.The OS, process, deploy, and Mongo info collection duplicates lines 326-347 and 384-388 in the
get()method. Extracting this into a helper function would reduce duplication and ensure both code paths stay in sync.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/meteor/app/statistics/server/lib/statistics.tsapps/meteor/server/lib/migrations.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/meteor/server/lib/migrations.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/app/statistics/server/lib/statistics.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (2)
apps/meteor/app/statistics/server/lib/statistics.ts (2)
41-41: LGTM!The
SystemLoggerimport is appropriate for the error handling in the newupdateDeploymentDatamethod.
603-652: LGTM!The method correctly retrieves the last statistics record, collects fresh deployment data, and inserts a new document with updated fields. The early return when no prior statistics exist is appropriate, and the destructuring to exclude
_idprevents duplicate key errors on insert. This addresses the PR objective of ensuring deployment info is current after server restarts.
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/meteor/app/statistics/server/lib/statistics.ts (1)
614-642: Consider extracting shared deployment data gathering logic.The deployment data collection (OS, process, deploy, mongo info) duplicates code from the
get()method (lines 326-388). Extracting this into a helper function would reduce duplication and ensure both methods stay in sync.Additionally, the
get()method also capturestagandbranchfromInfo(lines 109-110), which are not included here. If these fields should remain current, consider adding them:♻️ Suggested addition for tag and branch
if (Info) { deploymentInfo.version = Info.version; + deploymentInfo.tag = Info.tag; + deploymentInfo.branch = Info.branch; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/meteor/app/statistics/server/lib/statistics.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/app/statistics/server/lib/statistics.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (2)
apps/meteor/app/statistics/server/lib/statistics.ts (2)
41-41: LGTM!The
SystemLoggerimport is appropriate for structured error logging in the new method.
603-656: Implementation correctly addresses the PR objective.The method properly:
- Fetches the last statistics record and gracefully handles the case when none exists
- Gathers current deployment data (version, nodeVersion, pid, mongoVersion, etc.)
- Preserves existing statistics while updating only deployment-related fields
- Uses structured error logging via
SystemLoggerThe merge operation
{ ...baseStatistics, ...deploymentInfo, createdAt: new Date() }correctly overlays fresh deployment data onto the existing statistics.
KevLehman
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
Proposed changes (including videos or screenshots)
There is data not instantly being updated when upgrading/downgrading versions. Most precisely, the problem was in the statistics record not being updated (properties such as version, nodeVersion, pid, mongoVersion were not being reflected in the /admin/info view). To solve this I suggest running the statistics.save function during the platform startup so that after the server deployment, the user will see the latest information about it without pressing the refresh button.
Issue(s)
CORE-1180 Mismatch between server version and deployment version on info page after updating
Steps to test or reproduce
Further comments
Some context: The deployment card uses data pulled from the statistics collection, the upper card (which has the different version) uses build data.
At first I thought the issue was that the frontend was calling the get API endpoint api/v1/statistics with the refresh queryParam set to false. Although this is true, because when we first get into the /admin/info route, the fetch of statistics data is made with refresh=false, I considered this as a intended behavior because the statistics refresh is an expensive operation and it shouldn't be made every time we go to the info page (that was my reasoning). So, in the end I decided to add this operation during the startup in order to get this updated even before the users get to the info panel.
EDIT 1: The first modification I've made, was creating a separate function named updateDeploymentData for statistics, because this only updates the critical data (version, os, mongoVersion, mongoEngine) instead of everything of the stats as with the save function (which is super expensive if you have too many docs in your db). I've ended up performing the creation of statistics with the updated deployment data after upgrading/downgrading the platform version, making use of the onServerChangeVersion hook. Finally, I've added the changeset markdown file.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.