package server + example

[peterbourgon]

Iterating on the previous server implementation, this is still just a sketch...

[ChrisHines]

I've spent some time studying the example addsvc. It is instructive. I have a better feel for how package metrics works now. It is interesting to see how you hook up multiple transports. The server package is indeed quite minimal. In one of your comments on my logging PR (#16) you said that you are "biased toward functional composition whenever possible" and that is evident. But in some cases there are more layers of functional composition than seem necessary to me.

To be specific, I had to study the server.Gate function for quite a while before I could grok all the layers of functional indirection involved. First the code:

// Gate returns a middleware that gates requests. If the gating function
// returns an error, the request is aborted, and that error is returned.
func Gate(allow func(context.Context, Request) error) func(Endpoint) Endpoint {
    return func(next Endpoint) Endpoint {
        return func(ctx context.Context, req Request) (Response, error) {
            if err := allow(ctx, req); err != nil {
                return nil, err
            }
            return next(ctx, req)
        }
    }
}

Which gets used in main.main like so:

// `package server` domain
var e server.Endpoint
e = makeEndpoint(a)
e = server.Gate(zipkin.RequireInContext)(e) // must have Zipkin headers

I wonder if we can strip back one of the layers without loss of functionality like so:

func Gate(next Endpoint, allow func(context.Context, Request) error) Endpoint {
    return func(ctx context.Context, req Request) (Response, error) {
        if err := allow(ctx, req); err != nil {
            return nil, err
        }
        return next(ctx, req)
    }
}

// in main.main
e = server.Gate(e, zipkin.RequireInContext) // must have Zipkin headers

This approach is easier for me to understand, and the call site looks more idiomatic to me without the double function call. We lose the ability to store the Endpoint generator in a variable, but I'm not clear how important that is. The sample service does not do it.

The same pattern appears with grpcInstrument, httpInstrument, and cors.Middleware, and in all of those cases it seems like the arguments for the primary function and the returned anonymous function could be merged into a single function call.

I'm sure there is a rationale for this pattern that I am missing, please explain.

[peterbourgon]

@ChrisHines the rationale is simply that it enables the use of alice-style middleware chaining, which is slightly nicer to use by callers, providing a uniformity of type signature for everything that operates on Endpoints (in this case). A more sophisticated main.main may look like

e := server.NewChain(
    recoveryMiddleware,
    server.Gate(zipkin.RequireInContext),
    server.Check(otherProperty),
    myOtherFilter,
    // ...and so on...
).Then(makeEndpoint(a))

[tsenart]

@peterbourgon, @ChrisHines: In functional programming lingo, you're doing partial currying: http://en.wikipedia.org/wiki/Currying

It's partial because you still have a function that receives more than one argument.

[tsenart]

Since we're on the topic, this middleware chaining can be modelled as a right fold: http://en.wikipedia.org/wiki/Fold_%28higher-order_function%29

// FoldFunc folds two Endpoints into one.
type FoldFunc func(e, next Endpoint) Endpoint

// FoldR folds the given list of Endpoints from left to right with
// the given FoldFunc.
func FoldR(f FoldFunc, e Endpoint, es ...Endpoint) Endpoint {
    if len(es) == 0 {
        return e
    }
    return f(es[0], FoldR(f, e, es[1:]...))
}

// Chain is a FoldFunc that chains two Endpoints together in order.
// If e returns an error or a non-nil Response, next won't be called.
func Chain(e, next Endpoint) Endpoint {
    return func(ctx context.Context, req Request) (Response, error) {
        if resp, err := e(ctx, req); err != nil {
            return nil, err
        } else if resp != nil {
            return resp, nil
        } else {
            return next(ctx, req)
        }
    }
}

Then to construct the chain you'd use normal Endpoints.

e := FoldR(Chain, makeEndpoint(a),
    recoveryMiddleware,
    server.Gate(zipkin.RequireInContext),
    server.Check(otherProperty),
    myOtherFilter,
    // ...and so on...
)

[bhs]

strong suggestion to make the tracing support pluggable... happy to elaborate/justify as desired!

[eyberg]

I'd agree w/ @bensigelman on this - for example right now this is http only - what if you have a queue that is blowing up and it talks tcp but not http? - you prob wouldn't want to support everything in the kitchen sink so it'd make sense to be pluggable

[peterbourgon]

Alright, I think this has sat in a branch for long enough. If there aren't any huge outstanding problems, I'll merge to master later today.