-
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 regexp matching of HTTP response headers to the http probe #419
Changes from 2 commits
f6cdd9c
2c2a8ee
aa18657
b5c2f94
4bf57e1
6abb43e
4e794b3
7c95fb3
0ed9994
3527651
e7e7afb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -80,19 +80,27 @@ type Module struct { | |
|
||
type HTTPProbe struct { | ||
// Defaults to 2xx. | ||
ValidStatusCodes []int `yaml:"valid_status_codes,omitempty"` | ||
ValidHTTPVersions []string `yaml:"valid_http_versions,omitempty"` | ||
IPProtocol string `yaml:"preferred_ip_protocol,omitempty"` | ||
IPProtocolFallback bool `yaml:"ip_protocol_fallback,omitempty"` | ||
NoFollowRedirects bool `yaml:"no_follow_redirects,omitempty"` | ||
FailIfSSL bool `yaml:"fail_if_ssl,omitempty"` | ||
FailIfNotSSL bool `yaml:"fail_if_not_ssl,omitempty"` | ||
Method string `yaml:"method,omitempty"` | ||
Headers map[string]string `yaml:"headers,omitempty"` | ||
FailIfMatchesRegexp []string `yaml:"fail_if_matches_regexp,omitempty"` | ||
FailIfNotMatchesRegexp []string `yaml:"fail_if_not_matches_regexp,omitempty"` | ||
Body string `yaml:"body,omitempty"` | ||
HTTPClientConfig config.HTTPClientConfig `yaml:"http_client_config,inline"` | ||
ValidStatusCodes []int `yaml:"valid_status_codes,omitempty"` | ||
ValidHTTPVersions []string `yaml:"valid_http_versions,omitempty"` | ||
IPProtocol string `yaml:"preferred_ip_protocol,omitempty"` | ||
IPProtocolFallback bool `yaml:"ip_protocol_fallback,omitempty"` | ||
NoFollowRedirects bool `yaml:"no_follow_redirects,omitempty"` | ||
FailIfSSL bool `yaml:"fail_if_ssl,omitempty"` | ||
FailIfNotSSL bool `yaml:"fail_if_not_ssl,omitempty"` | ||
Method string `yaml:"method,omitempty"` | ||
Headers map[string]string `yaml:"headers,omitempty"` | ||
FailIfMatchesRegexp []string `yaml:"fail_if_matches_regexp,omitempty"` | ||
FailIfNotMatchesRegexp []string `yaml:"fail_if_not_matches_regexp,omitempty"` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should these two above be renamed to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I would rename them. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
FailIfHeaderMatchesRegexp []HeaderMatch `yaml:"fail_if_header_matches_regexp,omitempty"` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't a string, so I'd remove the _regexp from the name to avoid confusion. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where would the confusion arise from? Is there a convention that properties ending with I could rename the properties so it would be like this: # ...
fail_if_body_matches_regexp:
- 'Thou shall not pass'
fail_if_header_matches:
- header: Host
regexp: example.com It would lose the consistency between property names for header and body, though. Is it worth it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, the data structures are different There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
FailIfHeaderNotMatchesRegexp []HeaderMatch `yaml:"fail_if_header_not_matches_regexp,omitempty"` | ||
Body string `yaml:"body,omitempty"` | ||
HTTPClientConfig config.HTTPClientConfig `yaml:"http_client_config,inline"` | ||
} | ||
|
||
type HeaderMatch struct { | ||
Header string `yaml:"header,omitempty"` | ||
Regexp string `yaml:"regexp,omitempty"` | ||
AllowMissing bool `yaml:"allow_missing,omitempty"` | ||
} | ||
|
||
type QueryResponse struct { | ||
|
@@ -217,3 +225,21 @@ func (s *QueryResponse) UnmarshalYAML(unmarshal func(interface{}) error) error { | |
} | ||
return nil | ||
} | ||
|
||
// UnmarshalYAML implements the yaml.Unmarshaler interface. | ||
func (s *HeaderMatch) UnmarshalYAML(unmarshal func(interface{}) error) error { | ||
type plain HeaderMatch | ||
if err := unmarshal((*plain)(s)); err != nil { | ||
return err | ||
} | ||
|
||
if s.Header == "" { | ||
return errors.New("header name must be set for HTTP header matchers") | ||
} | ||
|
||
if !s.AllowMissing && s.Regexp == "" { | ||
return errors.New("regexp must be set for required HTTP headers") | ||
} | ||
|
||
return nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
modules: | ||
http_headers: | ||
prober: http | ||
timeout: 5s | ||
http: | ||
fail_if_header_not_matches_regexp: | ||
- header: Access-Control-Allow-Origin | ||
allow_missing: false |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -69,6 +69,55 @@ func matchRegularExpressions(reader io.Reader, httpConfig config.HTTPProbe, logg | |
return true | ||
} | ||
|
||
func matchRegularExpressionsOnHeaders(header http.Header, httpConfig config.HTTPProbe, logger log.Logger) bool { | ||
for _, headerMatchSpec := range httpConfig.FailIfHeaderMatchesRegexp { | ||
val := header.Get(headerMatchSpec.Header) | ||
if val == "" { | ||
if !headerMatchSpec.AllowMissing { | ||
level.Error(logger).Log("msg", "Missing required header", "header", headerMatchSpec.Header) | ||
return false | ||
} else { | ||
continue // no need to match any regex on missing headers | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "No", and full stop There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed here and in the other place. |
||
} | ||
} | ||
|
||
re, err := regexp.Compile(headerMatchSpec.Regexp) | ||
if err != nil { | ||
level.Error(logger).Log("msg", "Could not compile regular expression", "regexp", headerMatchSpec.Regexp, "err", err) | ||
return false | ||
} | ||
if re.MatchString(val) { | ||
level.Error(logger).Log("msg", "Header matched regular expression", "header", headerMatchSpec.Header, | ||
"val", val, "regexp", headerMatchSpec.Regexp) | ||
return false | ||
} | ||
} | ||
for _, headerMatchSpec := range httpConfig.FailIfHeaderNotMatchesRegexp { | ||
val := header.Get(headerMatchSpec.Header) | ||
if val == "" { | ||
if !headerMatchSpec.AllowMissing { | ||
level.Error(logger).Log("msg", "Missing required header", "header", headerMatchSpec.Header) | ||
return false | ||
} else { | ||
continue // no need to match any regex on missing headers | ||
} | ||
} | ||
|
||
re, err := regexp.Compile(headerMatchSpec.Regexp) | ||
if err != nil { | ||
level.Error(logger).Log("msg", "Could not compile regular expression", "regexp", headerMatchSpec.Regexp, "err", err) | ||
return false | ||
} | ||
if !re.MatchString(val) { | ||
level.Error(logger).Log("msg", "Header did not match regular expression", "header", headerMatchSpec.Header, | ||
"val", val, "regexp", headerMatchSpec.Regexp) | ||
return false | ||
} | ||
} | ||
|
||
return true | ||
} | ||
|
||
// roundTripTrace holds timings for a single HTTP roundtrip. | ||
type roundTripTrace struct { | ||
tls bool | ||
|
@@ -173,6 +222,11 @@ func ProbeHTTP(ctx context.Context, target string, module config.Module, registr | |
Help: "Indicates if probe failed due to regex", | ||
}) | ||
|
||
probeFailedDueToHeaders = prometheus.NewGauge(prometheus.GaugeOpts{ | ||
Name: "probe_failed_due_to_headers", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. probe_http_ falied_due_regex should already cover this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Thee are several other labels that do not have the EDIT: formatting There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They should be changed, but at least the ssl one is commonly used for alerting so we need to be a little careful so not now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, I will not touch the existing labels. But I am not sure if representing the failed header matches with If you do insist, I can reuse There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd suggest reusing, either that or have two more clearly named metrics. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, reused existing metric. Renaming existing ones would be a more dangerous breaking change indeed. |
||
Help: "Indicates if probe failed due to headers", | ||
}) | ||
|
||
probeHTTPLastModified = prometheus.NewGauge(prometheus.GaugeOpts{ | ||
Name: "probe_http_last_modified_timestamp_seconds", | ||
Help: "Returns the Last-Modified HTTP response header in unixtime", | ||
|
@@ -190,6 +244,7 @@ func ProbeHTTP(ctx context.Context, target string, module config.Module, registr | |
registry.MustRegister(statusCodeGauge) | ||
registry.MustRegister(probeHTTPVersionGauge) | ||
registry.MustRegister(probeFailedDueToRegex) | ||
registry.MustRegister(probeFailedDueToHeaders) | ||
|
||
httpConfig := module.HTTP | ||
|
||
|
@@ -320,6 +375,15 @@ func ProbeHTTP(ctx context.Context, target string, module config.Module, registr | |
level.Info(logger).Log("msg", "Invalid HTTP response status code, wanted 2xx", "status_code", resp.StatusCode) | ||
} | ||
|
||
if success && (len(httpConfig.FailIfHeaderMatchesRegexp) > 0 || len(httpConfig.FailIfHeaderNotMatchesRegexp) > 0) { | ||
success = matchRegularExpressionsOnHeaders(resp.Header, httpConfig, logger) | ||
if success { | ||
probeFailedDueToHeaders.Set(0) | ||
} else { | ||
probeFailedDueToHeaders.Set(1) | ||
} | ||
} | ||
|
||
if success && (len(httpConfig.FailIfMatchesRegexp) > 0 || len(httpConfig.FailIfNotMatchesRegexp) > 0) { | ||
success = matchRegularExpressions(resp.Body, httpConfig, logger) | ||
if success { | ||
|
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.
Indentation seems off here
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.
Yeah, does not look quite good. I got it from the prometheus config documentation, e.g. here when the same level contains both optional and non-optional parameters, then it looks like this (comments stripped):
In here, though, the
regex
should be namedregexp
and be made non-optional, so it would look like this:Would that be OK?
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.
Looking around, the only similar one we have is some of pagerduty for the AM. There we break it out as a different type.
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.
Another example right in this repo is
query_response
intcp_probe
, it does not break it out as a separate type. That's why I did not do it here. Can extract both or keep inlined like aboveThere 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.
For query_response they're all optional though.
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.
OK, extracted only the http header match spec.