Skip to content
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

Host field is unconditionally overwritten #61

Closed
magnusbaeck opened this issue Sep 25, 2016 · 6 comments
Closed

Host field is unconditionally overwritten #61

magnusbaeck opened this issue Sep 25, 2016 · 6 comments

Comments

@magnusbaeck
Copy link

It seems the http input unconditionally saves the remote host into the host field after the codecs are applied. This is fine with e.g. a plain codec but problematic with e.g. a json codec where the event payload might contain a host field itself.

I suggest the http input starts behaving like e.g. the tcp input, which only populates the host field if it's unset. Quoting tcp.rb:

event.set(HOST_FIELD, client_address) unless event.get(HOST_FIELD)

(One could argue that event.get(HOST_FIELD) should rather be event.include?(HOST_FIELD) to deal with the corner case that the host field is set to a non-truthy value.)

@jsvd
Copy link
Member

jsvd commented Sep 26, 2016

+1, what we can also do is have a target property that defaults the metadata setting to the root of the event. Either that or an override_metadata that defaults to true

@jdmac87
Copy link

jdmac87 commented Sep 26, 2016

+1

@nlowe
Copy link

nlowe commented Jul 28, 2017

Is there a workaround for this? I've been experiencing this while trying to setup a centralized logging solution for a few of my applications.

@jsvd
Copy link
Member

jsvd commented Jul 28, 2017

what do y'all think of #68?

this PR keeps the current behaviour but adds two configuration options (with terrible names, I accept suggestions) that allow you to override the default target for both remote host and request headers information.

@ebuildy
Copy link
Contributor

ebuildy commented May 9, 2018

let's merge it !!!

@jsvd
Copy link
Member

jsvd commented May 11, 2018

this has been solved by #68

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants