-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Support Activity Events and Links trace out of proc output #113905
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
Support Activity Events and Links trace out of proc output #113905
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 supports formatted output of Activity Events and Links for out-of-proc serialization. It adds new tests for verifying the formatting, updates the DiagLinkedList string conversion logic for ActivityLink and ActivityEvent types, adjusts the instantiation of empty collections in Activity.cs, and fixes a comment typo in TimeZoneInfo.cs.
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs | Introduces new tests to validate the ToString output for Activity Links and Events. |
| src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/DiagLinkedList.cs | Adds helper functions for converting ActivityLink and ActivityEvent objects to a formatted string. |
| src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/ActivitySource.cs | Removes an unnecessary check for listeners. |
| src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs | Replaces array initializations with DiagLinkedList for empty collections. |
| src/libraries/System.Private.CoreLib/src/System/TimeZoneInfo.cs | Corrects a typo in a comment to match the intended variable name. |
Files not reviewed (1)
- src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj: Language not supported
Comments suppressed due to low confidence (1)
src/libraries/System.Private.CoreLib/src/System/TimeZoneInfo.cs:980
- The comment now correctly spells 'daylightDisplayName'. Ensure that similar typos are corrected consistently in documentation if applicable.
<_standardDisplayName>;<_daylightDisplayName>;
|
@noahfalk @CodeBlanch @rajkumar-rangaraj could you please have a look at the change? Thanks! |
src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/DiagLinkedList.cs
Show resolved
Hide resolved
|
This is fine by me as long as our OTel.NET crew is happy with the serialized data format. |
src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/DiagLinkedList.cs
Show resolved
Hide resolved
|
@rajkumar-rangaraj @noahfalk @CodeBlanch is there anything that needs to be addressed, or can you approve it? |
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.
LGTM
Fixes #102924
This change supports formatting the Events and Links embedded in the
Activityobject to allow getting such formatted data when the activity data gets serialized out of proc. The Events and Links will show up like the following example in the ETW serialized data:Note: the Events and Links will have the zero width space

\u200Bembedded after,and:.