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

Metrics middleware #57

Open
1 of 4 tasks
davidpdrsn opened this issue Feb 12, 2021 · 0 comments
Open
1 of 4 tasks

Metrics middleware #57

davidpdrsn opened this issue Feb 12, 2021 · 0 comments
Labels
A-new-middleware Area: new middleware proposals

Comments

@davidpdrsn
Copy link
Member

davidpdrsn commented Feb 12, 2021

A middleware that provides high level metrics would be really cool.

Should at least cover:

  • Requests per second/minute
  • Number of in flight requests
  • Error rate (this requires the response classification stuff thats being worked here)
  • Throughput, bytes per second/minute

At Embark we use prometheus and the metrics crate. But tower-http should probably allow you to use whatever metrics setup you want.

@davidpdrsn davidpdrsn added A-new-middleware Area: new middleware proposals S-blocked Status: marked as blocked ❌ on something else such as a PR or other implementation work. labels Feb 12, 2021
davidpdrsn added a commit that referenced this issue May 21, 2021
Part of #57

I think the two killer features of tower-http is going to be easy setup of tracing/logging and metrics. Those are non-trivial pieces of functionality that pretty much everyone needs. Now that the [tracing middleware is in](#77) I would like to start thinking about metrics.

My initial goal is to cover these metrics:

- Traffic, requests per second/minute
- Number of in flight requests
- Error rate
- Throughput, bytes per second/minute

Each of those will be a separate middleware, starting with in-flight requests as I think its the simplest.

The module structure I'm thinking is:

- `tower_http::metrics` all middlewares related to metrics are here
- The `metrics` module exposes the most important pieces, so `tower_http::metrics::{InFlightRequests, InFlightRequestsLayer}` and so on when we have more middlewares
- Response futures, bodies and other lower level types are in `tower_http::metrics::in_flight_requests`.

This is similar to how `std::collections` do it.

Example usage of `InFlightRequests`:

```rust
use tower::{Service, ServiceExt, ServiceBuilder};
use tower_http::metrics::InFlightRequestsLayer;
use http::{Request, Response};
use hyper::Body;
use std::{time::Duration, convert::Infallible};

async fn handle(req: Request<Body>) -> Result<Response<Body>, Infallible> {
    // ...
}

async fn update_in_flight_requests_metric(count: usize) {
    // ...
}

// Create a `Layer` with an associated counter.
let (in_flight_requests_layer, counter) = InFlightRequestsLayer::pair();

// Spawn a task that will receive the number of in-flight requests every 10 seconds.
tokio::spawn(
    counter.run_emitter(Duration::from_secs(10), |count| async move {
        update_in_flight_requests_metric(count).await;
    }),
);

let mut service = ServiceBuilder::new()
    // Keep track of the number of in-flight requests. This will increment and decrement
    // `counter` automatically.
    .layer(in_flight_requests_layer)
    .service_fn(handle);

// Call the service.
let response = service
    .ready()
    .await?
    .call(Request::new(Body::empty()))
    .await?;
```

I'm also considering making a `tower-http-metrics` crate that provides integration between `tower-http` and the excellent [`metrics`](https://crates.io/crates/metrics) which we use at Embark.
@davidpdrsn davidpdrsn removed the S-blocked Status: marked as blocked ❌ on something else such as a PR or other implementation work. label May 26, 2021
ParkMyCar added a commit to MaterializeInc/materialize that referenced this issue Jul 13, 2023
### Motivation

Makes progress towards: #20212

This PR adds Prometheus metrics to our externally used HTTP endpoints,
including the new `api/webhook/...` endpoint.

Ideally we don't have to write our own `tower::Layer`, but there doesn't
seem to be any open source `Layer` that I would feel confident using.
There is `axum-prometheus` but it doesn't seem to be too widely used,
and this issue tower-rs/tower-http#57 which
hasn't received much traffic.

So this adds our own `PrometheusLayer` that tracks:
* Total number of requests, per-path, with status code
* Current number of inflight/concurrent requests, per-path
* Histogram of response time for requests, per-path

### Tips for reviewer

 * The diff is much smaller if viewed with whitespace hidden.

### Checklist

- [ ] This PR has adequate test coverage / QA involvement has been duly
considered.
- [ ] This PR has an associated up-to-date [design
doc](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/design/README.md),
is a design doc
([template](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/design/00000000_template.md)),
or is sufficiently small to not require a design.
  <!-- Reference the design in the description. -->
- [ ] If this PR evolves [an existing `$T ⇔ Proto$T`
mapping](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/command-and-response-binary-encoding.md)
(possibly in a backwards-incompatible way), then it is tagged with a
`T-proto` label.
- [ ] If this PR will require changes to cloud orchestration, there is a
companion cloud PR to account for those changes that is tagged with the
release-blocker label
([example](MaterializeInc/cloud#5021)).
<!-- Ask in #team-cloud on Slack if you need help preparing the cloud
PR. -->
- [x] This PR includes the following [user-facing behavior
changes](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/guide-changes.md#what-changes-require-a-release-note):
  - N/a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-new-middleware Area: new middleware proposals
Projects
None yet
Development

No branches or pull requests

1 participant