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

Rule: Add linting command for prometheus rules #1096

Closed
FUSAKLA opened this issue Apr 30, 2019 · 9 comments · Fixed by #1097
Closed

Rule: Add linting command for prometheus rules #1096

FUSAKLA opened this issue Apr 30, 2019 · 9 comments · Fixed by #1097

Comments

@FUSAKLA
Copy link
Member

FUSAKLA commented Apr 30, 2019

Thanos rule has additional config partial_response_strategy for rule groups in the rules file.

Prometheus unfortunately uses strict yaml unmarshaling so the promtool check rules ... fails with:

FAILED:
yaml: unmarshal errors:
  line 3: field partial_response_strategy not found in type rulefmt.RuleGroup

Possible solution could be adding own rule lint command to the thanos like thanos check rules which would check the syntax valid for thanos-rule.

@FUSAKLA
Copy link
Member Author

FUSAKLA commented Apr 30, 2019

I'll do this, just creating an issue if someone else hits this too.

@GiedriusS
Copy link
Member

Makes complete sense, thank you for doing this! However, I wonder if it would be better to move such functionality to separate binary file like thanostool? I remember someone else was making some "nice to have" things too so we could those there too as well. What do others think about this? 😄

@FUSAKLA
Copy link
Member Author

FUSAKLA commented May 1, 2019

This is valid point. We quickly acked this naming on slack with @bwplotka but if we agree than it can refactore it to separate tool.

But on the other hand the bucket commands are already there so if we decide to split it if would feel correct to take those commands out also.

@povilasv
Copy link
Member

povilasv commented May 1, 2019

If we seperate it out into different tools, thanos bucket ls, tahnos bucket inspect & other things should go out as well.

@bwplotka
Copy link
Member

bwplotka commented May 1, 2019

What's the point of separation? I think the single binary is quite a nice feature, as we have a single Go binary for everything, simple build step simplifies all.

Unless separation which gives us advantage like some separation of sensitivity or majority reduced binary size I don't see the benefit in splitting TBH (: But interested to see other points of views!

@bwplotka
Copy link
Member

bwplotka commented May 1, 2019

We can move the ls, bucket, linter functionalities to separate Golang package, and just hook it from single cmd/thanos package if that helps in organizing code ?

@FUSAKLA
Copy link
Member Author

FUSAKLA commented May 2, 2019

For me it's also ok to leave it in the one binary.
Not sure about organizing the code. Feels bit weird to have some of the commands some place else but on the other hand they are not regular "components".

@povilasv
Copy link
Member

povilasv commented May 2, 2019

Yeah I love our mono binary as well, and we should definitely figure a good package structure, so that people can import and extend if they need to :)

@GiedriusS
Copy link
Member

I don't have any strong opinions, just thought that maybe it would be worth to stop for a second and think about it. I am fine either way - I like the mono binary too! 😄

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 a pull request may close this issue.

4 participants