feat: add environment variable to disable statistics reporting #38020
Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
WalkthroughAdds an environment-driven opt-out for telemetry: Mocha spec discovery updated; sendUsageReport respects RC_DISABLE_STATISTICS_REPORTING (still saves local stats); usageReport cron exposes a startup warning when reporting is disabled; new unit tests cover both opt-out states. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Cron as UsageReportCron
participant Send as sendUsageReport
participant DB as Statistics DB
participant Token as Token Store
participant Collector as Collector API
Note over Cron: Cron job triggers sendUsageReport
Cron->>Send: invoke sendUsageReport()
Send->>DB: generate & save local statistics
Send->>Token: getWorkspaceAccessToken()
Send->>Send: compute shouldSendToCollector (env check)
alt shouldSendToCollector == true
Send->>Collector: POST stats (serverFetch) with token
Collector-->>Send: response
else shouldSendToCollector == false
Note right of Send: Skip external POST (opt-out)\nLocal stats still saved
end
Send-->>Cron: return undefined
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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)
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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #38020 +/- ##
===========================================
+ Coverage 70.65% 70.66% +0.01%
===========================================
Files 3143 3145 +2
Lines 108695 108761 +66
Branches 19557 19587 +30
===========================================
+ Hits 76799 76858 +59
- Misses 29894 29898 +4
- Partials 2002 2005 +3
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/07 17:52 (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, 07 Jan 2026 17:52:26 GMT |
ricardogarim
force-pushed
the
feat/disable-send-usage-metrics
branch
from
4c8d885 to
1f3e831
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 4 files
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/functions/sendUsageReport.ts">
<violation number="1" location="apps/meteor/app/statistics/server/functions/sendUsageReport.ts:55">
P2: Missing `return` statement causes unintended fallthrough. When `shouldSendToCollector` is `false` and there are recent unsent statistics, the code falls through to `statistics.save()`, regenerating statistics unnecessarily on every cron run instead of reusing existing data.</violation>
</file>
<file name="apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts">
<violation number="1" location="apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts:13">
P2: `statistics.save` mock lacks a return value. The implementation uses the returned stats object in the request body. Consider returning a mock stats object.</violation>
<violation number="2" location="apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts:16">
P2: `serverFetch` mock lacks a return value. The implementation calls `response.json()` on the fetch result, which will fail on undefined. Consider returning a mock response with a `json()` method.</violation>
<violation number="3" location="apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts:26">
P1: Missing mock for `@rocket.chat/tracing`. The implementation uses `tracerSpan` to wrap the function logic, but this module isn't mocked in the proxyquire setup. This may cause test failures or unexpected behavior.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/meteor/server/cron/usageReport.ts (1)
17-24: Consider extracting shouldSendToCollector to avoid duplication.The same logic
process.env.RC_DISABLE_STATISTICS_REPORTING?.toLowerCase() !== 'true'appears in both:
- Line 19 (this file)
- Line 39 in apps/meteor/app/statistics/server/functions/sendUsageReport.ts
While this duplication is minor, extracting it to a shared utility function would ensure consistency and make future changes easier. For example:
// In a shared utils file export const shouldSendStatisticsToCollector = () => process.env.RC_DISABLE_STATISTICS_REPORTING?.toLowerCase() !== 'true';This is a low-priority refactor that can be deferred if you prefer to keep the implementation simple.
📜 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 (4)
apps/meteor/.mocharc.jsapps/meteor/app/statistics/server/functions/sendUsageReport.spec.tsapps/meteor/app/statistics/server/functions/sendUsageReport.tsapps/meteor/server/cron/usageReport.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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/functions/sendUsageReport.spec.tsapps/meteor/app/statistics/server/functions/sendUsageReport.tsapps/meteor/server/cron/usageReport.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.spec.ts: Use descriptive test names that clearly communicate expected behavior in Playwright tests
Use.spec.tsextension for test files (e.g.,login.spec.ts)
Files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
🧠 Learnings (13)
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.tsapps/meteor/.mocharc.js
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.tsapps/meteor/.mocharc.js
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.tsapps/meteor/.mocharc.js
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : All test files must be created in `apps/meteor/tests/e2e/` directory
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.tsapps/meteor/.mocharc.js
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.tsapps/meteor/.mocharc.js
📚 Learning: 2025-10-06T20:30:45.540Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37152
File: packages/apps-engine/tests/test-data/storage/storage.ts:101-122
Timestamp: 2025-10-06T20:30:45.540Z
Learning: In `packages/apps-engine/tests/test-data/storage/storage.ts`, the stub methods (updatePartialAndReturnDocument, updateStatus, updateSetting, updateAppInfo, updateMarketplaceInfo) intentionally throw "Method not implemented." Tests using these methods must stub them using `SpyOn` from the test library rather than relying on actual implementations.
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.tsapps/meteor/.mocharc.js
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure clean state for each test execution in Playwright tests
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-12-10T21:00:54.909Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:54.909Z
Learning: Rocket.Chat monorepo: Jest testMatch pattern '<rootDir>/src/**/*.spec.(ts|js|mjs)' is valid in this repo and used across multiple packages (e.g., packages/tools, ee/packages/omnichannel-services). Do not flag it as invalid in future reviews.
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.tsapps/meteor/.mocharc.js
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.spec.ts : Use `.spec.ts` extension for test files (e.g., `login.spec.ts`)
Applied to files:
apps/meteor/.mocharc.js
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Store commonly used locators in variables/constants for reuse
Applied to files:
apps/meteor/.mocharc.js
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.{ts,tsx,js} : Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Applied to files:
apps/meteor/.mocharc.js
🧬 Code graph analysis (2)
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts (1)
apps/meteor/app/statistics/server/functions/sendUsageReport.ts (1)
sendUsageReport(36-67)
apps/meteor/server/cron/usageReport.ts (1)
apps/meteor/app/statistics/server/functions/sendUsageReport.ts (1)
sendUsageReport(36-67)
⏰ 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). (10)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
🔇 Additional comments (5)
apps/meteor/.mocharc.js (1)
31-31: LGTM!The addition of the statistics server test pattern enables discovery of the new unit tests and follows the established pattern in this configuration file.
apps/meteor/app/statistics/server/functions/sendUsageReport.ts (3)
37-39: LGTM! Clear intent and correct implementation.The shouldSendToCollector flag correctly implements the opt-out behavior per the PR objectives. The inline comment appropriately explains why local statistics generation is preserved even when reporting is disabled.
55-57: LGTM! Correct conditional sending in retry path.The retry path correctly respects the shouldSendToCollector flag, preventing transmission while still preserving the existing retry logic.
63-65: LGTM! Correct conditional sending for new statistics.The new statistics generation path correctly respects the shouldSendToCollector flag. Returning undefined when not sending is intentional and properly handled by the caller (sendUsageReportAndComputeRestriction falls back to the last token from the database).
apps/meteor/server/cron/usageReport.ts (1)
17-24: Logging behavior is correct and meets acceptance criteria.The warning logs once when
usageReportCron(logger)is invoked during startup (viastartCronJobs()inapps/meteor/server/startup/cron.ts). The warning check (lines 17-24) is in the function body but outside the cron job callback (lines 32-35), so it executes only once at initialization. The periodic cron executions do not re-log the warning, avoiding noisy per-event logs as required.
ricardogarim
force-pushed
the
feat/disable-send-usage-metrics
branch
from
5b4d49e to
ed164bf
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts (1)
43-62: Consider adding tests for case-insensitive handling and undefined env var.The PR objectives explicitly state the env var should be handled "case-insensitive" and that "absence leaves behavior unchanged." While the current tests verify the core binary behavior, adding tests for these specific requirements would confirm the normalization logic (e.g.,
.toLowerCase()) works correctly and prevent future regressions.💡 Suggested additional test cases
it('should handle case-insensitive "TRUE" value', async () => { process.env.RC_DISABLE_STATISTICS_REPORTING = 'TRUE'; const result = await sendUsageReport(mocks.logger); expect(mocks.statistics.save.called).to.be.true; expect(mocks.serverFetch.called).to.be.false; expect(result).to.be.undefined; }); it('should send to collector when RC_DISABLE_STATISTICS_REPORTING is not set', async () => { // Don't set the env var - it should default to sending const result = await sendUsageReport(mocks.logger); expect(mocks.statistics.save.called).to.be.true; expect(mocks.serverFetch.calledOnce).to.be.true; expect(result).to.be.undefined; });
📜 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 (4)
apps/meteor/.mocharc.jsapps/meteor/app/statistics/server/functions/sendUsageReport.spec.tsapps/meteor/app/statistics/server/functions/sendUsageReport.tsapps/meteor/server/cron/usageReport.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/meteor/server/cron/usageReport.ts
- apps/meteor/app/statistics/server/functions/sendUsageReport.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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/functions/sendUsageReport.spec.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.spec.ts: Use descriptive test names that clearly communicate expected behavior in Playwright tests
Use.spec.tsextension for test files (e.g.,login.spec.ts)
Files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
🧠 Learnings (19)
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : All test files must be created in `apps/meteor/tests/e2e/` directory
Applied to files:
apps/meteor/.mocharc.jsapps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/.mocharc.jsapps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-12-10T21:00:54.909Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:54.909Z
Learning: Rocket.Chat monorepo: Jest testMatch pattern '<rootDir>/src/**/*.spec.(ts|js|mjs)' is valid in this repo and used across multiple packages (e.g., packages/tools, ee/packages/omnichannel-services). Do not flag it as invalid in future reviews.
Applied to files:
apps/meteor/.mocharc.jsapps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
apps/meteor/.mocharc.jsapps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
apps/meteor/.mocharc.jsapps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/page-objects/**/*.ts : Utilize existing page objects pattern from `apps/meteor/tests/e2e/page-objects/`
Applied to files:
apps/meteor/.mocharc.js
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
apps/meteor/.mocharc.jsapps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.spec.ts : Use `.spec.ts` extension for test files (e.g., `login.spec.ts`)
Applied to files:
apps/meteor/.mocharc.js
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Store commonly used locators in variables/constants for reuse
Applied to files:
apps/meteor/.mocharc.js
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests
Applied to files:
apps/meteor/.mocharc.jsapps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.{ts,tsx,js} : Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Applied to files:
apps/meteor/.mocharc.js
📚 Learning: 2025-10-06T20:30:45.540Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37152
File: packages/apps-engine/tests/test-data/storage/storage.ts:101-122
Timestamp: 2025-10-06T20:30:45.540Z
Learning: In `packages/apps-engine/tests/test-data/storage/storage.ts`, the stub methods (updatePartialAndReturnDocument, updateStatus, updateSetting, updateAppInfo, updateMarketplaceInfo) intentionally throw "Method not implemented." Tests using these methods must stub them using `SpyOn` from the test library rather than relying on actual implementations.
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure clean state for each test execution in Playwright tests
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Follow Page Object Model pattern consistently in Playwright tests
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.spec.ts : Use descriptive test names that clearly communicate expected behavior in Playwright tests
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-11-24T17:08:26.531Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/test-cases.mdc:0-0
Timestamp: 2025-11-24T17:08:26.531Z
Learning: Applies to **/*test-case*.md : Define clear, measurable expected results in test cases that can be validated and later converted into automated tests
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-11-24T17:08:26.531Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/test-cases.mdc:0-0
Timestamp: 2025-11-24T17:08:26.531Z
Learning: Reference the test-cases.json file to ensure consistency with existing test case structures and standardized format
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.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 (5)
apps/meteor/.mocharc.js (1)
31-31: LGTM! Test discovery pattern added correctly.The addition of
'app/statistics/server/**/*.spec.ts'enables Mocha to discover the new statistics server test suite and follows the existing pattern conventions.apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts (4)
1-4: LGTM! Test imports are appropriate.The imports correctly bring in the necessary testing libraries (Chai for assertions, Mocha for test structure, Sinon for mocks, and proxyquire for dependency injection).
6-24: LGTM! Mock setup is appropriate for the test scope.The sandbox and mock structure provide the necessary stubs to isolate the function under test. The minimal mock definitions are intentional and sufficient for verifying the behavior being asserted.
26-32: LGTM! Dependency injection setup is correct.The proxyquire configuration properly isolates
sendUsageReportby replacing all external dependencies with controlled mocks, enabling reliable unit testing.
34-42: LGTM! Test isolation is properly maintained.The
beforeEachandafterEachhooks ensure clean state between tests by resetting mock history and cleaning up the environment variable.
ricardogarim
force-pushed
the
feat/disable-send-usage-metrics
branch
from
6978d60 to
24bb6ad
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts (1)
43-62: Tests provide adequate coverage for the binary opt-out behavior.The two test cases verify the core functionality: when disabled, statistics are saved locally without sending; when enabled, both save and send operations are attempted. The minimal mock approach is appropriate for verifying call patterns rather than full integration success.
Optional: Consider adding retry path coverage
While the current tests satisfy the acceptance criteria, you could add a test case that exercises the retry logic (lines 55-60 in sendUsageReport.ts) by mocking
Statistics.findLast()to return a recent stat without astatsToken. This would verify that retry attempts also respect theshouldSendToCollectorflag:it('should not send retry when reporting is disabled', async () => { process.env.RC_DISABLE_STATISTICS_REPORTING = 'true'; const recentStat = { _id: 'recent-id', createdAt: new Date(), // no statsToken - triggers retry path }; mocks.Statistics.findLast.resolves(recentStat); const result = await sendUsageReport(mocks.logger); expect(mocks.serverFetch.called).to.be.false; expect(result).to.be.undefined; });
📜 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 (4)
apps/meteor/.mocharc.jsapps/meteor/app/statistics/server/functions/sendUsageReport.spec.tsapps/meteor/app/statistics/server/functions/sendUsageReport.tsapps/meteor/server/cron/usageReport.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/meteor/server/cron/usageReport.ts
- apps/meteor/.mocharc.js
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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/functions/sendUsageReport.spec.tsapps/meteor/app/statistics/server/functions/sendUsageReport.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.spec.ts: Use descriptive test names that clearly communicate expected behavior in Playwright tests
Use.spec.tsextension for test files (e.g.,login.spec.ts)
Files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
🧠 Learnings (15)
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : All test files must be created in `apps/meteor/tests/e2e/` directory
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-10-06T20:30:45.540Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37152
File: packages/apps-engine/tests/test-data/storage/storage.ts:101-122
Timestamp: 2025-10-06T20:30:45.540Z
Learning: In `packages/apps-engine/tests/test-data/storage/storage.ts`, the stub methods (updatePartialAndReturnDocument, updateStatus, updateSetting, updateAppInfo, updateMarketplaceInfo) intentionally throw "Method not implemented." Tests using these methods must stub them using `SpyOn` from the test library rather than relying on actual implementations.
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure clean state for each test execution in Playwright tests
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-12-10T21:00:54.909Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:54.909Z
Learning: Rocket.Chat monorepo: Jest testMatch pattern '<rootDir>/src/**/*.spec.(ts|js|mjs)' is valid in this repo and used across multiple packages (e.g., packages/tools, ee/packages/omnichannel-services). Do not flag it as invalid in future reviews.
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.spec.ts : Use descriptive test names that clearly communicate expected behavior in Playwright tests
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Follow Page Object Model pattern consistently in Playwright tests
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-11-24T17:08:26.531Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/test-cases.mdc:0-0
Timestamp: 2025-11-24T17:08:26.531Z
Learning: Applies to **/*test-case*.md : Define clear, measurable expected results in test cases that can be validated and later converted into automated tests
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
📚 Learning: 2025-11-24T17:08:26.531Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/test-cases.mdc:0-0
Timestamp: 2025-11-24T17:08:26.531Z
Learning: Reference the test-cases.json file to ensure consistency with existing test case structures and standardized format
Applied to files:
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts
🧬 Code graph analysis (2)
apps/meteor/app/statistics/server/functions/sendUsageReport.spec.ts (1)
apps/meteor/app/statistics/server/functions/sendUsageReport.ts (1)
sendUsageReport(37-70)
apps/meteor/app/statistics/server/functions/sendUsageReport.ts (1)
apps/meteor/server/cron/usageReport.ts (1)
shouldReportStatistics(16-16)
⏰ 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). (7)
- GitHub Check: 🔎 Code Check / TypeScript
- GitHub Check: 🔎 Code Check / Code Lint
- GitHub Check: 🔨 Test Unit / Unit Tests
- GitHub Check: 🔨 Test Storybook / Test Storybook
- GitHub Check: 📦 Meteor Build (coverage)
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (1)
apps/meteor/app/statistics/server/functions/sendUsageReport.ts (1)
38-68: Implementation correctly separates local statistics generation from remote reporting.The conditional logic properly implements the opt-out mechanism: statistics are always generated locally (supporting air-gapped environments), while remote reporting respects the
shouldSendToCollectorflag. The early return at line 60 correctly prevents fallthrough when reporting is disabled.
ricardogarim
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
Proposed changes (including videos or screenshots)
As per CORE-1533, this PR introduces a startup-time, server-side mechanism to disable sending statistics/telemetry to Rocket.Chat via an environment variable.
Setting
RC_DISABLE_STATISTICS_REPORTING=trueprevents statistics from being sent to remote servers while continuing to generate and maintain statistics locally, ensuring compatibility with internal services that might rely on this data.The mechanism is intentionally non-discoverable, has no admin UI, is evaluated only at startup, and targets customers with strict compliance or regulatory requirements. When enabled, a single startup warning is logged. Default behavior remains unchanged when the env var is not set.
Issue(s)
Steps to test or reproduce
RC_DISABLE_STATISTICS_REPORTING=true yarn startSummary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.