Good questions.

A few notes:

1. It should be *extremely* rare for there to ever be a 10,000 event report.  The 90% case should be zero events.
2. Since the *vast* majority of events will not match the filter criteria, there would be very few cases where attempting to perform the filter earlier would actually allow us to avoid the concat operation or the hash build-up.
3. There will be one extra iteration over the event list to apply the filter, so that is a fair point.
4. The current design was largely influenced by ease of testing.  It's much easier to test and reason about if the filtering all happens in one place, rather than in two disparate places inside of branches of a conditional.
5. If we do version check it, it would be for Puppet < 3.3.x because they've fixed it in 3.3.x.  However, after chatting with the OSS team about it, it sounds like the change in behavior was a side effect of another change and they're not entirely certain that the behavior won't be re-introduced later; so, if we version check, then we risk having to do another release to blacklist more version numbers.

All of that said, my vote is to avoid premature optimization here.  Especially since the 90% case is that there will be zero (unfiltered) events.

Your points and questions are totally valid, though, so I'm happy to look into re-working it if you feel strongly about it.