[test] Wait for `RecordEnd` in `FakeClientCallAttemptTracer` destruction (#36396)

<!--

If you know who should review your pull request, please assign it to that
person, otherwise the pull request would get assigned randomly.

If your pull request is for a specific language, please add the appropriate
lang label.

-->

Closes #36396

COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36396 from yijiem:xds-cluster-end2end-test-twin 26062ea1b4
PiperOrigin-RevId: 626207011
pull/36403/head^2
Yijie Ma 12 months ago committed by Copybara-Service
parent f6ee70289d
commit ba78e4ff47
  1. 2
      src/cpp/ext/otel/otel_plugin.cc
  2. 1
      test/core/channel/call_tracer_test.cc
  3. 17
      test/core/util/fake_stats_plugin.h
  4. 7
      test/cpp/end2end/xds/xds_cluster_end2end_test.cc

@ -104,7 +104,7 @@ class OpenTelemetryPlugin::NPCMetricsKeyValueIterable
bool(opentelemetry::nostd::string_view, bool(opentelemetry::nostd::string_view,
opentelemetry::common::AttributeValue)> opentelemetry::common::AttributeValue)>
callback) const noexcept override { callback) const noexcept override {
for (size_t i = 0; i < label_keys_.size(); i++) { for (size_t i = 0; i < label_keys_.size(); ++i) {
if (!callback(AbslStrViewToOpenTelemetryStrView(label_keys_[i]), if (!callback(AbslStrViewToOpenTelemetryStrView(label_keys_[i]),
AbslStrViewToOpenTelemetryStrView(label_values_[i]))) { AbslStrViewToOpenTelemetryStrView(label_values_[i]))) {
return false; return false;

@ -94,6 +94,7 @@ TEST_F(CallTracerTest, MultipleClientCallAttemptTracers) {
attempt_tracer->RecordAnnotation("Test"); attempt_tracer->RecordAnnotation("Test");
EXPECT_EQ(annotation_logger_, EXPECT_EQ(annotation_logger_,
std::vector<std::string>({"Test", "Test", "Test"})); std::vector<std::string>({"Test", "Test", "Test"}));
attempt_tracer->RecordEnd(gpr_timespec());
} }
TEST_F(CallTracerTest, BasicServerCallTracerTest) { TEST_F(CallTracerTest, BasicServerCallTracerTest) {

@ -31,6 +31,7 @@
#include "src/core/lib/channel/metrics.h" #include "src/core/lib/channel/metrics.h"
#include "src/core/lib/channel/promise_based_filter.h" #include "src/core/lib/channel/promise_based_filter.h"
#include "src/core/lib/channel/tcp_tracer.h" #include "src/core/lib/channel/tcp_tracer.h"
#include "src/core/lib/gprpp/ref_counted.h"
namespace grpc_core { namespace grpc_core {
@ -59,12 +60,12 @@ void RegisterFakeStatsPlugin();
class FakeClientCallTracer : public ClientCallTracer { class FakeClientCallTracer : public ClientCallTracer {
public: public:
class FakeClientCallAttemptTracer class FakeClientCallAttemptTracer
: public ClientCallTracer::CallAttemptTracer { : public ClientCallTracer::CallAttemptTracer,
public RefCounted<FakeClientCallAttemptTracer> {
public: public:
explicit FakeClientCallAttemptTracer( explicit FakeClientCallAttemptTracer(
std::vector<std::string>* annotation_logger) std::vector<std::string>* annotation_logger)
: annotation_logger_(annotation_logger) {} : annotation_logger_(annotation_logger) {}
~FakeClientCallAttemptTracer() override {}
void RecordSendInitialMetadata( void RecordSendInitialMetadata(
grpc_metadata_batch* /*send_initial_metadata*/) override {} grpc_metadata_batch* /*send_initial_metadata*/) override {}
void RecordSendTrailingMetadata( void RecordSendTrailingMetadata(
@ -83,7 +84,7 @@ class FakeClientCallTracer : public ClientCallTracer {
grpc_metadata_batch* /*recv_trailing_metadata*/, grpc_metadata_batch* /*recv_trailing_metadata*/,
const grpc_transport_stream_stats* /*transport_stream_stats*/) const grpc_transport_stream_stats* /*transport_stream_stats*/)
override {} override {}
void RecordEnd(const gpr_timespec& /*latency*/) override {} void RecordEnd(const gpr_timespec& /*latency*/) override { Unref(); }
void RecordAnnotation(absl::string_view annotation) override { void RecordAnnotation(absl::string_view annotation) override {
annotation_logger_->push_back(std::string(annotation)); annotation_logger_->push_back(std::string(annotation));
} }
@ -113,9 +114,10 @@ class FakeClientCallTracer : public ClientCallTracer {
: annotation_logger_(annotation_logger) {} : annotation_logger_(annotation_logger) {}
~FakeClientCallTracer() override {} ~FakeClientCallTracer() override {}
CallAttemptTracer* StartNewAttempt(bool /*is_transparent_retry*/) override { CallAttemptTracer* StartNewAttempt(bool /*is_transparent_retry*/) override {
call_attempt_tracers_.emplace_back( auto call_attempt_tracer =
new FakeClientCallAttemptTracer(annotation_logger_)); MakeRefCounted<FakeClientCallAttemptTracer>(annotation_logger_);
return call_attempt_tracers_.back().get(); call_attempt_tracers_.emplace_back(call_attempt_tracer);
return call_attempt_tracer.release(); // Released in RecordEnd().
} }
void RecordAnnotation(absl::string_view annotation) override { void RecordAnnotation(absl::string_view annotation) override {
@ -132,8 +134,7 @@ class FakeClientCallTracer : public ClientCallTracer {
private: private:
std::vector<std::string>* annotation_logger_; std::vector<std::string>* annotation_logger_;
std::vector<std::unique_ptr<FakeClientCallAttemptTracer>> std::vector<RefCountedPtr<FakeClientCallAttemptTracer>> call_attempt_tracers_;
call_attempt_tracers_;
}; };
#define GRPC_ARG_INJECT_FAKE_CLIENT_CALL_TRACER_FACTORY \ #define GRPC_ARG_INJECT_FAKE_CLIENT_CALL_TRACER_FACTORY \

@ -355,13 +355,6 @@ TEST_P(CdsTest, MetricLabels) {
"mynamespace"), "mynamespace"),
::testing::Pair(OptionalLabelKey::kLocality, ::testing::Pair(OptionalLabelKey::kLocality,
LocalityNameString("locality1")))); LocalityNameString("locality1"))));
// TODO(yashkt, yijiem): This shutdown shouldn't actually be necessary. The
// only reason it's here is to add a delay before
// fake_client_call_tracer_factory goes out of scope, since there may be
// lingering callbacks in the call stack that are using the CallAttemptTracer
// even after we get here, which would then cause a crash. Find a cleaner way
// to fix this.
balancer_->Shutdown();
} }
// //

Loading…
Cancel
Save