-
Notifications
You must be signed in to change notification settings - Fork 206
Open
Description
With the new RpcServiceT the BatchResponse is set to MethodResult which is essentially a wrapper of the serialized JSON response but if one needs inspect each individual batch response in batch such as to create a metric for each individual call in the batch that becomes hard.
This prevents to write simple middleware such as:
- Metrics per call
- Easy logging/debugging
- Response ID correlation
To illustrate this have a look the following example:
#[derive(Clone)]
struct Identity<S>(S);
impl<S> RpcServiceT for Identity<S>
where
S: RpcServiceT<BatchResponse = MethodResponse> + Send + Sync + Clone + 'static,
{
type MethodResponse = S::MethodResponse;
type NotificationResponse = S::NotificationResponse;
type BatchResponse = MethodResponse;
fn batch<'a>(&self, batch: Batch<'a>) -> impl Future<Output = Self::BatchResponse> + Send + 'a {
let metrics = HashMap::new();
for entry in batch.iter() {
match entry {
Ok(BatchEntry::Call(c)) => {
metrics.insert(c.id, Instant::now());
}
Ok(BatchEntry::Notification(n)) => continue,
Err(e) => {
metrics.insert(e.id, Instant::now());
}
}
}
async {
let rp = self.0.batch(batch).await;
// need to deserialize JSON to extract the responses and id to match
// with our metrics which is annoying
rp
}
}
fn call<'a>(&self, request: Request<'a>) -> impl Future<Output = Self::MethodResponse> + Send + 'a {
self.0.call(request)
}
fn notification<'a>(&self, n: Notification<'a>) -> impl Future<Output = Self::NotificationResponse> + Send + 'a {
self.0.notification(n)
}
}Thus, I suggest to refactoring this as follows:
- Introduce a separate type BatchResponse (wrapper of Vec) where the individual responses can be iterated over easily
- Add response ID to the MethodResponse to make it easy to match a request with a response.
Metadata
Metadata
Assignees
Labels
No labels