Integrate instrumentation of response codes

[konradreiche]

I am still looking for a way to instrument the HTTP response codes from each handler. I did not find a way to seamlessly integrate it with the instrumentation middleware since the response code becomes only apparent when the response is encoded.

I was pointed to https://github.com/felixge/httpsnoop though. This still leaves me with a possible sub-optimal solution as the wrapping needs to happen in my transport.go where the single handlers are instantiated rather than abstracting it away into my instrumentation.go.

Are there any plans to implement this kind of feature or does someone have a better idea to get this integrated?

[basvanbeek]

http response codes are transport specific and will differ from for instance gRCP or other transport response handling.
Since Go kit abstracts transport concerns from your business logic and thus anything inside your endpoints, you will have to deal with it in your transport response encoding step which can separate what is considered typical http response codes, headers and body payloads.

Typically if the response code or other transport concern is conditional on something happening within your business logic it is needed to be part of the response from an endpoint. If it is conditional on non business related middleware it can be inject into context by serverBefore functions and EncodeResponse functions.

I think the main takeaway needs to be that a 1:1 mapping between your service methods' responses and JSON encoded response payloads on the HTTP transport is not a given.

[konradreiche]

@hardcoar How would that help in this context?

[peterbourgon]

I am open to the idea of amending transport/http.Server with something like an optional Finalizer function to perform tasks like transport-level instrumentation. But capturing the status code can only be reliably achieved with an intercepting http.ResponseWriter, and I'm not sure how to hook that up in a sane way. Thinking out loud...

type ServerFinalizerFunc func(ctx context.Context, code int, r *http.Request)

func ServerFinalizer(finalizer ServerFinalizerFunc) ServerOption {
    return func(s *Server) { s.finalizer = finalizer }
}

func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
    ctx := s.ctx

    if s.finalizer != nil {
        iw := interceptingWriter{w, http.StatusOK}
        defer func() { s.finalizer(ctx, iw.code, r) }()
        w = iw
    }

    for _, f := range s.before {
        ctx = f(ctx, r)
    }

    // ...
}

type interceptingWriter struct {
    http.ResponseWriter
    code int
}

// WriteHeader may not be explicitly called, so care must be taken to
// initialize w.code to its default value of http.StatusOK.
func (w *interceptingWriter) WriteHeader(code int) {
    w.code = code
    w.ResponseWriter.WriteHeader(code)
}

Thoughts?

[xh3b4sd]

What I am currently playing around with is this. Go-kit in its ideas is quite cool. What it does not do, and maybe for good reasons, is to provide a guidance on how to structure your code and implement your endpoints. In particular I am missing a common interface that suggests how to implement an endpoint. So I came up with one on my own.

// Endpoint represents the management of transport logic. An endpoint defines
// what it needs to work properly. Internally it holds a reference to the
// service object which implements business logic and executes any workload.
// That means that network transport and business logic are strictly separated
// and work hand in hand via well defined package APIs.
type Endpoint interface {
	// Decoder returns the kithttp.DecodeRequestFunc used to decode a request
	// before the actual endpoint is executed.
	Decoder() kithttp.DecodeRequestFunc
	// Decoder returns the kithttp.EncodeResponseFunc used to encode a response
	// after the actual endpoint was executed.
	Encoder() kithttp.EncodeResponseFunc
	// Endpoint returns the kitendpoint.Endpoint which receives a decoded response
	// and forwards any workload to the internal service object reference.
	Endpoint() kitendpoint.Endpoint
	// Method returns the HTTP verb used to register the endpoint.
	Method() string
	// Middlewares returns the middlewares the endpoint configures to be run
	// before the actual endpoint is being invoked.
	Middlewares() []kitendpoint.Middleware
	// Name returns the name of the endpoint which can be used to label metrics or
	// annotate logs.
	Name() string
	// Paath returns the HTTP request URL path used to register the endpoint.
	Path() string
}

Every endpoint in my microservice implements this interface. That way it provides everything it needs to work. So I create a list of all initialized endpoints in my server and iterate over each endpoint implementation to register the actual http.Handler, which is wrapped.

