Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drop_event does not work in a Metricbeat module filter #2249

Closed
andrewkroh opened this issue Aug 12, 2016 · 7 comments
Closed

drop_event does not work in a Metricbeat module filter #2249

andrewkroh opened this issue Aug 12, 2016 · 7 comments
Assignees

Comments

@andrewkroh
Copy link
Member

  • Version: 5.0.0-alpha5
  • Operating System: Any
  • Steps to Reproduce:
metricbeat:
  modules:
    - module: system
      metricsets: [filesystem]
      period: 15s 
      filters:
        - drop_event:
            when:
              regexp:
                mount_point: '/dev'
output.console:
  pretty: true

When the event is published this is what you get. Notice the null. What should happen is the whole event is dropped.

{
  "@timestamp": "2016-08-11T23:03:34.988Z",
  "beat": {
    "hostname": "myhost",
    "name": "myhost"
  },
  "metricset": {
    "module": "system",
    "name": "filesystem",
    "rtt": 197
  },
  "system": {
    "filesystem": null
  },
  "type": "metricsets"
}
@andrewkroh
Copy link
Member Author

@ruflin Should filters be renamed to processors in Metricbeat? The examples on this page show processors, but it's still filters in the code. https://www.elastic.co/guide/en/beats/metricbeat/current/filtering-and-enhancing-data.html

The examples on that page are wrong. The fields: ['redis.info.memory'] should be fields: ['memory'] due to how/when the filtering is applied. Or do you think we should change when the filtering is applied so that you can write redis.info.memory? Being able to use redis.info.memory is better because if there are two metricsets in the redis module producing a field named memory you can easily choose the correct metricset to filter.

@ruflin
Copy link
Member

ruflin commented Aug 12, 2016

I remember we had the discussion about filters vs processors in the past. I think the conclusion was to keep it filter here and if possible only expose the pure filter capabilities. Perhaps worth discussing again.

For the fields: As it is inside the module config I would expect fields: ['info.memory']. That would also work with multiple Metricsets.

@andrewkroh
Copy link
Member Author

After thinking about it a bit more, the benefit of having module specific filters is that you don't waste CPU cycles running the filter on all events and you don't need as many conditions on your filters since they only apply to that module.

If we require uses to specify the full field name like fields: ['redis.info.memory'], then we have consistent behavior with the global processors which is easier for us to explain and it's less likely to confuse someone IMO. The cost is that users need to include the module name in the field names (a small cost).

If we decide to go with fields: ['redis.info.memory'] then we should rename the module specific filters to processors for more consistency.

@andrewkroh
Copy link
Member Author

I opened PR #2256 to address the drop_event filtering issue. It does not change any of the naming or how the filters are applied within the module.

@ruflin
Copy link
Member

ruflin commented Aug 15, 2016

+1 on useing redis.info.memory as I agree it is very nice to have it consistent.

The reason I'm kind of hesitant to use processors is as I'm not sure yet if all future capabilities of processors will be / should be available already on the module level before the final event was generated. This would also mean that the filters are only a subset of processors and not all functionality is available on the module level.

@andrewkroh
Copy link
Member Author

I just realized that if we changed the filters to be applied post event creation that this would change the behavior of include_fields. If you set include_fields: ["redis.info.memory"] then this would end up dropping the metricset field because it's not one of the mandatory fields.

@ruflin
Copy link
Member

ruflin commented Aug 17, 2016

@andrewkroh Thanks for the fix. We should definitively apply "filters" before the event creation. To still use the full field names we could probably add some magic?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants