quic: consolidate stats collecting in QuicSession

PR-URL: #34741 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Rich Trott <[email protected]>
nodejs · Aug 17, 2020 · bfc3535 · bfc3535
1 parent 94aa291
commit bfc3535
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 22 deletions.
diff --git a/src/quic/node_quic_http3_application.cc b/src/quic/node_quic_http3_application.cc
@@ -79,11 +79,10 @@ Http3Application::Http3Application(
 // Push streams in HTTP/3 are a bit complicated.
 // First, it's important to know that only an HTTP/3 server can
 // create a push stream.
-// Second, it's important to recognize that a push stream is
-// essentially an *assumed* request. For instance, if a client
-// requests a webpage that has links to css and js files, and
-// the server expects the client to send subsequent requests
-// for those css and js files, the server can shortcut the
+// Second, a push stream is essentially an *assumed* request. For
+// instance, if a client requests a webpage that has links to css
+// and js files, and the server expects the client to send subsequent
+// requests for those css and js files, the server can shortcut the
 // process by opening a push stream for each additional resource
 // it assumes the client to make.
 // Third, a push stream can only be opened within the context

diff --git a/src/quic/node_quic_session.cc b/src/quic/node_quic_session.cc
@@ -2098,7 +2098,6 @@ bool QuicSession::Receive(
     UpdateIdleTimer();
 
   SendPendingData();
-  UpdateRecoveryStats();
   Debug(this, "Successfully processed received packet");
   return true;
 }
@@ -2893,19 +2892,6 @@ void QuicSessionOnCertDone(const FunctionCallbackInfo<Value>& args) {
 }
 }  // namespace
 
-// Recovery stats are used to allow user code to keep track of
-// important round-trip timing statistics that are updated through
-// the lifetime of a connection. Effectively, these communicate how
-// much time (from the perspective of the local peer) is being taken
-// to exchange data reliably with the remote peer.
-// TODO(@jasnell): Revisit
-void QuicSession::UpdateRecoveryStats() {
-  ngtcp2_conn_stat stat;
-  ngtcp2_conn_get_conn_stat(connection(), &stat);
-  SetStat(&QuicSessionStats::min_rtt, stat.min_rtt);
-  SetStat(&QuicSessionStats::latest_rtt, stat.latest_rtt);
-  SetStat(&QuicSessionStats::smoothed_rtt, stat.smoothed_rtt);
-}
 
 // Data stats are used to allow user code to keep track of important
 // statistics such as amount of data in flight through the lifetime
@@ -2918,6 +2904,13 @@ void QuicSession::UpdateDataStats() {
   ngtcp2_conn_stat stat;
   ngtcp2_conn_get_conn_stat(connection(), &stat);
 
+  SetStat(&QuicSessionStats::latest_rtt, stat.latest_rtt);
+  SetStat(&QuicSessionStats::min_rtt, stat.min_rtt);
+  SetStat(&QuicSessionStats::smoothed_rtt, stat.smoothed_rtt);
+  SetStat(&QuicSessionStats::receive_rate, stat.recv_rate_sec);
+  SetStat(&QuicSessionStats::send_rate, stat.delivery_rate_sec);
+  SetStat(&QuicSessionStats::cwnd, stat.cwnd);
+
   state_->bytes_in_flight = stat.bytes_in_flight;
   // The max_bytes_in_flight is a highwater mark that can be used
   // in performance analysis operations.

diff --git a/src/quic/node_quic_session.h b/src/quic/node_quic_session.h
@@ -204,7 +204,10 @@ enum QuicSessionStateFields {
   V(BLOCK_COUNT, block_count, "Block Count")                                   \
   V(MIN_RTT, min_rtt, "Minimum RTT")                                           \
   V(LATEST_RTT, latest_rtt, "Latest RTT")                                      \
-  V(SMOOTHED_RTT, smoothed_rtt, "Smoothed RTT")
+  V(SMOOTHED_RTT, smoothed_rtt, "Smoothed RTT")                                \
+  V(CWND, cwnd, "Cwnd")                                                        \
+  V(RECEIVE_RATE, receive_rate, "Receive Rate / Sec")                          \
+  V(SEND_RATE, send_rate, "Send Rate  Sec")
 
 #define V(name, _, __) IDX_QUIC_SESSION_STATS_##name,
 enum QuicSessionStatsIdx : int {
@@ -1272,8 +1275,6 @@ class QuicSession final : public AsyncWrap,
 
   bool WritePackets(const char* diagnostic_label = nullptr);
 
-  void UpdateRecoveryStats();
-
   void UpdateConnectionID(
       int type,
       const QuicCID& cid,