-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Porting promlint from prometheus/prometheus #739
Conversation
Signed-off-by: RainbowMango <renhongcai@huawei.com>
I can't see the Travis build log, |
46e70fb
to
6bd6811
Compare
lgtm but leaving last call to @beorn7 |
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.
LGTM 👍
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.
Just some nits here. I'll ask a more fundamental question separately in the conversation.
@@ -0,0 +1,367 @@ | |||
// Copyright 2017 The Prometheus Authors |
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.
It doesn't matter a lot, but we use to fill in the current year if we create a new file.
Text string | ||
} | ||
|
||
// newProblem is helper function to create a Problem. |
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.
🤣
} | ||
|
||
// New creates a new Linter that reads an input stream of Prometheus metrics. | ||
// Only the text exposition format is supported. |
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.
Perhaps "Only the Prometheus text exposition format is supported." so that people don't get confused with OpenMetrics?
Many thanks 🌈🥭, 🇧🇱🐟, and ☮️. I have a question: Do you feel this should in its own |
Signed-off-by: RainbowMango <renhongcai@huawei.com>
6bd6811
to
50cda50
Compare
I'm glad you ask this. Yes, I think if we put it in Indeed, we can take promlint as test util for now, because it can only check the gathered metrics. |
Actually, I started a program to run my thought months ago. I don't want to disrupt the Prometheus community, I just want the Would you mind I continue working on my program? If it really works, I will be happy to donate it to the Prometheus community. |
We already arrived at the conclusion that having the promlint package here in client_golang is a good idea. A stand-alone program would than simply import the package and do the boilerplate around it to turn it into e.g. a command line tool or whatever else. My question was more about having the moving parts directly in Let's go with Thanks everyone and in particular @RainbowMango . |
Refer prometheus/prometheus#5317.
I split this work into two commits:
promlint
in.I wish it will be easier for the reviewer to catch what has been changed during porting.