Reviewer comments

src/core/ext/xds/xds_cluster.cc

@@ -724,11 +724,11 @@ absl::StatusOr<std::shared_ptr<const XdsClusterResource>> CdsResourceParse(
         if (google_protobuf_Value_has_string_value(value)) {
           if (UpbStringToAbsl(google_protobuf_Struct_FieldsEntry_key(
                   fields_entry)) == "service_name") {
-            cds_update->service_telemetry_label = RefCountedStringValue(
+            cds_update->service_telemetry_label_ = RefCountedStringValue(
                 UpbStringToAbsl(google_protobuf_Value_string_value(value)));
           } else if (UpbStringToAbsl(google_protobuf_Struct_FieldsEntry_key(
                          fields_entry)) == "service_namespace") {
-            cds_update->namespace_telemetry_label = RefCountedStringValue(
+            cds_update->namespace_telemetry_label_ = RefCountedStringValue(
                 UpbStringToAbsl(google_protobuf_Value_string_value(value)));
           }
         }

src/core/ext/xds/xds_cluster.h

@@ -101,8 +101,8 @@ struct XdsClusterResource : public XdsResourceType::ResourceData {
 
   XdsHealthStatusSet override_host_statuses;
 
-  RefCountedStringValue service_telemetry_label;
-  RefCountedStringValue namespace_telemetry_label;
+  RefCountedStringValue service_telemetry_label_;
+  RefCountedStringValue namespace_telemetry_label_;
 
   bool operator==(const XdsClusterResource& other) const {
     return type == other.type && lb_policy_config == other.lb_policy_config &&

src/core/load_balancing/xds/xds_cluster_impl.cc

@@ -241,8 +241,8 @@ class XdsClusterImplLb final : public LoadBalancingPolicy {
 
     RefCountedPtr<CircuitBreakerCallCounterMap::CallCounter> call_counter_;
     uint32_t max_concurrent_requests_;
-    RefCountedStringValue service_telemetry_label;
-    RefCountedStringValue namespace_telemetry_label;
+    RefCountedStringValue service_telemetry_label_;
+    RefCountedStringValue namespace_telemetry_label_;
     RefCountedPtr<XdsEndpointResource::DropConfig> drop_config_;
     RefCountedPtr<XdsClusterDropStats> drop_stats_;
     RefCountedPtr<SubchannelPicker> picker_;
@@ -386,10 +386,10 @@ XdsClusterImplLb::Picker::Picker(XdsClusterImplLb* xds_cluster_impl_lb,
     : call_counter_(xds_cluster_impl_lb->call_counter_),
       max_concurrent_requests_(
           xds_cluster_impl_lb->cluster_resource_->max_concurrent_requests),
-      service_telemetry_label(
-          xds_cluster_impl_lb->cluster_resource_->service_telemetry_label),
-      namespace_telemetry_label(
-          xds_cluster_impl_lb->cluster_resource_->namespace_telemetry_label),
+      service_telemetry_label_(
+          xds_cluster_impl_lb->cluster_resource_->service_telemetry_label_),
+      namespace_telemetry_label_(
+          xds_cluster_impl_lb->cluster_resource_->namespace_telemetry_label_),
       drop_config_(xds_cluster_impl_lb->drop_config_),
       drop_stats_(xds_cluster_impl_lb->drop_stats_),
       picker_(std::move(picker)) {
@@ -405,13 +405,12 @@ LoadBalancingPolicy::PickResult XdsClusterImplLb::Picker::Pick(
   auto* call_attempt_tracer = call_state->GetCallAttemptTracer();
   if (call_attempt_tracer != nullptr) {
     call_attempt_tracer->SetOptionalLabel(
-        ClientCallTracer::CallAttemptTracer::OptionalLabelKey::
-            kXdsServiceName,
-        service_telemetry_label);
+        ClientCallTracer::CallAttemptTracer::OptionalLabelKey::kXdsServiceName,
+        service_telemetry_label_);
     call_attempt_tracer->SetOptionalLabel(
         ClientCallTracer::CallAttemptTracer::OptionalLabelKey::
             kXdsServiceNamespace,
-        namespace_telemetry_label);
+        namespace_telemetry_label_);
   }
   // Handle EDS drops.
   const std::string* drop_category;

src/cpp/ext/otel/otel_plugin.h

@@ -364,8 +364,6 @@ class OpenTelemetryPlugin : public grpc_core::StatsPlugin {
     grpc_core::RegisteredMetricCallback* key_;
   };
 
-  // StatsPlugin:
-
   // Returns the string form of \a key
   static absl::string_view OptionalLabelKeyToString(
       grpc_core::ClientCallTracer::CallAttemptTracer::OptionalLabelKey key);
@@ -376,6 +374,7 @@ class OpenTelemetryPlugin : public grpc_core::StatsPlugin {
       grpc_core::ClientCallTracer::CallAttemptTracer::OptionalLabelKey>
   OptionalLabelStringToKey(absl::string_view key);
 
+  // StatsPlugin:
   std::pair<bool, std::shared_ptr<grpc_core::StatsPlugin::ScopeConfig>>
   IsEnabledForChannel(
       const OpenTelemetryPluginBuilder::ChannelScope& scope) const override;

test/core/xds/xds_cluster_resource_type_test.cc

@@ -1635,8 +1635,8 @@ TEST_F(TelemetryLabelTest, ValidServiceLabelsConfig) {
   ASSERT_TRUE(decode_result.resource.ok()) << decode_result.resource.status();
   auto& resource =
       static_cast<const XdsClusterResource&>(**decode_result.resource);
-  EXPECT_EQ(resource.service_telemetry_label.as_string_view(), "abc");
-  EXPECT_EQ(resource.namespace_telemetry_label.as_string_view(), "xyz");
+  EXPECT_EQ(resource.service_telemetry_label_.as_string_view(), "abc");
+  EXPECT_EQ(resource.namespace_telemetry_label_.as_string_view(), "xyz");
 }
 
 TEST_F(TelemetryLabelTest, MissingMetadataField) {
@@ -1651,9 +1651,9 @@ TEST_F(TelemetryLabelTest, MissingMetadataField) {
   ASSERT_TRUE(decode_result.resource.ok()) << decode_result.resource.status();
   auto& resource =
       static_cast<const XdsClusterResource&>(**decode_result.resource);
-  EXPECT_THAT(resource.service_telemetry_label.as_string_view(),
+  EXPECT_THAT(resource.service_telemetry_label_.as_string_view(),
               ::testing::IsEmpty());
-  EXPECT_THAT(resource.namespace_telemetry_label.as_string_view(),
+  EXPECT_THAT(resource.namespace_telemetry_label_.as_string_view(),
               ::testing::IsEmpty());
 }
 
@@ -1672,9 +1672,9 @@ TEST_F(TelemetryLabelTest, MissingCsmFilterMetadataField) {
   ASSERT_TRUE(decode_result.resource.ok()) << decode_result.resource.status();
   auto& resource =
       static_cast<const XdsClusterResource&>(**decode_result.resource);
-  EXPECT_THAT(resource.service_telemetry_label.as_string_view(),
+  EXPECT_THAT(resource.service_telemetry_label_.as_string_view(),
               ::testing::IsEmpty());
-  EXPECT_THAT(resource.namespace_telemetry_label.as_string_view(),
+  EXPECT_THAT(resource.namespace_telemetry_label_.as_string_view(),
               ::testing::IsEmpty());
 }
 
@@ -1705,8 +1705,8 @@ TEST_F(TelemetryLabelTest, IgnoreNonServiceLabelEntries) {
   ASSERT_TRUE(decode_result.resource.ok()) << decode_result.resource.status();
   auto& resource =
       static_cast<const XdsClusterResource&>(**decode_result.resource);
-  EXPECT_THAT(resource.service_telemetry_label.as_string_view(), "service");
-  EXPECT_THAT(resource.namespace_telemetry_label.as_string_view(),
+  EXPECT_THAT(resource.service_telemetry_label_.as_string_view(), "service");
+  EXPECT_THAT(resource.namespace_telemetry_label_.as_string_view(),
               ::testing::IsEmpty());
 }