ECS compatibility #165

yaauie · 2021-01-18T06:05:25Z

Implements ECS Compatibility mode, supporting disabled (legacy), opt-in v1 (breaking), and an alias from v8 to v1.

In ECS mode, we add tcp connection metadata in a variety of descriptive fields under [@metadata][tcp], instead of the top-level fields that clash with ECS. Users can copy these metadata fields to their events if and when necessary, which allows us to avoid polluting an event with data that may or may-not align with ECS.

resolves #146

lib/logstash/inputs/tcp/chooser.rb

andsel

I've left a question on testing part plus a doubt:

should we also set the ecs.version as described in guidelines ?

spec/inputs/tcp_spec.rb

yaauie · 2021-06-22T18:53:12Z

should we also set the ecs.version as described in guidelines

Unfortunately, we don't have minor/patch versions available to us, so we can't comply :/

kares

👍

do have some suggestion on meta-data field names

kares · 2021-06-23T04:39:08Z

lib/logstash/inputs/tcp/decoder_impl.rb

@@ -1,7 +1,7 @@
 # encoding: utf-8
 require 'java'

-class DecoderImpl
+class LogStash::Inputs::Tcp::DecoderImpl


kares · 2021-06-23T05:17:46Z

lib/logstash/inputs/tcp.rb

    decorate(event)
    @output_queue << event
  end

+  # setup the field names, with respect to ECS compatibility.
+  def setup_fields!
+    @field_host       = ecs_select[disabled: "host",                    v1: "[@metadata][tcp][source][name]"        ].freeze


what about using source.address instead
so the underlying structure could be copied to a top-level and would be ecs-compliant.

There are a few reasons that make wholesale copy of this source metadata impractical.

While source and destination fields do have an address, ip, and port, they are used to describe network transactions captured in an event. Similarly, client and server have all three fields, but are used to describe the initiator or recipient of the network transaction.

While the name and ip of the event's source may make sense to copy into host or observer, the port is pretty meaningless and doesn't have a home there. When this plugin is run in server mode, the outbound port from the client is typically chosen at random by its host OS, and when the plugin is run in client mode, the port we are connecting to doesn't really describe the event.

I chose name here because it aligns with both host and observer.

We could align with the CEF codec, which has a new device option that accepts host or observer, and could populate the relevant bits if this is set?

thanks for sharing the details - fine by me to keep as is - prefixed source.name

kares · 2021-06-23T05:24:28Z

lib/logstash/inputs/tcp.rb

+    @field_host       = ecs_select[disabled: "host",                    v1: "[@metadata][tcp][source][name]"        ].freeze
+    @field_host_ip    = ecs_select[disabled: "[@metadata][ip_address]", v1: "[@metadata][tcp][source][ip]"          ].freeze
+    @field_port       = ecs_select[disabled: "port",                    v1: "[@metadata][tcp][source][port]"        ].freeze
+    @field_proxy_host = ecs_select[disabled: "proxy_host",              v1: "[@metadata][tcp][proxy][ip]"           ].freeze


guess we do not have a better (ecs-like) names for proxy.host and proxy.ip here ...

lib/logstash/inputs/tcp.rb

andsel

LGTM left only a few nitpicks that you are free to skip

spec/inputs/tcp_spec.rb

Also fixes a missing ip address of source when a TCP Proxy is present.

CHANGELOG.md

kares

🍦

karenzone

Suggestions inline for your consideration.

docs/index.asciidoc

karenzone · 2021-06-24T20:00:44Z

docs/index.asciidoc

+The value of this setting affects the placement of a TCP connection's metadata on events:
+
+.Source Metadata Location by `ecs_compatibility` value


Suggested change

The value of this setting affects the placement of a TCP connection's metadata on events:

.Source Metadata Location by `ecs_compatibility` value

The value of this setting affects the placement of a TCP connection's metadata on events.

On other plugins we've standardized putting the metadata table in the ECS metadata section. There's a compelling argument for either location. If you don't have strong feelings, perhaps we could move this under the ECS metadata section.

Nice job on the table. It renders quite nicely.

I've moved the table, and re-worded the relevant links/intros.

docs/index.asciidoc

Co-authored-by: Karol Bucek <karol.bucek@elastic.co> Co-authored-by: Karen Metts <karen.metts@elastic.co>