// NewRouter returns a HTTP handler for the server. Here we register all
// endpoints listed in the endpoint collection.
func (s *Server) NewRouter() *mux.Router {
	router := mux.NewRouter()

	// We go through all endpoints this server defines and register them to the
	// router.
	for _, e := range s.Endpoints {
		ctx := context.Background()

		decoder := e.Decoder()
		encoder := e.Encoder()
		endpoint := e.Endpoint()
		method := e.Method()
		middlewares := e.Middlewares()
		name := e.Name()
		path := e.Path()

		// Prepare the actual endpoint depending on the provided middlewares of the
		// endpoint implementation. There might be cases in which there are none or
		// only one middleware. The go-kit interface is not that nice so we need to
		// make it fit here.
		{
			if len(middlewares) == 1 {
				endpoint = kitendpoint.Chain(middlewares[0])(endpoint)
			}
			if len(middlewares) > 1 {
				endpoint = kitendpoint.Chain(middlewares[0], middlewares[1:]...)(endpoint)
			}
		}

		// Combine all options this server defines.
		options := []kithttp.ServerOption{
			kithttp.ServerBefore(s.RequestFuncs()...),
			kithttp.ServerErrorEncoder(s.ErrorEncoder()),
		}

		// Register all endpoints to the router depending on their HTTP methods and
		// request paths. The registered http.Handler is instrumented using
		// prometheus. We track counts of execution and duration it took to complete
		// the http.Handler.
		router.Methods(method).Path(path).Handler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
			// Here we define the metrics relevant labels. These will be used to
			// instrument the current request.
			endpointName := strings.Replace(name, "/", "_", -1)
			endpointMethod := strings.ToLower(method)
			endpointCode := http.StatusOK

			// This defered callback will be executed at the very end of the request.
			// When it is executed we know all necessary information to instrument the
			// complete request, including its response status code.
			defer func(t time.Time) {
				// At the time this code is executed the status code is properly set. So
				// we can use it for our instrumentation.
				endpointCode := strconv.Itoa(endpointCode)
				endpointTotal.WithLabelValues(endpointMethod, endpointName, endpointCode).Inc()
				endpointTime.WithLabelValues(endpointMethod, endpointName, endpointCode).Set(float64(time.Since(t) / time.Millisecond))
			}(time.Now())

			// Here we create a new wrapper for the htt.ResponseWriter of the current
			// request. We inject it into the called http.Handler so it can track the
			// status code we are interested in. It will help us gathering the
			// response status code after it was written by the underlying
			// http.ResponseWriter.
			responseWriterConfig := DefaultResponseWriterConfig()
			responseWriterConfig.ResponseWriter = w
			responseWriter, err := NewResponseWriter(responseWriterConfig)
			if err != nil {
				panic(err)
			}
			w = responseWriter

			// Now we execute the actual endpoint handler.
			kithttp.NewServer(
				ctx,
				endpoint,
				decoder,
				encoder,
				options...,
			).ServeHTTP(w, r)

			// Here we now the status code.
			endpointCode = responseWriter.StatusCode()
		}))
	}

	return router
}

The wrapping response writer is basically this.

package server

import "net/http"

// ResponseWriterConfig represents the configuration used to create a new
// response writer.
type ResponseWriterConfig struct {
	// Settings.
	ResponseWriter http.ResponseWriter
	StatusCode     int
}

// DefaultResponseWriterConfig provides a default configuration to create a new
// response writer by best effort.
func DefaultResponseWriterConfig() ResponseWriterConfig {
	return ResponseWriterConfig{
		// Settings.
		ResponseWriter: nil,
		StatusCode:     http.StatusOK,
	}
}

// New creates a new configured response writer.
func NewResponseWriter(config ResponseWriterConfig) (*ResponseWriter, error) {
	// Settings.
	if config.ResponseWriter == nil {
		return nil, maskAnyf(invalidConfigError, "response writer must not be empty")
	}
	if config.StatusCode == 0 {
		return nil, maskAnyf(invalidConfigError, "status code must not be empty")
	}

	newResponseWriter := &ResponseWriter{
		responseWriter: config.ResponseWriter,
		statusCode:     config.StatusCode,
	}

	return newResponseWriter, nil
}

