Skip to content

perf: Optimize telemetry latency logging to reduce overhead #715

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.

Sign up for GitHub

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

Jump to bottom
Merged
samikshya-db merged 22 commits into from
Nov 27, 2025
Merged

perf: Optimize telemetry latency logging to reduce overhead #715

samikshya-db merged 22 commits into from
Nov 27, 2025

Conversation

@samikshya-db
Copy link
Contributor

@samikshya-db samikshya-db commented Nov 24, 2025 •
edited
Loading

Summary

Changes

  1. Eliminated extractor pattern - Replaced wrapper classes (TelemetryExtractor, CursorExtractor, ResultSetDownloadHandlerExtractor) with direct attribute access functions
  2. Feature flag early exit - Checks cached telemetry_enabled flag to skip heavy work when telemetry is disabled

Performance Impact

Internal doc : https://docs.google.com/document/d/1sV_N3IXhzH5aAoAOOhc4sA49ZdINGii_wXvYNyZdVEo/edit?tab=t.0#heading=h.ezkw4po8o1mx

~ 8% improvement across test cases.

Behavior Changes

  • The decorator now always logs latency at DEBUG level for debugging purposes, even when telemetry is disabled
  • Exits early using cached connection.telemetry_enabled flag (avoids dictionary lookup + instance check on every operation)
  • Only performs data extraction and object creation when telemetry is actually enabled

Test Plan

@samikshya-db
test
876ed88
@samikshya-db
perf: Optimize telemetry latency logging to reduce overhead
0687a29 
Optimizations implemented:
1. Eliminated extractor pattern - replaced wrapper classes with direct
   attribute access functions, removing object creation overhead
2. Switched from time.perf_counter() to time.monotonic() for faster timing
3. Added feature flag early exit - checks cached telemetry_enabled flag
   to skip heavy work when telemetry is disabled
4. Simplified code structure with early returns for better readability

Performance impact:
- When telemetry disabled: ~95% overhead reduction (only timing + debug log)
- When telemetry enabled: ~50-70% overhead reduction
- Overall: Reduces telemetry overhead from ~10% to 0.5-3%

The decorator now:
- Always logs latency at DEBUG level for debugging
- Exits early using cached connection.telemetry_enabled flag (avoids dict lookup)
- Only performs data extraction and object creation when telemetry is enabled
@github-actions
Copy link

github-actions bot commented Nov 24, 2025

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

@samikshya-db samikshya-db marked this pull request as draft November 24, 2025 09:02
@samikshya-db
perf: Optimize telemetry latency logging and feature flag caching
695ea7f 
Optimizations implemented:

1. Latency Logger (latency_logger.py):
   - Eliminated extractor pattern - replaced wrapper classes with direct
     attribute access functions, removing object creation overhead
   - Switched from time.perf_counter() to time.monotonic() for faster timing
   - Added feature flag early exit - checks cached telemetry_enabled flag
     to skip heavy work when telemetry is disabled
   - Simplified code structure with early returns for better readability

2. Feature Flag Caching (feature_flag.py):
   - Changed cache key from session_id to host
   - Feature flags now shared across multiple connections to same host
   - Reduces network calls - first connection fetches, subsequent reuse cache

Performance impact:
- When telemetry disabled: significantly reduced overhead (only timing + debug log)
- When telemetry enabled: reduced overhead from data extraction optimizations
- Feature flag: Only fetched once per host instead of per session
- Overall: Reduces telemetry overhead substantially

The decorator now:
- Always logs latency at DEBUG level for debugging
- Exits early using cached connection.telemetry_enabled flag
- Only performs data extraction and object creation when telemetry is enabled
@github-actions
Copy link

github-actions bot commented Nov 24, 2025

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

@samikshya-db
perf: Skip feature flag fetch when telemetry is force enabled
9f19c88 
When force_enable_telemetry=True, skip the feature flag network call
entirely since we already know telemetry should be enabled.

This optimization:
- Eliminates unnecessary network call during connection init
- Faster connection setup when telemetry is forced on
- Clearer control flow with early returns
@github-actions
Copy link

github-actions bot commented Nov 24, 2025

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

@samikshya-db
style: Fix Black formatting for latency_logger.py
c76610e 
Split long lines to comply with Black's 88 character limit
@github-actions
Copy link

github-actions bot commented Nov 24, 2025

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

@samikshya-db
test: Update telemetry test for lock-free queue
b97359d 
Changed _events_batch to _events_queue.qsize() to match the
lock-free queue implementation
@github-actions
Copy link

github-actions bot commented Nov 24, 2025

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

@github-actions
Copy link

github-actions bot commented Nov 24, 2025

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

@samikshya-db
fix: Prevent exception suppression in log_latency decorator
564827f 
The decorator was using 'return' statements in the finally block,
which suppressed exceptions raised in decorated methods. This caused
downloader tests to fail as exceptions (ConnectionError, TimeoutError)
were being swallowed.

Fixed by nesting telemetry logic inside an if statement instead of
using early returns, ensuring exceptions propagate correctly.
@github-actions
Copy link

github-actions bot commented Nov 24, 2025

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

@github-actions
Copy link

github-actions bot commented Nov 26, 2025

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

nikhilsuri-db
nikhilsuri-db approved these changes Nov 26, 2025
View reviewed changes
Copy link
Contributor

@nikhilsuri-db nikhilsuri-db left a comment

Choose a reason for hiding this comment

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

I am mostly aligned with your PR too, just not able to pin how these changes improved the benchmarking numbers. It will be great if we can measure the changes with the improvements they bring to the benchmarking numbers.

@samikshya-db
Copy link
Contributor Author

samikshya-db commented Nov 26, 2025

just not able to pin how these changes improved the benchmarking numbers

Most of the improvements I see in benchmarking in my opinion is coming from extractor pattern removal (this cuts object creation and method call)

@samikshya-db @claude
Merge branch 'main' into improvePerf
2bbc3e6 
Resolved merge conflict in telemetry_client.py by keeping Queue optimization
while incorporating new telemetry push client with circuit breaker support.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions
Copy link

github-actions bot commented Nov 26, 2025

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

@github-actions
Copy link

github-actions bot commented Nov 26, 2025

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

@databricks databricks deleted a comment from github-actions bot Nov 26, 2025
@databricks databricks deleted a comment from github-actions bot Nov 26, 2025
@databricks databricks deleted a comment from github-actions bot Nov 26, 2025
@databricks databricks deleted a comment from github-actions bot Nov 26, 2025
@samikshya-db samikshya-db merged commit 8d5e155 into main Nov 27, 2025
29 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nikhilsuri-db nikhilsuri-db nikhilsuri-db approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@samikshya-db @nikhilsuri-db @jayantsing-db @vikrantpuppala