Packages registry/{consul,etcd}

[bhnedo]

It would be useful to provide the service registry capability for Consul, etcd and eventually for the Eureka discovery server (if the support for this publisher is on the roadmap).

[basvanbeek]

How do you envision that? Currently the publishers are architected so you can have any kind of data for your services in the discovery server. You as go-kit user provide your own factory function to convert that data in the discovery server to a go-kit endpoint. This allows completely free form service registry. By spawning a publishers' discovery client you have access to the exported functions of the underlying client library to place data into the discovery server. What that data needs to be, is completely up to you.

[peterbourgon]

The publishers allow Go kit services to receive information from the source of authority, to make outgoing connections. I guess the idea here is to allow Go kit services to publish information to the source of authority, to announce their presence. I need to do a bit of research to see if this is generalizable, and if it would actually be useful to bring into Go kit, or if each platform (Consul, etcd, ...) is sufficiently different that there's no point.

[bhnedo]

@basvanbeek When I say service registry, I mean the ability of the framework to expose the services, so any client capable to talk with discovery server can locate and consume them. I don't have strong experience with Go kit, but as far as I can see there is no such mechanism. I had to use Consul API to register the instance of the service, so it can be queried from the Java Spring application through Spring Cloud Consul and consumed using the well known RestTemplate abstraction.

consulc, _ := consul.NewClient(consul.DefaultConfig())
consulc.Catalog().Register(&consul.CatalogRegistration{
    Node:    "archrabbit",
    Address: "127.0.0.1",
    Service: &consul.AgentService{
        ID:      "1",
        Service: "todo-microservice",
        Port:    8000,
    },

}, nil)

That's basically what @peterbourgon states about.

[basvanbeek]

I understand what you mean by service registry, I just don't see the benefit of abstracting away or wrapping this in a generic way as the various stores we want to support can be quite different in their scope. Consul can also be used as K/V store, would we need to add the ability to control these to our interface? With for instance zookeeper the kind of data placed in its znodes can be anything as it's just byte strings. And organization of these items is done by a filesystem like tree. What about future systems and their functionality that can be used for service discovery?

What I'm trying to say is I would not want to build or use a generic interface that forces me to do service discovery a certain way, limiting what can be done with the chosen discovery server or be incompatible with non go-kit services. To keep functionality wide open the go-kit interface would be either extremely verbose up to the point of getting annoyingly unclear compared to the properly scoped api of a native discovery servers' client. Looking at your example it shows that consul is opinionated in how it treats service discovery and which variables are important and in which structure. This does not necessarily map with a roll your own zookeeper set-up and in that sense I'd like go-kit to have no opinion.

[bhnedo]

I get your point of view, but I shouldn't consider it such a big deal. go-micro provides an out of the box service registry support for Consul discovery servers. There is no need to abstract every single detail of the underlying store - just the service registry functionality which is IMHO one of the essential microservice patterns. We can start by finding the common denominator of all discovery solutions, which could be the connection configuration (hostname, port, etc) and the entity that should be published to server. I have too much on the plate this month, otherwise, I would try to implement the PoC of this.

[peterbourgon]

@bhnedo To be clear, you're advocating for a package registry/consul (or whatever) with the code you posted above? And potentially other package registry/foo with similar functionality? Anything else additionally?

[bhnedo]

@peterbourgon Exactly, I think that would be sufficient.

[bhnedo]

Here is a fork with two new packages:

1. registry\consul - service registrator for Consul servers

cr := registrator.ConsulRegistrator{Config: nil}
err := cr.Register("zombie", 8000)

1. demux - integration with Gorilla demux router to enable truly RESTful APIs

dmx := demux.Demux{Ctx: ctx, Mux: mux.NewRouter()}
dmx.NewRoute("/zombie/{zombieId}", FindZombieEndpoint(service), 
                         "GET", 
                          decodeZombieEndpoint)

dmx.NewRoute("/zombie/kill/{zombieId}", KillZombieEndpoint(service), 
                         "POST", 
                          decodeZombieKillEndpoint)

http.ListenAndServe(":8000", dmx.Mux)

[peterbourgon]

Thanks for the contribution. Re: registrator, I think it's pretty straightforward, I'm still trying to figure out if it's providing enough value-add to warrant inclusion in Go kit. I think we can probably try it out. Re: demux, anything which modifies the endpoint.Endpoint definition is unfortunately a non-starter.

I'll pick this up in a couple days when I have some downtime on the holidays! :)

[brendan-munro]

If it is at all helpful, I am currently using both Consul and go-kit in a production environment with self registering services and can provide some input on how useful this would be to us as a feature. The code snippet provided above does accomplish service registration, but it introduces a few negative aspects which would make it difficult or dangerous to use in a production environment.

First, it does not integrate health Consul health checks, which are an extremely important aspect of self registering services in that environment. Without health checks present, a service could go offline (through a crash or other unforeseen outage) and your clients and their service discovery would have no way of detecting and excluding those failed endpoints.

