Feature Request: Consider flexible conditions for exclusions

[Skeen]

trafficstars

Currently metrics can be specified to only be applied to specific files, rather than all files, using the path or paths argument, as shown here:

- type: counter
  name: grok_example_lines_total
  help: Counter metric example with labels.
  match: '%{DATE} %{TIME} %{USER:user} %{NUMBER}'
  paths: /tmp/example/app.log
  labels:
    user: '{{.user}}'
    logfile: '{{base .logfile}}'

How would you feel about generalizing this to allowing filtering on any context variable, with a syntax such as:

- type: counter
  name: grok_example_lines_total
  help: Counter metric example with labels.
  match: '%{DATE} %{TIME} %{USER:user} %{NUMBER}'
  conditions:
    - '{{ eq .logfile /tmp/example/app.log }}'
  labels:
    user: '{{.user}}'
    logfile: '{{base .logfile}}'

This would allow for a generic approach to filtering;

- type: counter
  name: admin_errors_total
  help: Counts number of errors seen by admin users.
  match: '%{DATE} %{TIME} %{USER:user} %{NUMBER:log_level}'
  conditions:
    - '{{ eq (base .logfile) app.log }}'
    - '{{ eq (gsub .user ".*admin.*" "admin") admin }}'
    - '{{ gt .log_level 20 }}'
  labels:
    user: '{{.user}}'

Which would say, only count lines originating in files named app.log, with the username containing 'admin', and which are significantly bad.

An alternative to having a list of go templates evaluate to true/false, could be to have a list of regexes matched against a single key:

- type: counter
  name: admin_errors_total
  help: Counts number of errors seen by admin users.
  match: '%{DATE} %{TIME} %{USER:user} %{NUMBER:log_level}'
  conditions:
    logfile: '.*/app.log'
    user: '.*admin.*'
    log_level: '([2-9]\d|[1-9]\d{2,})'
  labels:
    user: '{{.user}}'

This would be less flexible, but might be easier to grasp, it could potentially be made more flexible by allowing the left-hand-side to be go-templates, even thought that might just overly complicate things.

[fstab]

Thanks for this proposal. Is this "just" about improving readability, or is there anything you can do with this approach that you cannot do with the current implementation? I'm wondering if you can achieve the same behavior by adapting match patterns, or if there is anything that you cannot express with the current match configuration?

Not saying that improving readability isn't valuable, I'd just like to make sure I understand this correctly.

[Skeen]

Hi, I originally had two main goals with this proposal.

1. Seperating parse and conditioning
2. Supporting extra context filtering (assuming #95 gets merged)

Seperating parse and conditioning

Assume I want to parse httpd access logs.

In my match I would like to utilize the logstash core pattern for HTTPD_COMMONLOG, however I'm only interested in log lines, from a specific client-ip, method, or similar. So i cannot use the pattern directly (as it would match too much), rather I'll have to replicate the pattern, and then restrict the specific part of it, with the part that I want to restrict.

Now my configuration is less maintainable because it is not clear which part of the pattern is changed, and how.

If however the two were seperate stages, I could utilize a standard grok pattern for the parsing, and do conditioning based on the standard-parsed context.

This however is "just" improving readability and maintainability, it is not increasing the expressiveness of the system, which leads me to argument 2.

Supporting extra context filtering

Assuming #95 gets merged, additional context will be available, and as such the following:

{"message": "Login occured", "user": "Skeen", "ip": "1.1.1.1"}'
{"message": "Login occured", "user": "John", "ip": "8.8.8.8"}'
{"message": "Login occured", "user": "Fabian", "ip": "8.8.4.4"}'
{"message": "Login occured", "user": "Skeen", "ip": "1.1.1.1"}'
...

Could be parsed and filtered like so:

- type: counter
  name: vip_user_logins_total
  help: Counts number of times VIP users have logged in.
  match: 'Login occured'
  conditions:
    - '{{ eq (index .extra "user") "Fabian" "Skeen" }}'
  labels:
    user: '{{ index .extra "user" }}'

Yielding login metrics for only "Fabian" and "Skeen", but not for "John" or any other users, thereby keeping the cardinality of the metric low.

As this is not being parsed and rejected via the match regex, there would not be any other way to reject based upon the extra context, thus this will in fact increase the expressiveness of the system.

Assuming we merge #95, varies other factors could also be utilized for filtering, assuming we utilize the new extra, for instance:

• input which generated the log lines (assuming multiple inputs are supported, say webhook and file(s)), and we for some reason want to only collect for one of them.
• Meta information, such as the length of log lines or parsing times. Assuming we want to collect metrics on sources that produce excessively log lines, or grok patterns that are taking a long time to parse.
• ... The list goes on, and only the imagination sets the limits.

[fstab]

Thanks for elaborating on this. Yes, it sounds like a good idea.

It would be good to first define the configuration format before implementing this.

The following example is the result of just adding a new conditions field, but keeping the current path or paths as it is:

- type: counter
  name: alice_occurrences_total
  help: number of log lines containing alice
  match: 'alice'
  labels:
      logfile: '{{base .logfile}}'
  path: /tmp/example/*.log
  conditions:
    - '{{ eq (index .extra "user") "Fabian" "Skeen" }}'

It looks a bit inconsistent, because path is a kind of condition that's handled outside of the conditions, but I can only think of alternatives that make the configuration more complex, so I would tend to go with this one. What do you think?

[Skeen]

Hi,

I was considering what to do with path / paths, and I had the following ideas:

1. Keep them as is; this maintains backwards compatability, but has two systems in the code for conditions.
2. Keep them in configuration, but as syntactical sugar for conditions: - '{{ eq .logfile /tmp/example/*.log }}' or similar. This maintains backwards compatability, but has only one system in the code for conditions.
3. Same as 2, except we deprecate them and remove them from documentation, but keeps them in the code for backwards compatability.
4. Same as 3, but removing the code and thus not maintaining backwards compatability.

I am torn between 2 and 3, the argument for 2 should be that the current syntax is indeed easier to read, but it will leave two seperate systems for filtering and may be confusing.

Let me know what you think.

[fstab]

Backwards compatibility should not be a problem, because the config files have a version number. We can always keep support for version 3 while we implement new features in version 4. This will not break any existing installations.

However, I'd like to keep grok_exporter as simple as possible for people who just want to get it up and running quickly without learning much about it, and I think the current path(s) are intuitive to understand, and Go template functions may not be so intuitive.

So I think we should keep the path(s) as they are, and implement the conditions independent of that. We can add conditions within the current config version 3, and we can still refactor the configuration file later if it turns out this was a bad idea.

[fstab]

Note: #101 has another use case for the conditions.