type ResponseWriter struct {
	responseWriter http.ResponseWriter
	statusCode     int
}

func (rw *ResponseWriter) Header() http.Header {
	return rw.responseWriter.Header()
}

func (rw *ResponseWriter) StatusCode() int {
	return rw.statusCode
}

func (rw *ResponseWriter) Write(b []byte) (int, error) {
	return rw.responseWriter.Write(b)
}

func (rw *ResponseWriter) WriteHeader(c int) {
	rw.responseWriter.WriteHeader(c)
	rw.statusCode = c
}

This then looks like this from the metrics side.

$ curl localhost:8080/metrics
# HELP endpoint_milliseconds Time taken to execute the HTTP handler of an endpoint, in milliseconds
# TYPE endpoint_milliseconds gauge
endpoint_milliseconds{code="200",endpoint="option",method="option"} 0
endpoint_milliseconds{code="401",endpoint="cluster_creator",method="post"} 0
# HELP endpoint_total Number of times we have execute the HTTP handler of an endpoint
# TYPE endpoint_total counter
endpoint_total{code="200",endpoint="option",method="option"} 1
endpoint_total{code="401",endpoint="cluster_creator",method="post"} 1

I plan to create a general package for all of our microservices so that you only need to implement the endpoint interface and configure it with your service implementations. I plan to also put the cobra framework into it so that it provides a basic command line tool. Then you can develop microservices super fast.

All this works for me. This might not be perfect for everybody. What I would like though is that maybe something of this can flow into go-kit. Especially the interface is really helpful to build generic code. I think wrapping the actual http.Handler and then also the http.ResponseWriter is a good way to go to solve the issue described here.

The shown code is like one day old at the time writing this comment. So bear with me. There might be bugs. My coding style is also somehow explicit and verbose, but was always a good foundation to build on. I hope I could draw a picture though. Cheers.

[peterbourgon]

This is an interesting approach, though I view it with some guarded suspicion ;) as the collection of all of the Endpoint "things" into an interface strikes me as an abuse of that primitive. More concretely, you're coupling pure-endpoint-domain concepts (Endpoint, Middleware, arguably Name) with the HTTP transport specifically (Decoder, Encoder, Method, Path) — so maybe you want to call that interface HTTPEndpoint? But in any case, this does look promising; I would be very happy to see the outcome!

[xh3b4sd]

You are right. This is the HTTP transport version. As I see it now this makes sense though. Endpoints somehow have to be transport agnostic. Having endpoint specific configuration flying around in different package is not sufficient from my current point of view. So I think each endpoint should be defined in its own package and does therefore care about the transport it is build for. You can have gRPC or whatever kind of endpoint interface though and use the same service references in all of them. I see that technically it would make sense to reuse an encoder for multiple endpoints, but I think an endpoint has to encode payloads for its own purpose. AFAIK it is also common in golang to not prefer the DRY pattern over everything else. Separation of concerns and stable package APIs are far more important for me. All this is off topic for the actual issue. So thanks for listening anyway. :)

[peterbourgon]

I think that you're arguing to couple endpoint+transport as a single thing. But they are decoupled for well-defined reasons. Specifically, there is a lot of value-add that can be applied to abstract endpoints—things like rate limiters and circuit breakers—which are completely independent of transport.

So, to clarify:

So I think each endpoint should be defined in its own package and does therefore care about the transport it is build for.

Endpoints (in the Go kit sense) explicitly don't care about the transport they are attached to. They are designed so that exact same endpoint can be attached to multiple transports simultaneously.

I see that technically it would make sense to reuse an encoder for multiple endpoints, but I think an endpoint has to encode payloads for its own purpose.

In the Go kit view of the world, encoding (and decoding) is a concern of the transport layer, not the endpoint layer. The endpoint is completely agnostic to how its requests and responses are serialized and deserialized.

Hopefully this helps to clarify, not to confuse :)

[peterbourgon]

@konradreiche I went ahead and did what I proposed in #408. Care to take a look? I'll merge sometime in the next 48h, modulo any concerns you raise.

[edwincabezas7]

Hi, what about instrumenting response body ? Im implementing serverFinalizer but the response body is not present, exists a better way to achieve it ?