feat: ✨ add amplitude domain proxy #4024
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
4 Skipped Deployments
|
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
packages/web/hooks/use-amplitude-analytics.tsOops! Something went wrong! :( ESLint: 8.50.0 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/packages/web/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. WalkthroughThe pull request modifies the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (2)
packages/web/hooks/use-amplitude-analytics.ts (2)
Line range hint
1-90: Add documentation for the proxy configurationThe code would benefit from documentation explaining the proxy setup and configuration requirements.
Add a comment block above the useAmplitudeAnalytics hook explaining:
- The purpose of the proxy
- Required environment variables
- Configuration instructions
+/** + * Amplitude analytics hook with proxy support to prevent ad-blockers from blocking analytics. + * + * Configuration: + * - NEXT_PUBLIC_AMPLITUDE_API_KEY: Your Amplitude API key + * - NEXT_PUBLIC_AMPLITUDE_SERVER_URL: URL of your proxy server that forwards requests to Amplitude + * + * @example + * ``` + * export function MyComponent() { + * useAmplitudeAnalytics({ init: true }); + * // ... + * } + * ``` + */ export function useAmplitudeAnalytics({
62-64: Consider adding debug logging for proxy configurationSince the DEBUG flag is already implemented, extend it to include proxy-related information.
if (init) { if (process.env.NEXT_PUBLIC_AMPLITUDE_API_KEY !== undefined) { + if (DEBUG) { + console.info('Initializing Amplitude with proxy:', { + apiKey: process.env.NEXT_PUBLIC_AMPLITUDE_API_KEY, + proxyUrl: process.env.NEXT_PUBLIC_AMPLITUDE_SERVER_URL + }); + } amplitudeInit(process.env.NEXT_PUBLIC_AMPLITUDE_API_KEY, undefined, { serverUrl: process.env.NEXT_PUBLIC_AMPLITUDE_SERVER_URL, }); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/web/hooks/use-amplitude-analytics.ts(1 hunks)
🔇 Additional comments (1)
packages/web/hooks/use-amplitude-analytics.ts (1)
62-64: Verify the proxy URL configuration in the deployment environment
The implementation correctly adds the proxy server URL configuration. However, ensure that:
- The proxy server URL is properly configured in all deployment environments
- The URL is properly sanitized and validated before use
Consider implementing URL validation before passing it to amplitudeInit to prevent potential security issues.
| amplitudeInit(process.env.NEXT_PUBLIC_AMPLITUDE_API_KEY, undefined, { | ||
| serverUrl: process.env.NEXT_PUBLIC_AMPLITUDE_SERVER_URL, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for proxy server failures
The current implementation doesn't handle scenarios where the proxy server might be unavailable or return errors.
Consider adding a fallback mechanism:
amplitudeInit(process.env.NEXT_PUBLIC_AMPLITUDE_API_KEY, undefined, {
serverUrl: process.env.NEXT_PUBLIC_AMPLITUDE_SERVER_URL,
+ transport: {
+ retry: {
+ maxRetries: 3,
+ backoffFactor: 2
+ },
+ onError: (error) => {
+ console.error('Amplitude proxy error:', error);
+ // Optionally fall back to direct connection
+ }
+ }
});Committable suggestion skipped: line range outside the PR's diff.
💡 Codebase verification
Server-side proxy implementation is missing
Based on the verification results, while the client-side is configured to use a proxy server URL through NEXT_PUBLIC_AMPLITUDE_SERVER_URL, there is no corresponding server-side implementation found in the codebase:
- No API routes handling Amplitude proxy requests
- No proxy configuration in Next.js config
- No server middleware implementation
The proxy server implementation needs to be added to handle:
- Request forwarding to Amplitude's servers
- CORS and security headers
- Rate limiting
- Error handling
🔗 Analysis chain
Verify the proxy server implementation
The changes only show the client-side configuration for the proxy. Ensure that the corresponding proxy server implementation exists and handles:
- Request forwarding to Amplitude
- CORS configuration
- Rate limiting
- Error handling
- Security headers
Let's try another search strategy to find the server-side implementation.
Let's try one more time with corrected file type flags:
Let's check one final time for API routes with a corrected path:
Let's check the Next.js configuration for any proxy setup:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Look for proxy server implementation
echo "Checking for proxy server implementation..."
fd -e ts -e js -e tsx -e jsx --full-path ".*proxy.*amplitude.*"
# Check for deployment configurations
echo "Checking for deployment configurations..."
fd -e yaml -e yml --full-path ".*deploy.*"
Length of output: 300
Script:
#!/bin/bash
# Search for Amplitude-related files and their content
echo "Searching for Amplitude-related files..."
rg -l "amplitude" --type ts --type js --type tsx --type jsx
# Show content of found files to analyze the implementation
echo -e "\nSearching for server-related Amplitude code..."
rg "amplitude.*server" -i --type ts --type js --type tsx --type jsx -B 2 -A 2
Length of output: 395
Script:
#!/bin/bash
# Search for Amplitude-related files
echo "Searching for Amplitude-related files..."
rg -l "amplitude"
# Look for server implementations or configurations
echo -e "\nChecking for server implementations..."
rg -i "server.*amplitude|amplitude.*server" -A 3 -B 3
# Check for API routes that might handle proxying
echo -e "\nChecking for API routes..."
fd -e ts -e js "api" "pages"
Length of output: 1867
Script:
#!/bin/bash
# Check for API routes in the web package
echo "Checking for API routes in web package..."
fd -e ts -e js "api" "packages/web/pages"
# Look for Next.js configuration that might contain proxy settings
echo -e "\nChecking Next.js configuration..."
fd -e js -e ts "next.config" "packages/web"
# Check environment variables related to Amplitude
echo -e "\nChecking environment variable files..."
fd -g "*.env*" "packages/web"
Length of output: 408
Script:
#!/bin/bash
# Check Next.js config content for proxy settings
echo "Checking Next.js config content..."
cat packages/web/next.config.js
# Look for any rewrites or proxy configurations in the codebase
echo -e "\nSearching for proxy or rewrite configurations..."
rg -i "rewrites|proxy" packages/web/next.config.js -A 5 -B 5
Length of output: 3588
DavideSegullo
deleted the
davidesegullo/fe-1233-amplitude-using-proxy-to-redirect-event-logs-to-amplitude
branch
What is the purpose of the change:
Right now due to uBlock or stuffs like that, our request to amplitude are blocked for some users, this create a lot of missing data in our metrics. This PR should address this issue by using a domain proxy!
Linear Task
https://linear.app/osmosis/issue/FE-1233/amplitude-or-using-proxy-to-redirect-event-logs-to-amplitude
Brief Changelog
Testing and Verifying