search

logstash-plugins / logstash-codec-json

Apache License 2.0
23 stars 29 forks source link

Feat: event target => namespace support (for ECS) #37

Closed kares closed 3 years ago

kares commented 4 years ago

We're introducing a target => ... configuration option for the codec, to aid ECS support. When target isn't set in ECS mode, we do the usual info log message.

The code is relying on our new plugin mixin to provide event_factory.new_event support.

Performance numbers were posted on https://github.com/logstash-plugins/logstash-mixin-event_support/pull/2

jsvd commented 3 years ago

One thing I noted that is outside the scope of this change but is related to the mixins, is that any cloning of the plugin parameters causes ECS-related logging to be printed. Also, at least for codecs, the loggin entries are printed without the codec id, which make debugging difficult.

Example:

pipelines.yml

- pipeline.id: test
  pipeline.workers: 1
  pipeline.batch.size: 1
  config.string: "input { generator { threads=> 1 codec => json {  ecs_compatibility => \"v1\"} message => '{\"message\": \"hi\", \"data\": {\"num\": 1 } }' count => 1 } } output { stdout { } }"

Starting logstash with bin/logstash -r and then tweaking something in the pipeline and causing it to reload the user will see the following logging:

[2021-06-29T10:44:30,136][INFO ][logstash.codecs.json     ] ECS compatibility is enabled but `target` option was not specified. This may cause fields to be set at the top-level of the event where they are likely to clash with the Elastic Common Schema. It is recommended to set the `target` option to avoid potential schema conflicts (if your data is ECS compliant or non-conflicting, feel free to ignore this message)
[2021-06-29T10:44:30,143][INFO ][logstash.codecs.json     ] ECS compatibility is enabled but `target` option was not specified. This may cause fields to be set at the top-level of the event where they are likely to clash with the Elastic Common Schema. It is recommended to set the `target` option to avoid potential schema conflicts (if your data is ECS compliant or non-conflicting, feel free to ignore this message)
[2021-06-29T10:44:30,161][INFO ][logstash.pipelineaction.reload] Reloading pipeline {"pipeline.id"=>:test}
[2021-06-29T10:44:30,317][INFO ][logstash.codecs.json     ] ECS compatibility is enabled but `target` option was not specified. This may cause fields to be set at the top-level of the event where they are likely to clash with the Elastic Common Schema. It is recommended to set the `target` option to avoid potential schema conflicts (if your data is ECS compliant or non-conflicting, feel free to ignore this message)
[2021-06-29T10:44:30,324][INFO ][logstash.codecs.json     ] ECS compatibility is enabled but `target` option was not specified. This may cause fields to be set at the top-level of the event where they are likely to clash with the Elastic Common Schema. It is recommended to set the `target` option to avoid potential schema conflicts (if your data is ECS compliant or non-conflicting, feel free to ignore this message)

So, for 1 codec each reload prints the warning 4 times without any codec instance information.

kares commented 3 years ago

Thank you so much for the detailed review(s) :bow:

So, for 1 codec each reload prints the warning 4 times without any codec instance information.

Opened https://github.com/logstash-plugins/logstash-mixin-ecs_compatibility_support/issues/7 (we might consider moving the check from register into initialize). The user experience with those logs is very poor, so I would rather hold off shipping this with 7.14.