This could be mitigated by introducing health checks into the registration library, but that leads down a very long path of additional features to get to a functional inclusion into a general use library, especially given the differences in how various discovery systems implement service health.

The second issue comes with the random generation of the service id. This is a potentially dangerous practice in Consul. The issue here comes down to how Consul identifies duplicate services. By using a randomly generated service ID the service will re-register a new instance with Consul every time it is stopped and started, rather than replacing the existing instance. This means that if a given instance is restarted more times than its peers, and you are using a round-robin load balance methodology, that instance will receive more requests than its peers.

The service id could be generated using a naming format which generates the same ID each time to resolve this issue. For example, in our infrastructure we make use of service-name:host-name:port for our service ID, but this seems like something which should be determined by an individual implementer.

I think what @bhnedo has posted here is a good start, but the issues I have outlined above would need to be addressed in order for a feature like this to be usable in a production environment. Unfortunately, it seems like there isn't a large amount of common ground in this area between various discovery systems (Consul, etcd, Zookeeper). If there is interest in pursuing this as part of the go-kit and addressing the issues I've outlined above I would be very interested in contributing to that development.

Sorry for the long comment, it's very quiet in the office this time of year and I think it's making me ramble more than usual.

[bhnedo]

@EcofitBrendan I completely agree with you. The current implementation is not usable in a production environment (it also lacks an unregister functionality). Moreover, it's just a proof of concept, a piece of functionality I needed to finish up my blog post (Spring Cloud and Go microservice integration). I would really like to see something more robust. Thanks for a detailed comment.

[brendan-munro]

@bhnedo I read your blog post, it's very well thought out! Hopefully we can identify a good way to integrate service identification into go-kit.

[bhnedo]

@EcofitBrendan thank you. Yes, that would be awesome! I'm going to look into Consul health checks soon to make the proof of concept a bit more elaborated.

btw. adorable avatar

[peterbourgon]

For the record I am convinced this is a good-enough idea to make prototypes for each supported service discovery backend (Consul, ZooKeeper, etcd...). I get the feeling it may result in a restructuring of the package layout, e.g. loadbalancer/ becomes sd/ or something.

[weitzj]

Side note about my Consul experiences: I am not entirely sure, if these are helpful, but hopefully when restructuring e.g. 'loadbalancer' this might be something to keep in mind.

I have one service, which is reachable with 2 protocols, e.g. MQTT and websocket.

This might be something to consider:

• "Is a service bound to one protocol, so each protocol variant will be another service?
• Can I have "sticky services"? In my case: Does the load balancer get me to the same MQTT broker as before (e.g. my US broker)
• What about security for "sticky services" with multiple protocols? Who does TLS termination?
• Should I have 2 consul health checks for each service? e.g. one healthcheck behind the loadbalancer and one healthcheck before the loadbalancer? Which service should be announched?

Some of these answers are for us:

• announcing services with 2 protocols as 2 different services with each one protocol (so that you can still use the Consul DNS interface and use consul-template)
• wildcard dns for the loadbalancer and SNI for all consuming clients. Tls termination at the service.
• health checks and registration behind the load balancer to be included in the loadbalancer
• health checks and registration in front of loadbalancer. These are the actual services, which are consumed from clients

[peterbourgon]

@weitzj Thanks for that, actually very insightful!

• Totally agree about different protocols being different SD entries as the degenerate case, though I guess it depends on the API of the SD system
• Stickiness (affinity) as you describe it cannot be effectively guaranteed in a generalized way — requires application-level semantics, so I think we agree here
• Health checks I think belong in multiple points in the system: SD can healthcheck an instance before advertising it in the first place; the LB can healthcheck advertised instances before adding them to pools; clients can healthcheck LB endpoints as part of validation (though this usually happens implicitly with circuitbreaking) — a good infrastructure will use some/all of these methods simultaneously

[peterbourgon]

@bhnedo Right now in the linked PR, the Consul registry publisher is invoking AgentServiceRegistration. Is that the right one to use? The docs strongly advise me to use it, but (I think) implicit in that recommendation is that you're always talking to a local Consul agent, and I'm not sure if that's a valid assumption for Go kit. What do you think?

If you think it is a valid assumption, then I'll add some documentation stipulating that to the Publisher constructor.

If you think that's not a valid assumption, then I guess we won't be able to use AgentService and will have to use the lower-level CatalogRegistration... do you agree?

Forgive all of my questions, it's been a little while since I've used Consul in anger :)

[peterbourgon]

@EcofitBrendan You might be able to answer that ^^ question, too...

[bhnedo]

@peterbourgon keep in mind I'm not an expert on Consul. As long as I could figure out from the docs, local agent mode is recommended over the server mode which offers nodes to participate in consensus quorum process. AgentService would be the way to go. @EcofitBrendan, please correct me if I'm wrong.

[peterbourgon]

We now have a Consul registrar, and Zookeeper/etcd registrars are requested in #284. Closing this ticket. Thanks for the motivation and sorry it took so long! :)