Get mesh_id local label from "CSM_MESH_ID" environment variable, rather than parsing from bootstrap file (#11621)

2024-10-16 16:12:27 -07:00 · 2024-10-16 16:12:27 -07:00 · 84d30afad6
parent b692b9d26e
commit 84d30afad6
3 changed files with 27 additions and 134 deletions
--- a/gcp-csm-observability/src/main/java/io/grpc/gcp/csm/observability/MetadataExchanger.java
+++ b/gcp-csm-observability/src/main/java/io/grpc/gcp/csm/observability/MetadataExchanger.java
@ -16,7 +16,6 @@

 package io.grpc.gcp.csm.observability;

-import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.io.BaseEncoding;
 import com.google.protobuf.Struct;
@ -29,12 +28,9 @@ import io.grpc.ServerCall;
 import io.grpc.ServerCallHandler;
 import io.grpc.ServerInterceptor;
 import io.grpc.Status;
-import io.grpc.internal.JsonParser;
-import io.grpc.internal.JsonUtil;
 import io.grpc.opentelemetry.InternalOpenTelemetryPlugin;
 import io.grpc.protobuf.ProtoUtils;
 import io.grpc.xds.ClusterImplLoadBalancerProvider;
-import io.grpc.xds.InternalGrpcBootstrapperImpl;
 import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.common.AttributesBuilder;
@ -43,8 +39,6 @@ import io.opentelemetry.sdk.autoconfigure.ResourceConfiguration;
 import java.net.URI;
 import java.util.Map;
 import java.util.function.Consumer;
-import java.util.logging.Level;
-import java.util.logging.Logger;

 /**
 * OpenTelemetryPlugin implementing metadata-based workload property exchange for both client and
@ -52,7 +46,6 @@ import java.util.logging.Logger;
 * and remote details to metrics.
 */
 final class MetadataExchanger implements InternalOpenTelemetryPlugin {
-  private static final Logger logger = Logger.getLogger(MetadataExchanger.class.getName());

  private static final AttributeKey<String> CLOUD_PLATFORM =
      AttributeKey.stringKey("cloud.platform");
@ -89,11 +82,10 @@ final class MetadataExchanger implements InternalOpenTelemetryPlugin {
  public MetadataExchanger() {
    this(
        addOtelResourceAttributes(new GCPResourceProvider().getAttributes()),
-        System::getenv,
-        InternalGrpcBootstrapperImpl::getJsonContent);
+        System::getenv);
  }

-  MetadataExchanger(Attributes platformAttributes, Lookup env, Supplier<String> xdsBootstrap) {
+  MetadataExchanger(Attributes platformAttributes, Lookup env) {
    String type = platformAttributes.get(CLOUD_PLATFORM);
    String canonicalService = env.get("CSM_CANONICAL_SERVICE_NAME");
    Struct.Builder struct = Struct.newBuilder();
@ -121,7 +113,7 @@ final class MetadataExchanger implements InternalOpenTelemetryPlugin {
    localMetadata = BaseEncoding.base64().encode(struct.build().toByteArray());

    localAttributes = Attributes.builder()
-        .put("csm.mesh_id", nullIsUnknown(getMeshId(xdsBootstrap)))
+        .put("csm.mesh_id", nullIsUnknown(env.get("CSM_MESH_ID")))
        .put("csm.workload_canonical_service", nullIsUnknown(canonicalService))
        .build();
  }
@ -162,29 +154,6 @@ final class MetadataExchanger implements InternalOpenTelemetryPlugin {
    return builder.build();
  }

-  @VisibleForTesting
-  static String getMeshId(Supplier<String> xdsBootstrap) {
-    try {
-      @SuppressWarnings("unchecked")
-      Map<String, ?> rawBootstrap = (Map<String, ?>) JsonParser.parse(xdsBootstrap.get());
-      Map<String, ?> node = JsonUtil.getObject(rawBootstrap, "node");
-      String id = JsonUtil.getString(node, "id");
-      Preconditions.checkNotNull(id, "id");
-      String[] parts = id.split("/", 6);
-      if (!(parts.length == 6
-          && parts[0].equals("projects")
-          && parts[2].equals("networks")
-          && parts[3].startsWith("mesh:")
-          && parts[4].equals("nodes"))) {
-        throw new Exception("node id didn't match mesh format: " + id);
-      }
-      return parts[3].substring("mesh:".length());
-    } catch (Exception e) {
-      logger.log(Level.INFO, "Failed to determine mesh ID for CSM", e);
-      return null;
-    }
-  }
-
  private void addLabels(AttributesBuilder to, Struct struct) {
    to.putAll(localAttributes);
    Map<String, Value> remote = struct.getFieldsMap();
--- a/gcp-csm-observability/src/test/java/io/grpc/gcp/csm/observability/CsmObservabilityTest.java
+++ b/gcp-csm-observability/src/test/java/io/grpc/gcp/csm/observability/CsmObservabilityTest.java
@ -77,17 +77,14 @@ public final class CsmObservabilityTest {

  @Test
  public void unknownDataExchange() throws Exception {
-    String xdsBootstrap = "";
    MetadataExchanger clientExchanger = new MetadataExchanger(
        Attributes.builder().build(),
-        ImmutableMap.<String, String>of()::get,
-        () -> xdsBootstrap);
+        ImmutableMap.<String, String>of()::get);
    CsmObservability.Builder clientCsmBuilder = new CsmObservability.Builder(clientExchanger)
        .sdk(openTelemetryTesting.getOpenTelemetry());
    MetadataExchanger serverExchanger = new MetadataExchanger(
        Attributes.builder().build(),
-        ImmutableMap.<String, String>of()::get,
-        () -> xdsBootstrap);
+        ImmutableMap.<String, String>of()::get);
    CsmObservability.Builder serverCsmBuilder = new CsmObservability.Builder(serverExchanger)
        .sdk(openTelemetryTesting.getOpenTelemetry());

@ -140,11 +137,9 @@ public final class CsmObservabilityTest {

  @Test
  public void nonCsmServer() throws Exception {
-    String xdsBootstrap = "";
    MetadataExchanger clientExchanger = new MetadataExchanger(
        Attributes.builder().build(),
-        ImmutableMap.<String, String>of()::get,
-        () -> xdsBootstrap);
+        ImmutableMap.<String, String>of()::get);
    CsmObservability.Builder clientCsmBuilder = new CsmObservability.Builder(clientExchanger)
        .sdk(openTelemetryTesting.getOpenTelemetry());

@ -205,19 +200,16 @@ public final class CsmObservabilityTest {

  @Test
  public void nonCsmClient() throws Exception {
-    String xdsBootstrap = "";
    MetadataExchanger clientExchanger = new MetadataExchanger(
        Attributes.builder()
            .put(stringKey("cloud.platform"), "gcp_kubernetes_engine")
            .build(),
-        ImmutableMap.<String, String>of()::get,
-        () -> xdsBootstrap);
+        ImmutableMap.<String, String>of()::get);
    CsmObservability.Builder clientCsmBuilder = new CsmObservability.Builder(clientExchanger)
        .sdk(openTelemetryTesting.getOpenTelemetry());
    MetadataExchanger serverExchanger = new MetadataExchanger(
        Attributes.builder().build(),
-        ImmutableMap.<String, String>of()::get,
-        () -> xdsBootstrap);
+        ImmutableMap.<String, String>of()::get);
    CsmObservability.Builder serverCsmBuilder = new CsmObservability.Builder(serverExchanger)
        .sdk(openTelemetryTesting.getOpenTelemetry());

@ -262,11 +254,6 @@ public final class CsmObservabilityTest {

  @Test
  public void k8sExchange() throws Exception {
-    // Purposefully use a different project ID in the bootstrap than the resource, as the mesh could
-    // be in a different project than the running account.
-    String clientBootstrap = "{\"node\": {"
-        + "\"id\": \"projects/12/networks/mesh:mymesh/nodes/a6420022-cbc5-4e10-808c-507e3fc95f2e\""
-        + "}}";
    MetadataExchanger clientExchanger = new MetadataExchanger(
        Attributes.builder()
            .put(stringKey("cloud.platform"), "gcp_kubernetes_engine")
@ -277,13 +264,10 @@ public final class CsmObservabilityTest {
            .build(),
        ImmutableMap.of(
            "CSM_CANONICAL_SERVICE_NAME", "canon-service-is-a-client",
-            "CSM_WORKLOAD_NAME", "best-client")::get,
-        () -> clientBootstrap);
+            "CSM_WORKLOAD_NAME", "best-client",
+            "CSM_MESH_ID", "mymesh")::get);
    CsmObservability.Builder clientCsmBuilder = new CsmObservability.Builder(clientExchanger)
        .sdk(openTelemetryTesting.getOpenTelemetry());
-    String serverBootstrap = "{\"node\": {"
-        + "\"id\": \"projects/34/networks/mesh:meshhh/nodes/4969ef19-24b6-44c0-baf3-86d188ff5967\""
-        + "}}";
    MetadataExchanger serverExchanger = new MetadataExchanger(
        Attributes.builder()
            .put(stringKey("cloud.platform"), "gcp_kubernetes_engine")
@ -295,8 +279,8 @@ public final class CsmObservabilityTest {
            .build(),
        ImmutableMap.of(
            "CSM_CANONICAL_SERVICE_NAME", "server-has-a-single-name",
-            "CSM_WORKLOAD_NAME", "fast-server")::get,
-        () -> serverBootstrap);
+            "CSM_WORKLOAD_NAME", "fast-server",
+            "CSM_MESH_ID", "meshhh")::get);
    CsmObservability.Builder serverCsmBuilder = new CsmObservability.Builder(serverExchanger)
        .sdk(openTelemetryTesting.getOpenTelemetry());

@ -366,11 +350,6 @@ public final class CsmObservabilityTest {

  @Test
  public void gceExchange() throws Exception {
-    // Purposefully use a different project ID in the bootstrap than the resource, as the mesh could
-    // be in a different project than the running account.
-    String clientBootstrap = "{\"node\": {"
-        + "\"id\": \"projects/12/networks/mesh:mymesh/nodes/a6420022-cbc5-4e10-808c-507e3fc95f2e\""
-        + "}}";
    MetadataExchanger clientExchanger = new MetadataExchanger(
        Attributes.builder()
            .put(stringKey("cloud.platform"), "gcp_compute_engine")
@ -379,13 +358,10 @@ public final class CsmObservabilityTest {
            .build(),
        ImmutableMap.of(
            "CSM_CANONICAL_SERVICE_NAME", "canon-service-is-a-client",
-            "CSM_WORKLOAD_NAME", "best-client")::get,
-        () -> clientBootstrap);
+            "CSM_WORKLOAD_NAME", "best-client",
+            "CSM_MESH_ID", "mymesh")::get);
    CsmObservability.Builder clientCsmBuilder = new CsmObservability.Builder(clientExchanger)
        .sdk(openTelemetryTesting.getOpenTelemetry());
-    String serverBootstrap = "{\"node\": {"
-        + "\"id\": \"projects/34/networks/mesh:meshhh/nodes/4969ef19-24b6-44c0-baf3-86d188ff5967\""
-        + "}}";
    MetadataExchanger serverExchanger = new MetadataExchanger(
        Attributes.builder()
            .put(stringKey("cloud.platform"), "gcp_compute_engine")
@ -395,8 +371,8 @@ public final class CsmObservabilityTest {
            .build(),
        ImmutableMap.of(
            "CSM_CANONICAL_SERVICE_NAME", "server-has-a-single-name",
-            "CSM_WORKLOAD_NAME", "fast-server")::get,
-        () -> serverBootstrap);
+            "CSM_WORKLOAD_NAME", "fast-server",
+            "CSM_MESH_ID", "meshhh")::get);
    CsmObservability.Builder serverCsmBuilder = new CsmObservability.Builder(serverExchanger)
        .sdk(openTelemetryTesting.getOpenTelemetry());

@ -456,9 +432,6 @@ public final class CsmObservabilityTest {

  @Test
  public void trailersOnly() throws Exception {
-    String clientBootstrap = "{\"node\": {"
-        + "\"id\": \"projects/12/networks/mesh:mymesh/nodes/a6420022-cbc5-4e10-808c-507e3fc95f2e\""
-        + "}}";
    MetadataExchanger clientExchanger = new MetadataExchanger(
        Attributes.builder()
            .put(stringKey("cloud.platform"), "gcp_compute_engine")
@ -467,13 +440,11 @@ public final class CsmObservabilityTest {
            .build(),
        ImmutableMap.of(
            "CSM_CANONICAL_SERVICE_NAME", "canon-service-is-a-client",
-            "CSM_WORKLOAD_NAME", "best-client")::get,
-        () -> clientBootstrap);
+            "CSM_WORKLOAD_NAME", "best-client",
+            "CSM_MESH_ID", "mymesh")::get);
    CsmObservability.Builder clientCsmBuilder = new CsmObservability.Builder(clientExchanger)
        .sdk(openTelemetryTesting.getOpenTelemetry());
-    String serverBootstrap = "{\"node\": {"
-        + "\"id\": \"projects/34/networks/mesh:meshhh/nodes/4969ef19-24b6-44c0-baf3-86d188ff5967\""
-        + "}}";
+
    MetadataExchanger serverExchanger = new MetadataExchanger(
        Attributes.builder()
            .put(stringKey("cloud.platform"), "gcp_compute_engine")
@ -483,8 +454,8 @@ public final class CsmObservabilityTest {
            .build(),
        ImmutableMap.of(
            "CSM_CANONICAL_SERVICE_NAME", "server-has-a-single-name",
-            "CSM_WORKLOAD_NAME", "fast-server")::get,
-        () -> serverBootstrap);
+            "CSM_WORKLOAD_NAME", "fast-server",
+            "CSM_MESH_ID", "meshhh")::get);
    CsmObservability.Builder serverCsmBuilder = new CsmObservability.Builder(serverExchanger)
        .sdk(openTelemetryTesting.getOpenTelemetry());

--- a/gcp-csm-observability/src/test/java/io/grpc/gcp/csm/observability/MetadataExchangerTest.java
+++ b/gcp-csm-observability/src/test/java/io/grpc/gcp/csm/observability/MetadataExchangerTest.java
@ -33,56 +33,11 @@ import org.junit.runners.JUnit4;
 /** Tests for {@link MetadataExchanger}. */
@RunWith(JUnit4.class)
 public final class MetadataExchangerTest {
-  @Test
-  public void getMeshId_findsMeshId() {
-    assertThat(MetadataExchanger.getMeshId(() ->
-        "{\"node\":{\"id\":\"projects/12/networks/mesh:mine/nodes/uu-id\"}}"))
-        .isEqualTo("mine");
-    assertThat(MetadataExchanger.getMeshId(() ->
-          "{\"node\":{\"id\":\"projects/1234567890/networks/mesh:mine/nodes/uu-id\", "
-          + "\"unknown\": \"\"}, \"unknown\": \"\"}"))
-        .isEqualTo("mine");
-  }
-
-  @Test
-  public void getMeshId_returnsNullOnBadMeshId() {
-    assertThat(MetadataExchanger.getMeshId(
-            () -> "[\"node\"]"))
-        .isNull();
-    assertThat(MetadataExchanger.getMeshId(
-            () -> "{\"node\":[\"id\"]}}"))
-        .isNull();
-    assertThat(MetadataExchanger.getMeshId(
-            () -> "{\"node\":{\"id\":[\"projects/12/networks/mesh:mine/nodes/uu-id\"]}}"))
-        .isNull();
-
-    assertThat(MetadataExchanger.getMeshId(
-            () -> "{\"NODE\":{\"id\":\"projects/12/networks/mesh:mine/nodes/uu-id\"}}"))
-        .isNull();
-    assertThat(MetadataExchanger.getMeshId(
-            () -> "{\"node\":{\"ID\":\"projects/12/networks/mesh:mine/nodes/uu-id\"}}"))
-        .isNull();
-    assertThat(MetadataExchanger.getMeshId(
-            () -> "{\"node\":{\"id\":\"projects/12/networks/mesh:mine\"}}"))
-        .isNull();
-    assertThat(MetadataExchanger.getMeshId(
-            () -> "{\"node\":{\"id\":\"PROJECTS/12/networks/mesh:mine/nodes/uu-id\"}}"))
-        .isNull();
-    assertThat(MetadataExchanger.getMeshId(
-            () -> "{\"node\":{\"id\":\"projects/12/NETWORKS/mesh:mine/nodes/uu-id\"}}"))
-        .isNull();
-    assertThat(MetadataExchanger.getMeshId(
-            () -> "{\"node\":{\"id\":\"projects/12/networks/MESH:mine/nodes/uu-id\"}}"))
-        .isNull();
-    assertThat(MetadataExchanger.getMeshId(
-            () -> "{\"node\":{\"id\":\"projects/12/networks/mesh:mine/NODES/uu-id\"}}"))
-        .isNull();
-  }

  @Test
  public void enablePluginForChannel_matches() {
    MetadataExchanger exchanger =
-        new MetadataExchanger(Attributes.builder().build(), (name) -> null, () -> "");
+        new MetadataExchanger(Attributes.builder().build(), (name) -> null);
    assertThat(exchanger.enablePluginForChannel("xds:///testing")).isTrue();
    assertThat(exchanger.enablePluginForChannel("xds:/testing")).isTrue();
    assertThat(exchanger.enablePluginForChannel(
@ -92,7 +47,7 @@ public final class MetadataExchangerTest {
  @Test
  public void enablePluginForChannel_doesNotMatch() {
    MetadataExchanger exchanger =
-        new MetadataExchanger(Attributes.builder().build(), (name) -> null, () -> "");
+        new MetadataExchanger(Attributes.builder().build(), (name) -> null);
    assertThat(exchanger.enablePluginForChannel("dns:///localhost")).isFalse();
    assertThat(exchanger.enablePluginForChannel("xds:///[]")).isFalse();
    assertThat(exchanger.enablePluginForChannel("xds://my-xds-server/testing")).isFalse();
@ -101,7 +56,7 @@ public final class MetadataExchangerTest {
  @Test
  public void addLabels_receivedWrongType() {
    MetadataExchanger exchanger =
-        new MetadataExchanger(Attributes.builder().build(), (name) -> null, () -> "");
+        new MetadataExchanger(Attributes.builder().build(), (name) -> null);
    Metadata metadata = new Metadata();
    metadata.put(Metadata.Key.of("x-envoy-peer-metadata", Metadata.ASCII_STRING_MARSHALLER),
        BaseEncoding.base64().encode(Struct.newBuilder()
@ -122,7 +77,7 @@ public final class MetadataExchangerTest {
  @Test
  public void addLabelsFromExchange_unknownGcpType() {
    MetadataExchanger exchanger =
-        new MetadataExchanger(Attributes.builder().build(), (name) -> null, () -> "");
+        new MetadataExchanger(Attributes.builder().build(), (name) -> null);
    Metadata metadata = new Metadata();
    metadata.put(Metadata.Key.of("x-envoy-peer-metadata", Metadata.ASCII_STRING_MARSHALLER),
        BaseEncoding.base64().encode(Struct.newBuilder()
@ -153,8 +108,7 @@ public final class MetadataExchangerTest {
            .build(),
        ImmutableMap.of(
            "CSM_CANONICAL_SERVICE_NAME", "myservice1",
-            "CSM_WORKLOAD_NAME", "myworkload1")::get,
-        () -> "");
+            "CSM_WORKLOAD_NAME", "myworkload1")::get);
    Metadata metadata = new Metadata();
    exchanger.newClientCallPlugin().addMetadata(metadata);

@ -182,8 +136,7 @@ public final class MetadataExchangerTest {
            .build(),
        ImmutableMap.of(
            "CSM_CANONICAL_SERVICE_NAME", "myservice1",
-            "CSM_WORKLOAD_NAME", "myworkload1")::get,
-        () -> "");
+            "CSM_WORKLOAD_NAME", "myworkload1")::get);
    Metadata metadata = new Metadata();
    exchanger.newClientCallPlugin().addMetadata(metadata);