Skip to content

Commit

Permalink
Improve performance of MonitoringInterceptors (#18)
Browse files Browse the repository at this point in the history
* Improve performance of MonitoringInterceptors

The current implementation has multiple GrpcMethod.of() calls which are unnecessary. The method in turn calls three String methods (.substring twice, .lastIndexOf once) which add unnecessary overhead which should be avoided when collecting metrics.

* Fix typo
  • Loading branch information
dadadom authored and dinowernli committed Aug 11, 2019
1 parent 1688b5a commit f7c056c
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,10 @@ static class Factory {
}
}

/** Creates a {@link ClientMetrics} for the supplied method. */
<R, S> ClientMetrics createMetricsForMethod(MethodDescriptor<R, S> methodDescriptor) {
/** Creates a {@link ClientMetrics} for the supplied gRPC method. */
ClientMetrics createMetricsForMethod(GrpcMethod grpcMethod) {
return new ClientMetrics(
GrpcMethod.of(methodDescriptor),
grpcMethod,
rpcStarted,
rpcCompleted,
streamMessagesReceived,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,12 @@ private MonitoringClientInterceptor(
@Override
public <R, S> ClientCall<R, S> interceptCall(
MethodDescriptor<R, S> methodDescriptor, CallOptions callOptions, Channel channel) {
ClientMetrics metrics = clientMetricsFactory.createMetricsForMethod(methodDescriptor);
GrpcMethod grpcMethod = GrpcMethod.of(methodDescriptor);
ClientMetrics metrics = clientMetricsFactory.createMetricsForMethod(grpcMethod);
return new MonitoringClientCall<>(
channel.newCall(methodDescriptor, callOptions),
metrics,
GrpcMethod.of(methodDescriptor),
grpcMethod,
configuration,
clock);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,12 @@ public <R, S> ServerCall.Listener<R> interceptCall(
ServerCall<R, S> call,
Metadata requestHeaders,
ServerCallHandler<R, S> next) {
MethodDescriptor<R, S> method = call.getMethodDescriptor();
ServerMetrics metrics = serverMetricsFactory.createMetricsForMethod(method);
GrpcMethod grpcMethod = GrpcMethod.of(method);
MethodDescriptor<R, S> methodDescriptor = call.getMethodDescriptor();
GrpcMethod grpcMethod = GrpcMethod.of(methodDescriptor);
ServerMetrics metrics = serverMetricsFactory.createMetricsForMethod(grpcMethod);
ServerCall<R,S> monitoringCall = new MonitoringServerCall(call, clock, grpcMethod, metrics, configuration);
return new MonitoringServerCallListener<>(
next.startCall(monitoringCall, requestHeaders), metrics, GrpcMethod.of(method));
next.startCall(monitoringCall, requestHeaders), metrics, grpcMethod);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,10 @@ static class Factory {
}
}

/** Creates a {@link ServerMetrics} for the supplied method. */
<R, S> ServerMetrics createMetricsForMethod(MethodDescriptor<R, S> methodDescriptor) {
/** Creates a {@link ServerMetrics} for the supplied gRPC method. */
ServerMetrics createMetricsForMethod(GrpcMethod grpcMethod) {
return new ServerMetrics(
GrpcMethod.of(methodDescriptor),
grpcMethod,
serverStarted,
serverHandled,
serverStreamMessagesReceived,
Expand Down

1 comment on commit f7c056c

@nmayakuntla
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dadadom @dinowernli Would there be a new release with this change?

Please sign in to comment.