fix(analytics): remove amplitude sampling to restore full reporting#2526
Merged
ygrishajev merged 3 commits intomainfrom Jan 19, 2026
Merged
fix(analytics): remove amplitude sampling to restore full reporting#2526ygrishajev merged 3 commits intomainfrom
ygrishajev merged 3 commits intomainfrom
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR removes Amplitude sampling configuration and sampling-related logic across API and frontend. Environment variables and schema entries for AMPLITUDE_SAMPLING / NEXT_PUBLIC_AMPLITUDE_SAMPLING were deleted; AnalyticsService implementations were simplified to drop hash-based sampling and related dependencies, with lazy initialization added on the frontend. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AnalyticsService
participant Amplitude
participant Logger
participant GA
Client->>AnalyticsService: identify(userId, properties?)
AnalyticsService->>AnalyticsService: if not initialized -> initAmplitude()
AnalyticsService->>Amplitude: identify(userId, properties) rgba(0,128,255,0.5)
AnalyticsService->>Logger: debug(ANALYTICS_USER_IDENTIFIED, payload) rgba(0,255,128,0.5)
AnalyticsService->>GA: optional GA identify/event sync rgba(255,128,0,0.5)
Client->>AnalyticsService: track(userId, eventName, properties?)
AnalyticsService->>AnalyticsService: ensure amplitude initialized
AnalyticsService->>Amplitude: track(event with user_id & properties) rgba(0,128,255,0.5)
AnalyticsService->>Logger: debug(ANALYTICS_EVENT_REPORTED, payload) rgba(0,255,128,0.5)
AnalyticsService->>GA: GA event emit rgba(255,128,0,0.5)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (79.29%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #2526 +/- ##
==========================================
- Coverage 50.78% 50.45% -0.33%
==========================================
Files 1063 1053 -10
Lines 29476 29097 -379
Branches 6535 6462 -73
==========================================
- Hits 14968 14681 -287
+ Misses 14146 14001 -145
- Partials 362 415 +53
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
![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
🤖 Fix all issues with AI agents
In `@apps/deploy-web/src/services/analytics/analytics.service.ts`:
- Around line 207-221: The call to amplitudeClient.init inside initAmplitude
currently passes the options object as the second argument (so SDK treats it as
userId); change the call in initAmplitude to pass undefined as the second
argument and supply the options object as the third argument (use
this.options.amplitude.apiKey, undefined, and then the options object built from
serverUrl), leaving amplitudeInitialized logic unchanged.
🧹 Nitpick comments (1)
apps/deploy-web/src/services/analytics/analytics.service.spec.ts (1)
115-213: Remove stale “Initialize sampling” comments in track tests.Sampling is no longer present, so these comments now mislead and add noise.
🧹 Suggested cleanup
- service.identify({ id: faker.string.uuid() }); // Initialize sampling + service.identify({ id: faker.string.uuid() }); ... - service.identify({ id: faker.string.uuid() }); // Initialize sampling + service.identify({ id: faker.string.uuid() });As per coding guidelines, avoid unnecessary comments.
ygrishajev
force-pushed
the
fix/analytics
branch
from
4190146 to
72ef590
Compare
baktun14
approved these changes
Jan 17, 2026
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
2 participants
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.
refs #2525
Summary by CodeRabbit
Chores
New Features
✏️ Tip: You can customize this high-level summary in your review settings.