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. Michael > > The HitFreqsRuleTiming plugin performs more complex analysis, and that level > of complexity - which will only be used by a subset of users, typically rules > developers when engaged in rule performance tuning or masschecks - is, I > agree, properly placed in a plugin. > > As I said, I believe that both have a place. > > Does anyone else have an objection to this change being in the base code > rather than a plugin? > > -- > John Hardin KA7OHZ http://www.impsec.org/~jhardin/ > [email protected] FALaholic #11174 pgpk -a [email protected] > key: 0xB8732E79 -- 2D8C 34F4 6411 F507 136C AF76 D822 E6E6 B873 2E79 > ----------------------------------------------------------------------- > If "healthcare is a Right" means that the government is obligated > to provide the people with hospitals, physicians, treatments and > medications at low or no cost, then the right to free speech means > the government is obligated to provide the people with printing > presses and public address systems, the right to freedom of > religion means the government is obligated to build churches for the > people, and the right to keep and bear arms means the government is > obligated to provide the people with guns, all at low or no cost. > ----------------------------------------------------------------------- > 227 days since the first successful private orbital launch (SpaceX)
