Skip to content

feat(analytics): proxies amplitude requests to avoid failed requests due to blockers#2441

Merged
ygrishajev merged 1 commit intomainfrom
feature/analytics
Dec 31, 2025
Merged

feat(analytics): proxies amplitude requests to avoid failed requests due to blockers#2441
ygrishajev merged 1 commit intomainfrom
feature/analytics

Conversation

@ygrishajev
Copy link
Contributor

@ygrishajev ygrishajev commented Dec 31, 2025

Summary by CodeRabbit

  • New Features

    • Added support for configuring a custom proxy endpoint for analytics event delivery, enabling more flexible routing of analytics data through alternative endpoints.
  • Chores

    • Updated analytics service initialization to support optional custom server URL configuration, allowing analytics to be routed through specified proxy endpoints when configured.

✏️ Tip: You can customize this high-level summary in your review settings.

@ygrishajev ygrishajev requested a review from a team as a code owner December 31, 2025 14:00
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 31, 2025

📝 Walkthrough

Walkthrough

This change implements an Amplitude analytics events proxy by introducing a configurable proxy URL environment variable, Next.js rewrite rule, and analytics service initialization logic to route events through a custom endpoint.

Changes

Cohort / File(s) Summary
Environment & Schema Configuration
apps/deploy-web/env/.env, apps/deploy-web/src/config/env-config.schema.ts, apps/deploy-web/src/config/browser-env.config.ts
Added NEXT_PUBLIC_AMPLITUDE_PROXY_URL environment variable with value "/api/collect". Defined schema as optional string with descriptive message and exported in browser env config object.
Next.js Rewrite Configuration
apps/deploy-web/next.config.js
Env validation result now stored in browserEnv variable. Added conditional rewrites configuration that routes NEXT_PUBLIC_AMPLITUDE_PROXY_URL to https://api2.amplitude.com/2/httpapi when proxy URL is defined.
Analytics Service Integration
apps/deploy-web/src/services/analytics/analytics.service.ts, apps/deploy-web/src/services/app-di-container/app-di-container.ts
Added optional serverUrl property to AnalyticsOptions.amplitude type. Updated AnalyticsService initialization to conditionally pass serverUrl to amplitudeClient.init(). Wired proxy URL from environment into DI container's analytics initialization.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • stalniy

Poem

🐰 A proxy path springs to life,
From /api/collect it'll send,
To Amplitude's distant shore,
Rewrites guide the journey through,
Analytics flowing free and true! 📊

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main change: implementing a proxy for Amplitude analytics requests to circumvent blocker issues, which is reflected across all modified files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/deploy-web/next.config.js (1)

26-35: Critical: browserEnv can be undefined when accessed.

When env-config.schema.js is not found or validation fails, browserEnv remains undefined. Line 175 then attempts to access browserEnv.NEXT_PUBLIC_AMPLITUDE_PROXY_URL, which will throw a runtime error and break the build.

🔎 Proposed fix
 let browserEnv;
 try {
   const { browserEnvSchema } = require("./env-config.schema");
 
   browserEnv = browserEnvSchema.parse(process.env);
 } catch (error) {
   if (error.message.includes("Cannot find module")) {
     console.warn("No env-config.schema.js found, skipping env validation");
   }
+  browserEnv = {};
 }

Or use optional chaining in the rewrites function:

   rewrites: async () =>
-    browserEnv.NEXT_PUBLIC_AMPLITUDE_PROXY_URL
+    browserEnv?.NEXT_PUBLIC_AMPLITUDE_PROXY_URL
       ? [
           {
-            source: browserEnv.NEXT_PUBLIC_AMPLITUDE_PROXY_URL,
+            source: browserEnv?.NEXT_PUBLIC_AMPLITUDE_PROXY_URL,
             destination: "https://api2.amplitude.com/2/httpapi"
           }
         ]
       : []
🧹 Nitpick comments (1)
apps/deploy-web/env/.env (1)

35-35: Remove unnecessary quotes from environment variable value.

The value doesn't require quotes in .env files. Remove them for consistency with dotenv conventions.

🔎 Suggested fix
-NEXT_PUBLIC_AMPLITUDE_PROXY_URL="/api/collect"
+NEXT_PUBLIC_AMPLITUDE_PROXY_URL=/api/collect
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ab32673 and c091842.

📒 Files selected for processing (6)
  • apps/deploy-web/env/.env
  • apps/deploy-web/next.config.js
  • apps/deploy-web/src/config/browser-env.config.ts
  • apps/deploy-web/src/config/env-config.schema.ts
  • apps/deploy-web/src/services/analytics/analytics.service.ts
  • apps/deploy-web/src/services/app-di-container/app-di-container.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

**/*.{ts,tsx,js}: Never use type any or cast to type any. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.

Files:

  • apps/deploy-web/src/services/analytics/analytics.service.ts
  • apps/deploy-web/src/config/env-config.schema.ts
  • apps/deploy-web/src/config/browser-env.config.ts
  • apps/deploy-web/next.config.js
  • apps/deploy-web/src/services/app-di-container/app-di-container.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: baktun14
Repo: akash-network/console PR: 1725
File: apps/api/src/utils/constants.ts:5-5
Timestamp: 2025-07-24T17:00:52.361Z
Learning: In the Akash Network Console project, when cross-cutting concerns or broader refactoring issues are identified during PR review, the preferred approach is to create a separate GitHub issue to track the work rather than expanding the scope of the current PR. This maintains focus and allows for proper planning of architectural improvements.
📚 Learning: 2025-08-12T13:52:38.708Z
Learnt from: stalniy
Repo: akash-network/console PR: 1800
File: apps/deploy-web/next.config.js:163-165
Timestamp: 2025-08-12T13:52:38.708Z
Learning: In the Akash Console project, akashnetwork/env-loader is used at the top of next.config.js files to automatically load environment variables from env/.env files into process.env. SENTRY_ORG and SENTRY_PROJECT are stored as public configuration values in apps/deploy-web/env/.env and are loaded this way, while only SENTRY_AUTH_TOKEN is handled as a GitHub secret in workflows.

Applied to files:

  • apps/deploy-web/env/.env
  • apps/deploy-web/src/config/browser-env.config.ts
  • apps/deploy-web/next.config.js
📚 Learning: 2025-08-12T13:52:38.708Z
Learnt from: stalniy
Repo: akash-network/console PR: 1800
File: apps/deploy-web/next.config.js:163-165
Timestamp: 2025-08-12T13:52:38.708Z
Learning: In the Akash Console project, SENTRY_ORG and SENTRY_PROJECT are stored as public configuration values in apps/deploy-web/env/.env file and loaded by akashnetwork/env-loader, not as GitHub secrets. Only SENTRY_AUTH_TOKEN should be handled as a secret.

Applied to files:

  • apps/deploy-web/env/.env
🧬 Code graph analysis (1)
apps/deploy-web/next.config.js (2)
apps/stats-web/next.config.js (2)
  • require (2-2)
  • require (5-5)
apps/deploy-web/src/config/env-config.schema.ts (1)
  • browserEnvSchema (6-44)
🪛 dotenv-linter (4.0.0)
apps/deploy-web/env/.env

[warning] 35-35: [QuoteCharacter] The value has quote characters (', ")

(QuoteCharacter)

⏰ 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: validate / validate-app
  • GitHub Check: test-build
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
apps/deploy-web/next.config.js (1)

174-182: The endpoint https://api2.amplitude.com/2/httpapi is correct for the standard (global) Amplitude HTTP API v2 event tracking. No changes needed.

apps/deploy-web/src/services/analytics/analytics.service.ts (1)

216-224: Amplitude SDK serverUrl configuration is correctly implemented.

The code properly uses the serverUrl option in the init method, which is supported in @amplitude/analytics-browser v2.11.11. The pattern of conditionally passing { serverUrl } to init is correct, and https://api2.amplitude.com/2/httpapi is the documented standard endpoint for event tracking.

@codecov
Copy link

codecov bot commented Dec 31, 2025

Codecov Report

❌ Patch coverage is 71.42857% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.93%. Comparing base (ab32673) to head (c091842).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...oy-web/src/services/analytics/analytics.service.ts 66.66% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2441      +/-   ##
==========================================
- Coverage   51.23%   50.93%   -0.30%     
==========================================
  Files        1072     1062      -10     
  Lines       29375    29034     -341     
  Branches     6428     6413      -15     
==========================================
- Hits        15050    14789     -261     
+ Misses      13920    13839      -81     
- Partials      405      406       +1     
Flag Coverage Δ *Carryforward flag
api 80.03% <ø> (ø) Carriedforward from ab32673
deploy-web 31.41% <71.42%> (-0.01%) ⬇️
log-collector ?
notifications 87.94% <ø> (ø) Carriedforward from ab32673
provider-console 81.48% <ø> (ø) Carriedforward from ab32673
provider-proxy 84.35% <ø> (ø) Carriedforward from ab32673

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines Coverage Δ
apps/deploy-web/src/config/browser-env.config.ts 100.00% <ø> (ø)
apps/deploy-web/src/config/env-config.schema.ts 72.72% <ø> (ø)
.../src/services/app-di-container/app-di-container.ts 76.80% <100.00%> (+0.18%) ⬆️
...oy-web/src/services/analytics/analytics.service.ts 79.04% <66.66%> (-2.59%) ⬇️

... and 11 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments