Custom HTTP headers for the Elasticsearch output

[tsg]

Configuration looks like this:

output.elasticsearch.headers:
  X-My-Header: Contents of the header

To use from the CLI:

metricbeat -E "output.elasticsearch.headers.X-test=Test value"

It's not possible to set the same header name more than once with different values,
but it is possible to separate header values with a comma, which has the same meaning
as per the RFC.

Closes #1768.

[tsg]

@hub-cap @alexbrasetvik @joegallo Can you have a quick look at this one to see if it seems to do what you need/expect?

[joegallo]

That seems workable to me. If you can get me a build I can even drop it into things on my end and try it out.

[urso]

@tsg seems this PR contains some more changes then just HTTP headers.

Have you considered this solution?

output.elasticsearch.headers:
- X-My-Header: Contents of the header

[joegallo]

I like @urso's suggestion -- there's some alignment with how fields are defined, then.

[tsg]

@joegallo @urso that's an array of dictionaries, right? So not like the fields :)

For @urso's version, I worry that people are going to mess up the syntax. I considered making it simply a dictionary, but it gets more complex when you want to provide the same header twice.

[tsg]

seems this PR contains some more changes then just HTTP headers.

That's because it needs a rebase on top of #3394. Have a look at the second commit only for this change. Or did you mean something else?

[joegallo]

I don't really know yaml that well, so I'm not sure what things are called, but what I was getting was that there's a little bit of symmetry between the configs if they were like this:

fields:
  my_field_1: "some value"
  my_field_2: "some other value"

output.elasticsearch.headers:
  X-My-Header: "blah"
  X-Something: "blah blah"

But yeah, I haven't thought about the implications for multiple values for the same header, and I don't really know the ins and outs of what the above would mean for yaml.

I definitely wouldn't say my preference is at all strong, and I'm happy to defer to you.

[ruflin]

An alternative option here would be:

output.elasticsearch.headers:
- `X-My-Header: Contents of the header`

So the header would be string value. This would allow to set multiple headers with the same name. It is allowed under certain circumstances (https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2) but not sure if that is a real use case. Also it would help in case there are some special chars in the field name which are not compatible with yaml, but not sure if this can happen.

Currently I like the one proposed by @tsg best because it is the most expressive and extensible one.

[tsg]

@joegallo Yeah, the only issue with the your proposed version, which I agree is the cleanest, is the multiple values for the same header case. I know that's probably not even useful in practice, but it feels wrong not to support it.

[urso]

@tsg in heartbeat headers can already be configured using the dictionary approach: https://github.com/elastic/beats/blob/master/heartbeat/monitors/active/http/config.go#L42

Having a configurable HTTP client in multiple places in beats (elasticsearch output, heartbeat, metricbeat) we should consider refactoring the HTTP client abstracting in order to provide a common and consistent configuration experience.

[tsg]

Rebased to master.

@urso +1 on unifying the client handling, but my understanding is that the cloud team is waiting for this feature, so I wouldn't block it on that. If we plan to make it generic, it's even more important that we support the case of multiple headers with the same name, right?

[alexbrasetvik]

No strong opinions on duplicate headers here. We don't need it, I see mentions of contrived Set-Cookie and Accept examples on the interwebs for it. We'd like to not wait for a full rewrite of the http client handling, though. :)

[tsg]

We discussed it again and decided to go for the simplest version for the user:

output.elasticsearch.headers:
  X-My-Header: Contents of the header

and from the CLI:

metricbeat -E "output.elasticsearch.headers.X-test=Test value"

The reason is that if someone needs multiple values for a single header, they can separate them with commas, which per the RFC should have the same meaning for the server.

The PR and description is updated to reflect the above.

[monicasarbu]

Maybe mention somewhere in the config file that you can pass multiple values separated by comma.

[tsg]

I added a line in the configuration.

[monicasarbu]

s/Contents of the header/Header contents

[urso]

checking HTTP module, using commas should work according to RFC. Headers are represented as map[string][]string. Every array entry is a new line, but there doesn't seem to be any kind of escaping on commas: https://github.com/golang/go/blob/master/src/net/http/header.go#L147