Fix EventSource WriteEvent varargs overloads dropped from EventPipe#130525
Fix EventSource WriteEvent varargs overloads dropped from EventPipe#130525steveisok wants to merge 1 commit into
Conversation
The WriteEvent(int, params object[]) / params EventSourcePrimitive[] overloads were silently dropped from EventPipe sessions enabled with keyword 0. The specialized WriteEvent overloads gate on metadata.EnabledForEventPipe (via IsEnabledCommon, which honors the 'matchAnyKeyword == 0 means match all' convention), but the object[] varargs path re-checks EventProviderImpl.IsEnabled(level, keywords), which did not honor that convention. Since every manifest event descriptor carries reserved session-keyword bits (0xF00000000000) and EventPipe enables named providers with keyword 0, the varargs gate always failed. Fix IsEnabled to treat _anyKeywordMask == 0 as match-all, aligning it with IsEnabledCommon. Behavior is unchanged when _anyKeywordMask != 0. Adds a regression test that enables a provider with keyword 0 and verifies both specialized and varargs events reach EventPipe. Fixes dotnet#124518 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes EventSource.WriteEvent varargs events being incorrectly filtered out of EventPipe sessions when the provider is enabled with keyword 0 (match-all), by aligning EventProviderImpl.IsEnabled keyword-mask semantics with EventSource.IsEnabledCommon. It also adds a regression test under the EventPipe tracing tests to ensure varargs and specialized overloads both show up in the collected trace.
Changes:
- Update
EventProviderImpl.IsEnabled(byte level, long keywords)to treat_anyKeywordMask == 0as “match all keywords”. - Add a new EventPipe test project (
writeeventvarargs) that enables a provider with keyword0and validates emitted events are observed in the trace. - Add the test implementation that generates both strongly-typed and varargs events and validates counts in the EventPipe stream.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventProvider.cs | Fix keyword matching so EventPipe sessions enabled with keyword 0 don’t drop varargs events. |
| src/tests/tracing/eventpipe/writeeventvarargs/writeeventvarargs.csproj | New EventPipe test project for validating WriteEvent varargs behavior. |
| src/tests/tracing/eventpipe/writeeventvarargs/writeeventvarargs.cs | Regression test that emits specialized + varargs events and validates both appear in the EventPipe stream. |
| // Boxing the arguments forces the WriteEvent(int, params object?[]) overload, which | ||
| // serializes through EventProvider.WriteEvent(object[]). This path was dropped for | ||
| // EventPipe sessions enabled with keyword 0 because the keyword check did not treat | ||
| // an "any" keyword mask of 0 as "match all keywords". | ||
| [Event(2)] | ||
| public void VarargsEvent(int id, string name) => WriteEvent(2, (object)id, (object)name); |
| // | ||
| if ((keywords == 0) || | ||
| (((keywords & _anyKeywordMask) != 0) && | ||
| ((_anyKeywordMask == 0 || (keywords & _anyKeywordMask) != 0) && |
There was a problem hiding this comment.
I think all of these are occurring to cause the discrepancy:
-
Events are initialized with an EventDescriptor OR'd with the 4 reserved bits session mask
-
VarArgs events will use
WriteEventVarargsthat invokes the EventProvider.WriteEvent overloadthat layers anIsEnabled(<level>, <keywords>)checkusing the raw metadata descriptor with the sessionMask bits flipped. As a result, it misses thekeywords == 0check above because it's using the (real keywords | sessionMask). -
dotnet-trace will treat no specified provider keywords as
0whereas PerfView collect and logman will treat no specified keywords as all enabled (thus_anyKeywordMaskactually matches the keywords OR'd with the sessionMask All)
It feels like we should clear the sessionMask bits from the EventDescriptor before passing it back into this IsEnabled, e.g.
long keywords = eventDescriptor.Keywords &
unchecked((long)~SessionMask.All.ToEventKeywords())
if (IsEnabled(eventDescriptor.Level, keywords))at
There was a problem hiding this comment.
Good catch — the descriptor keywords carry the session-mask bits, so the keywords == 0 short-circuit never fires. IsEnabledByDefault already strips those bits before IsEnabledCommon (EventSource.cs#L2384), so that's the right pattern.
One caveat: the strip alone still drops an event with a declared keyword under a keyword-0 session (_anyKeywordMask == 0 && keywords != 0 → false).
I think we should do both. Strip the session bits and treat _anyKeywordMask == 0 as a match-all. Basically the inner check mirrors IsEnabledCommon.
The WriteEvent(int, params object[]) / params EventSourcePrimitive[] overloads were silently dropped from EventPipe sessions enabled with keyword 0. The specialized WriteEvent overloads gate on metadata.EnabledForEventPipe (via IsEnabledCommon, which honors the 'matchAnyKeyword == 0 means match all' convention), but the object[] varargs path re-checks EventProviderImpl.IsEnabled(level, keywords), which did not honor that convention. Since every manifest event descriptor carries reserved session-keyword bits (0xF00000000000) and EventPipe enables named providers with keyword 0, the varargs gate always failed.
Fix IsEnabled to treat _anyKeywordMask == 0 as match-all, aligning it with IsEnabledCommon. Behavior is unchanged when _anyKeywordMask != 0.
Adds a regression test that enables a provider with keyword 0 and verifies both specialized and varargs events reach EventPipe.
Fixes #124518