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

Refactor the API to use go-kit #3416

Closed
krasi-georgiev opened this Issue Nov 6, 2017 · 21 comments

Comments

Projects
None yet
4 participants
@krasi-georgiev
Copy link
Member

krasi-georgiev commented Nov 6, 2017

Started looking at the api code and I think it can be improved by using the go-kit package.
https://github.com/go-kit/kit
https://youtu.be/JXEjAwNWays?t=1451
https://www.youtube.com/watch?v=NX0sHF8ZZgw

@krasi-georgiev

This comment has been minimized.

Copy link
Member Author

krasi-georgiev commented Feb 12, 2018

@tomwilkie what do you have against go-kit?
I haven't used it, but it looks like a good fit.

@gauravsitlani

This comment has been minimized.

Copy link

gauravsitlani commented Feb 26, 2018

@krasi-georgiev Can I take the initiative to help out .
This is the code right ?
https://github.com/prometheus/prometheus/blob/master/web/api/v2/api.go

@krasi-georgiev

This comment has been minimized.

Copy link
Member Author

krasi-georgiev commented Feb 26, 2018

that would be great!
I started a topic in the def list, but hasn't been discusses yet so need a bit more input from the other maintainers.
https://groups.google.com/forum/#!topic/prometheus-developers/IebVSsE2_Gk

I am leaning towards gokit + swagger spec generation via annotations.
https://goswagger.io/generate/spec.html

@gauravsitlani

This comment has been minimized.

Copy link

gauravsitlani commented Feb 26, 2018

@krasi-georgiev I'll go through the api code and check out go-kit + swagger and follow the thread.

@krasi-georgiev

This comment has been minimized.

Copy link
Member Author

krasi-georgiev commented Feb 26, 2018

Thanks.
post your findings or questions in the Google groups thread.

@Bo0km4n

This comment has been minimized.

Copy link

Bo0km4n commented Mar 14, 2018

Hi I’m Katsuya,
I’m thinking to participate GSoC’18, and I’d like to work on Prometheus.
I found this issue Prometheus’s ideas page,
It’s sounds very interesting.
Could you tell me more detail of this issue?
If there is anything I can do, I want to help.

@krasi-georgiev

This comment has been minimized.

Copy link
Member Author

krasi-georgiev commented Mar 14, 2018

I did a quick research and started a dicsussion in the Prometheus mailing list.
https://groups.google.com/forum/#!topic/prometheus-developers/IebVSsE2_Gk

also started a doc to take some notes
https://docs.google.com/spreadsheets/d/1juaYH-unAYIp1-rlmYRNojxxwRswK4w7lDPHtUQj398/edit#gid=0

overall I am leaning towards go-kit with some code annotations to auto generate the code based on the code annotations/comments next to each endpoint.

if you have an opinion can post it in the mailing list to get some more comment and can also ping me on irc #prometheus-dev if you have any questions.

@krasi-georgiev

This comment has been minimized.

Copy link
Member Author

krasi-georgiev commented Mar 14, 2018

@Bo0km4n should hang around a bit longer in the IRC. Saw your question, but you were not in the irc anymore. if you put krasi in front of the question I will get a notification and better chance to get a quick reply.

@Bo0km4n

This comment has been minimized.

Copy link

Bo0km4n commented Mar 14, 2018

@krasi-georgiev

This comment has been minimized.

Copy link
Member Author

krasi-georgiev commented Mar 14, 2018

it might be a bit difficult of you have more than one question , but lets try , fire away.

@Bo0km4n

This comment has been minimized.

Copy link

Bo0km4n commented Mar 14, 2018

@krasi-georgiev

This comment has been minimized.

Copy link
Member Author

krasi-georgiev commented Mar 14, 2018

send me a message in the main room

@Bo0km4n

This comment has been minimized.

Copy link

Bo0km4n commented Mar 14, 2018

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented Mar 15, 2018

What's the actual problem we're trying to solve here?

@krasi-georgiev

This comment has been minimized.

Copy link
Member Author

krasi-georgiev commented Mar 15, 2018

AFAIR the code is not well organised , v1,v2, admin are all separate and difficult to follow.
versioning is difficult and non DRY , API documentation is all manual and easy to drift.

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented Mar 15, 2018

I don't see how using a framework would resolve any of that, it'd just add an extra layer of abstraction and complicate the code. That versioning was never properly designed wouldn't be fixed either, and I personally think we should be considering deleting v2 rather than fixing it.

@krasi-georgiev

This comment has been minimized.

Copy link
Member Author

krasi-georgiev commented Mar 15, 2018

can't be sure on a 100% , but I would like at least to try and refactor it.
If it ends up the same or more complicated than we don't have to merge it.

I am against using frameworks as well so will try to avoid it at all cost unless it brings some real time and maintaining savings.

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented Mar 15, 2018

As is stands there's very little maintenance required on the APIs, and adding new endpoints isn't that hard.

@krasi-georgiev

This comment has been minimized.

Copy link
Member Author

krasi-georgiev commented Mar 15, 2018

It was long ago so will check again and will close the proposal if that is the case.

@krasi-georgiev

This comment has been minimized.

Copy link
Member Author

krasi-georgiev commented Mar 23, 2018

I had a look at the code again as part of troubleshooting a race condition and must say it is extremely unorganised , difficult to follow and difficult to troubleshoot.
It uses 2 different routers with some rules defined in the New function and others in the Run function.
Overall adding more things to the api . versioning etc. would make it really difficult to maintain in its current state.

Now I am even more convinced than before that this can get a lot better.

@krasi-georgiev

This comment has been minimized.

Copy link
Member Author

krasi-georgiev commented Nov 28, 2018

the last dev summit it was decided to try open api for the alertmanager and it all good will consider it for Prometheus as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.