gcp-observability: add custom tags for all 3 - metrics, logging, trac…

…es and remove old env-vars impl (#9402) * gcp-observability: add custom tags for all 3 - metrics, logging, traces and remove old env-vars impl
grpc · Jul 22, 2022 · 10979b2 · 10979b2
1 parent 027d36e
commit 10979b2
Show file tree

Hide file tree

Showing 6 changed files with 91 additions and 66 deletions.
diff --git a/gcp-observability/src/main/java/io/grpc/gcp/observability/GcpObservability.java b/gcp-observability/src/main/java/io/grpc/gcp/observability/GcpObservability.java
@@ -66,10 +66,10 @@ public final class GcpObservability implements AutoCloseable {
    */
   public static synchronized GcpObservability grpcInit() throws IOException {
     if (instance == null) {
-      GlobalLoggingTags globalLoggingTags = new GlobalLoggingTags();
+      GlobalLocationTags globalLocationTags = new GlobalLocationTags();
       ObservabilityConfigImpl observabilityConfig = ObservabilityConfigImpl.getInstance();
       Sink sink = new GcpLogSink(observabilityConfig.getDestinationProjectId(),
-          globalLoggingTags.getLocationTags(), globalLoggingTags.getCustomTags(),
+          globalLocationTags.getLocationTags(), observabilityConfig.getCustomTags(),
           observabilityConfig.getFlushMessageCount());
       // TODO(dnvindhya): Cleanup code for LoggingChannelProvider and LoggingServerProvider
       // once ChannelBuilder and ServerBuilder are used

diff --git a/.../gcp/observability/GlobalLoggingTags.java → ...gcp/observability/GlobalLocationTags.java b/.../gcp/observability/GlobalLoggingTags.java → ...gcp/observability/GlobalLocationTags.java
@@ -16,8 +16,6 @@
 
 package io.grpc.gcp.observability;
 
-import static com.google.common.base.Preconditions.checkNotNull;
-
 import com.google.api.client.http.HttpTransport;
 import com.google.api.client.http.javanet.NetHttpTransport;
 import com.google.api.client.util.Strings;
@@ -35,20 +33,16 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
-/** A container of all global custom tags used for logging (for now). */
-final class GlobalLoggingTags {
-  private static final Logger logger = Logger.getLogger(GlobalLoggingTags.class.getName());
+/** A container of all global location tags used for observability. */
+final class GlobalLocationTags {
+  private static final Logger logger = Logger.getLogger(GlobalLocationTags.class.getName());
 
-  private static final String ENV_KEY_PREFIX = "GRPC_OBSERVABILITY_";
   private final Map<String, String> locationTags;
-  private final Map<String, String> customTags;
 
-  GlobalLoggingTags() {
+  GlobalLocationTags() {
     ImmutableMap.Builder<String, String> locationTagsBuilder = ImmutableMap.builder();
-    ImmutableMap.Builder<String, String> customTagsBuilder = ImmutableMap.builder();
-    populate(locationTagsBuilder, customTagsBuilder);
+    populate(locationTagsBuilder);
     locationTags = locationTagsBuilder.buildOrThrow();
-    customTags = customTagsBuilder.buildOrThrow();
   }
 
   private static String applyTrim(String value) {
@@ -62,40 +56,36 @@ Map<String, String> getLocationTags() {
     return locationTags;
   }
 
-  Map<String, String> getCustomTags() {
-    return customTags;
-  }
-
   @VisibleForTesting
-  static void populateFromMetadataServer(ImmutableMap.Builder<String, String> customTags) {
+  static void populateFromMetadataServer(ImmutableMap.Builder<String, String> locationTags) {
     MetadataConfig metadataConfig = new MetadataConfig(new DefaultHttpTransportFactory());
     metadataConfig.init();
-    customTags.putAll(metadataConfig.getAllValues());
+    locationTags.putAll(metadataConfig.getAllValues());
   }
 
   @VisibleForTesting
-  static void populateFromKubernetesValues(ImmutableMap.Builder<String, String> customTags,
+  static void populateFromKubernetesValues(ImmutableMap.Builder<String, String> locationTags,
       String namespaceFile,
       String hostnameFile, String cgroupFile) {
     // namespace name: contents of file /var/run/secrets/kubernetes.io/serviceaccount/namespace
-    populateFromFileContents(customTags, "namespace_name",
-        namespaceFile, GlobalLoggingTags::applyTrim);
+    populateFromFileContents(locationTags, "namespace_name",
+        namespaceFile, GlobalLocationTags::applyTrim);
 
     // pod_name: hostname i.e. contents of /etc/hostname
-    populateFromFileContents(customTags, "pod_name", hostnameFile,
-        GlobalLoggingTags::applyTrim);
+    populateFromFileContents(locationTags, "pod_name", hostnameFile,
+        GlobalLocationTags::applyTrim);
 
     // container_id: parsed from /proc/self/cgroup . Note: only works for Linux-based containers
-    populateFromFileContents(customTags, "container_id", cgroupFile,
+    populateFromFileContents(locationTags, "container_id", cgroupFile,
         (value) -> getContainerIdFromFileContents(value));
   }
 
   @VisibleForTesting
-  static void populateFromFileContents(ImmutableMap.Builder<String, String> customTags, String key,
-      String filePath, Function<String, String> parser) {
+  static void populateFromFileContents(ImmutableMap.Builder<String, String> locationTags,
+      String key, String filePath, Function<String, String> parser) {
     String value = parser.apply(readFileContents(filePath));
     if (value != null) {
-      customTags.put(key, value);
+      locationTags.put(key, value);
     }
   }
 
@@ -139,25 +129,7 @@ private static String readFileContents(String file) {
     return null;
   }
 
-  private static void populateFromEnvironmentVars(ImmutableMap.Builder<String, String> customTags) {
-    populateFromMap(System.getenv(), customTags);
-  }
-
-  @VisibleForTesting
-  static void populateFromMap(Map<String, String> map,
-      final ImmutableMap.Builder<String, String> customTags) {
-    checkNotNull(map);
-    map.forEach((k, v) -> {
-      if (k.startsWith(ENV_KEY_PREFIX)) {
-        String customTagKey = k.substring(ENV_KEY_PREFIX.length());
-        customTags.put(customTagKey, v);
-      }
-    });
-  }
-
-  static void populate(ImmutableMap.Builder<String, String> locationTags,
-      ImmutableMap.Builder<String, String> customTags) {
-    populateFromEnvironmentVars(customTags);
+  static void populate(ImmutableMap.Builder<String, String> locationTags) {
     populateFromMetadataServer(locationTags);
     populateFromKubernetesValues(locationTags,
         "/var/run/secrets/kubernetes.io/serviceaccount/namespace",

diff --git a/gcp-observability/src/main/java/io/grpc/gcp/observability/ObservabilityConfig.java b/gcp-observability/src/main/java/io/grpc/gcp/observability/ObservabilityConfig.java
@@ -20,6 +20,7 @@
 import io.grpc.observabilitylog.v1.GrpcLogRecord.EventType;
 import io.opencensus.trace.Sampler;
 import java.util.List;
+import java.util.Map;
 
 @Internal
 public interface ObservabilityConfig {
@@ -47,6 +48,9 @@ public interface ObservabilityConfig {
   /** Get sampler for TraceConfig - when Cloud Tracing is enabled. */
   Sampler getSampler();
 
+  /** Map of all custom tags used for logging, metrics and traces. */
+  Map<String, String> getCustomTags();
+
   /**
    * POJO for representing a filter used in configuration.
    */

diff --git a/gcp-observability/src/main/java/io/grpc/gcp/observability/ObservabilityConfigImpl.java b/gcp-observability/src/main/java/io/grpc/gcp/observability/ObservabilityConfigImpl.java
@@ -20,6 +20,7 @@
 
 import com.google.common.base.Charsets;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import io.grpc.internal.JsonParser;
 import io.grpc.internal.JsonUtil;
 import io.grpc.observabilitylog.v1.GrpcLogRecord.EventType;
@@ -48,6 +49,7 @@ final class ObservabilityConfigImpl implements ObservabilityConfig {
   private List<LogFilter> logFilters;
   private List<EventType> eventTypes;
   private Sampler sampler;
+  private Map<String, String> customTags;
 
   static ObservabilityConfigImpl getInstance() throws IOException {
     ObservabilityConfigImpl config = new ObservabilityConfigImpl();
@@ -120,6 +122,17 @@ private void parseConfig(Map<String, ?> config) {
           this.sampler = Samplers.probabilitySampler(samplingRate);
         }
       }
+      Map<String, ?> rawCustomTags = JsonUtil.getObject(config, "custom_tags");
+      if (rawCustomTags != null) {
+        ImmutableMap.Builder<String, String> builder = new ImmutableMap.Builder<>();
+        for (Map.Entry<String, ?> entry: rawCustomTags.entrySet()) {
+          checkArgument(
+                  entry.getValue() instanceof String,
+                  "'custom_tags' needs to be a map of <string, string>");
+          builder.put(entry.getKey(), (String) entry.getValue());
+        }
+        customTags = builder.build();
+      }
     }
   }
 
@@ -191,4 +204,9 @@ public List<EventType> getEventTypes() {
   public Sampler getSampler() {
     return sampler;
   }
+
+  @Override
+  public Map<String, String> getCustomTags() {
+    return customTags;
+  }
 }
diff --git a/.../observability/GlobalLoggingTagsTest.java → ...observability/GlobalLocationTagsTest.java b/.../observability/GlobalLoggingTagsTest.java → ...observability/GlobalLocationTagsTest.java
@@ -30,7 +30,7 @@
 import org.junit.runners.JUnit4;
 
 @RunWith(JUnit4.class)
-public class GlobalLoggingTagsTest {
+public class GlobalLocationTagsTest {
   private static String FILE_CONTENTS =
       "12:perf_event:/kubepods/burstable/podc43b6442-0725-4fb8-bb1c-d17f5122155c/"
           + "fe61ca6482b58f4a9831d08d6ea15db25f9fd19b4be19a54df8c6c0eab8742b7\n"
@@ -51,32 +51,23 @@ public class GlobalLoggingTagsTest {
   @Rule public TemporaryFolder hostnameFolder = new TemporaryFolder();
   @Rule public TemporaryFolder cgroupFolder = new TemporaryFolder();
 
-  @Test
-  public void testPopulateFromMap() {
-    ImmutableMap.Builder<String, String> customTags = ImmutableMap.builder();
-    GlobalLoggingTags.populateFromMap(
-        ImmutableMap.of("GRPC_OBSERVABILITY_KEY1", "VALUE1", "ANOTHER_KEY2", "VALUE2",
-            "GRPC_OBSERVABILITY_KEY3", "VALUE3"), customTags);
-    assertThat(customTags.buildOrThrow()).containsExactly("KEY1", "VALUE1", "KEY3", "VALUE3");
-  }
-
   @Test
   public void testContainerIdParsing_lastLine() {
-    String containerId = GlobalLoggingTags.getContainerIdFromFileContents(FILE_CONTENTS_LAST_LINE);
+    String containerId = GlobalLocationTags.getContainerIdFromFileContents(FILE_CONTENTS_LAST_LINE);
     assertThat(containerId).isEqualTo("e19a54df");
   }
 
   @Test
   public void testContainerIdParsing_fewerFields_notFound() {
-    String containerId = GlobalLoggingTags.getContainerIdFromFileContents(
+    String containerId = GlobalLocationTags.getContainerIdFromFileContents(
         "12:/kubepods/burstable/podc43b6442-0725-4fb8-bb1c-d17f5122155c/"
             + "fe61ca6482b58f4a9831d08d6ea15db25f9fd19b4be19a54df8c6c0eab8742b7\n");
     assertThat(containerId).isNull();
   }
 
   @Test
   public void testContainerIdParsing_fewerPaths_notFound() {
-    String containerId = GlobalLoggingTags.getContainerIdFromFileContents(
+    String containerId = GlobalLocationTags.getContainerIdFromFileContents(
         "12:xdf:/kubepods/podc43b6442-0725-4fb8-bb1c-d17f5122155c/"
             + "fe61ca6482b58f4a9831d08d6ea15db25f9fd19b4be19a54df8c6c0eab8742b7\n");
     assertThat(containerId).isNull();
@@ -92,10 +83,10 @@ public void testPopulateKubernetesValues() throws IOException {
     Files.write("test-hostname2\n".getBytes(StandardCharsets.UTF_8), hostnameFile);
     Files.write(FILE_CONTENTS.getBytes(StandardCharsets.UTF_8), cgroupFile);
 
-    ImmutableMap.Builder<String, String> customTags = ImmutableMap.builder();
-    GlobalLoggingTags.populateFromKubernetesValues(customTags, namespaceFile.getAbsolutePath(),
+    ImmutableMap.Builder<String, String> locationTags = ImmutableMap.builder();
+    GlobalLocationTags.populateFromKubernetesValues(locationTags, namespaceFile.getAbsolutePath(),
         hostnameFile.getAbsolutePath(), cgroupFile.getAbsolutePath());
-    assertThat(customTags.buildOrThrow()).containsExactly("container_id",
+    assertThat(locationTags.buildOrThrow()).containsExactly("container_id",
         "fe61ca6482b58f4a9831d08d6ea15db25f9fd19b4be19a54df8c6c0eab8742b7", "namespace_name",
         "test-namespace1", "pod_name", "test-hostname2");
   }
@@ -109,10 +100,10 @@ public void testNonKubernetesInstanceValues() throws IOException {
     Files.write("test-hostname2\n".getBytes(StandardCharsets.UTF_8), hostnameFile);
     Files.write(FILE_CONTENTS.getBytes(StandardCharsets.UTF_8), cgroupFile);
 
-    ImmutableMap.Builder<String, String> customTags = ImmutableMap.builder();
-    GlobalLoggingTags.populateFromKubernetesValues(customTags,
+    ImmutableMap.Builder<String, String> locationTags = ImmutableMap.builder();
+    GlobalLocationTags.populateFromKubernetesValues(locationTags,
         namespaceFilePath, hostnameFile.getAbsolutePath(), cgroupFile.getAbsolutePath());
-    assertThat(customTags.buildOrThrow()).containsExactly("container_id",
+    assertThat(locationTags.buildOrThrow()).containsExactly("container_id",
         "fe61ca6482b58f4a9831d08d6ea15db25f9fd19b4be19a54df8c6c0eab8742b7",
         "pod_name", "test-hostname2");
   }

diff --git a/gcp-observability/src/test/java/io/grpc/gcp/observability/ObservabilityConfigImplTest.java b/gcp-observability/src/test/java/io/grpc/gcp/observability/ObservabilityConfigImplTest.java
@@ -33,6 +33,7 @@
 import java.nio.file.Files;
 import java.nio.file.Paths;
 import java.util.List;
+import java.util.Map;
 
 import org.junit.Rule;
 import org.junit.Test;
@@ -106,6 +107,23 @@ public class ObservabilityConfigImplTest {
       + "    \"global_trace_sampling_rate\": -0.75\n"
       + "}";
 
+  private static final String CUSTOM_TAGS = "{\n"
+          + "    \"enable_cloud_logging\": true,\n"
+          + "    \"custom_tags\": {\n"
+          + "      \"SOURCE_VERSION\" : \"J2e1Cf\",\n"
+          + "      \"SERVICE_NAME\" : \"payment-service\",\n"
+          + "      \"ENTRYPOINT_SCRIPT\" : \"entrypoint.sh\"\n"
+          + "    }\n"
+          + "}";
+
+  private static final String BAD_CUSTOM_TAGS = "{\n"
+          + "    \"enable_cloud_monitoring\": true,\n"
+          + "    \"custom_tags\": {\n"
+          + "      \"SOURCE_VERSION\" : \"J2e1Cf\",\n"
+          + "      \"SERVICE_NAME\" : { \"SUB_SERVICE_NAME\" : \"payment-service\"},\n"
+          + "      \"ENTRYPOINT_SCRIPT\" : \"entrypoint.sh\"\n"
+          + "    }\n"
+          + "}";
 
   ObservabilityConfigImpl observabilityConfig = new ObservabilityConfigImpl();
 
@@ -257,4 +275,26 @@ public void configFileLogFilters() throws Exception {
     assertThat(logFilters.get(1).headerBytes).isNull();
     assertThat(logFilters.get(1).messageBytes).isNull();
   }
+
+  @Test
+  public void customTags() throws IOException {
+    observabilityConfig.parse(CUSTOM_TAGS);
+    assertTrue(observabilityConfig.isEnableCloudLogging());
+    Map<String, String> customTags = observabilityConfig.getCustomTags();
+    assertThat(customTags).hasSize(3);
+    assertThat(customTags).containsEntry("SOURCE_VERSION", "J2e1Cf");
+    assertThat(customTags).containsEntry("SERVICE_NAME", "payment-service");
+    assertThat(customTags).containsEntry("ENTRYPOINT_SCRIPT", "entrypoint.sh");
+  }
+
+  @Test
+  public void badCustomTags() throws IOException {
+    try {
+      observabilityConfig.parse(BAD_CUSTOM_TAGS);
+      fail("exception expected!");
+    } catch (IllegalArgumentException iae) {
+      assertThat(iae.getMessage()).isEqualTo(
+              "'custom_tags' needs to be a map of <string, string>");
+    }
+  }
 }