metrics: correctly reflect closed connections in route metrics

[zaharidichev]

This PR fixes a problem that was introduced in the following commit: 5ac1352

The problem is that if a particular route is configured to fail a connection, the poll_ready call on the inner service will return the error and the service will never get called. Therefore in order to capture that in metrics, we need to increment the metrics counter if we encounter errors while calling poll_ready on the inner service.

Signed-off-by: Zahari Dichev zaharidichev@gmail.com

[cratelyn]

this isn't a comment on this particular change so much as the architecture of this middleware, which predates this PR, but...

a "closed" counter is usually, in my experience, a good place to lean on RAII guards that increment a closed counter when dropped. that could be stored as an Option<Deferred> value that will increment the "unexpected" counter by default when dropped, to work around tricky edge cases like this.

that'd be a more significant change though, so i'll just note the idea here for posterity.

[olix0r]

The problem is that if a particular route is configured to fail a connection, the poll_ready call on the inner service will return the error and the service will never get called.

The poll contract isn't really tied to the connection contract: calling poll_ready doesn't indicate that connection is being attempted, strictly speaking. Services may be polled eagerly or more than once. It would be fragile to encode this here. Also, callers may cache error values returned from poll_ready, so this may not be called as expected.

I think a preferable solution is to modify the stack to replace the push_filter with a service that runs the filter in the request-response path...

[zaharidichev]

@olix0r Addressed feedback in 9c3dc5a

I think there is a way to make this middleware more generic so it can be reused across tls and tcp stacks but did not want to overcomplicate

[olix0r]

I can push a commit that addresses this.