Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upSplitting discovery to a separate repo #2911
Comments
This comment has been minimized.
This comment has been minimized.
|
/cc @nathanielc and @pauldix from InfluxData might find this interesting as well. |
This comment has been minimized.
This comment has been minimized.
|
I agree that it probably makes sense to split the generic service discovery code out into it's own library, but given the list of immediate needs in @brian-brazil email I think this is probably a nice to have at the moment. It's still probably a good idea to discuss and document what would need to happen for this effort to go forward. |
This comment has been minimized.
This comment has been minimized.
|
@cstyan On the other hand, splitting it out into a first-class open-source citizen would increase the visibility and prestige of it as a project. It would also encourage others to use, adopt, and help maintain it. So IMO it'd probably be worth it. |
This comment has been minimized.
This comment has been minimized.
|
I think this is a good idea, if there is already interest from other projects. |
This comment has been minimized.
This comment has been minimized.
|
I dumped my thoughts on IRC before seeing this thread. I cleaned it up and copied it below: From a Go perspective, it doesn't matter at all whether discovery is a directory in prometheus/prometheus or its own repo. It already is its own library. Projects interested in using it draw no technical benefit from a changed import path. This also came up for PromQL in the past – essentially it's proposing moving towards a model where all the cool stuff is its own repo (tsdb, discovery, promql) and just stichting it together in prometheus/prometheus and adding and API. That's what k8s is moving towards.... but they literally have a bunch of people working on each of those things they are extracting. We don't. Factoring stuff out into their own repos comes with a significant cost. We have seen that with common/model, common/route, common/log. Even prometheus/prometheus is notoriously out of sync with their HEAD. (I started all those btw and caused this pain to begin with, much with the same good intentions.) In the end we've to think what does factoring discovery, promql, etc. out buy us? Some more visibility for sure. But practically it doesn't get any more usable as a standalone package than before. Improving prometheus-independent usability is a package internal concern and I think discovery is doing quite well there. In prometheus/tsdb that reason was having tight (self-)control for no prometheus specific details to leak into TSDB (i.e. doing hacks in tsdb instead of refactoring the big picture to accommodate use cases.). Also, we have interest in doing infrequent and careful syncs into prometheus/prometheus as we will inevitably break TSDB in ways that may affect persisted data. Broken Prometheus releases that cannot be recovered from by deploying an updated binary must be avoided. tl;dr I recommend only splitting it out if there is a human problem in the development cycle to solve – breaking out repos always makes development harder for an individual developer. All those things are and will be subject to constant reconsideration as the project evolves – right now we have an imminent issue of maintainability to solve and a bunch of people seem to be signing up to volunteer on the mailing list thread. Practically, those won't all convert into long-term maintainers – but we should do our best to aim for this of course. |
This comment has been minimized.
This comment has been minimized.
|
At Qubit we make use of the service discocery code, we also use promql as a
library in our tools for Devs to submit alerts and recording rules. The
major downside is that some 50 LoC tools end up vendoring the whole of
prom. That's painful, but manageable.
Splitting them out would be great for us, but it's not essential.
…On Fri, 7 Jul 2017, 07:30 Fabian Reinartz, ***@***.***> wrote:
I dumped my thoughts on IRC before seeing this thread. I copied it below:
From a Go perspective, it doesn't matter at all whether discovery is a
directory in prometheus/prometheus or its own repo. *It already is its
own library*.
This also came up for PromQL in the past – essentially it's proposing
moving towards a model where all the cool stuff is its own repo (tsdb,
discovery, promql) and just stichting it together in prometheus/prometheus
and adding and API.
That's what k8s is moving towards.... but they literally have a bunch of
people working on each of those things they are extracting. We don't.
We have a rather low number of active developers, thus we can be pretty
liberal with changes in different packages to make something work in others
– we don't have to worry much about breaking other pieces of the project.
Being more diligent about only doing so when necessary for certain packages
is something that can be done without splitting it into its own repo. In
fact, we have hardly touched exported APIs of discovery and PromQL for a
long time now.
Factoring stuff out into their own repos comes with a significant cost. We
have seen that with common/model, common/route, common/log. (I started all
those btw and caused this pain to begin with, much for the same reasons.)
prometheus/prometheus is effectively where stuff happens – Alertmanager
ist kinda big as well, but much more spiky in its activity and they are
totally different aside from the notion of labels.
In the end we've to think what does factoring discovery, promql, etc. out
buy us? Some more visibility for sure. But practically it doesn't get any
more usable as a standalone package than before. Improving
prometheus-independent usability is a package internal concern and I think
discovery is doing quite well there.
tl;dr I recommend only splitting it out if there is a human problem in the
development cycle to solve – breaking out repos always makes development
harder for an individual developer.
In prometheus/tsdb that reason was having tight (self-)control for no
prometheus specific details to leak into tsdb (i.e. doing hacks in tsdb
instead of refactoring the big picture to accommodate use cases.)
All those things are and will be subject to constant reconsideration as
the project evolves – right now we have an imminent issue of
maintainability to solve and a bunch of people seem to be signing up to
volunteer on the mailing list thread.
But maybe making a such a big structural change is giving us more of a
feeling of progress rather than actual one at this point
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#2911 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAEo8-sf2fUty-fv-KWB7_gaie9VNUheks5sLdCigaJpZM4OQKuZ>
.
|
This comment has been minimized.
This comment has been minimized.
|
A proper Go vendoring tool won't vendor the whole prometheus/prometheus tree but just the packages you are using and its transitive dependencies. Glide, dep, govendor, and others all do this. Which one are you using? |
This comment has been minimized.
This comment has been minimized.
|
My thoughts are generally in line with Fabian. I do believe that SD is something it'd be good to offer as distinct library to users at some point, independent of its use in the Prometheus monitoring system. I also believe the state of that subsystem and the current level of contributors for project generally doesn't allow us to offer up what would be another major component to users. |
This comment has been minimized.
This comment has been minimized.
|
I just checked, and we have very little deps on other Prometheus packages. But naturally on a lot of external ones. The latter can be fixed by moving to a composing pattern where |
This comment has been minimized.
This comment has been minimized.
My argument was also that it already is a distinct library that can be used outside of Prometheus easily. And people doing so is proof of that. This is not the property a separate repo addresses. |
This comment has been minimized.
This comment has been minimized.
It can be used, however we don't currently support that as it falls under our default rule that we reserve the right to break internals. If we plan to officially support using this beyond Prometheus internals, then moving to a different repo would be one clear way to signal that. |
This comment has been minimized.
This comment has been minimized.
|
FWIW, we don't have such a non-breakage policy for top-level repos either. Ultimately, the Go world vendors and occasionally breaking packages to allow them to evolve is not much of an issue. |
This comment has been minimized.
This comment has been minimized.
|
@fabxc That's a good point. The trade-off here is visibility / prestige of this library as an independently useful project vs. the convenience for the specific use and development of it in Prometheus. We don't have to make this decision right now, but I still feel that if we want to become the standard Go library for SD (with all the benefits that gives the Prometheus project in turn), having it as a separate repository would help. |
This comment has been minimized.
This comment has been minimized.
What would those benefits be precisely? And how exactly does the additional I'm sure @peterbourgon has relevant insight on all of this. |
This comment has been minimized.
This comment has been minimized.
|
Experience has burned this hard lesson into me: the ongoing costs to keep multiple repos in sync cannot be understated. IMO such an action requires quite concrete technical or process justification, speculation about how it would be nice for some secondary use cases is wildly insufficient. |
This comment has been minimized.
This comment has been minimized.
|
@fabxc The benefits of it becoming the standard Go SD library? I would expect that it would attract more maintainers and contributions, leading to stabilization, testing, and eventually more mechanisms. It would also reflect well on the Prometheus project in general. |
This comment has been minimized.
This comment has been minimized.
|
On Fri, 7 Jul 2017, 08:19 Fabian Reinartz, ***@***.***> wrote:
A proper Go vendoring tool won't vendor the whole prometheus/prometheus
tree. Which on are you using?
Variously godep glide and now dep. We also vendor in the Config structs in
some places.
I'm happy vendoring from the main repo. Though I can imagine there might
be some advantage to splitting some of the discovered out, to allow for
separate maintainers.
|
This comment has been minimized.
This comment has been minimized.
That seems like a very high-set goal that ultimately depends on credible projects adopting it. Influx seems like one of those projects and it apparently was not a blocker. It's purely a visibility thing (imposed by the GitHub UI) and it seems like this can be significantly improved via other means. Such as giving the package a README, documenting usage examples and projects using it, linking to it from other places... As a developer, I only care about whether the package does not pull an entire project as transitive dependencies, which is an orthogonal issue. |
brian-brazil
added
the
component/service discovery
label
Jul 7, 2017
This comment has been minimized.
This comment has been minimized.
I think it's pretty realistic, given that there's not much competition, and given that there'll be more and more interest in SD in the future, as infrastructure becomes more dynamic. The only worry here would be that others would start building their own implementations if they get the impression that ours is not meant to be used outside of Prometheus, or not sufficiently standalone / stable.
Not really, when you look for a library to use, you also care about subtle psychological aspects like it being its own repo or not, and all that subtly implies. Again, I think given the doubts, we should shelve this for now, but I think the idea still has merit to potentially explore further at a later point. |
This comment has been minimized.
This comment has been minimized.
Fair, this is one factor, which we have not yet confirmed to be limiting to our goal. Yesterday I still thought we were just looking for more maintainers – so those ambitions come a bit surprisingly right now.
All those can be addressed in a more proactive and effective manner than eliminating a potential subconscious indicator for them. I see all your arguments but as Peter said, this transition does not just have a one-time cost but maintainers will pay the price again and again forever. And we have to consider whether the single reason of (not) sending a subconscious signal is worth it – especially if we haven't determined whether this is actually true. By the way, some day CNCF may come around and start their official SD project, which we might notably steer or not. Then all of this is may become obsolete again. |
This comment has been minimized.
This comment has been minimized.
|
Yeah, I see your points of course. Let's agree to close this for now and see first how it goes with new maintainers, then possibly reconsider at a later point? Maybe if we get more opinions from existing or prospective external users, or the field is developing in some relevant way. |
This comment has been minimized.
This comment has been minimized.
|
To be clear, I'm not arguing this strongly for the sake of this single package. But because promql/ will be the natural next choice. The further in we get the lower the bar for extraction will become. And suddenly we are doing 2-5 sync-merges a day, encounter unexpected breakage because contracts are broken. Then we may get into automated tooling to trigger testing across repos. The rabbit hole doesn't really end and it will need at least half an FTE to maintain everything brought on by this structure. |
This comment has been minimized.
This comment has been minimized.
nathanielc
commented
Jul 10, 2017
|
Adding my thoughts here from Influx since we already pulled in Having an independent repo is not particularly important but improving the visibility of the package as consumable externally is a good goal. Here are some points already made in this thread but I'll reiterate the ones that are important to us.
Slightly unrelated but I thought I would add here for completeness, we (influxdata/kapacitor) are currently also making use of the TL;DR A separate repo is not important, communicating that the package can be used externally is valuable. |
This comment has been minimized.
This comment has been minimized.
|
Thanks for the insight from an external users' perspective @nathanielc |
gouthamve commentedJul 6, 2017
I think we have the best service-discovery library for Go. @juliusv Once told me that one thing that puts Prometheus apart is its ability to detect targets. It would be amazing if other projects could also use this.
Those familiar with Prometheus have already started using it (Loki and Influxdb). But putting it in its own repo would increase the usage and IMO also attract new contributors who would be using it.
Current users I know of:
Potential users:
*** other lbs, health-check systems and who knows?
/cc @fabxc @brian-brazil @TheTincho @cstyan (guessed it from the thread on -dev, sorry if wrong)