From d9c4f307627573a1a7da83916ccc50428f995b4b Mon Sep 17 00:00:00 2001
From: ZHANG Dapeng <zdapeng@google.com>
Date: Fri, 29 Oct 2021 11:46:00 -0700
Subject: [PATCH] xds: lazily init MessagePrinter (#8639)

Just for cleanup. The printer might be used in other class e.g. to convert RLS proto to string/Map.
---
 .../java/io/grpc/xds/AbstractXdsClient.java   | 19 +++--
 .../main/java/io/grpc/xds/MessagePrinter.java | 70 ++++++++++---------
 .../java/io/grpc/xds/MessagePrinterTest.java  |  9 ++-
 3 files changed, 54 insertions(+), 44 deletions(-)

diff --git a/xds/src/main/java/io/grpc/xds/AbstractXdsClient.java b/xds/src/main/java/io/grpc/xds/AbstractXdsClient.java
index a6c3c2feb99a..0e609ff74587 100644
--- a/xds/src/main/java/io/grpc/xds/AbstractXdsClient.java
+++ b/xds/src/main/java/io/grpc/xds/AbstractXdsClient.java
@@ -78,7 +78,6 @@ public void uncaughtException(Thread t, Throwable e) {
           throw new AssertionError(e);
         }
       });
-  private final MessagePrinter msgPrinter = new MessagePrinter();
   private final InternalLogId logId;
   private final XdsLogger logger;
   private final ManagedChannel channel;
@@ -580,8 +579,9 @@ public void onNext(final io.envoyproxy.envoy.api.v2.DiscoveryResponse response)
                 public void run() {
                   ResourceType type = ResourceType.fromTypeUrl(response.getTypeUrl());
                   if (logger.isLoggable(XdsLogLevel.DEBUG)) {
-                    logger.log(XdsLogLevel.DEBUG, "Received {0} response:\n{1}",
-                        type, msgPrinter.print(response));
+                    logger.log(
+                        XdsLogLevel.DEBUG, "Received {0} response:\n{1}", type,
+                        MessagePrinter.print(response));
                   }
                   handleRpcResponse(type, response.getVersionInfo(), response.getResourcesList(),
                       response.getNonce());
@@ -633,7 +633,9 @@ void sendDiscoveryRequest(ResourceType type, String versionInfo, Collection<Stri
       }
       io.envoyproxy.envoy.api.v2.DiscoveryRequest request = builder.build();
       requestWriter.onNext(request);
-      logger.log(XdsLogLevel.DEBUG, "Sent DiscoveryRequest\n{0}", msgPrinter.print(request));
+      if (logger.isLoggable(XdsLogLevel.DEBUG)) {
+        logger.log(XdsLogLevel.DEBUG, "Sent DiscoveryRequest\n{0}", MessagePrinter.print(request));
+      }
     }
 
     @Override
