Skip to content

Make splunk to use record host field#276

Closed
lukesteensen wants to merge 1 commit intomasterfrom
fix-splunk-host
Closed

Make splunk to use record host field#276
lukesteensen wants to merge 1 commit intomasterfrom
fix-splunk-host

Conversation

@lukesteensen
Copy link
Copy Markdown
Member

This fix ended up having some behavior consequences, hence the separate PR from #269.

The end result here is that the splunk sink will look first for the host_field configuration, then any explicitly set "host" field in the structured data, and then fall back the the built in host field on the record. It will also fall back in the case that host_field is set but these is no value in that field.

Does that sound sane to everyone? /cc @binarylogic

Something else we might want to do is a less case-sensitive lookup for the default "host" field.

@lukesteensen lukesteensen added sink: splunk_hec Anything `splunk_hec` sink related meta: regression This issue represents a regression labels Apr 11, 2019
@lukesteensen lukesteensen mentioned this pull request Apr 11, 2019
@michaelfairley
Copy link
Copy Markdown
Contributor

michaelfairley commented Apr 11, 2019

I'm not convinced that moving the host field back onto record was the right call:

  1. All of the arguments for not having a special cased field still stand. (This PR demonstrates some of the awkwardness that special cases cause.)
  2. It was done to micro-optimize a very specific benchmark, without addressing all of the other performance issues that that hashmap has in other sources and transforms.
  3. There's now no way to prevent the host field from being sent to ES (or to other sinks that straight json encode a record).

@binarylogic
Copy link
Copy Markdown
Contributor

Just noting, this will probably be superseded by the work done in #295, which @michaelfairley is scheduled to do shortly.

@binarylogic
Copy link
Copy Markdown
Contributor

Given that #308 was merged, do we want to close this?

@lukesteensen
Copy link
Copy Markdown
Member Author

Yep!

@binarylogic binarylogic deleted the fix-splunk-host branch July 1, 2019 05:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

meta: regression This issue represents a regression sink: splunk_hec Anything `splunk_hec` sink related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants