From 84d30afad6fb966ab9b59fff0b381cc57129d958 Mon Sep 17 00:00:00 2001 From: Vindhya Ningegowda Date: Wed, 16 Oct 2024 16:12:27 -0700 Subject: [PATCH] Get mesh_id local label from "CSM_MESH_ID" environment variable, rather than parsing from bootstrap file (#11621) --- .../csm/observability/MetadataExchanger.java | 37 +---------- .../observability/CsmObservabilityTest.java | 65 +++++-------------- .../observability/MetadataExchangerTest.java | 59 ++--------------- 3 files changed, 27 insertions(+), 134 deletions(-) diff --git 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 index bf76c2532bc..5f05d52c7e7 100644 --- 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.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 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 @@ * and remote details to metrics. */ final class MetadataExchanger implements InternalOpenTelemetryPlugin { - private static final Logger logger = Logger.getLogger(MetadataExchanger.class.getName()); private static final AttributeKey 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 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 @@ public MetadataExchanger() { 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 @@ private static Attributes addOtelResourceAttributes(Attributes platformAttribute return builder.build(); } - @VisibleForTesting - static String getMeshId(Supplier xdsBootstrap) { - try { - @SuppressWarnings("unchecked") - Map rawBootstrap = (Map) JsonParser.parse(xdsBootstrap.get()); - Map 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 remote = struct.getFieldsMap(); diff --git 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 index 878bf30ce34..aba2c43c44f 100644 --- 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 void tearDown() { @Test public void unknownDataExchange() throws Exception { - String xdsBootstrap = ""; MetadataExchanger clientExchanger = new MetadataExchanger( Attributes.builder().build(), - ImmutableMap.of()::get, - () -> xdsBootstrap); + ImmutableMap.of()::get); CsmObservability.Builder clientCsmBuilder = new CsmObservability.Builder(clientExchanger) .sdk(openTelemetryTesting.getOpenTelemetry()); MetadataExchanger serverExchanger = new MetadataExchanger( Attributes.builder().build(), - ImmutableMap.of()::get, - () -> xdsBootstrap); + ImmutableMap.of()::get); CsmObservability.Builder serverCsmBuilder = new CsmObservability.Builder(serverExchanger) .sdk(openTelemetryTesting.getOpenTelemetry()); @@ -140,11 +137,9 @@ public void unknownDataExchange() throws Exception { @Test public void nonCsmServer() throws Exception { - String xdsBootstrap = ""; MetadataExchanger clientExchanger = new MetadataExchanger( Attributes.builder().build(), - ImmutableMap.of()::get, - () -> xdsBootstrap); + ImmutableMap.of()::get); CsmObservability.Builder clientCsmBuilder = new CsmObservability.Builder(clientExchanger) .sdk(openTelemetryTesting.getOpenTelemetry()); @@ -205,19 +200,16 @@ public void nonCsmServer() throws Exception { @Test public void nonCsmClient() throws Exception { - String xdsBootstrap = ""; MetadataExchanger clientExchanger = new MetadataExchanger( Attributes.builder() .put(stringKey("cloud.platform"), "gcp_kubernetes_engine") .build(), - ImmutableMap.of()::get, - () -> xdsBootstrap); + ImmutableMap.of()::get); CsmObservability.Builder clientCsmBuilder = new CsmObservability.Builder(clientExchanger) .sdk(openTelemetryTesting.getOpenTelemetry()); MetadataExchanger serverExchanger = new MetadataExchanger( Attributes.builder().build(), - ImmutableMap.of()::get, - () -> xdsBootstrap); + ImmutableMap.of()::get); CsmObservability.Builder serverCsmBuilder = new CsmObservability.Builder(serverExchanger) .sdk(openTelemetryTesting.getOpenTelemetry()); @@ -262,11 +254,6 @@ public void nonCsmClient() throws Exception { @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 void k8sExchange() throws Exception { .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 void k8sExchange() throws Exception { .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 void k8sExchange() throws Exception { @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 void gceExchange() throws Exception { .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 void gceExchange() throws Exception { .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 void gceExchange() throws Exception { @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 void trailersOnly() throws Exception { .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 void trailersOnly() throws Exception { .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()); diff --git 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 index 20665e502f7..cc3472be182 100644 --- 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 @@ /** 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 void enablePluginForChannel_matches() { @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 void enablePluginForChannel_doesNotMatch() { @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 void addLabels_receivedWrongType() { @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 void addMetadata_k8s() throws Exception { .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 void addMetadata_gce() throws Exception { .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);