@@ -657,8 +659,9 @@ public void onNext(final DiscoveryResponse response) {
             public void run() {
               ResourceType type = ResourceType.fromTypeUrl(response.getTypeUrl());
               if (logger.isLoggable(XdsLogLevel.DEBUG)) {
-                logger.log(XdsLogLevel.DEBUG, "Received {0} response:\n{1}",
-                    type, msgPrinter.print(response));
+                logger.log(
+                    XdsLogLevel.DEBUG, "Received {0} response:\n{1}", type,
+                    MessagePrinter.print(response));
               }
               handleRpcResponse(type, response.getVersionInfo(), response.getResourcesList(),
                   response.getNonce());
@@ -710,7 +713,9 @@ void sendDiscoveryRequest(ResourceType type, String versionInfo, Collection<Stri
       }
       DiscoveryRequest request = builder.build();
       requestWriter.onNext(request);
-      logger.log(XdsLogLevel.DEBUG, "Sent DiscoveryRequest\n{0}", msgPrinter.print(request));
+      if (logger.isLoggable(XdsLogLevel.DEBUG)) {
+        logger.log(XdsLogLevel.DEBUG, "Sent DiscoveryRequest\n{0}", MessagePrinter.print(request));
+      }
     }
 
     @Override
diff --git a/xds/src/main/java/io/grpc/xds/MessagePrinter.java b/xds/src/main/java/io/grpc/xds/MessagePrinter.java
index edddcb7a4658..249e1f0e9659 100644
--- a/xds/src/main/java/io/grpc/xds/MessagePrinter.java
+++ b/xds/src/main/java/io/grpc/xds/MessagePrinter.java
@@ -38,43 +38,49 @@
  * containing {@link com.google.protobuf.Any} fields.
  */
 final class MessagePrinter {
-  private final JsonFormat.Printer printer;
 
-  MessagePrinter() {
-    TypeRegistry registry =
-        TypeRegistry.newBuilder()
-            .add(Listener.getDescriptor())
-            .add(io.envoyproxy.envoy.api.v2.Listener.getDescriptor())
-            .add(HttpConnectionManager.getDescriptor())
-            .add(io.envoyproxy.envoy.config.filter.network.http_connection_manager.v2
-                .HttpConnectionManager.getDescriptor())
-            .add(HTTPFault.getDescriptor())
-            .add(io.envoyproxy.envoy.config.filter.http.fault.v2.HTTPFault.getDescriptor())
-            .add(RBAC.getDescriptor())
-            .add(RBACPerRoute.getDescriptor())
-            .add(Router.getDescriptor())
-            .add(io.envoyproxy.envoy.config.filter.http.router.v2.Router.getDescriptor())
-            // UpstreamTlsContext and DownstreamTlsContext in v3 are not transitively imported
-            // by top-level resource types.
-            .add(UpstreamTlsContext.getDescriptor())
-            .add(DownstreamTlsContext.getDescriptor())
-            .add(RouteConfiguration.getDescriptor())
-            .add(io.envoyproxy.envoy.api.v2.RouteConfiguration.getDescriptor())
-            .add(Cluster.getDescriptor())
-            .add(io.envoyproxy.envoy.api.v2.Cluster.getDescriptor())
-            .add(ClusterConfig.getDescriptor())
-            .add(io.envoyproxy.envoy.config.cluster.aggregate.v2alpha.ClusterConfig
-                .getDescriptor())
-            .add(ClusterLoadAssignment.getDescriptor())
-            .add(io.envoyproxy.envoy.api.v2.ClusterLoadAssignment.getDescriptor())
-            .build();
-    printer = JsonFormat.printer().usingTypeRegistry(registry);
+  private MessagePrinter() {}
+
+  // The initialization-on-demand holder idiom.
+  private static class LazyHolder {
+    static final JsonFormat.Printer printer = newPrinter();
+
+    private static JsonFormat.Printer newPrinter() {
+      TypeRegistry registry =
+          TypeRegistry.newBuilder()
+              .add(Listener.getDescriptor())
+              .add(io.envoyproxy.envoy.api.v2.Listener.getDescriptor())
+              .add(HttpConnectionManager.getDescriptor())
+              .add(io.envoyproxy.envoy.config.filter.network.http_connection_manager.v2
+                  .HttpConnectionManager.getDescriptor())
+              .add(HTTPFault.getDescriptor())
+              .add(io.envoyproxy.envoy.config.filter.http.fault.v2.HTTPFault.getDescriptor())
+              .add(RBAC.getDescriptor())
+              .add(RBACPerRoute.getDescriptor())
+              .add(Router.getDescriptor())
+              .add(io.envoyproxy.envoy.config.filter.http.router.v2.Router.getDescriptor())
+              // UpstreamTlsContext and DownstreamTlsContext in v3 are not transitively imported
+              // by top-level resource types.
+              .add(UpstreamTlsContext.getDescriptor())
+              .add(DownstreamTlsContext.getDescriptor())
+              .add(RouteConfiguration.getDescriptor())
+              .add(io.envoyproxy.envoy.api.v2.RouteConfiguration.getDescriptor())
+              .add(Cluster.getDescriptor())
+              .add(io.envoyproxy.envoy.api.v2.Cluster.getDescriptor())
+              .add(ClusterConfig.getDescriptor())
+              .add(io.envoyproxy.envoy.config.cluster.aggregate.v2alpha.ClusterConfig
+                  .getDescriptor())
+              .add(ClusterLoadAssignment.getDescriptor())
+              .add(io.envoyproxy.envoy.api.v2.ClusterLoadAssignment.getDescriptor())
+              .build();
+      return JsonFormat.printer().usingTypeRegistry(registry);
+    }
   }
 
-  String print(MessageOrBuilder message) {
+  static String print(MessageOrBuilder message) {
     String res;
     try {
-      res = printer.print(message);
+      res = LazyHolder.printer.print(message);
     } catch (InvalidProtocolBufferException e) {
       res = message + " (failed to pretty-print: " + e + ")";
     }
diff --git a/xds/src/test/java/io/grpc/xds/MessagePrinterTest.java b/xds/src/test/java/io/grpc/xds/MessagePrinterTest.java
index bb6c5517ab99..7b6942583756 100644
--- a/xds/src/test/java/io/grpc/xds/MessagePrinterTest.java
+++ b/xds/src/test/java/io/grpc/xds/MessagePrinterTest.java
@@ -64,7 +64,6 @@
  */
 @RunWith(JUnit4.class)
 public class MessagePrinterTest {
-  private final MessagePrinter printer = new MessagePrinter();
 
   @Test
   public void printLdsResponse_v3() {
@@ -151,7 +150,7 @@ public void printLdsResponse_v3() {
         + "  \"typeUrl\": \"type.googleapis.com/envoy.config.listener.v3.Listener\",\n"
         + "  \"nonce\": \"0000\"\n"
         + "}";
-    String res = printer.print(response);
+    String res = MessagePrinter.print(response);
     assertThat(res).isEqualTo(expectedString);
   }
 
@@ -202,7 +201,7 @@ public void printRdsResponse_v3() {
         + "  \"typeUrl\": \"type.googleapis.com/envoy.config.route.v3.RouteConfiguration\",\n"
         + "  \"nonce\": \"0000\"\n"
         + "}";
-    String res = printer.print(response);
+    String res = MessagePrinter.print(response);
     assertThat(res).isEqualTo(expectedString);
   }
 
@@ -267,7 +266,7 @@ public void printCdsResponse_v3() {
         + "  \"typeUrl\": \"type.googleapis.com/envoy.config.cluster.v3.Cluster\",\n"
         + "  \"nonce\": \"0000\"\n"
         + "}";
-    String res = printer.print(response);
+    String res = MessagePrinter.print(response);
     assertThat(res).isEqualTo(expectedString);
   }
 
@@ -356,7 +355,7 @@ public void printEdsResponse_v3() {
         + ".ClusterLoadAssignment\",\n"
         + "  \"nonce\": \"0000\"\n"
         + "}";
-    String res = printer.print(response);
+    String res = MessagePrinter.print(response);
     assertThat(res).isEqualTo(expectedString);
   }
 }