Convert fields at grok match

[z0rc]

Instead of doing dedicated mutate convert action, we can convert known fields to integer or float right in grok match.

Ref: https://www.elastic.co/guide/en/logstash/5.4/plugins-filters-grok.html#_grok_basics

Optionally you can add a data type conversion to your grok pattern. By default all semantics are saved as strings. If you wish to convert a semantic’s data type, for example change a string to an integer then suffix it with the target data type. For example %{NUMBER:num:int} which converts the num semantic from a string to an integer. Currently the only supported conversions are int and float.

[z0rc]

Interesting, I'll check what's the case with test failures.

[whyscream]

You got what you asked for :)

  1) Failure:
TestGrokPatterns#test_postscreen_0013 [test/test.rb:56]:
Missing expected data in field 'postfix_postscreen_violation_time'.
Expected [0.0] to include "0".

The list of results, which is a single float 0.0, does not contain the string 0.

[z0rc]

Well, yeah. I need to update expected values in fixtures, as grok does conversion in place with this change.

[z0rc]

@whyscream test runner explicitly converts expected data to string here. I'm not sure how to address this without updating the ton of other fixtures, if to_s conversion is removed.

I'm leaving this for your judgement.

[whyscream]

The conversion in the test was probably added to work around this issue (reversed) with upstream patterns that are producing integers or floats. You changed quite a lot of result fields, so I'd expect that quite some tests will need updates too. I have no problems with changing the test suite in a way that causes the tests to detect proper data types too.

The explicit conversion you're removing takes some extra cycles I assume, but does it cause that much additional load that your optimization actually helps? Just curious on how you got the idea for the proposed change.

[z0rc]

The explicit conversion you're removing takes some extra cycles I assume, but does it cause that much additional load that your optimization actually helps? Just curious on how you got the idea for the proposed change.

It's habit I developed over time writing custom grok patterns. It's easier and less error prone this way. For example, the removed mutate filter references fields that aren't present in grok template anymore, like postfix_uid.

As of test suite, I'll see what can be done with type conventions. Seems it's worth to refactor it to allow proper type matching.

[z0rc]

I tried to remove to_s conversion for expected data and work through failing tests. This removal fixes couple of test cases, but introduces a bunch of new failures because of jls-grok bug.

The issue is jordansissel/ruby-grok#29.

Again, I'm not sure how to proceed from here as testing suite is FUBAR. Maybe switching to tests using logstash way is better here, ref: https://github.com/logstash-plugins/logstash-patterns-core/tree/master/spec.

[whyscream]

Thanks for trying!
I looked into it myself too, and am coming to the same conclusion. I guess that the patterns with type coercion appended won't work reliable inside logstash either, so without changes to thet grok library this will never work.

For now I'm leaving the type coercion as-is, when the grok library is fixed, we can revisit this issue, which looks like a nice improvement overall.

[z0rc]

Okay, I'm closing this for now and will track issue with jls-grok.

Will create a new PR when mentioned issue resolved.