Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support routes generated by grpc-gateway #107

Open
ronenh opened this issue Jan 28, 2022 · 0 comments
Open

Support routes generated by grpc-gateway #107

ronenh opened this issue Jan 28, 2022 · 0 comments

Comments

@ronenh
Copy link

ronenh commented Jan 28, 2022

Hello. Thank you for creating and maintaining this wonderful library. I've used it in a number of contexts and never encountered a situation I couldn't solve out-of-the-box using the available config options, or by implementing my own Reporter and handler if I needed non-standard behavior.

However, I recently wanted to collect metrics from the HTTP gateway of a gRPC service and realized that I had to fork Middleware in order to support the routes generated by grpc-gateway.
The discussion in this issue covers all of the relevant details if you feel like going into the weeds, but the gist of it is that the grpc-gateway muxer is a bit of a black box. It generates routes based on annotations to protobuf definitions and packages them all in a single HTTP handler that you can attach to a path prefix on your server (e.g. /api/*).

The result is that no middleware that sits before the grpc-gateway handler can know which route the incoming request is going to be dispatched to which, as far as metrics go, leaves us with two equally bad options:

  1. Use the same handler ID for all the generated routes, which makes these metrics utterly useless. Or,
  2. Use the full URL path, which results in an explosion of metrics in routes that include path parameters (e.g. /api/users/{user_id}/policies/{policy_id}) and again renders the results pretty much useless.

Fortunately, a solution was offered in this post. The idea is to provide hook function that is called by the grpc-gateway muxer after the route has been selected but before the request is actually served.

The problem is that the Measure function in Middleware, expects the handler ID to be known before calling the underlying handler, which doesn't work in this case.

The way I solved it locally is by forking Middleware and slightly modifying Measure to give reporters that didn't provide a handler ID before the call, a second chance to do it after.

The change is very small but that's all I needed to support this use case, which I believe many others may run into. Below is my modified implementation of Measure (the only change is in the deferred func). If it makes sense to you I'd be delighted to open a PR. I can also include my handler and hook function to help others collect grpc-gateway metrics without having to write their own Reporter.

Many thanks!

// Measure abstracts the HTTP handler implementation by only requesting a reporter, this
// reporter will return the required data to be measured.
// it accepts a next function that will be called as the wrapped logic before and after
// measurement actions.
func (m Middleware) Measure(handlerID string, reporter middleware.Reporter, next func()) {
	ctx := reporter.Context()

	// If there isn't predefined handler ID we
	// set that ID as the URL path.
	hid := handlerID
	if handlerID == "" {
		hid = reporter.URLPath()
	}

	// Measure inflights if required.
	if !m.cfg.DisableMeasureInflight && hid != "" {
		props := metrics.HTTPProperties{
			Service: m.cfg.Service,
			ID:      hid,
		}
		m.cfg.Recorder.AddInflightRequests(ctx, props, 1)
		defer m.cfg.Recorder.AddInflightRequests(ctx, props, -1)
	}

	// Start the timer and when finishing measure the duration.
	start := time.Now()
	defer func() {
		duration := time.Since(start)

		// If we need to group the status code, it uses the
		// first number of the status code because is the least
		// required identification way.
		var code string
		if m.cfg.GroupedStatus {
			code = fmt.Sprintf("%dxx", reporter.StatusCode()/100)
		} else {
			code = strconv.Itoa(reporter.StatusCode())
		}

                //************ Modification ***************
                // If reporter was unable to determine the path before calling the underlying handler, try again after.
                // This is the only departure from the original implementation.
		if hid == "" {
			hid = reporter.URLPath()
		}
               //************ End Modification ***************

		props := metrics.HTTPReqProperties{
			Service: m.cfg.Service,
			ID:      hid,
			Method:  reporter.Method(),
			Code:    code,
		}
		m.cfg.Recorder.ObserveHTTPRequestDuration(ctx, props, duration)

		// Measure size of response if required.
		if !m.cfg.DisableMeasureSize {
			m.cfg.Recorder.ObserveHTTPResponseSize(ctx, props, reporter.BytesWritten())
		}
	}()

	// Call the wrapped logic.
	next()
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant