Skip to content
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

Change healthcheck query to use the "site" label #29

Merged
merged 2 commits into from
May 29, 2019

Conversation

robertodauria
Copy link
Contributor

@robertodauria robertodauria commented May 22, 2019

Now that we have the "site" label on all the blackbox-exporter targets, we can avoid using label_replace.


This change is Reviewable

Now that we have the "site" label on all the blackbox-exporter
targets, we can avoid using label_replace.
Copy link

@stephen-soltesz stephen-soltesz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 2 of 2 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @evfirerob)


healthcheck/prometheus.go, line 20 at r1 (raw file):

	// tests on the node is applied. This makes sure we never reboot a node
	// unnecessarily or lose data.
	NodeQuery = `sum_over_time(probe_success{service="ssh", module="ssh_v4_online"}[%[1]dm]) == 0

Ah, nice. This reads much more easily.


promtest/prometheus_testing.go, line 28 at r1 (raw file):

type response struct {
	value model.Value
	err   api.Error

Huh. Strange choice on the part of the prometheus client authors, imo. I would expect an error type that could be type asserted into the underlying api.Error. This pattern is common in system packages for example. I wonder what encouraged them to use the more explicit approach.

@robertodauria robertodauria merged commit 0bc3e56 into master May 29, 2019
@robertodauria robertodauria deleted the sandbox-roberto-fix-query branch May 29, 2019 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants