Skip to content
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

Limit analytics send rate #3420

Merged
merged 13 commits into from
Mar 3, 2025
Merged

Limit analytics send rate #3420

merged 13 commits into from
Mar 3, 2025

Conversation

NickKhalow
Copy link
Contributor

@NickKhalow NickKhalow commented Feb 24, 2025

Pull Request Description

What does this PR change?

  • Fixed Bug with MAX_BATCH_LIMIT fix MAX_BATCH_SIZE const meilisearch/segment#17 . batcher.rs should be replaced to repo's version when the change is merged.
  • Introduced queue_batcher to replace auto_batcher. Batch sending is manual and requires a trigger from the managed side. It is being triggered periodically per 1 minute in the PR.
  • Added Analytics tab to the debug panel. It displays amount of unflushed batches in the queue and has a button to flush the oldest batch manually.
  • Removed flushAt with 20 value to prevent the overuse of flushing and to distress the web requests.

Test Instructions

Prerequisites

  1. Get access to segment admin panel
  2. Go to the debug tab under Development Unity channel

Test Steps

  1. Launch the client from this branch (test both mac and windows)
  2. Play normally with the opened analytics tab in the debug panel
  3. Validate that changes are being sent per minute and not faster
  4. Try to manually flush your changes with the manual flash button, check the changes are appeared immediately (maybe in 5 seconds due the network and display delays)
  5. Stress test the changes with analytic events (chat, moving, etc) and validate the queue grows in the debug panel

Additional Testing Notes

  • Note any edge cases to verify - stressful events with many players, chatting functionality
  • Mention specific areas that need careful testing - described in the test steps
  • List known limitations or potential issues - no potential issues are known

Quality Checklist

  • [+] Changes have been tested locally
  • [+] Performance impact has been considered

Code Review Reference

Please review our Code Review Standards before submitting.

@NickKhalow NickKhalow requested review from a team as code owners February 24, 2025 09:37
@NickKhalow NickKhalow linked an issue Feb 24, 2025 that may be closed by this pull request
Copy link
Contributor

github-actions bot commented Feb 24, 2025

badge

New build in progress, come back later!

Copy link

@DafGreco DafGreco left a comment

Choose a reason for hiding this comment

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

✔️ PR reviewed and approved by QA on both platforms following instructions playing both happy and un-happy path

Regressions for this ticket had been performed in order to verify that the normal flow is working as expected:

  • ✅ Backpack and wearables in world
  • ✅ Emotes in world and in backpack
  • ✅ Teleport with map/coordinates/Jump In
  • ✅ Chat and multiplayer
  • ✅ Profile card
  • ✅ Camera
  • ✅ Skybox

Tested the following scenarios in both platforms:

  • Flush manually
  • Checked the number of calls
  • Teleport multiple times
  • Generate 250 avatars with the devtool and works as expected

# Conflicts:
#	Explorer/Assets/Plugins/RustSegment/SegmentServerWrap/RustSegmentAnalyticsService.cs
#	Explorer/Assets/Scripts/Global/Dynamic/BootstrapContainer.cs
@NickKhalow NickKhalow enabled auto-merge (squash) March 3, 2025 13:10
@NickKhalow NickKhalow merged commit 1ec4f5a into dev Mar 3, 2025
3 of 5 checks passed
@NickKhalow NickKhalow deleted the 3419-limit-analytics-send-rate branch March 3, 2025 13:35
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.

Limit Analytics Send Rate
4 participants