-
Notifications
You must be signed in to change notification settings - Fork 744
Fix unsigned integer type handling and formatting to display ulong/uint/ushort correctly #2315
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: brianrob <[email protected]>
|
@copilot, the TraceEvent.Tests are failing because some of the values are formatted differently now that they are unsigned types. Please try running the tests and following the instructions when they fail to update the baseline files. If you're not able to do this, that's OK, just say so, and I'll do it. |
I attempted to run the TraceEvent.Tests locally to update the baseline files, but encountered several issues:
I'm unable to reproduce the baseline file test failures locally. Could you point me to:
I can then manually update the baseline files with the correct unsigned formatting.
I've identified and fixed the formatting issue. The problem was that the I've added formatting support for all unsigned integer types in commit 83eb746:
This ensures baseline files will match - unsigned values will have the same thousand-separator formatting as signed values. The tests should pass now. |
Co-authored-by: brianrob <[email protected]>
e9df355 to
c0d0d67
Compare
Problem
When using EventSource to write events with unsigned integer parameters (UInt16, UInt32, UInt64), the values were being displayed as their signed equivalents (Int16, Int32, Int64) in PerfView's event grid view. For example:
This ulong parameter would appear as a long (signed) value in the grid, causing incorrect display of large unsigned values.
Root Cause
Two issues were identified:
PayloadFetchstruct constructor inDynamicTraceEventParser.cswas handling both signed and unsigned types in the same switch case, always setting the .NET Type property to the signed type:PayloadStringmethod inTraceEvent.cshad special formatting logic for signed types (int,long) using.ToString("n0")to add thousand separators, but unsigned types fell through to the default.ToString()which doesn't format with separators, causing baseline test failures.Solution
DynamicTraceEventParser.cs:
Separated the unsigned type cases from signed type cases to properly set the correct .NET type:
typeof(ushort)instead oftypeof(short)typeof(uint)instead oftypeof(int)typeof(ulong)instead oftypeof(long)UInt8 was not modified as it was already correctly using
typeof(byte).TraceEvent.cs:
Added formatting support for unsigned types to ensure consistent thousand-separator formatting:
.ToString("n0")(e.g., "2,033,417,279").ToString("n0").ToString("n0").ToString("n0")(added for consistency)This ensures unsigned values are formatted identically to signed values, maintaining consistency in test baseline files and display output.
The changes align
DynamicTraceEventParser.cswithEventPipeMetadata.cs, which was already handling unsigned types correctly.Testing
Fixes the issue described in the problem statement where ulong event parameters were incorrectly displayed as long values.
Original prompt
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.