- Notifications
You must be signed in to change notification settings - Fork79
Fixes creating .csv file in OpenAIUsageDebuggingPlugin. Closes #1460#1463
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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.
Pull request overview
This PR addresses issue#1460 by moving CSV file creation logic fromInitializeAsync toAfterResponseAsync in the OpenAIUsageDebuggingPlugin. The change ensures the output file is created on-demand when the first OpenAI request is processed, rather than during plugin initialization.
Key changes:
- Removed the
InitializeAsyncoverride that created the CSV file with headers during plugin initialization - Added file existence check and header creation logic directly in
AfterResponseAsyncbefore appending usage data
You can also share your feedback on Copilot code review for a chance to win a $100 gift card.Take the survey.
| awaitFile.AppendAllLinesAsync(outputFileName,[UsageRecord.GetHeader()],cancellationToken); | ||
| } | ||
| awaitFile.AppendAllLinesAsync(outputFileName,[usage.ToString()],cancellationToken); |
CopilotAINov 28, 2025
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.
Potential file corruption: The file append operation is not protected by any synchronization mechanism. If multiple requests are processed concurrently, their writes to the CSV file could interleave, corrupting the file format. Consider protecting both the header initialization and data appending with the same lock mechanism.
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.
@copilot open a new pull request to apply changes based onthis feedback
| if(!File.Exists(outputFileName)) | ||
| { | ||
| Logger.LogDebug("Creating output file {FileName} with header",outputFileName); | ||
| awaitFile.AppendAllLinesAsync(outputFileName,[UsageRecord.GetHeader()],cancellationToken); | ||
| } |
CopilotAINov 28, 2025
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.
Race condition: Multiple concurrent requests could pass theFile.Exists check simultaneously before the header is written, potentially causing the header to be written multiple times or file corruption. Consider using a lock or a flag to ensure thread-safe file initialization.
Example solution:
privatereadonlyobject_fileLock=new();privatebool_fileInitialized=false;// In AfterResponseAsync:if(!_fileInitialized){lock(_fileLock){if(!_fileInitialized){Logger.LogDebug("Creating output file {FileName} with header",outputFileName);awaitFile.AppendAllLinesAsync(outputFileName,[UsageRecord.GetHeader()],cancellationToken);_fileInitialized=true;}}}
Fixes creating .csv file in OpenAIUsageDebuggingPlugin.Closes#1460