[GitHub] [incubator-ratis] anshkhannasbu commented on a change in pull request #174: RATIS-1029. NPE at MetricServerInterceptor.interceptCall

[GitBox <gi...@apache.org>]

anshkhannasbu commented on a change in pull request #174:
URL: https://github.com/apache/incubator-ratis/pull/174#discussion_r467547624



##########
File path: ratis-grpc/src/main/java/org/apache/ratis/grpc/metrics/intercept/server/MetricServerInterceptor.java
##########
@@ -62,14 +62,14 @@ private String getMethodMetricPrefix(MethodDescriptor<?, ?> method){
       Metadata requestHeaders,
       ServerCallHandler<R, S> next) {
     MethodDescriptor<R, S> method = call.getMethodDescriptor();
-    if(identifier == null){
+    if (identifier == null) {
       try {
         identifier = peerIdSupplier.get().toString();
-      } catch (Exception e){
+      } catch (Exception e) {
         identifier = defaultIdentifier;
       }
-      metrics = new MessageMetrics(identifier, "server");
     }
+    metrics = new MessageMetrics(identifier, "server");

Review comment:
       This will probably create a new metrics info for every invocation. I think we should move it to an else in case identifier is not null and check if metrics are set or not.
   ```suggestion
       else{
            if(metrics == null){
               metrics = new MessageMetrics(identifier, "server");
            }
       }
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org