Adding option in client to specify extra headers #276
Conversation
| @@ -209,5 +209,9 @@ func (client *Client) createRequest(method, api string, reqbody interface{}) (*h | |||
| if bodyReader != nil { | |||
| req.Header.Add("Content-Type", "application/json") | |||
| } | |||
| for k, v := range client.ExtraHeader { | |||
| req.Header.Add(k, v) | |||
There was a problem hiding this comment.
do we need to replace if a user specifies a header that is already set?
There was a problem hiding this comment.
It looks like they get appended using curl, I would follow this implementation in case for whatever reason the same key can be used multiple times.
➜ ~ curl -H "Content-Type:application/json" -H "Content-Type:test" -vvv localhost
* Rebuilt URL to: localhost/
* Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 80 (#0)
> GET / HTTP/1.1
> Host: localhost
> User-Agent: curl/7.54.0
> Accept: */*
> Content-Type:application/json
> Content-Type:test
There was a problem hiding this comment.
Ahh good point 😄 Is the order important here? Should/does extra headers always come at the end?
There was a problem hiding this comment.
the order is not important because it's sent in the request, and the server can process it however it wants.
| func TestExtraHeader(t *testing.T) { | ||
| t.Run("No Extra Header for backward compatibility", func(t *testing.T) { | ||
| c := datadog.NewClient("foo", "bar") | ||
| assert.Empty(t, c.ExtraHeader) |
There was a problem hiding this comment.
Can we also add a test that ensures an extra header was set?
There was a problem hiding this comment.
That's what I wanted to do originally but it would require some refactoring of the code as there are no interfaces, and all the methods I am interested in to test are private in the datadog package.
Do you think it's worth a refactoring ?
There was a problem hiding this comment.
I think we're just currently testing that the client now has a new attribute 😄 But if its refactoring the whole suite, perhaps not currently worthwhile.
👋 In order to better identify Agents making calls to the API we need to be able to configure the request's context.
Having the extra options at the client level will also allow for other header options to be configured.