Thanos: Add linter for ensuring correct Prometheus metric name.

[bwplotka]

Thanks to @miekg and code they added here: https://github.com/coredns/coredns/blob/master/test/metric_naming_test.go

[sresthas]

I would like to take this up as my first issue. Any pointers to start would be helpful.
Thanks :-)

[bwplotka]

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 :hugs:

[yeya24]

A command-line linter would be super useful! This also benefits all the projects using Prometheus metrics in the Go ecosystem

[sresthas]

Should I add it as a test? Or a separate go command?

[bwplotka]

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 [e-mail]
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
https://github.com/thanos-io/thanos/issues/2430#issuecomment-615400443,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ABVA3O6IEYCGV3ILRNTXFJLRNCON3ANCNFSM4MHTGY4A
.

[sresthas]

@bwplotka no I haven't checked that, I was learning go, should I check that first and maybe open an issue there?

[bwplotka]

Just check their docs, I am sure it's easy to add and there must be some process

[sresthas]

https://github.com/golangci/golangci-lint/wiki/How-to-add-a-custom-linter
I found this, so should I try adding the linter in golint ci?

[bwplotka]

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! (:

[pracucci]

@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.

[mcristina422]

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.
https://github.com/prometheus/prometheus/issues/5317

[stale[bot]]

Hello 👋 Looks like there was no activity on this issue for last 30 days.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity for next week, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

[yeya24]

Valid and help wanted!

[stale[bot]]

Hello 👋 Looks like there was no activity on this issue for last 30 days.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity for next week, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

[stale[bot]]

Closing for now as promised, let us know if you need this to be reopened! 🤗

[GiedriusS]

Still relevant AFAIK.

[kakkoyun]

Something is baking https://github.com/yeya24/promlinter @yeya24

[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).

[miekg]

[ Quoting [e-mail] 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; https://github.com/coredns/coredns/pull/4021
(you have a metric, but forget to register it)

A linter should probabaly check for that as well.

/Miek

--
Miek Gieben

[bwplotka]

@miekg Actually for this one you don't need linter, you ensure that through library itself. Check out promauto.With and discussion here: https://github.com/thanos-io/thanos/issues/2102

Since we moved all metric constructions to this, it's not longer a problem (:

[miekg]

[ Quoting [e-mail] 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

[stale[bot]]

Hello 👋 Looks like there was no activity on this issue for last 30 days.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity for next week, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

[GiedriusS]

@yeya24 seems like there is interest from golangci-lint. Care to update your PR there? This would be very much appreciated

[yeya24]

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.

[stale[bot]]

Hello 👋 Looks like there was no activity on this issue for the last two months.
Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity in the next two weeks, this issue will be closed (we can always reopen an issue if we need!). Alternatively, use remind command if you wish to be reminded at some point in future.

[stale[bot]]

Closing for now as promised, let us know if you need this to be reopened! 🤗

[yeya24]

Still valid. Will get back to this soon