New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#78 changed behaviour of host field and added new ip_address field #79

Merged
merged 1 commit into from Jul 13, 2017

Conversation

Projects
None yet
4 participants
@original-brownbear
Contributor

original-brownbear commented Jul 13, 2017

fixes #78

  • host field now does the reverse lookup
  • ip_address now contains the IP of the peer or is empty for proxy connections (is the behavior for proxy ok here?)
  • bumped to 4.3.0 since this is a behavior change and not a patch
@original-brownbear

This comment has been minimized.

Contributor

original-brownbear commented Jul 13, 2017

@suyograo

Naming change needs to match beats.

@@ -107,6 +107,7 @@ class LogStash::Inputs::Tcp < LogStash::Inputs::Base
config :ssl_extra_chain_certs, :validate => :array, :default => []
HOST_FIELD = "host".freeze
HOST_IP_FIELD = "ip_address".freeze

This comment has been minimized.

@suyograo

suyograo Jul 13, 2017

Contributor

this needs to be "[@metdata][ip_address]" to match beats convention

This comment has been minimized.

@original-brownbear

original-brownbear Jul 13, 2017

Contributor

@suyograo ah damn one level of nesting slipped by me :D sec fixing :)

@original-brownbear

This comment has been minimized.

Contributor

original-brownbear commented Jul 13, 2017

@suyograo fixed :)

@suyograo

This comment has been minimized.

Contributor

suyograo commented Jul 13, 2017

@original-brownbear the version bump needs to be 5.0.0 since it is a breaking change w.r.t. field value + mapping.

LGTM

@original-brownbear

This comment has been minimized.

Contributor

original-brownbear commented Jul 13, 2017

@suyograo right, changing version and merging then, thanks!

@jakelandis

This comment has been minimized.

Contributor

jakelandis commented Jul 13, 2017

@original-brownbear - we will also to pin LS 5.x to TCP input 4.x

@original-brownbear original-brownbear merged commit f382056 into logstash-plugins:master Jul 13, 2017

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
CLA Commit author has signed the CLA
Details

@original-brownbear original-brownbear deleted the original-brownbear:78 branch Jul 13, 2017

@jakelandis

This comment has been minimized.

Contributor

jakelandis commented Jul 13, 2017

did a quick test... and all looks great !
(link is the name of my desktop)

{
    "@timestamp" => 2017-07-13T14:02:41.678Z,
          "port" => 42458,
      "@version" => "1",
          "host" => "link",
      "@metdata" => {
        "ip_address" => "10.0.0.99"
    },
       "message" => "from link"
}
{
    "@timestamp" => 2017-07-13T14:03:13.209Z,
          "port" => 61054,
      "@version" => "1",
          "host" => "localhost",
      "@metdata" => {
        "ip_address" => "0:0:0:0:0:0:0:1"
    },
       "message" => "from localhost"
}
@original-brownbear

This comment has been minimized.

Contributor

original-brownbear commented Jul 13, 2017

@jakelandis nice, and thanks for checking!

@@ -107,6 +107,7 @@ class LogStash::Inputs::Tcp < LogStash::Inputs::Base
config :ssl_extra_chain_certs, :validate => :array, :default => []
HOST_FIELD = "host".freeze
HOST_IP_FIELD = "[@metdata][ip_address]".freeze

This comment has been minimized.

@redbaron4

redbaron4 Jan 1, 2018

This should be "[@metadata]". The "a" is missing.

This comment has been minimized.

@original-brownbear

original-brownbear Jan 1, 2018

Contributor

@redbaron4 oops thanks for spotting this! Will fix this within the next 2 days once I'm back from vacation.

This comment has been minimized.

@redbaron4

redbaron4 Jan 2, 2018

@original-brownbear Looks like this has been fixed in the master (though not yet released)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment