Skip to content

Conversation

@tarekgh
Copy link
Member

@tarekgh tarekgh commented Mar 25, 2025

Fixes #102924

This change supports formatting the Events and Links embedded in the Activity object 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:

Events->"[(TestEvent1,​2025-03-27T23:34:10.6225721+00:00,​[E11:​EV1,​E12:​EV2]),​(TestEvent2,​2025-03-27T23:34:11.6276895+00:00,​[E21:​EV21,​E22:​EV22])]"
Links->"[(19b6e8ea216cb2ba36dd5d957e126d9f,​98f7abcb3418f217,​Recorded,​null,​false,​[alk1:​alv1,​alk2:​alv2]),​(2d409549aadfdbdf5d1892584a5f2ab2,​4f3526086a350f50,​None,​null,​false)]"

Note: the Events and Links will have the zero width space \u200B embedded after , and :.
image

@Copilot Copilot AI review requested due to automatic review settings March 25, 2025 23:39
Copy link
Contributor

Copilot AI left a 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>;

@tarekgh
Copy link
Member Author

tarekgh commented Mar 25, 2025

@noahfalk @CodeBlanch @rajkumar-rangaraj could you please have a look at the change? Thanks!

@noahfalk
Copy link
Member

This is fine by me as long as our OTel.NET crew is happy with the serialized data format.

@tarekgh
Copy link
Member Author

tarekgh commented Apr 1, 2025

@rajkumar-rangaraj @noahfalk @CodeBlanch is there anything that needs to be addressed, or can you approve it?

Copy link

@rajkumar-rangaraj rajkumar-rangaraj left a comment

Choose a reason for hiding this comment

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

LGTM

@tarekgh tarekgh merged commit c15177a into dotnet:main Apr 1, 2025
139 of 143 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators May 2, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

System.Diagnostics.Activity out of proc monitoring can't access Activity Events or Links

4 participants