[SUPERCEDED] package sd

[peterbourgon]

This is a big change.

Package loadbalancer has been deleted, and is replaced by package sd. What was previously loadbalancer.Publisher becomes sd.Subscriber, which I believe better reflects its purpose. That is, the sd/consul.Subscriber is subscribing to updates from Consul.

// Subscriber listens to a service discovery system and yields a set of
// identical services on demand. Typically, this means a set of identical
// instances of a microservice. An error indicates a problem with connectivity
// to the service discovery system, or within the system itself; a subscriber
// may yield no services without error.
type Subscriber interface {
    Services() ([]service.Service, error)
}

Additionally, we introduce a new type, sd.Registrar, responsible for advertising the lifecycle of the Go kit service into the corresponding service discovery system. That is, sd/consul.Registrar registers updates to Consul.

// Registrar registers instance information to a service discovery system when
// an instance becomes alive and healthy, and deregisters that information when
// the service becomes unhealthy or goes away.
//
// Registrar implementations exist for various service discovery systems. Note
// that identifying instance information (e.g. host:port) must be given via the
// concrete constructor; this interface merely signals lifecycle changes.
type Registrar interface {
    Register()
    Deregister()
}

This is optional, as many users prefer to or must have their orchestration system manage entries in their service discovery system. But for those users who do self-registration, this will be useful. Addresses #167.

Astute readers will note another big change: the Subscriber no longer emits raw endpoint.Endpoints, but a new type, service.Service. A Service is a logical collection of named endpoints intended to represent a complete microservice.

// Service represents a collection of endpoints (i.e. methods). It may be one
// instance of a microservice, or it may abstract over multiple identical
// instances of the same microservice.
type Service interface {
    Endpoint(method string) (endpoint.Endpoint, error)
}

A Service is probably implemented as a simple map[string]endpoint.Endpoint. Method names have no particular schema and are totally up to users. The factory methods given to an sd.Subscriber must now convert an instance string (host:port) into a Service rather than an Endpoint. This makes much more sense: previously, users would need near-identical factories (and loadbalancer.Publishers) for every individual endpoint they wanted to interact with, which was very tedious.

With those important distinctions in mind, I've mostly just ported the existing loadbalancer code to sd. A few additional changes:

• The Consul implementation was refactored (attn: @xla), and sd/consul.Publisher added
• The endpoint cache was simplified and moved to sd/internal/cache

There are a few additional things I want to do, which I will probably leave to subsequent PRs:

• Add sd/etcd.Publisher
• Add sd/zk.Publisher
• Add service.AdaptMiddleware, which adapts an endpoint.Middleware to a service.Middleware (there are subtleties here that make it nontrivial)

Comments, pointers to things I may have forgotten, and bitter complaints that I am changing core Go kit semantics — all are greatly appreciated :)

[peterbourgon]

Oh — I obviously haven't deleted package loadbalancer from this PR yet :) Updated examples to come soon.

[nussjustin]

Maybe I'm misunderstanding this, but instead of doing a load+cas in a loop couldn't this just use an atomic add? For example:

old := atomic.AddUint64(&rr.c, 1) -1 // subtract 1 to get the previous value

[peterbourgon]

Yes, I think you are right :) In my defense, this code was left over from a long time ago and I didn't really rethink it. Thank you for the spot!

[ChrisHines]

Is sd the best name for the new package? Maybe this is a common abbreviation amongst micro-service practitioners, but I find it cryptic. What other names were rejected in favor of sd?

[peterbourgon]

I'm pretty happy with sd. It is a tiny bit cryptic, but it is (as you suspect) a well-known abbreviation, it is very small, and I can't think of a good alternative. package servicediscovery is no good.

[basvanbeek]

Just thinking out loud. Maybe Publisher isn't the best name for the advertisement of a new service to the discovery system as it has previously been used in loadbalancer.

How about Registrar as the Interface would be responsible for registering / de-registering the availability of the Go kit service to the discovery system?

[peterbourgon]

Yes, Registrar may make more sense. Something like this?

// Registrar registers instance information to a service discovery system when
// an instance becomes alive and healthy, and deregisters that information when
// the service becomes unhealthy or goes away.
//
// Registrar implementations exist for various service discovery systems. Note
// that identifying instance information (e.g. host:port) must be given via the
// concrete constructor; this interface merely signals lifecycle changes.
type Registrar interface {
    Register()
    Deregister()
}

[basvanbeek]

I just added a PR to the sd branch for a lb.Retry function. It will allow one to have the same retry logic per Endpoint as available in the old loadbalancer package, to be used with the new style per service sd.Balancer load balancers.

[basvanbeek]

Is there a particular reason the cache package is made internal? This would be annoying for people wanting to use the cache for writing their own subscribers to proprietary discovery systems.

[peterbourgon]

I did not anticipate cache being used by third-party Subscriber implementations. I made it internal primarily for cleanliness, I'm happy to move it out if you think there might be a desire for it.

[basvanbeek]

It would help me out. I have a subscriber implementation based on curator-go :)

[xla]

It probably should be noted why this filtering is done in the client, how using prepared queries is desired yet not feasible currently.

Did you dig deeper into the issue of prepared queries not supporting blocking calls? Otherwise I'm happy to explore in that direction.

[peterbourgon]

Yes, good point, I will add some documentation.

re: lack of support, the Consul issue for querying on multiple tags was closed when prepared queries were introduced, but the execute endpoint "does not support blocking queries" so

¯\_(ツ)_/¯

[peterbourgon]

Porting the examples reveals some unwanted friction between factories, services, and endpoints. I need to iterate on the interaction between those components. Thanks to @basvanbeek for highlighting some of these problems early.

[peterbourgon]

The Service abstraction created a number of problems.

• Refactoring existing services that used multiple independent (loadbalancer.Publisher → endpoint.Endpoint) codepaths to use a single (sd.Subscriber → service.Service) codepath, and then split out endpoints after, was a lot messier than expected, with a net increase of SLoC.
• I really wanted a user-friendly way to adapt existing endpoint.Middlewares to apply to all endpoints in a service (service.AdaptMiddleware). But I wasn't able to figure out a way to allow endpoint.Middlewares to set up their closure state (e.g. a token bucket) once, while applying the actual middleware equally to multiple endpoints. At least, not with a good UX.
• No equivalent generic aggregation type exists in e.g. gRPC; it manages client-side aggregation via generated code.

Consequently, I'm not comfortable moving forward with the Service abstraction. But, everything else in the PR seems OK. So I will rework this PR. For my own edification, the work to be done is

• Refactor sd.Subscriber to yield individual endpoint.Endpoints
• Use sd.Registrar + implementations
• Use new endpoint cache, Consul subscriber implementation, and other mechanical bits
• Refactor addsvc example
• Refactor profilesvc example, adding a client package
• Refactor apigateway example
• Make minimum changes necessary to shipping example
• Update examples/README + refactor stringsvc examples