feat(analytics): explicit source param + 'system' default for capture/captureException#3653
Conversation
… default Adds optional `source` parameter to analyticsService.capture() and a `'system'` baseline default so events from cron tasks, webhook handlers, and worker callbacks no longer have null source attribution in PostHog. Precedence: explicit source > properties.source > req.posthogContext.source > 'system'.
Extends captureException to accept an explicit source override and arbitrary properties merged into the $exception event, with the same 'system' default fallback as capture(). Unblocks the Winston error transport (#3651) and per-call-site source attribution downstream.
…source test Add jest.resetModules() in capture source attribution beforeEach (latent flake risk). Add properties.source > system precedence test for captureException.
|
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: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR extends the analytics service with explicit ChangesAnalytics Source Attribution
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3653 +/- ##
==========================================
+ Coverage 89.14% 89.15% +0.01%
==========================================
Files 135 135
Lines 4641 4646 +5
Branches 1433 1437 +4
==========================================
+ Hits 4137 4142 +5
Misses 392 392
Partials 112 112
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@lib/services/tests/analytics.captureException.unit.tests.js`:
- Around line 1-2: Add the missing JSDoc module header by inserting the comment
/** Module dependencies. */ directly above the import statements in
analytics.captureException.unit.tests.js so it matches the other test files;
locate the top-level import block (the line starting with "import { jest,
beforeEach, afterEach, describe, test, expect }") and place the comment
immediately above it.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5ccef826-c203-48d1-9a52-442a910f0555
📒 Files selected for processing (5)
README.mdlib/services/analytics.jslib/services/tests/analytics.capture.unit.tests.jslib/services/tests/analytics.captureException.unit.tests.jslib/services/tests/analytics.service.unit.tests.js
There was a problem hiding this comment.
Pull request overview
Adds explicit source attribution to PostHog analytics emissions so server-side events and captured exceptions no longer end up with source: null, while preserving a clear precedence order and updating unit tests + documentation accordingly.
Changes:
- Extend
AnalyticsService.capture()to accept an explicitsourceoverride and defaultsource: 'system'when no other source is provided. - Extend
AnalyticsService.captureException()to acceptctx.sourceand mergectx.properties, withsource: 'system'as the baseline default. - Add/adjust unit tests for source precedence and update README with the source taxonomy convention.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents source attribution precedence and canonical source values. |
| lib/services/analytics.js | Implements source default + override precedence for capture() and captureException(), and merges ctx.properties for exceptions. |
| lib/services/tests/analytics.capture.unit.tests.js | Updates existing expectations for source: 'system' and adds source precedence tests for capture(). |
| lib/services/tests/analytics.captureException.unit.tests.js | New unit tests covering captureException() source precedence and ctx.properties merge behavior. |
| lib/services/tests/analytics.service.unit.tests.js | Updates $exception assertion to include the new default source: 'system'. |
| The devkit ships an `analyticsService` backed by `posthog-node`. Enable via `config.analytics.posthog.enabled = true` + a valid `key`. | ||
|
|
||
| ### Source attribution convention | ||
|
|
||
| Every `analytics.capture()` and `analytics.captureException()` event gets a `source` property. The lookup order is: | ||
|
|
||
| 1. Explicit `source` param: `analytics.capture({ ..., source: 'cron' })` |
Nit fixed: JSDoc module header added to captureException test file (commit 596fae0 + style fix pushed)
Summary
analytics.capture({ ..., source })with an explicitsourceoverride parameter and a'system'baseline default, so server-side events (cron, webhook, worker callback) no longer producenullsource in PostHog.analytics.captureException(err, { source, properties })with the samesourceparam + arbitraryctx.propertiesmerge — unblocks the Winston transport (feat(observability): forward Winstonerror-level logs to PostHog Error Tracking via custom transport #3651) and per-call-site tagging downstream.source>properties.source>req.posthogContext.source>'system'.README.md.Closes #3652.
Part of Wave 4 PostHog Observability plan — Phase 2 (foundation for Phase 1 Winston transport + Phase 3 Trawl call-site tagging).
Plan:
infra/docs/superpowers/plans/2026-05-11-posthog-observability-wave4.mdFiles touched
lib/services/analytics.jscapture()+captureException()extendedlib/services/tests/analytics.capture.unit.tests.jslib/services/tests/analytics.captureException.unit.tests.jslib/services/tests/analytics.service.unit.tests.jssource: 'system')README.mdTest plan
npm run lint— greennpm run test:unit— 1369 tests, all green (+11 vs baseline 1358)npm run test:coverage— thresholds unchanged (pre-existingbilling.extraBalance.listLedger.perf.integrationMongoDB version failure unrelated to this diff)OK with nits, nits fixed before pushSummary by CodeRabbit
Release Notes
New Features
Documentation