Add HTTP headers support to common HTTP client. #416
Conversation
roidelapluie
force-pushed
the
headers
branch
from
27796f3
to
49aba83
Compare
|
Mailing list discussion: https://groups.google.com/g/prometheus-developers/c/p9HTlPzwUEo |
|
Wondering if this will enable what was intended in #326 |
roidelapluie
force-pushed
the
headers
branch
2 times, most recently
from
a7c7b9d
to
bb90569
Compare
config/headers.go
Outdated
| Headers map[string][]string `yaml:"headers,omitempty" json:"headers,omitempty"` | ||
| SecretHeaders map[string][]Secret `yaml:"secret_headers,omitempty" json:"secret_headers,omitempty"` | ||
| Files map[string][]string `yaml:"files,omitempty" json:"files,omitempty"` |
There was a problem hiding this comment.
I find this entire scheme a bit confusing, both in the non-uniformity of the names (we have normal headers, secret headers, and then just files, which I understand why can't be file headers but still), and also in the names themselves (secret headers could be interpreted as intended, a header with a secret value, or some sort of a "secret" header). Of course they'll be docs to clear this up but I still think it's less than ideal. Therefore, I propose we instead have an iterated:
type header struct {
name/key string
value string
secret secret
file string
}Which I think would be more clear and also allow for more complicated header appends. Generally a small difference though so feel free to ignore.
config/headers.go
Outdated
| if _, ok := reservedHeaders[http.CanonicalHeaderKey(k)]; ok { | ||
| return fmt.Errorf("setting header %q is not allowed", http.CanonicalHeaderKey(k)) | ||
| } |
There was a problem hiding this comment.
It would make more sense for this check to come first
|
@roidelapluie @LeviHarrison what is status in this PR? At least I am interested of this feature |
|
Me too! Adding headers to requests is a relatively trivial operation and multiple APIs support that |
|
Just some small revisions @roidelapluie friendly ping |
This is named `http_headers` so it does not clash with blackbox exporter's headers and Prometheus remote client's headers, which are simple maps. Signed-off-by: Julien Pivotto <roidelapluie@o11y.eu>
|
@LeviHarrison here you go 🙂 |
This is named
http_headersso it does not clash with blackbox exporter's headers and Prometheus remote client's headers, which are simple maps.Signed-off-by: Julien Pivotto roidelapluie@o11y.eu