Is there any way to restructure WriteToAllListeners (both overloads) / DispatchToAllListeners such that we can delay creating/populating the EventWrittenEventArgs until / if it's actually needed the first time? As it stands, it seems like we're creating the fairly-expensive per-event object even if it'll never be used.

I just took a second look and yes, I think I should be able to by querying whether any of the listeners have the events enabled before calling WriteToAllListeners from WriteImpl - this would mean that in the case it is actually enabled by an instance of listener, it needs incur a little bit of more perf cost (since needs to filter it again at the DispatchToAllListeners) but I think it still makes sense to do what you said for the sake of pay-for-play. I'll give that a try. Thanks Stephen!

We already have a check "event[eventId].enabledForAnyListener" that precedes the object construction.

Similar to above, if these filters aren't working it would be good to understand why not and if they should be fixed vs. creating a second filtering implementation.

We already have a check "event[eventId].enabledForAnyListener" that precedes the object construction.

Similar to above, if these filters aren't working it would be good to understand why not and if they should be fixed vs. creating a second filtering implementation.

This logic still has some trouble. One example would be m_level =Informational, m_keywords=0x0, level = verbose, keywords=0x0. Expected result: false (listener level is too low), Actual result: true.

You probably should be calling EventSource.IsEnabledCommon() except reviewing that code it also appears to have bugs here and here : (. EventPipe native code has a correct implementation though the special case in the levelEnabled check for LogAlways is unnecessary complexity. LogAlways is the lowest value so sessionLevel >= LogAlways is always true.

The fact that we've got all these all these filtering bugs lurking probably means we are in need of EventListener level/keyword filtering tests for both the self-describing case and the manifested events.

I think you could fix IsEnabledCommon() to look like this:

        private static bool IsEnabledCommon(bool enabled, EventLevel currentLevel, EventKeywords currentMatchAnyKeyword,
                                                          EventLevel eventLevel, EventKeywords eventKeywords, EventChannel eventChannel)
        {
            if (!enabled)
                return false;

            // does is pass the level test?
            if (currentLevel < eventLevel)
                return false;

            // if yes, does it pass the keywords test?
            if (eventKeywords != 0)
            {
#if FEATURE_MANAGED_ETW_CHANNELS
                // is there a channel with keywords that match currentMatchAnyKeyword?
                if (eventChannel != EventChannel.None && this.m_channelData != null && this.m_channelData.Length > (int)eventChannel)
                {
                    EventKeywords channel_keywords = unchecked((EventKeywords)(m_channelData[(int)eventChannel] | (ulong)eventKeywords));
                    if (channel_keywords != 0 && (channel_keywords & currentMatchAnyKeyword) == 0)
                        return false;
                }
                else
#endif
                {
                    if ((unchecked((ulong)eventKeywords & (ulong)currentMatchAnyKeyword)) == 0)
                        return false;
                }
            }
            return true;
        }

Once you had that you could invoke it with Channel.None to get the right filtering behavior.

I also notice the code in WriteStringToAllListeners() has a suspicious loop over m_EventEnabled[] that tries to infer if the listener is enabled or not. I could imagine that code may also not be doing the right thing for self-describing events? It would worth testing as part of the filtering tests.

I think its legal to create an EventSource using manifested types and then call Write<T>(...) to do one-off serialization of self-describing events. In this case SelfDescribingEvents would be false, but it would still be necessary to track level/keywords.

(If you are curious I only spotted this because the code above that set level and keywords didn't check SelfDescribingEvents and I noticed the invariant wasn't consistent)