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

util/promlint: consider making it easier for 3rd parties to import #5317

Closed
mdlayher opened this issue Mar 8, 2019 · 14 comments · Fixed by #7209
Closed

util/promlint: consider making it easier for 3rd parties to import #5317

mdlayher opened this issue Mar 8, 2019 · 14 comments · Fixed by #7209
Assignees

Comments

@mdlayher
Copy link
Contributor

mdlayher commented Mar 8, 2019

Proposal

I originally added promlint for use in promtool check metrics. It is very useful for that purpose, however, it would also be nice to allow the package to be easily vendored and used in other applications for unit tests and such.

Because this main Prometheus repository is fairly large and also contains a great deal of unrelated code, I propose moving util/promlint to github.com/prometheus/common/promlint or similar. This way, library code lives with a library instead of an entire application, and is much easier to vendor.

At that point, we can vendor in the updated copy of common and switch the import statements used within promtool.

@mdlayher
Copy link
Contributor Author

mdlayher commented Mar 8, 2019

Talked with @SuperQ on IRC and it appears I was wrong about needing to vendor the entire repo. dep at least appears to pick out the package by itself if I dep ensure -add github.com/prometheus/prometheus/util/promlint.

✔ ~/src/xxx/vendor/github.com/prometheus/prometheus [master|✚ 2…1⚑ 4] 
14:37 $ ls -laR
.:
total 28
drwxr-xr-x 3 matt matt  4096 Mar  8 14:34 .
drwxr-xr-x 7 matt matt  4096 Mar  8 14:34 ..
-rw-r--r-- 1 matt matt 11357 Mar  8 14:34 LICENSE
-rw-r--r-- 1 matt matt  2769 Mar  8 14:34 NOTICE
drwxr-xr-x 3 matt matt  4096 Mar  8 14:34 util

./util:
total 12
drwxr-xr-x 3 matt matt 4096 Mar  8 14:34 .
drwxr-xr-x 3 matt matt 4096 Mar  8 14:34 ..
drwxr-xr-x 2 matt matt 4096 Mar  8 14:34 promlint

./util/promlint:
total 16
drwxr-xr-x 2 matt matt 4096 Mar  8 14:34 .
drwxr-xr-x 3 matt matt 4096 Mar  8 14:34 ..
-rw-r--r-- 1 matt matt 6733 Mar  8 14:34 promlint.go

I do still think it'd be reasonable to move this to common, though.

@brian-brazil
Copy link
Contributor

It's only used in one of our repos, so I see no need to move it to common. Common is for stuff shared across our repos.

@RainbowMango
Copy link
Contributor

If github.com/prometheus/common isn't the proper place, then how about github.com/prometheus/client_golang?

Maybe we can move it to client_golang/prometheus/testutil.

Involve @brancz here.

@mcristina422
Copy link

@brian-brazil @mdlayher Thoughts on moving promlint into http://github.com/prometheus/client_golang like @RainbowMango suggested?

Or maybe even http://github.com/prometheus/promlint ? Moving it out of github.com/prometheus/prometheus would be extremely helpful

@brian-brazil
Copy link
Contributor

brian-brazil commented Apr 17, 2020

As indicated in kubernetes/kubernetes#86477 you'd probably be best using it is a library binary. As-is the code appropriately located from a Prometheus standpoint, as it is only used in promtool.

@RichiH
Copy link
Member

RichiH commented Apr 20, 2020

We are currently doing a bug scrub.

@beorn is considering to move this to client_golang. @brancz would you be interested in doing that?

@beorn7
Copy link
Member

beorn7 commented Apr 20, 2020

To be precise: client_golang/prometheus/testutil could make sense as a location.

@RichiH RichiH changed the title util/promlint: consider moving to github.com/prometheus/common util/promlint: consider making it easier to import Apr 20, 2020
@brian-brazil brian-brazil changed the title util/promlint: consider making it easier to import util/promlint: consider making it easier for 3rd parties to import Apr 20, 2020
@beorn7
Copy link
Member

beorn7 commented Apr 20, 2020

In particular, client_golang would not be a good place for a stand-alone linter binary, but promtool can stay where it is and just import client_golang/prometheus/testutil to get the code.

@RainbowMango
Copy link
Contributor

I'm interested in this, can I join or take this?
@RichiH @beorn7 @brancz

@brancz
Copy link
Member

brancz commented Apr 21, 2020

Go for it @RainbowMango :)

@beorn7
Copy link
Member

beorn7 commented Apr 22, 2020

I'll brace for impact then. (o:

@RainbowMango
Copy link
Contributor

Working on it. Thanks.

@RainbowMango
Copy link
Contributor

@beorn7 Can you assign this issue to me? It can help me find this issue quickly in the future. :)

@beorn7
Copy link
Member

beorn7 commented May 7, 2020

Sure, done.

@prometheus prometheus locked as resolved and limited conversation to collaborators Nov 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants