On Sun, Jul 24, 2011 at 05:00:46PM -0500, Michael Parker wrote: > > On Jul 24, 2011, at 2:55 PM, John Hardin wrote: > > > On Sun, 24 Jul 2011, Michael Parker wrote: > > > >> On Jul 24, 2011, at 12:04 PM, John Hardin wrote: > >> > >>> On Sun, 24 Jul 2011, Michael Parker wrote: > >>> > >>>> How is this change different from what is provided in the > >>>> HitFreqsRuleTiming plugin? > >>> > >>> The plugin is a lot heavier-weight and provides a lot more analysis than > >>> my change. I feel my change would be a better way for an end-user to > >>> quickly isolate a poorly-performing rule by enabling a single channel in > >>> an interactive debug run without affecting the production install, while > >>> the plugin provides more in-depth analysis for rule developers. > >>> > >>> I believe that both have a place, and I'd suggest my change is indeed > >>> justified. > >> > >> I would rather see your change implemented as a plugin, that way people > >> can include things if they would like. > > > > By that logic all of the debug channels should be implemented instead as > > plugins. > > > > My change gives very basic elapsed time stats only if a specific debugging > > channel is enabled. I view it as equivalent to the "rules" debug channel > > logging the text that the rule matched. This sort of thing will be > > regularly used by many end users when interactively troubleshooting > > performance problems, and does not require any changes to the production > > system's configuration (such as enabling a new plugin). > > It does however further complicate an already over complicated piece of code. > The ran_rule plugin hook was added for exactly this use case: > > https://issues.apache.org/SpamAssassin/show_bug.cgi?id=4517 > > As someone who has to delve into this code often enough to get sweaty palms > and the shakes when edits are mentioned I propose we stick with the plugin > call.
Checking rule timings should not be a task that requires lot of effort. I don't want to go enabling and disabling plugins each time for a simple task. I'm all +1 for an internal debug channel to see those timings. Why isn't this discussed in a bug so we can have a proper vote and code review?
