Pass warning back from response so we can tell if there is a problem with prometheus

[eroteme]

@krasi-georgiev
Was having a problem with prometheus not giving back expected results. Seems that warnings where always passing back nil (from the client). So added a simple fix to merge the warnings back from the client with warnings back rom the response.

Signed-off-by: Brian Gibbins brian.eroteme@gmail.com

[krasi-georgiev]

I wonder (c *httpClient) Do doesn't handle the warnings there?

[joe-elliott]

As you mentioned warnings is hardcoded to nil in the client:
https://github.com/prometheus/client_golang/blob/master/api/client.go#L123

Is there any reason to not just remove this from the interface/client? What meaningful warning could this return that would not just be an error?

Regarding the change in question. I would like to see the prometheus warnings returned from apiClient Do method.

@beorn7, IMO the full change should be:

• Remove the warnings return values from the client since they are hardcoded to nil:
  https://github.com/prometheus/client_golang/blob/master/api/client.go#L64
  https://github.com/prometheus/client_golang/blob/master/api/client.go#L123
• Return the apiResonse warnings here. No need to merge them with anything b/c the client no longer return warnings:
  https://github.com/prometheus/client_golang/blob/master/api/prometheus/v1/api.go#L870
• Add a comment to the documentation that the warnings should be logged only and not considered part of the API. i.e. Don't parse them and write code to respond to them since they are controlled in prometheus proper

[beorn7]

Thanks @joe-elliott for all the insight. I have a closer look ASAP.

[beorn7]

First of all, my apologies for the long delay. It was a really busy month full of conferences.

@joe-elliott what you are writing makes sense to me. The warnings in the "top level" client were introduced in 1335ef4 by @jacksontj .

@jacksontj was there a reason to have warnings in the api.Client.Do method?

If not, I'd just go forward with @joe-elliott 's suggestions.

Thanks, everyone.

[jacksontj]

The warnings should be passed back -- they were initially added in #562 so the warnings shouldn't always be nil -- how can I reproduce the error you are seeing (not getting warnings back)?

[beorn7]

It looks to me that the methods of api.Client always return nil for the warnings. @jacksontj can you point me to any code path where that's not the case?

This PR illustrates where warnings are not propagated, i.e. result.Warnings is never accessed.

[jacksontj]

@beorn7 re-reading the code I also don't see how it ever worked. It seems that the tests there pass because it mocks out the client as well -- so not a great test.

So +1 from me for this PR :)

[beorn7]

Thanks for clarifying.

I still think we should do wath @joe-elliott suggested. @eroteme are you up for it? Otherwise, @joe-elliott or I could do it.

[beorn7]

@joe-elliott has kindly picked up this issue and created #699, which supersedes this PR.