-
Notifications
You must be signed in to change notification settings - Fork 2.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 linter for ensuring correct Prometheus metric name. #2430
Comments
I would like to take this up as my first issue. Any pointers to start would be helpful. |
Well, I know as much as you! I got that awesome link, and we need to have the same on Thanos side. Ideally, it is a separate Go command as we do in copyright check: https://github.com/thanos-io/thanos/blob/55cb8ca38b3539381dc6a781e637df15c694e50a/scripts/copyright/copyright.go At some point we could propose it as official linter for https://github.com/golangci/golangci-lint 🤗 |
A command-line linter would be super useful! This also benefits all the projects using Prometheus metrics in the Go ecosystem |
Should I add it as a test? Or a separate go command? |
Separate Go command please, to be consistent with rest.
BTW have you checked how to add linter to golint ci project?
…On Fri, 17 Apr 2020 at 19:35, Srestha Srivastava ***@***.***> wrote:
Should I add it as a test? Or a separate go command?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#2430 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABVA3O6IEYCGV3ILRNTXFJLRNCON3ANCNFSM4MHTGY4A>
.
|
@bwplotka no I haven't checked that, I was learning go, should I check that first and maybe open an issue there? |
Just check their docs, I am sure it's easy to add and there must be some process |
https://github.com/golangci/golangci-lint/wiki/How-to-add-a-custom-linter |
We can try, question is if they will accept Prometheus instrumentation to be popular enough. IMO it is native way for metrics. Let's see (: In the worst case we can use the linter you will build separatedly to golangci-lint! (: |
@twisted-sres Feel free to add me as reviewer too. I would like to do the same in Cortex too, and we may reuse what you're coming up with. |
I would love to help/see this eventually as a customer linter in golangci-lint. I think we're going to run into the same problem that k8s and others ran into with promlint, adding it will add a TON of unrelated dependencies. |
Hello 👋 Looks like there was no activity on this issue for last 30 days. |
Valid and help wanted! |
Hello 👋 Looks like there was no activity on this issue for last 30 days. |
Closing for now as promised, let us know if you need this to be reopened! 🤗 |
Still relevant AFAIK. |
Something is baking https://github.com/yeya24/promlinter @yeya24 |
Opened a pr in golangci-lint, I am not sure whether it will be accepted or not, but it works (though not all the cases). |
[ Quoting <[email protected]> in "Re: [thanos-io/thanos] Add linter f..." ]
Opened a pr in golangci-lint, I am not sure whether it will be accepted or not,
but it works (though not all the cases).
We also experiencing these kind of PR too much; coredns/coredns#4021
(you have a metric, but forget to register it)
A linter should probabaly check for that as well.
/Miek
…--
Miek Gieben
|
@miekg Actually for this one you don't need linter, you ensure that through library itself. Check out Since we moved all metric constructions to this, it's not longer a problem (: |
[ Quoting <[email protected]> in "Re: [thanos-io/thanos] Add linter f..." ]
@miekg Actually for this one you don't need linter, you ensure that through
library itself. Check out promauto.With and discussion here: #2102
Since we moved all metric constructions to this, it's not longer a problem (:
oohhhh, nice.
Thanks for the pointer
|
Hello 👋 Looks like there was no activity on this issue for last 30 days. |
@yeya24 seems like there is interest from golangci-lint. Care to update your PR there? This would be very much appreciated |
Thanks @GiedriusS. Yes, I want to finish that as well. But tbh that tool is not very mature and complete currently and it cannot handle some cases. So I am wondering whether it is a good time to add it to golangci-lint or not. Anyway, I will update that pr soon. |
Hello 👋 Looks like there was no activity on this issue for the last two months. |
Closing for now as promised, let us know if you need this to be reopened! 🤗 |
Still valid. Will get back to this soon |
Hello 👋 Looks like there was no activity on this issue for the last two months. |
valid |
Hello 👋 Looks like there was no activity on this issue for the last two months. |
Thanks to @miekg and code they added here: https://github.com/coredns/coredns/blob/master/test/metric_naming_test.go
The text was updated successfully, but these errors were encountered: