observability: change the config parsing to not require logging_config (

#9023) and make Observability implement AutoCloseable
grpc · Mar 28, 2022 · 9b1023b · 9b1023b
1 parent 4a84c6f
commit 9b1023b
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 24 deletions.
diff --git a/observability/src/main/java/io/grpc/observability/Observability.java b/observability/src/main/java/io/grpc/observability/Observability.java
@@ -29,7 +29,7 @@
 
 /** The main class for gRPC Observability features. */
 @ExperimentalApi("https://github.com/grpc/grpc-java/issues/8869")
-public final class Observability {
+public final class Observability implements AutoCloseable {
   private static Observability instance = null;
   private final Sink sink;
 
@@ -64,10 +64,11 @@ public static synchronized Observability grpcInit() throws IOException {
   }
 
   /** Un-initialize/shutdown grpc-observability. */
-  public void grpcShutdown() {
+  @Override
+  public void close() {
     synchronized (Observability.class) {
       if (instance == null) {
-        throw new IllegalStateException("Observability already shutdown!");
+        throw new IllegalStateException("Observability already closed!");
       }
       LoggingChannelProvider.shutdown();
       LoggingServerProvider.shutdown();

diff --git a/observability/src/main/java/io/grpc/observability/ObservabilityConfigImpl.java b/observability/src/main/java/io/grpc/observability/ObservabilityConfigImpl.java
@@ -43,8 +43,7 @@ static ObservabilityConfigImpl getInstance() throws IOException {
   @SuppressWarnings("unchecked")
   void parse(String config) throws IOException {
     checkArgument(config != null, CONFIG_ENV_VAR_NAME + " value is null!");
-    parseLoggingConfig(
-        JsonUtil.getObject((Map<String, ?>) JsonParser.parse(config), "logging_config"));
+    parseLoggingConfig((Map<String, ?>) JsonParser.parse(config));
   }
 
   private void parseLoggingConfig(Map<String,?> loggingConfig) {

diff --git a/observability/src/test/java/io/grpc/observability/ObservabilityConfigImplTest.java b/observability/src/test/java/io/grpc/observability/ObservabilityConfigImplTest.java
@@ -31,15 +31,12 @@
 @RunWith(JUnit4.class)
 public class ObservabilityConfigImplTest {
   private static final String EVENT_TYPES = "{\n"
-      + "  \"logging_config\": {\n"
       + "    \"enable_cloud_logging\": false,\n"
       + "    \"event_types\": "
       + "[\"GRPC_CALL_REQUEST_HEADER\", \"GRPC_CALL_HALF_CLOSE\", \"GRPC_CALL_TRAILER\"]\n"
-      + "  }\n"
       + "}";
 
   private static final String LOG_FILTERS = "{\n"
-      + "  \"logging_config\": {\n"
       + "    \"enable_cloud_logging\": true,\n"
       + "    \"destination_project_id\": \"grpc-testing\",\n"
       + "    \"log_filters\": [{\n"
@@ -51,19 +48,15 @@ public class ObservabilityConfigImplTest {
       + "        \"pattern\": \"service1/Method2\"\n"
       + "    }"
       + "    ]\n"
-      + "  }\n"
       + "}";
 
   private static final String DEST_PROJECT_ID = "{\n"
-      + "  \"logging_config\": {\n"
       + "    \"enable_cloud_logging\": true,\n"
       + "    \"destination_project_id\": \"grpc-testing\"\n"
-      + "  }\n"
       + "}";
 
   private static final String DISABLE_CLOUD_LOGGING = "{\n"
-      + "  \"logging_config\": {\n"
-      + "    \"enable_cloud_logging\": false\n" + "  }\n"
+      + "    \"enable_cloud_logging\": false\n"
       + "}";
 
   ObservabilityConfigImpl observabilityConfig = new ObservabilityConfigImpl();

diff --git a/observability/src/test/java/io/grpc/observability/ObservabilityTest.java b/observability/src/test/java/io/grpc/observability/ObservabilityTest.java
@@ -42,23 +42,24 @@ public void initFinish() {
         InternalLoggingChannelInterceptor.Factory.class);
     InternalLoggingServerInterceptor.Factory serverInterceptorFactory = mock(
         InternalLoggingServerInterceptor.Factory.class);
-    Observability observability = Observability.grpcInit(sink, channelInterceptorFactory,
-        serverInterceptorFactory);
-    assertThat(ManagedChannelProvider.provider()).isInstanceOf(LoggingChannelProvider.class);
-    assertThat(ServerProvider.provider()).isInstanceOf(ServerProvider.class);
-    Observability observability1 = Observability.grpcInit(sink, channelInterceptorFactory,
-        serverInterceptorFactory);
-    assertThat(observability1).isSameInstanceAs(observability);
+    Observability observability1;
+    try (Observability observability = Observability.grpcInit(sink, channelInterceptorFactory,
+        serverInterceptorFactory)) {
+      assertThat(ManagedChannelProvider.provider()).isInstanceOf(LoggingChannelProvider.class);
+      assertThat(ServerProvider.provider()).isInstanceOf(ServerProvider.class);
+      observability1 = Observability.grpcInit(sink, channelInterceptorFactory,
+              serverInterceptorFactory);
+      assertThat(observability1).isSameInstanceAs(observability);
 
-    observability.grpcShutdown();
+    }
     verify(sink).close();
     assertThat(ManagedChannelProvider.provider()).isSameInstanceAs(prevChannelProvider);
     assertThat(ServerProvider.provider()).isSameInstanceAs(prevServerProvider);
     try {
-      observability.grpcShutdown();
-      fail("should have failed for calling grpcShutdown() second time");
+      observability1.close();
+      fail("should have failed for calling close() second time");
     } catch (IllegalStateException e) {
-      assertThat(e).hasMessageThat().contains("Observability already shutdown!");
+      assertThat(e).hasMessageThat().contains("Observability already closed!");
     }
   }
 }