QuicStatsGatherer destructor flushes pending logs

Signed-off-by: Paul Sohn <[email protected]>
pksohn · May 6, 2024 · 514d822 · 514d822
1 parent e2998e1
commit 514d822
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 0 deletions.
diff --git a/source/common/quic/quic_stats_gatherer.cc b/source/common/quic/quic_stats_gatherer.cc
@@ -7,6 +7,12 @@
 namespace Envoy {
 namespace Quic {
 
+QuicStatsGatherer::~QuicStatsGatherer() {
+  if (!logging_done_) {
+    maybeDoDeferredLog(false);
+  }
+}
+
 void QuicStatsGatherer::OnPacketAcked(int acked_bytes,
                                       quic::QuicTime::Delta /* delta_largest_observed */) {
   bytes_outstanding_ -= acked_bytes;

diff --git a/source/common/quic/quic_stats_gatherer.h b/source/common/quic/quic_stats_gatherer.h
@@ -18,6 +18,8 @@ class QuicStatsGatherer : public quic::QuicAckListenerInterface {
 public:
   explicit QuicStatsGatherer(Envoy::TimeSource* time_source) : time_source_(time_source) {}
 
+  ~QuicStatsGatherer();
+
   // QuicAckListenerInterface
   void OnPacketAcked(int acked_bytes, quic::QuicTime::Delta delta_largest_observed) override;
   void OnPacketRetransmitted(int retransmitted_bytes) override;

diff --git a/test/integration/quic_http_integration_test.cc b/test/integration/quic_http_integration_test.cc
@@ -1579,6 +1579,41 @@ TEST_P(QuicHttpIntegrationTest, DeferredLoggingWithRetransmission) {
   EXPECT_GE(std::stoi(metrics.at(0)), std::stoi(metrics.at(1)));
 }
 
+TEST_P(QuicHttpIntegrationTest, DeferredLoggingWithAbortedClientConnection) {
+  config_helper_.addRuntimeOverride("envoy.reloadable_features.quic_defer_logging_to_ack_listener",
+                                    "true");
+  useAccessLog(
+      "%PROTOCOL%,%ROUNDTRIP_DURATION%,%REQUEST_DURATION%,%RESPONSE_DURATION%,%RESPONSE_"
+      "CODE%,%BYTES_RECEIVED%,%ROUTE_NAME%,%VIRTUAL_CLUSTER_NAME%,%RESPONSE_CODE_DETAILS%,%"
+      "CONNECTION_TERMINATION_DETAILS%,%START_TIME%,%UPSTREAM_HOST%,%DURATION%,%BYTES_SENT%,%"
+      "RESPONSE_FLAGS%,%DOWNSTREAM_LOCAL_ADDRESS%,%UPSTREAM_CLUSTER%,%STREAM_ID%,%DYNAMIC_"
+      "METADATA("
+      "udp.proxy.session:bytes_sent)%,%REQ(:path)%,%STREAM_INFO_REQ(:path)%");
+  initialize();
+
+  codec_client_ = makeHttpConnection(makeClientConnection(lookupPort("http")));
+  IntegrationStreamDecoderPtr response =
+      codec_client_->makeHeaderOnlyRequest(default_request_headers_);
+  waitForNextUpstreamRequest(0, TestUtility::DefaultTimeout);
+  upstream_request_->encodeHeaders(default_response_headers_, true);
+
+  codec_client_->close(Envoy::Network::ConnectionCloseType::Abort);
+
+  std::string log = waitForAccessLog(access_log_name_);
+
+  std::vector<std::string> metrics = absl::StrSplit(log, ',');
+  ASSERT_EQ(metrics.size(), 21);
+  EXPECT_EQ(/* PROTOCOL */ metrics.at(0), "HTTP/3");
+  EXPECT_EQ(/* ROUNDTRIP_DURATION */ metrics.at(1), "-");
+  EXPECT_GE(/* REQUEST_DURATION */ std::stoi(metrics.at(2)), 0);
+  EXPECT_GE(/* RESPONSE_DURATION */ std::stoi(metrics.at(3)), 0);
+  EXPECT_EQ(/* RESPONSE_CODE */ metrics.at(4), "200");
+  EXPECT_EQ(/* BYTES_RECEIVED */ metrics.at(5), "0");
+  // Ensure that request headers from top-level access logger parameter and stream info are
+  // consistent.
+  EXPECT_EQ(/* request headers */ metrics.at(19), metrics.at(20));
+}
+
 TEST_P(QuicHttpIntegrationTest, InvalidTrailer) {
   initialize();
   // Empty string in trailer key is invalid.