-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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
Promtool: Validate service discovery files #8950
Conversation
Signed-off-by: Levi Harrison <git@leviharrison.dev>
Signed-off-by: Levi Harrison <git@leviharrison.dev>
Signed-off-by: Levi Harrison <git@leviharrison.dev>
cmd/promtool/main.go
Outdated
if tg.Labels == nil { | ||
tg.Labels = model.LabelSet{} | ||
} | ||
tg.Labels["__<filepath or url>"] = "<path or URL of your targets>" |
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.
Why do we need this?
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.
I'm trying to replicate the SD code as much as possible so we give the most accurate representation of what will be available on the server.
prometheus/discovery/file/file.go
Line 414 in 922f9ba
tg.Labels[fileSDFilepathLabel] = model.LabelValue(filename) |
prometheus/discovery/http/http.go
Line 179 in 922f9ba
tg.Labels[httpSDURLLabel] = model.LabelValue(d.url) |
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.
if we do this, we would probably use the SD interfaces, instead of doing a per-sd thing.
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.
What do you mean by per-sd thing?
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.
I'm not sure this helps but I guess I'll throw in here that in the standalone validation command, which will actually use this output, we won't know if the input was from a file or from HTTP.
curl localhost:1000 | promtool check sd
cat sd.yml | promtool check sd
Either way Promtool receives a JSON or YAML (well not for HTTP) encoded text blob.
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.
The idea of the validation command:
promtool check sd --config.file=prometheus --job=prometheus --wait-time=5s
--wait time is used is the SD is not a "refresh SD". It is the time we wait in the discovery channel to get the targets.
The output would be:
{targets: [ { job_name: prometheus, targets: [before_relabeling: {labels...}, after_relabeling: {labels....}]}
That enables to use oauth2, tls, etc, and works with any service discovery. it also enables testing relabeling.
WDYT?
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.
That's pretty cool, I really like it.
My idea behind curl localhost:1000 | promtool check sd
was that if you were developing a file or HTTP SD, you could quickly check your output without having to set up a config file or anything. I think that still might have a place, maybe if we don't get any flags we check if there's anything in stdin, and parse that?
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.
there is more in the HTTP SD that the format. There is also the headers (content-type), the security, the return code ..
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.
True true, I guess we'll keep it at promtool check sd --config.file=prometheus --job=prometheus --wait-time=5s
. For this PR though, I'll remove the checkSDOutput
function now that we don't need it for anything else and move it into the checkSDFile
function.
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.
We also don't need the output now.
cc @dgl |
Signed-off-by: Levi Harrison <git@leviharrison.dev>
Signed-off-by: Levi Harrison <git@leviharrison.dev>
cmd/promtool/main.go
Outdated
|
||
for i, tg := range targetGroups { | ||
if tg == nil { | ||
return errors.Errorf("nil target group item found (index %v)", i) |
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.
return errors.Errorf("nil target group item found (index %v)", i) | |
return errors.Errorf("nil target group item found (index %d)", i) |
cmd/promtool/main_test.go
Outdated
require.Equalf(t, test.err, err.Error(), "Expected error %q, got %q", test.err, err.Error()) | ||
return | ||
} | ||
require.NoErrorf(t, err, "Got error %q, expected none", err) |
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.
require.NoErrorf(t, err, "Got error %q, expected none", err) | |
require.Ok(t, err) |
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.
require.Ok
doesn't exist.
Signed-off-by: Levi Harrison <git@leviharrison.dev>
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.
Thanks!
This PR adds support to validate service discovery files specified in a config Promtool is checking.
Fixes #8935