bpaquet / node-logstash

Simple logstash implmentation in nodejs : file log collection, sent with zeromq
Other
517 stars 141 forks source link

Consistently respect `only_type` and `only_field_*` params #62

Closed ryepup closed 10 years ago

ryepup commented 10 years ago

This fixes around a bug in BaseComponent.prototype.loadConfig where your config won't be processed unless your filter has some specific configuration.

only_type and only_field_* params are implicitly processed by BaseComponent.prototype.loadConfig, but loadConfig has a "do we need to load config" check at the top of the function that might fail, at which point the implicit options is ignored.

For example, a custom filter filter://my_thing://only_type=syslog. my_thing doesn't need any other parameters, so it's constructor looks like

function FilterMyThing() {
  base_filter.BaseFilter.call(this);
  this.mergeConfig({
    name: 'MyThing'
  });
}

In the previous implementation, BaseComponent.prototype.loadConfig will decide no config needs to be loaded, ignore the only_type param, and my_thing filter will apply to all messages.

This patch removes the if statement, and re-indents. The loadConfig function is only called once when the filter is loaded, so any performance penalty by processing all the options is a one-time cost.

ryepup commented 10 years ago

all tests pass now

bpaquet commented 10 years ago

Fixed, but not with your PR.

Thx you for reporting the bug.