Skip to content

[Part 5/6] feat(telemetry): add activity monitor with event-driven snapshots#8124

Merged
jacob314 merged 6 commits intogoogle-gemini:mainfrom
eLyiN:split/activity-monitor
Oct 20, 2025
Merged

[Part 5/6] feat(telemetry): add activity monitor with event-driven snapshots#8124
jacob314 merged 6 commits intogoogle-gemini:mainfrom
eLyiN:split/activity-monitor

Conversation

@eLyiN
Copy link
Copy Markdown
Contributor

@eLyiN eLyiN commented Sep 9, 2025

Summary

Introduces ActivityMonitor to track user events and trigger throttled memory snapshots when performance monitoring is active. This component bridges user activity detection with memory monitoring for intelligent telemetry collection.

Dependencies

All dependencies merged to main:

Branch Status: Rebased on latest main (all dependencies included).

Related PRs - Enhanced Performance Monitoring Split

This is Part 5 of 6 in the split implementation of #2127:

Changes in This PR

🆕 Files Added:

  • packages/core/src/telemetry/activity-monitor.ts - ActivityMonitor implementation
  • packages/core/src/telemetry/activity-monitor.test.ts - Comprehensive unit tests
  • packages/core/src/telemetry/index.ts - ActivityMonitor exports

ActivityMonitor (packages/core/src/telemetry/activity-monitor.ts)

Event-Driven Memory Snapshots:

  • Smart triggering: Monitors user events and triggers memory snapshots when appropriate
  • Performance-aware: Only active when isPerformanceMonitoringActive() returns true
  • Throttled recording: Default config with minimal throttle to avoid telemetry noise
  • Event integration: Hooks into key user interactions (input start, message added, tool calls, streams)

Key Features:

  • Configurable triggers: Define which events should trigger memory snapshots
  • Memory monitor integration: Uses getMemoryMonitor() from Part 4 for actual memory collection
  • Activity detection integration: Leverages isUserActive() from Part 2 for smart timing
  • Rate limiting: Uses primitives from Part 1 to prevent telemetry spam

Default Configuration:

// Triggers memory snapshots on key user interactions
const defaultTriggers = [
  'user_input_start',
  'message_added', 
  'tool_call_scheduled',
  'stream_start'
];

API Interface:

// Initialize activity monitor
initializeActivityMonitor(config);

// Get monitor instance
const monitor = getActivityMonitor();

// Lifecycle management
startGlobalActivityMonitoring();
stopGlobalActivityMonitoring();

// Manual event triggering
monitor.recordEvent('custom_event');

Testing

ActivityMonitor unit tests:

  • activity-monitor.test.ts - Event triggering, memory snapshot integration, throttling behavior
  • Tests cover performance monitoring integration, event configuration, and memory monitor coordination
  • Mock-based testing for external dependencies (memory monitor, activity detector)
  • All 22 tests passing

Technical Architecture

Component Integration Stack:

  1. Listens for events → ActivityMonitor receives user interaction events
  2. Checks performance config → Only proceeds if monitoring is active (Part 3)
  3. Validates activity → Uses activity detection to ensure user is active (Part 2)
  4. Triggers memory snapshot → Calls MemoryMonitor to capture current state (Part 4)
  5. Applies rate limiting → Uses primitives to prevent spam (Part 1)

Rebase Status

Successfully rebased on main (commit: 603ec2b)
All preflight checks passing

  • Build: ✅
  • Tests: ✅ (5,237 tests passing)
  • TypeCheck: ✅
  • Lint: ✅
  • Format: ✅

Next Steps: Part 6 will add CLI hooks to wire ActivityMonitor into the actual user interface event flows.

@jacob314
Copy link
Copy Markdown
Contributor

jacob314 commented Oct 8, 2025

It would be great to get this in! Please merge to reflect the changes already landed for memory monitor and then we can take a look.

@eLyiN eLyiN force-pushed the split/activity-monitor branch from 47a4ec0 to ecd4673 Compare October 8, 2025 18:46
@eLyiN eLyiN marked this pull request as ready for review October 8, 2025 18:46
@eLyiN eLyiN requested a review from a team as a code owner October 8, 2025 18:46
@eLyiN
Copy link
Copy Markdown
Contributor Author

eLyiN commented Oct 8, 2025

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

Thank you for this contribution. The new ActivityMonitor is a great addition for event-driven telemetry. I've found a couple of high-severity issues that should be addressed to improve the robustness and maintainability of the new module. My main concerns are around the lifecycle management of listeners and a function name collision that could cause confusion. Please see my detailed comments below.

@eLyiN
Copy link
Copy Markdown
Contributor Author

eLyiN commented Oct 8, 2025

It would be great to get this in! Please merge to reflect the changes already landed for memory monitor and then we can take a look.

Hi @jacob314, ready for review!

Copy link
Copy Markdown
Contributor

@jacob314 jacob314 left a comment

Choose a reason for hiding this comment

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

lgtm

@jacob314 jacob314 added this pull request to the merge queue Oct 13, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 13, 2025
@eLyiN eLyiN force-pushed the split/activity-monitor branch from b22a59b to 792981a Compare October 14, 2025 06:29
@eLyiN eLyiN requested a review from jacob314 October 14, 2025 06:45
@eLyiN
Copy link
Copy Markdown
Contributor Author

eLyiN commented Oct 14, 2025

The test file-system-interactive.test.ts introduced in #11046 was failing due to a race condition. The test wasn't awaiting rig.waitForTelemetryReady() after the write operation, which meant it read the file before the file system had fully synced (especially problematic in sandboxed environments).

@jacob314 take a look for re-approval, so we can continue in #8125 , thanks

@eLyiN eLyiN force-pushed the split/activity-monitor branch from 9d74e9b to dc1433a Compare October 17, 2025 06:37
@eLyiN eLyiN force-pushed the split/activity-monitor branch from dc1433a to de2e7f6 Compare October 20, 2025 10:47
@eLyiN
Copy link
Copy Markdown
Contributor Author

eLyiN commented Oct 20, 2025

@jacob314 fixed conflict on rebase & format fix, ready for merge

@jacob314 jacob314 enabled auto-merge October 20, 2025 17:20
Copy link
Copy Markdown
Contributor

@jacob314 jacob314 left a comment

Choose a reason for hiding this comment

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

lgtm

@jacob314 jacob314 added this pull request to the merge queue Oct 20, 2025
Merged via the queue into google-gemini:main with commit 71ecc40 Oct 20, 2025
25 checks passed
cocosheng-g pushed a commit to cocosheng-g/gemini-cli that referenced this pull request Oct 21, 2025
…apshots (google-gemini#8124)

Co-authored-by: Jacob Richman <jacob314@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority/p2 Important but can be addressed in a future release.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants