feat(observability): replace sentry with posthog#1844
Closed
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
📝 WalkthroughWalkthroughThis PR removes Sentry instrumentation and dependencies from CI, server, and worker entrypoints, replaces Sentry error capture with PostHog-based exception reporting across backend functions, and adds client-side PostHog exception capture and earlier PostHog initialization. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant EdgeFunction
participant BackgroundTask
participant PostHog
participant Discord
Client->>EdgeFunction: request triggers error
EdgeFunction->>BackgroundTask: enqueue capturePosthogException(error, metadata)
BackgroundTask->>PostHog: POST /capture with exception payload
BackgroundTask->>Discord: optional alert (depending on error)
PostHog-->>BackgroundTask: 200 / non-OK logged
EdgeFunction-->>Client: returns HTTP response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
supabase/functions/_backend/utils/posthog.ts (1)
68-72: Minor: Redundant condition check.The
!existInEnv(c, 'POSTHOG_API_KEY')check is redundant. IfapiKeyis truthy (non-empty string), thenexistInEnvmust return true. The!apiKeycheck alone handles both "key doesn't exist" and "key exists but is empty" cases.♻️ Suggested simplification
async function sendPostHogCapture(c: Context, body: PostHogCapturePayload) { const apiKey = getEnv(c, 'POSTHOG_API_KEY') - if (!apiKey || !existInEnv(c, 'POSTHOG_API_KEY')) { + if (!apiKey) { cloudlog({ requestId: c.get('requestId'), message: 'PostHog not configured' }) return false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/_backend/utils/posthog.ts` around lines 68 - 72, The condition in sendPostHogCapture is redundant: remove the existInEnv(c, 'POSTHOG_API_KEY') check and only check the apiKey value returned by getEnv(c, 'POSTHOG_API_KEY'); update the if to use if (!apiKey) so the block that logs "PostHog not configured" and returns false runs when the key is missing or empty, referencing the sendPostHogCapture function and the getEnv/existInEnv calls to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@supabase/functions/_backend/utils/posthog.ts`:
- Around line 104-106: The helper parseIntOrUndefined incorrectly treats parsed
value 0 as missing because it returns Number.parseInt(input || '', 10) ||
undefined; update parseIntOrUndefined to first handle undefined input, then
parse with Number.parseInt and return undefined only when Number.isNaN(parsed)
(or otherwise detect invalid parse); specifically, keep the function name
parseIntOrUndefined and change its logic to return the parsed number (including
0) or undefined when parsing fails.
---
Nitpick comments:
In `@supabase/functions/_backend/utils/posthog.ts`:
- Around line 68-72: The condition in sendPostHogCapture is redundant: remove
the existInEnv(c, 'POSTHOG_API_KEY') check and only check the apiKey value
returned by getEnv(c, 'POSTHOG_API_KEY'); update the if to use if (!apiKey) so
the block that logs "PostHog not configured" and returns false runs when the key
is missing or empty, referencing the sendPostHogCapture function and the
getEnv/existInEnv calls to locate the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 538bd666-eab1-4bd5-a0d5-00c412bf79f8
⛔ Files ignored due to path filters (2)
bun.lockis excluded by!**/*.locksupabase/functions/deno.lockis excluded by!**/*.lock
📒 Files selected for processing (30)
.github/workflows/build_and_deploy.ymlcloudflare_workers/api/index.tscloudflare_workers/files/index.tscloudflare_workers/plugin/index.tspackage.jsonsrc/main.tssrc/services/posthog.tssupabase/functions/.env.examplesupabase/functions/_backend/utils/hono.tssupabase/functions/_backend/utils/on_error.tssupabase/functions/_backend/utils/posthog.tssupabase/functions/apikey/index.tssupabase/functions/app/index.tssupabase/functions/build/index.tssupabase/functions/bundle/index.tssupabase/functions/channel/index.tssupabase/functions/channel_self/index.tssupabase/functions/deno.jsonsupabase/functions/device/index.tssupabase/functions/files/index.tssupabase/functions/ok/index.tssupabase/functions/organization/index.tssupabase/functions/private/index.tssupabase/functions/replication/index.tssupabase/functions/statistics/index.tssupabase/functions/stats/index.tssupabase/functions/triggers/index.tssupabase/functions/updates/index.tssupabase/functions/updates_debug/index.tssupabase/functions/webhooks/index.ts
💤 Files with no reviewable changes (2)
- supabase/functions/deno.json
- .github/workflows/build_and_deploy.yml
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@supabase/functions/_backend/utils/posthog.ts`:
- Around line 77-87: The PostHog POST currently awaits fetch(posthogUrl, ...)
with no timeout; add an AbortController-based deadline so the request fails fast
on slow edge. Create an AbortController before calling fetch, pass
controller.signal into fetch options, start a timer (e.g., 500–2000ms) that
calls controller.abort() on expiry, and clear the timer after fetch completes;
keep the existing body payload (apiKey and ...body) and handle the aborted fetch
as a fast failure inside the same try block (the symbols to update are
posthogUrl, apiKey, body and the existing fetch call).
- Around line 61-75: getPostHogCaptureUrl and sendPostHogCapture currently
produce malformed URLs when POSTHOG_API_HOST ends with "capture" (with or
without trailing slash) and the helper is called before validation; update
getPostHogCaptureUrl to defensively normalize the host by stripping any trailing
"capture" or "capture/" segment before appending the "capture/" path, and move
the call to getPostHogCaptureUrl inside sendPostHogCapture's try-block after
checking apiKey so malformed hosts can be caught and cause sendPostHogCapture to
return false; ensure sendPostHogCapture handles URL construction errors (invalid
host) by logging and returning false.
- Around line 317-333: trackPosthogEvent currently uses a global 'anonymous'
distinct_id which merges unauthenticated traffic; change distinctId to
payload.user_id || c.get('requestId') || crypto.randomUUID(), and when user_id
is absent set properties.$process_person_profile = false and omit
properties.$set (or only include $set when user_id exists) to avoid mutating
person profiles; update the properties object and the call to sendPostHogCapture
accordingly (see trackPosthogEvent and sendPostHogCapture references).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c02824c8-f9e0-40cc-b723-700462c67df7
📒 Files selected for processing (1)
supabase/functions/_backend/utils/posthog.ts
Comment on lines
+61
to
+75
| function getPostHogCaptureUrl(c: Context) { | ||
| const host = getEnv(c, 'POSTHOG_API_HOST') || POSTHOG_CAPTURE_URL | ||
| const posthogUrl = host.endsWith('/capture/') | ||
| return host.endsWith('/capture/') | ||
| ? host | ||
| : new URL('capture/', host.endsWith('/') ? host : `${host}/`).toString() | ||
| } | ||
|
|
||
| const distinctId = payload.user_id || 'anonymous' | ||
|
|
||
| const properties = { | ||
| ...(payload.tags || {}), | ||
| channel: payload.channel, | ||
| description: payload.description, | ||
| $set: payload.tags, | ||
| async function sendPostHogCapture(c: Context, body: PostHogCapturePayload) { | ||
| const apiKey = getEnv(c, 'POSTHOG_API_KEY') | ||
| if (!apiKey) { | ||
| cloudlog({ requestId: c.get('requestId'), message: 'PostHog not configured' }) | ||
| return false | ||
| } | ||
|
|
||
| const body = { | ||
| api_key: apiKey, | ||
| event: payload.event, | ||
| distinct_id: distinctId, | ||
| properties, | ||
| ip: payload.ip, | ||
| timestamp: new Date().toISOString(), | ||
| } | ||
| const posthogUrl = getPostHogCaptureUrl(c) |
Contributor
There was a problem hiding this comment.
Normalize POSTHOG_API_HOST more defensively.
If POSTHOG_API_HOST is configured as .../capture without the trailing slash, Line 65 resolves it to .../capture/capture/. Also, getPostHogCaptureUrl(c) runs before the try, so a malformed host will escape this helper instead of degrading to false.
🔧 Proposed fix
function getPostHogCaptureUrl(c: Context) {
const host = getEnv(c, 'POSTHOG_API_HOST') || POSTHOG_CAPTURE_URL
+ if (host.endsWith('/capture'))
+ return `${host}/`
return host.endsWith('/capture/')
? host
: new URL('capture/', host.endsWith('/') ? host : `${host}/`).toString()
}
async function sendPostHogCapture(c: Context, body: PostHogCapturePayload) {
const apiKey = getEnv(c, 'POSTHOG_API_KEY')
if (!apiKey) {
cloudlog({ requestId: c.get('requestId'), message: 'PostHog not configured' })
return false
}
- const posthogUrl = getPostHogCaptureUrl(c)
-
try {
+ const posthogUrl = getPostHogCaptureUrl(c)
const res = await fetch(posthogUrl, {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/functions/_backend/utils/posthog.ts` around lines 61 - 75,
getPostHogCaptureUrl and sendPostHogCapture currently produce malformed URLs
when POSTHOG_API_HOST ends with "capture" (with or without trailing slash) and
the helper is called before validation; update getPostHogCaptureUrl to
defensively normalize the host by stripping any trailing "capture" or "capture/"
segment before appending the "capture/" path, and move the call to
getPostHogCaptureUrl inside sendPostHogCapture's try-block after checking apiKey
so malformed hosts can be caught and cause sendPostHogCapture to return false;
ensure sendPostHogCapture handles URL construction errors (invalid host) by
logging and returning false.
Comment on lines
77
to
87
| try { | ||
| const res = await fetch(posthogUrl, { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| body: JSON.stringify(body), | ||
| body: JSON.stringify({ | ||
| api_key: apiKey, | ||
| ...body, | ||
| }), | ||
| }) |
Contributor
There was a problem hiding this comment.
Add a deadline to the PostHog fetch.
This helper is awaited from request-scoped code, so a slow PostHog edge can hold the API path open until the platform timeout. Observability failures should fail fast here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/functions/_backend/utils/posthog.ts` around lines 77 - 87, The
PostHog POST currently awaits fetch(posthogUrl, ...) with no timeout; add an
AbortController-based deadline so the request fails fast on slow edge. Create an
AbortController before calling fetch, pass controller.signal into fetch options,
start a timer (e.g., 500–2000ms) that calls controller.abort() on expiry, and
clear the timer after fetch completes; keep the existing body payload (apiKey
and ...body) and handle the aborted fetch as a fast failure inside the same try
block (the symbols to update are posthogUrl, apiKey, body and the existing fetch
call).
Comment on lines
+317
to
+333
| export async function trackPosthogEvent(c: Context, payload: Pick<TrackOptions, 'event'> & { user_id?: string } & Pick<TrackOptions, 'channel' | 'description'> & { ip?: string, tags?: Record<string, any> }) { | ||
| const distinctId = payload.user_id || 'anonymous' | ||
|
|
||
| const properties = { | ||
| ...(payload.tags || {}), | ||
| channel: payload.channel, | ||
| description: payload.description, | ||
| $set: payload.tags, | ||
| } | ||
|
|
||
| return sendPostHogCapture(c, { | ||
| event: payload.event, | ||
| distinct_id: distinctId, | ||
| properties, | ||
| ip: payload.ip, | ||
| timestamp: new Date().toISOString(), | ||
| }) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the posthog.ts file to see the full context
cd supabase/functions/_backend/utils && head -400 posthog.ts | tail -100Repository: Cap-go/capgo
Length of output: 3107
🏁 Script executed:
# Look for capturePosthogException function
rg -A 15 "function capturePosthogException|const capturePosthogException" supabase/functions/_backend/Repository: Cap-go/capgo
Length of output: 1450
🏁 Script executed:
# Check how requestId is set in middleware
rg -B 5 -A 10 "requestId" supabase/functions/_backend/utils/ --type tsRepository: Cap-go/capgo
Length of output: 50369
🏁 Script executed:
# Search for other usages of c.get('requestId') in backend code
rg "c\.get\('requestId'\)" supabase/functions/_backend/ --type tsRepository: Cap-go/capgo
Length of output: 50369
Use per-request identity for unauthenticated PostHog events.
Currently, all unauthenticated events get distinct_id: 'anonymous', merging unrelated traffic into a single person profile where $set fields overwrite each other. Mirror the pattern in capturePosthogException() (line 346): use c.get('requestId') || crypto.randomUUID() as fallback and set $process_person_profile = false when user_id is absent to prevent profile mutations.
Fix
export async function trackPosthogEvent(c: Context, payload: Pick<TrackOptions, 'event'> & { user_id?: string } & Pick<TrackOptions, 'channel' | 'description'> & { ip?: string, tags?: Record<string, any> }) {
- const distinctId = payload.user_id || 'anonymous'
+ const distinctId = payload.user_id || c.get('requestId') || crypto.randomUUID()
- const properties = {
+ const properties: Record<string, any> = {
...(payload.tags || {}),
channel: payload.channel,
description: payload.description,
$set: payload.tags,
}
+ if (!payload.user_id)
+ properties.$process_person_profile = false
return sendPostHogCapture(c, {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/functions/_backend/utils/posthog.ts` around lines 317 - 333,
trackPosthogEvent currently uses a global 'anonymous' distinct_id which merges
unauthenticated traffic; change distinctId to payload.user_id ||
c.get('requestId') || crypto.randomUUID(), and when user_id is absent set
properties.$process_person_profile = false and omit properties.$set (or only
include $set when user_id exists) to avoid mutating person profiles; update the
properties object and the call to sendPostHogCapture accordingly (see
trackPosthogEvent and sendPostHogCapture references).
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary (AI generated)
Motivation (AI generated)
Capgo already uses PostHog for product analytics, but operational errors were still split across Sentry-specific middleware and deploy wiring. Consolidating on PostHog removes the extra vendor-specific maintenance surface and keeps error events, product events, and user identity on the same platform.
Business Impact (AI generated)
This reduces observability integration overhead, removes Sentry-specific secret management from CI/CD, and gives support and product teams a single place to inspect both usage signals and application failures. It also lowers the risk of drift between analytics and error-reporting identity data.
Test Plan (AI generated)
bun run lint:fixbun run lint:backendbun run typecheckbun run test:backendGenerated with AI
Summary by CodeRabbit
Chores
Bug Fixes