Extend PredefinedDynamicTraceEventParser to Support Dynamic Events from EventPipeEventSource
This PR extends PredefinedDynamicTraceEventParser to support dynamic events from EventPipeEventSource in addition to ETWTraceEventSource.
Problem
Previously, PredefinedDynamicTraceEventParser.OnUnhandledEvent only handled ETW events using RegisteredTraceEventParser.TryLookupWorker(), which doesn't work for EventPipe events. This meant that predefined dynamic event templates couldn't be used with EventPipe trace sources.
Solution
Modified the OnUnhandledEvent method to:
- Check source type: Use
data.Source as EventPipeEventSourceto detect EventPipe sources - EventPipe path: For EventPipe sources, use
EventPipeEventSource.TryGetTemplateFromMetadata()to retrieve event templates - ETW path: For other sources, preserve existing behavior using
RegisteredTraceEventParser.TryLookupWorker() - Unified processing: Both paths converge to the same template registration logic
// Before (ETW only)
DynamicTraceEventData dynamicTraceEventData = RegisteredTraceEventParser.TryLookupWorker(data);
// After (ETW + EventPipe)
DynamicTraceEventData dynamicTraceEventData = null;
EventPipeEventSource eventPipeSource = data.Source as EventPipeEventSource;
if (eventPipeSource != null)
{
eventPipeSource.TryGetTemplateFromMetadata(data, out dynamicTraceEventData);
}
else
{
dynamicTraceEventData = RegisteredTraceEventParser.TryLookupWorker(data);
}
Benefits
- ✅ Enables strongly-typed access to EventPipe dynamic events via predefined templates
- ✅ Zero breaking changes to existing ETW functionality
- ✅ Minimal code footprint (15 lines added, 2 modified)
- ✅ Follows established patterns used in
EventPipeTraceEventParser.TryLookup - ✅ Maintains backward compatibility
The implementation follows the same architectural pattern already established in the codebase and enables unified dynamic event handling across both ETW and EventPipe trace sources.
Fixes #2231.
@copilot, this code looks correct. I'd now like for you to update UniversalEventsTraceEventParser to inherit from PredefinedDynamicTraceEventParser. Be sure to update each of the events as well. The event payloads are the same and don't need to be changed, but the events need to be converted to the pattern that PredefinedDynamicTraceEventParser follows.
@copilot, this code looks correct. I'd now like for you to update
UniversalEventsTraceEventParserto inherit fromPredefinedDynamicTraceEventParser. Be sure to update each of the events as well....
I've updated UniversalEventsTraceEventParser to inherit from PredefinedDynamicTraceEventParser and converted the events to follow the PredefinedDynamicTraceEventParser pattern. The changes include:
- Changed inheritance from
TraceEventParsertoPredefinedDynamicTraceEventParser - Created
CpuSampleEventandCswitchSampleEventclasses that inherit fromPredefinedDynamicEvent - Registered the event templates in the constructor using
RegisterTemplate() - Removed the custom
EnumerateTemplatesmethod to use the base class implementation - Maintained backward compatibility by keeping the existing event signatures with
SampleTraceData
The implementation provides both dynamic event support (for EventPipe sources) and backward compatibility for existing code. Commit: 87a570a
@copilot please update the cpu and cswitch events in UniversalEventsTraceEventParser to properly use the registered dynamic events.
@copilot please update the
cpuandcswitchevents inUniversalEventsTraceEventParserto properly use the registered dynamic events.
I've updated the cpu and cswitch events in UniversalEventsTraceEventParser to properly use the registered dynamic events. The changes include:
- Changed event signatures from
Action<SampleTraceData>toAction<CpuSampleEvent>andAction<CswitchSampleEvent> - Updated event implementation to use
AddCallbackForEvent<T>()andRemoveCallback<T>()instead of manual template registration - Updated consuming code in
PerfViewData.csandNettraceUniversalConverter.csto use the new event types
The events now properly leverage the PredefinedDynamicTraceEventParser infrastructure and work with the registered dynamic event templates. Commit: 48c23bd
Copilot Post-Mortem:
- Copilot did a good job getting started initially, but ultimately was not able to produce code that worked.
- I believe a big issue here is that I was not able to adequately coach it. Copilot was able to produce code that worked with a source type of
EventPipeEventSource, but it did not adequately understand how things would work when the input source type isTraceLog, and I could not adequately coach it because I didn't realize exactly howDynamicTraceEventParserworks with aTraceLogas input. - It will be interesting to keep track of the types of issues that copilot can solve without the user being able to adequately coach it towards a solution - this is certainly one that it could not, and I had to fallback to writing and testing the code myself.
- I tried a new way to coach copilot in this PR, which is to check-in a repro test case that doesn't work and ask it to solve it. It didn't work this time, but it did acknowledge that it hadn't fully fixed it. I intend to try this again with future work.
Looks good. I have a preference for new C# features that reduce the boilerplate on alot of this, but this seems like it should work.
Thanks much! I was thinking I might do an experiment to bump the language version and see what kinds of readability improvements we can make.