Http response fixes #3814
Http response fixes #3814
Conversation
… event of a network error
…th a code representing the result_type
… failed to
compile or the plugin failed to read the response body
…tags/fields are expected to be present/absent and with what values
…processing the net/http library was not reliably outputing accurate error types for these, so they were dropped
|
@danielnelson This PR is ready for review. One thing that bothers me, I discovered that the plugin doesn't capitalize the method used before adding it as a tag, so if you set |
| h.compiledStringMatch, err = regexp.Compile(h.ResponseStringMatch) | ||
| if err != nil { | ||
| log.Printf("E! Failed to compile regular expression %s : %s", h.ResponseStringMatch, err) | ||
| return err |
There was a problem hiding this comment.
You can just return the decorated error here and it will be logged:
return fmt.Errorf("failed to compile regular expression %s: %s", h.ResponseStringMatch, err)
| @@ -113,18 +119,63 @@ func (h *HTTPResponse) createHttpClient() (*http.Client, error) { | |||
| return client, nil | |||
| } | |||
|
|
|||
| func set_result(result_string string, fields *map[string]interface{}, tags *map[string]string) { | |||
There was a problem hiding this comment.
No need for pointers to maps, since they are shallow copies anyway. Also use normal Go camelCase naming convention on function names and variable names:
func setResult(resultString string, fields map[string]interface{}, tags map[string]string) {
| } | ||
|
|
||
| func set_error(err error, fields *map[string]interface{}, tags *map[string]string) error { | ||
| if timeoutError, ok := err.(net.Error); ok && timeoutError.Timeout() { |
There was a problem hiding this comment.
I think this function can be improved by using a type switch
| @@ -24,6 +24,10 @@ This input plugin will test HTTP/HTTPS connections. | |||
| # {'fake':'data'} | |||
| # ''' | |||
|
|
|||
| # Log network errors in the Telegraf log | |||
| # Turned off by default to avoid filling the logs with exessive repetitive strings | |||
| # log_network_errors = false | |||
There was a problem hiding this comment.
Instead of this option log the network issues at debug level with log.Printf("D! foo")
|
|
||
| ### Example Output: | ||
|
|
||
| ``` | ||
| http_response,method=GET,server=http://www.github.com http_response_code=200i,response_time=6.223266528 1459419354977857955 | ||
| http_response,method=GET,server=http://www.github.com,status_code="200",result="sucess" http_response_code=200i,response_time=6.223266528,result_type="sucess",result_code="0" 1459419354977857955 |
There was a problem hiding this comment.
Example output should have int result_code, also typo in "success".
|
On the method thing, I agree that it would have been nice to uppercase but I think just leave it how it is to avoid a breaking change. |
Remove `log_network_errors` option in favor of logging all errors annotated so they only show in debug mode
Fix typos in docs
Return error in case the regex fails to compile, instead of logging and returning no metrics
Simplify funcions signatures and use camelcase instead of underscores for a couple of function names
Remove log_network_errors option from tests
Add type switch in the error handling routines to make them more clear
|
Thanks, it's in for 1.6. |
This merge request aims to solve #3803 #3802 and #3801. These issues pertain mainly some bugs in the http_response input plugin and making the metrics that it generates Prometheus friendly
Work in progress, do not merge
Required for all PRs: