From c5390c99a11c854bb15bb86434d38ec7329719e8 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Mon, 8 Apr 2024 15:38:50 +0000 Subject: [PATCH] Fixes --- BUILD | 1 + src/core/BUILD | 3 +++ src/core/ext/xds/xds_cluster.cc | 4 ++-- src/core/ext/xds/xds_cluster.h | 8 +++++--- src/core/lib/channel/call_tracer.h | 1 + src/core/load_balancing/xds/xds_cluster_impl.cc | 6 ++++-- src/core/load_balancing/xds/xds_wrr_locality.cc | 1 + src/core/resolver/xds/xds_dependency_manager.cc | 2 ++ test/core/xds/xds_cluster_resource_type_test.cc | 16 ++++++++-------- 9 files changed, 27 insertions(+), 15 deletions(-) diff --git a/BUILD b/BUILD index 0926e9321b3..891ece92987 100644 --- a/BUILD +++ b/BUILD @@ -1720,6 +1720,7 @@ grpc_cc_library( "//src/core:context", "//src/core:error", "//src/core:metadata_batch", + "//src/core:ref_counted_string", "//src/core:slice_buffer", ], ) diff --git a/src/core/BUILD b/src/core/BUILD index 2e5ea79b716..c8140bfdce2 100644 --- a/src/core/BUILD +++ b/src/core/BUILD @@ -5348,10 +5348,12 @@ grpc_cc_library( "match", "pollset_set", "ref_counted", + "ref_counted_string", "resolved_address", "subchannel_interface", "validation_errors", "xds_dependency_manager", + "//:call_tracer", "//:config", "//:debug_location", "//:endpoint_addresses", @@ -5433,6 +5435,7 @@ grpc_cc_library( "lb_policy_factory", "lb_policy_registry", "pollset_set", + "ref_counted_string", "validation_errors", "//:config", "//:debug_location", diff --git a/src/core/ext/xds/xds_cluster.cc b/src/core/ext/xds/xds_cluster.cc index 62e36d17fe0..572ce831c53 100644 --- a/src/core/ext/xds/xds_cluster.cc +++ b/src/core/ext/xds/xds_cluster.cc @@ -724,11 +724,11 @@ absl::StatusOr> 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))); } } diff --git a/src/core/ext/xds/xds_cluster.h b/src/core/ext/xds/xds_cluster.h index cd3fdf0cc24..445c443a8e5 100644 --- a/src/core/ext/xds/xds_cluster.h +++ b/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 && @@ -111,7 +111,9 @@ struct XdsClusterResource : public XdsResourceType::ResourceData { connection_idle_timeout == other.connection_idle_timeout && max_concurrent_requests == other.max_concurrent_requests && outlier_detection == other.outlier_detection && - override_host_statuses == other.override_host_statuses; + override_host_statuses == other.override_host_statuses && + service_telemetry_label == other.service_telemetry_label && + namespace_telemetry_label == other.namespace_telemetry_label; } std::string ToString() const; diff --git a/src/core/lib/channel/call_tracer.h b/src/core/lib/channel/call_tracer.h index 53fbf3fff5c..7c4d2447df4 100644 --- a/src/core/lib/channel/call_tracer.h +++ b/src/core/lib/channel/call_tracer.h @@ -33,6 +33,7 @@ #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/channel/context.h" #include "src/core/lib/channel/tcp_tracer.h" +#include "src/core/lib/gprpp/ref_counted_string.h" #include "src/core/lib/iomgr/error.h" #include "src/core/lib/resource_quota/arena.h" #include "src/core/lib/slice/slice_buffer.h" diff --git a/src/core/load_balancing/xds/xds_cluster_impl.cc b/src/core/load_balancing/xds/xds_cluster_impl.cc index 7eaaea4ef21..ef766148021 100644 --- a/src/core/load_balancing/xds/xds_cluster_impl.cc +++ b/src/core/load_balancing/xds/xds_cluster_impl.cc @@ -44,6 +44,7 @@ #include "src/core/ext/xds/xds_client_grpc.h" #include "src/core/ext/xds/xds_client_stats.h" #include "src/core/ext/xds/xds_endpoint.h" +#include "src/core/lib/channel/call_tracer.h" #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/config/core_configuration.h" #include "src/core/lib/debug/trace.h" @@ -52,6 +53,7 @@ #include "src/core/lib/gprpp/orphanable.h" #include "src/core/lib/gprpp/ref_counted.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" +#include "src/core/lib/gprpp/ref_counted_string.h" #include "src/core/lib/gprpp/sync.h" #include "src/core/lib/gprpp/validation_errors.h" #include "src/core/lib/iomgr/pollset_set.h" @@ -387,9 +389,9 @@ XdsClusterImplLb::Picker::Picker(XdsClusterImplLb* xds_cluster_impl_lb, max_concurrent_requests_( xds_cluster_impl_lb->cluster_resource_->max_concurrent_requests), service_telemetry_label_( - xds_cluster_impl_lb->cluster_resource_->service_telemetry_label_), + xds_cluster_impl_lb->cluster_resource_->service_telemetry_label), namespace_telemetry_label_( - xds_cluster_impl_lb->cluster_resource_->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)) { diff --git a/src/core/load_balancing/xds/xds_wrr_locality.cc b/src/core/load_balancing/xds/xds_wrr_locality.cc index 5f5059f0725..2a773941983 100644 --- a/src/core/load_balancing/xds/xds_wrr_locality.cc +++ b/src/core/load_balancing/xds/xds_wrr_locality.cc @@ -39,6 +39,7 @@ #include "src/core/lib/gprpp/debug_location.h" #include "src/core/lib/gprpp/orphanable.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" +#include "src/core/lib/gprpp/ref_counted_string.h" #include "src/core/lib/gprpp/validation_errors.h" #include "src/core/lib/iomgr/pollset_set.h" #include "src/core/lib/json/json.h" diff --git a/src/core/resolver/xds/xds_dependency_manager.cc b/src/core/resolver/xds/xds_dependency_manager.cc index d7313e129ac..b9e37bca0bf 100644 --- a/src/core/resolver/xds/xds_dependency_manager.cc +++ b/src/core/resolver/xds/xds_dependency_manager.cc @@ -18,6 +18,8 @@ #include "src/core/resolver/xds/xds_dependency_manager.h" +#include + #include "absl/strings/str_join.h" #include "src/core/ext/xds/xds_routing.h" diff --git a/test/core/xds/xds_cluster_resource_type_test.cc b/test/core/xds/xds_cluster_resource_type_test.cc index b30e2633417..2537fc8af31 100644 --- a/test/core/xds/xds_cluster_resource_type_test.cc +++ b/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(**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(**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(**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(**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()); }