-
Notifications
You must be signed in to change notification settings - Fork 3
feat: telemetry and security hardening #51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…on guide This commit introduces a detailed PostHog instrumentation blueprint for the OpenChat monorepo, covering web, server, and extension components. It outlines baseline setup, event taxonomy with triggers and properties, common properties, dashboards and insights strategies, implementation notes by file, and next steps for full analytics coverage. Key additions include event naming conventions, super-property registrations, enhanced event properties for auth, chat, settings, sync, and extension features, as well as recommended dashboard KPIs and monitoring plans to track acquisition, onboarding, chat health, personalization, and extension engagement. This documentation will guide engineering alignment and implementation to enable actionable analytics through PostHog. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
…trumentation - Reduced and optimized core event sets to focus on high-leverage insights, minimizing low-signal noise. - Standardized event naming conventions to lowercase snake_case with scope.action structure. - Updated super-properties registration for better session identification including guest handling. - Enhanced event properties for chat creation, streaming, attachments, and OpenRouter API key lifecycle. - Added new telemetry events for sync connection state, fallback storage usage, and chat rate limiting. - Revised dashboard and funnel definitions to reflect streamlined event model and metrics. - Improved implementation notes for consistency and expanded coverage across web, server, and extension components. This update enables effective monitoring of acquisition, chat usage, reliability, and onboarding activation without excess event volume. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
- Add base super properties and sanitize event properties for client and server PostHog usage - Track fallback storage usage in server router for chats and messages - Add detailed client event tracking in web chat handler and components - Introduce PosthogBootstrap component for consistent client identification and property registration - Enhance event tracking with user workspace IDs, auth states, theme info, and OpenRouter API key status - Track marketing CTA clicks and landing visits with detailed metadata - Improve rate limit and error event captures for chat and OpenRouter APIs - Add client property registration on model selection and OpenRouter key management - Add sync connection state events to PostHog This update improves analytics granularity and consistency across client and server, enabling better user behavior insights and troubleshooting. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
…og-features-qkjs37
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
This PR implements comprehensive PostHog telemetry instrumentation across the OpenChat application with security hardening measures. The changes add systematic event tracking for17 key user flows including marketing analytics (landing page visits, CTA clicks), chat functionality (creation, messages, attachments), and system monitoring (WebSocket connections, API rate limiting, fallback storage usage).
The implementation introduces structured telemetry with consistent event naming, property sanitization to remove undefined values, and environment-aware metadata collection. Security improvements include custom JavaScript escaping for user ID sanitization before client-side bootstrapping and IP address hashing in server logs. The telemetry system supports both authenticated users and anonymous guests through workspace-based user identification.
Key architectural additions include a new PosthogBootstrap component for centralized initialization, enhanced super properties with deployment metadata, and privacy-conscious data collection that excludes sensitive user input while capturing meaningful interaction patterns. The changes integrate seamlessly with the existing authentication system and maintain backward compatibility.
Important Files Changed
Changed Files
| Filename | Score | Overview |
|---|---|---|
posthog.md |
5/5 | Added comprehensive telemetry documentation and implementation plan |
apps/web/src/components/posthog-bootstrap.tsx |
4/5 | New component for centralized PostHog user identification and property tracking |
apps/web/src/app/dashboard/layout.tsx |
4/5 | Added security hardening with custom JavaScript escaping for user ID sanitization |
apps/web/src/lib/posthog.ts |
4/5 | Enhanced client-side PostHog configuration with property sanitization and workspace grouping |
apps/server/src/lib/posthog.ts |
4/5 | Added server-side PostHog configuration with environment metadata and property sanitization |
apps/web/src/components/hero-section.tsx |
4/5 | Converted to client component with marketing analytics for CTAs and landing page visits |
apps/web/src/app/api/chat/chat-handler.ts |
4/5 | Added comprehensive chat API telemetry with IP hashing for security |
apps/server/src/routers/index.ts |
4/5 | Added telemetry for chat operations and fallback storage monitoring |
apps/web/src/components/providers.tsx |
4/5 | Enhanced pageview tracking with referrer analysis and PostHog bootstrap integration |
apps/web/src/components/app-sidebar.tsx |
4/5 | Added user identification, chat creation events, and dashboard entry tracking |
apps/web/src/components/chat-composer.tsx |
4/5 | Added file attachment telemetry with privacy protection for user input |
apps/web/src/lib/posthog-server.ts |
4/5 | Enhanced server-side PostHog with environment properties and event sanitization |
apps/web/src/components/account-settings-modal.tsx |
4/5 | Added OpenRouter API key management telemetry with privacy-conscious data masking |
apps/web/src/components/model-selector.tsx |
4/5 | Added model selection tracking for understanding user preferences |
apps/web/src/lib/sync.ts |
5/5 | Added WebSocket connection state telemetry for reliability monitoring |
apps/web/src/components/openrouter-link-modal.tsx |
5/5 | Added API key prompt telemetry with deduplication logic |
Confidence score: 4/5
- This PR implements a comprehensive telemetry system with solid security practices and should be safe to merge with careful testing
- Score reflects the extensive scope of changes across critical files and potential for unintended side effects, though no obvious bugs were found
- Pay close attention to the client-side JavaScript injection security hardening and PostHog initialization order dependencies
Sequence Diagram
sequenceDiagram
participant User
participant ChatRoom as ChatRoom Component
participant ChatHandler as /api/chat Handler
participant PosthogClient as PostHog Client
participant PosthogServer as PostHog Server
participant ServerRouter as Server Router
participant SyncHub as Sync Hub
User->>ChatRoom: "Submits chat message"
ChatRoom->>PosthogClient: "captureClientEvent('chat_message_submitted')"
ChatRoom->>ChatHandler: "POST /api/chat with message data"
ChatHandler->>PosthogServer: "captureServerEvent('chat_message_stream') - start"
ChatHandler->>ServerRouter: "persistMessage() - user message"
ServerRouter->>PosthogServer: "capturePosthogEvent('workspace.fallback_storage_used')"
Note over ServerRouter: If database fails, fallback storage used
ChatHandler->>ChatHandler: "Stream text generation"
ChatHandler->>ServerRouter: "persistMessage() - assistant chunks"
alt Rate Limited
ChatHandler->>PosthogServer: "captureServerEvent('chat.rate_limited')"
ChatHandler->>ChatRoom: "HTTP 429 Too Many Requests"
ChatRoom->>PosthogClient: "captureClientEvent('chat.rate_limited')"
else Success
ChatHandler->>PosthogServer: "captureServerEvent('chat_message_stream') - complete"
ChatHandler->>SyncHub: "publish() - chat updates"
ChatHandler->>ChatRoom: "Stream response chunks"
end
ChatRoom->>User: "Display assistant response"
Context used:
- Context from
dashboard- AGENTS.md (source)
16 files reviewed, 6 comments
| process.env.POSTHOG_DEPLOYMENT_REGION ?? process.env.VERCEL_REGION ?? "local"; | ||
|
|
||
| const BASE_SUPER_PROPERTIES = Object.freeze({ | ||
| app: "openchat-server", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: app name should be 'openchat-web' since this is the web app's posthog-server.ts file, not the server app
| React.useEffect(() => { | ||
| if (!selectedValue) return | ||
| registerClientProperties({ model_id: selectedValue }) | ||
| }, [selectedValue]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Potential duplicate tracking - this useEffect fires on mount and when selectedValue changes, but the onSelect handler also calls registerClientProperties. Consider if both are needed.
| if (char === "\\" || char === "]" || char === "^" || char === "-") { | ||
| return `\\${char}`; | ||
| } | ||
| if (char === "/") return "\\/"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: The forward slash escaping logic is duplicated - line 15 maps '/' to '\u002F' but line 32 returns '/' for the same character
| client.group("workspace", workspaceId); | ||
| registerClientProperties({ workspace_id: workspaceId }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Consider using client.setPersonPropertiesOnce({ workspace_id: workspaceId }) instead of registerClientProperties to avoid overwriting existing workspace associations when users switch workspaces
|
|
||
| useEffect(() => { | ||
| if (!resolvedWorkspaceId) return; | ||
| if (identifyRef.current === resolvedWorkspaceId && session?.user) return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: This condition may cause unnecessary re-identification when session changes from null to user object with same ID
| useEffect(() => { | ||
| ensureGuestIdClient(); | ||
| }, []); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Guest ID initialization runs on every component mount but has no cleanup
Summary