-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for body_path
in HTTP probe config.
#392
Conversation
config/config.go
Outdated
@@ -135,6 +139,13 @@ func (s *HTTPProbe) UnmarshalYAML(unmarshal func(interface{}) error) error { | |||
if err := s.HTTPClientConfig.Validate(); err != nil { | |||
return err | |||
} | |||
if s.BodyPath != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be opened and read in the request. It should also be an error for both to be specified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to be opened within ProbeHTTP
. I left the current os.Open
call in config.go
in order to fail fast on bad configs. This is different from BearerToken
/BearerTokenFile
(https://github.com/prometheus/common/blob/master/config/http_config.go#L81).
Let me know if I should drop opening the file during config validation.
Fixes prometheus#391. Signed-off-by: Emmanuel Gomez <[email protected]>
Signed-off-by: Emmanuel Gomez <[email protected]>
Signed-off-by: Emmanuel Gomez <[email protected]>
06c592f
to
427d68b
Compare
Added DCO. I'm not sure why the Travis build is failing, but from my (short) look, it appears related to git remotes, and not the content of these changes. |
config/config.go
Outdated
return errors.New("at most one of body & body_file may be configured") | ||
} | ||
if s.BodyFile != "" { | ||
_, err := os.Open(s.BodyFile) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should close the file
if httpConfig.Body != "" { | ||
body = strings.NewReader(httpConfig.Body) | ||
} | ||
// If a body path is configured, add it to the request. | ||
if httpConfig.BodyFile != "" { | ||
file, err := os.Open(httpConfig.BodyFile) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure this gets closed
prober/http_test.go
Outdated
HTTP: config.HTTPProbe{ | ||
IPProtocolFallback: true, | ||
Method: "POST", | ||
BodyFile: "./http.go", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can create a testdata directory, and a file with known contents there - plus verify that's what's coming through.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should still change this to use explicit testdata files, rather than the source itself
c71b8b7
to
b72856c
Compare
This has gone stale, let us know if you still wish to work on this. |
Fixes #391.