magnusbaeck / logstash-filter-verifier

Apache License 2.0
192 stars 27 forks source link

temp pipeline configurations broken by large integers #104

Closed Amorik closed 3 years ago

Amorik commented 3 years ago

Logstash-filter-verifer:

Operating System:

Logstash:

When setting large integers in Logstash configurations Logstash will represent them as exponent values (ie 2000000 becomes 2e6). This is typically fine as its all back end processing by Logstash, except in LFV's case when copying/cloning the respective configurations into its designated /tmp directory. LFVs copy function(s) appear to replace the numbers in the configuration with the string exponent equivalent. The problem now is that Logstash, despite some sources saying otherwise, does not support exponent notation in place of integers; As a result Logstash crashes due to parsing errors.

NOTE: The same errors can be seen when running Logstash without LFV when using exponent substitution.




Filter Test configuration

{
    "testcases": [
        {
            "description": "Simple Number Test",
            "input": [
                "placeholder"
            ],
            "expected": [
                {}
            ]
        }
    ]
}

Filter configuration (working)

filter {
    mutate { add_field => { "test.123" => 2000 } }
}

Filter configuration (broken)

filter {
    mutate { add_field => { "test.123" => 2000000} }
}

tmp Filter configuration (working)

~:/tmp$ find /tmp/ -name filter.conf -exec cat {} \;
filter {
  mutate {
    add_field => {
      "test.123" => 2000
    }
  }
}

tmp Filter Configuration (broken)

~:/tmp$ find ./ -name filter.conf -exec cat {} \;
filter {
  mutate {
    add_field => {
      "test.123" => 2e+06
    }
  }
}

Output (working)

~:/d/lfv-simple-test$ logstash-filter-verifier --logstash-path=/d/logstash/logstash-7.9.3/bin/logstash --loglevel=DEBUG test.json filter.conf
2021/03/25 13:56:03 Reading test case file: test.json (/d/lfv-simple-test/test.json)
2021/03/25 13:56:03 Current TestCaseSet after converting fields: {File: Codec:line IgnoredFields:[@version] InputFields:map[] InputLines:[placeholder] ExpectedEvents:[map[]] TestCases:[{InputLines:[placeholder] ExpectedEvents:[map[]] Description:Simple Number Test}] descriptions:[Simple Number Test]}
2021/03/25 13:56:03 Logstash path candidate accepted: /d/logstash/logstash-7.9.3/bin/logstash
2021/03/25 13:56:03 Preparing configuration file directory /tmp/169531170/pipeline.d with these files: [filter.conf]
Running tests in test.json...
2021/03/25 13:56:03 Starting "/d/logstash/logstash-7.9.3/bin/logstash" with args ["-w" "1" "--debug" "-f" "/tmp/169531170/pipeline.d" "-b" "1" "-l" "/tmp/169531170/log" "--path.settings" "/tmp/169531170/config" "--path.data" "/tmp/169531170/data"].
2021/03/25 13:56:03 Waiting for child with pid 12646 to terminate.
☐ Comparing message 1 of 1 (Simple Number Test) from test.json:
--- /tmp/910917555/test.json/1/expected 2021-03-25 13:56:33.760787800 -0400
+++ /tmp/910917555/test.json/1/actual   2021-03-25 13:56:33.759020000 -0400
@@ -1 +1,6 @@
-{}
+{
+  "@timestamp": "2021-03-25T17:56:32.118Z",
+  "host": "CA-L4DCS9Y2",
+  "message": "placeholder",
+  "test.123": "2000"
+}

Summary: ☐ All tests: 0/1
         ☐ test.json: 0/1

Output (broken)

see debug log attached

lfv-bignumbers-test.debug.txt

breml commented 3 years ago

Hi @Amorik

Thanks for your detailed bug report. Thanks to all the details you provided, I was quickly able to find the root cause for this issue.

The problem is not within LFV but in the logstash-config package (which is maintained by myself), which is used by LFV to parse and process Logstash config files. I was able to reproduce the issue in a test case and I will try to fix this issue as soon as possible.

breml commented 3 years ago

@magnusbaeck I fixed logstash-config and I have a branch (https://github.com/breml/logstash-filter-verifier/tree/bugfix-104) with the updated go.mod file to reference the fixed version of logstash-config. But we do not yet have a maintenance branch for LFV v1.x, so I can not create the pull request.

magnusbaeck commented 3 years ago

@breml I've created a maint-1.x branch based on the commit prior to the one that started with the v2 refactoring so you should be all set for a PR. Thanks!

magnusbaeck commented 3 years ago

A PR that fixes the problem has been merged now. I'll issue a 1.6.3 release tomorrow.