-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix: add missing trace info and cancellation events #791
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
Conversation
📋 Review SummaryThis PR introduces valuable telemetry improvements by adding user cancellation event logging for both request cancellations and tool call cancellations. It also fixes an important issue where trace information was being lost during response merging in the content generation pipeline. The changes are well-structured and include appropriate test coverage. 🔍 General Feedback
🎯 Specific Feedback🟢 Medium
🔵 Low
✅ Highlights
|
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
| } | ||
|
|
||
| logUserCancellationEvent( | ||
| event: import('../types.js').UserCancellationEvent, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This import statements should be place on top of this file.
|
LGTM! |
* fix: add missing trace info and cancellation events * fix: re-organize tool/request cancellation logging
* fix: add missing trace info and cancellation events * fix: re-organize tool/request cancellation logging
TLDR
This PR adds comprehensive telemetry logging for user cancellation events, including both request cancellations and tool call cancellations. It also fixes missing trace information in the content generation pipeline by ensuring essential response properties are properly copied during response merging.
Dive Deeper
Key Changes
1. Added User Cancellation Logging
2. Fixed Missing Trace Information
responseIdandcreateTimewere getting lost3. Updated Telemetry System
Reviewer Test Plan
Testing User Cancellation Events
Request Cancellation Testing
Tool Call Cancellation Testing
Testing Matrix
Linked issues / bugs
None