karenzone

Suggestion to removed numbered table title. Otherwise, LGTM! 🎉

docs/index.asciidoc

remove numbered table header Co-authored-by: Karen Metts <35154725+karenzone@users.noreply.github.com>

yaauie requested a review from andsel January 18, 2021 06:05

kares reviewed Jan 18, 2021

View reviewed changes

lib/logstash/inputs/tcp/chooser.rb Outdated Show resolved Hide resolved

andsel reviewed Jan 18, 2021

View reviewed changes

spec/inputs/tcp_spec.rb Outdated Show resolved Hide resolved

andsel mentioned this pull request Jan 20, 2021

Ecs compatibility logstash-plugins/logstash-input-beats#404

Merged

roaksoax added the status:needs-review label Apr 6, 2021

yaauie added 2 commits June 22, 2021 17:32

noop: avoid polluting top-level namespace

5484e2d

noop: make field names per-instance (not global)

28e85bf

yaauie force-pushed the ecs-compatibility branch from dda27f7 to 28e85bf Compare June 22, 2021 18:49

yaauie force-pushed the ecs-compatibility branch 2 times, most recently from 0fdff63 to 9bc1c4e Compare June 22, 2021 19:27

yaauie requested review from kares and andsel June 22, 2021 19:38

kares reviewed Jun 23, 2021

View reviewed changes

andsel approved these changes Jun 23, 2021

View reviewed changes

spec/inputs/tcp_spec.rb Outdated Show resolved Hide resolved

spec/inputs/tcp_spec.rb Outdated Show resolved Hide resolved

spec/inputs/tcp_spec.rb Outdated Show resolved Hide resolved

roaksoax added status:approved and removed status:needs-review labels Jun 23, 2021

implement ecs_compatibility mode

4b53619

Also fixes a missing ip address of source when a TCP Proxy is present.

yaauie force-pushed the ecs-compatibility branch from 9bc1c4e to 4b53619 Compare June 23, 2021 21:39

yaauie requested a review from karenzone June 23, 2021 21:40

kares reviewed Jun 24, 2021

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

kares approved these changes Jun 24, 2021

View reviewed changes

karenzone reviewed Jun 24, 2021

View reviewed changes

docs: move ecs table up to ecs_metadata section, other minor fixes

fe8ec28

Co-authored-by: Karol Bucek <karol.bucek@elastic.co> Co-authored-by: Karen Metts <karen.metts@elastic.co>

yaauie force-pushed the ecs-compatibility branch from 9e09de5 to fe8ec28 Compare June 27, 2021 15:01

yaauie requested a review from karenzone June 28, 2021 01:36

karenzone approved these changes Jun 28, 2021

View reviewed changes

docs/index.asciidoc Outdated Show resolved Hide resolved

Update docs/index.asciidoc

d250cf2

remove numbered table header Co-authored-by: Karen Metts <35154725+karenzone@users.noreply.github.com>

yaauie merged commit a833fa6 into logstash-plugins:master Jun 28, 2021

yaauie mentioned this pull request Jul 28, 2021

Implement ECS-Compatibility Mode #149

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ECS compatibility #165

ECS compatibility #165

yaauie commented Jan 18, 2021 •

edited

Loading

andsel left a comment

yaauie commented Jun 22, 2021

kares left a comment

kares Jun 23, 2021

kares Jun 23, 2021

yaauie Jun 23, 2021

kares Jun 24, 2021

kares Jun 23, 2021

andsel left a comment

kares left a comment

karenzone left a comment

karenzone Jun 24, 2021

karenzone Jun 24, 2021

karenzone Jun 24, 2021

yaauie Jun 27, 2021

karenzone left a comment

		The value of this setting affects the placement of a TCP connection's metadata on events:

		.Source Metadata Location by `ecs_compatibility` value

ECS compatibility #165

ECS compatibility #165

Conversation

yaauie commented Jan 18, 2021 • edited Loading

andsel left a comment

Choose a reason for hiding this comment

yaauie commented Jun 22, 2021

kares left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andsel left a comment

Choose a reason for hiding this comment

kares left a comment

Choose a reason for hiding this comment

karenzone left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karenzone left a comment

Choose a reason for hiding this comment

yaauie commented Jan 18, 2021 •

edited

Loading