-
Notifications
You must be signed in to change notification settings - Fork 5.3k
[NET] Remove unnecessary Keyword property from internal log events #123488
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
|
Tagging subscribers to this area: @karelz, @dotnet/ncl |
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 removes unnecessary Keywords properties from internal log events in System.Net libraries. The Keywords were redundant with the LogLevel information and their presence required dotnet trace to explicitly mask for them using flags (:0xf), making log collection more cumbersome.
Changes:
- Removed the Keywords class definition from NetEventSource.Common.cs
- Removed
Keywords = Keywords.DefaultorKeywords = Keywords.Debugparameters from all Event attributes across System.Net libraries - Applied changes consistently to production code and test logging infrastructure
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| NetEventSource.Common.cs | Removed the Keywords class definition that contained Default and Debug keyword constants |
| NetEventSource.Common.DumpBuffer.cs | Removed Keywords parameter from DumpBuffer event attribute |
| NetEventSource.Common.Associate.cs | Removed Keywords parameter from Associate event attribute |
| NetEventSource.Security.cs | Removed Keywords parameter from 27 Security-related event attributes |
| NetEventSource.Security.Windows.cs | Removed Keywords parameter from 8 Windows Security-related event attributes |
| NetEventSource.Http.cs | Removed Keywords parameter from 8 HTTP-related event attributes |
| NetEventSource.Sockets.cs | Removed Keywords parameter from 4 Socket-related event attributes |
| NetEventSource.WebSockets.cs | Removed Keywords parameter from 14 WebSocket-related event attributes |
| EventSourceTestLogging.cs | Removed Keywords parameter from 3 test event attributes |
MihaZupan
left a comment
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.
Thanks
rzikm
left a comment
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, too bad we still need to keep using the flags for existing releases.
Do we have any public documentation talking about these internal diagnostics which we should update?
|
We mention it here https://learn.microsoft.com/en-us/dotnet/fundamentals/networking/telemetry/events#internal-diagnostics, but just via the EventListener API. We could add a snippet for how to do it with |
|
dotnet/docs#51284 We'll need to update with .NET 11 though. |
|
/ba-g enterprise tests are unrelated |
They were just containing the very same information as the
LogLeveland having them specified preventsdotnet traceto collect the logs without masking for them in flags (see :0xf):Tested manually.
cc @rzikm