FEAT(server, client): Add rolling connection quality stats

Previously, only the total packet statistics since the user had connected were tracked. While a good start, these stats are not really helping to understand sudden connection problems. This commit adds additional server-side rolling packet stats which are updated every few seconds/minutes. These new stats are sent using additional fields in the UserStats protocol message and rendered in the UserInformation dialog in the client. Closes mumble-voip#5872
Hartmnt · Jan 13, 2025 · 97143df · 97143df
1 parent 47797b7
commit 97143df
Show file tree

Hide file tree

Showing 8 changed files with 419 additions and 102 deletions.
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
@@ -85,6 +85,7 @@ set(SHARED_SOURCES
 
 	"crypto/CryptographicHash.cpp"
 	"crypto/CryptographicRandom.cpp"
+	"crypto/CryptState.cpp"
 	"crypto/CryptStateOCB2.cpp"
 
 	"${3RDPARTY_DIR}/arc4random/arc4random_uniform.cpp"

diff --git a/src/Mumble.proto b/src/Mumble.proto
@@ -553,6 +553,13 @@ message UserStats {
 	// True if the user has a strong certificate.
 	optional bool strong_certificate = 18 [default = false];
 	optional bool opus = 19 [default = false];
+
+	// Rolling packet statistics time window as defined on the server.
+	optional uint32 rolling_time_window = 20;
+	// Rolling packet statistics for packets received from the client.
+	optional Stats rolling_from_client = 21;
+	// Rolling packet statistics for packets sent by the server.
+	optional Stats rolling_from_server = 22;
 }
 
 // Used by the client to request binary data from the server. By default large

diff --git a/src/crypto/CryptState.cpp b/src/crypto/CryptState.cpp
@@ -0,0 +1,49 @@
+// Copyright The Mumble Developers. All rights reserved.
+// Use of this source code is governed by a BSD-style license
+// that can be found in the LICENSE file at the root of the
+// Mumble source tree or at <https://www.mumble.info/LICENSE>.
+
+#include "CryptState.h"
+
+void CryptState::updateRollingStats() {
+	std::chrono::time_point< std::chrono::steady_clock > now = std::chrono::steady_clock::now();
+
+	// Update no more than every few seconds
+	if ((now - m_rollingLastSampleTime) < m_rollingScanInterval) {
+		return;
+	}
+
+	m_rollingLastSampleTime = now;
+
+	PacketStatsSnapshot snapshotLocal;
+	snapshotLocal.stats     = m_statsLocal;
+	snapshotLocal.timestamp = now;
+	m_statsLocalReference.push(snapshotLocal);
+
+	PacketStatsSnapshot snapshotRemote;
+	snapshotRemote.stats     = m_statsRemote;
+	snapshotRemote.timestamp = now;
+	m_statsRemoteReference.push(snapshotRemote);
+
+	while (!m_statsLocalReference.empty() && (now - m_statsLocalReference.front().timestamp) > m_rollingWindow) {
+		m_statsLocalReference.pop();
+	}
+
+	while (!m_statsRemoteReference.empty() && (now - m_statsRemoteReference.front().timestamp) > m_rollingWindow) {
+		m_statsRemoteReference.pop();
+	}
+
+	if (!m_statsLocalReference.empty()) {
+		m_statsLocalRolling.good   = m_statsLocal.good - m_statsLocalReference.front().stats.good;
+		m_statsLocalRolling.late   = m_statsLocal.late - m_statsLocalReference.front().stats.late;
+		m_statsLocalRolling.lost   = m_statsLocal.lost - m_statsLocalReference.front().stats.lost;
+		m_statsLocalRolling.resync = m_statsLocal.resync - m_statsLocalReference.front().stats.resync;
+	}
+
+	if (!m_statsRemoteReference.empty()) {
+		m_statsRemoteRolling.good   = m_statsRemote.good - m_statsRemoteReference.front().stats.good;
+		m_statsRemoteRolling.late   = m_statsRemote.late - m_statsRemoteReference.front().stats.late;
+		m_statsRemoteRolling.lost   = m_statsRemote.lost - m_statsRemoteReference.front().stats.lost;
+		m_statsRemoteRolling.resync = m_statsRemote.resync - m_statsRemoteReference.front().stats.resync;
+	}
+}
diff --git a/src/crypto/CryptState.h b/src/crypto/CryptState.h
@@ -7,6 +7,8 @@
 #define MUMBLE_CRYPTSTATE_H_
 
 #include "Timer.h"
+#include <chrono>
+#include <queue>
 #include <string>
 
 struct PacketStats {
@@ -16,12 +18,32 @@ struct PacketStats {
 	unsigned int resync = 0;
 };
 
+struct PacketStatsSnapshot {
+	PacketStats stats;
+	std::chrono::time_point< std::chrono::steady_clock > timestamp;
+};
+
 class CryptState {
 private:
 	Q_DISABLE_COPY(CryptState)
+
+	const std::chrono::seconds m_rollingScanInterval                             = std::chrono::seconds(5);
+	std::chrono::time_point< std::chrono::steady_clock > m_rollingLastSampleTime = {};
+
+	std::queue< PacketStatsSnapshot > m_statsLocalReference;
+	std::queue< PacketStatsSnapshot > m_statsRemoteReference;
+
+protected:
+	void updateRollingStats();
+
 public:
-	PacketStats m_statsLocal  = {};
-	PacketStats m_statsRemote = {};
+	PacketStats m_statsLocal         = {};
+	PacketStats m_statsRemote        = {};
+	PacketStats m_statsLocalRolling  = {};
+	PacketStats m_statsRemoteRolling = {};
+
+	/// This is the packet statistics sliding time window size in seconds
+	std::chrono::duration< unsigned int, std::ratio< 1 > > m_rollingWindow = std::chrono::minutes(5);
 
 	Timer tLastGood;
 	Timer tLastRequest;

diff --git a/src/crypto/CryptStateOCB2.cpp b/src/crypto/CryptStateOCB2.cpp
@@ -220,6 +220,7 @@ bool CryptStateOCB2::decrypt(const unsigned char *source, unsigned char *dst, un
 		m_statsLocal.lost -= static_cast< unsigned int >(std::abs(lost));
 	}
 
+	updateRollingStats();
 	tLastGood.restart();
 	return true;
 }

diff --git a/src/mumble/UserInformation.cpp b/src/mumble/UserInformation.cpp
@@ -163,8 +163,13 @@ void UserInformation::update(const MumbleProto::UserStats &msg) {
 	qlTCPVar->setText(QString::number(msg.tcp_ping_var() > 0.0f ? sqrtf(msg.tcp_ping_var()) : 0.0f, 'f', 2));
 	qlUDPVar->setText(QString::number(msg.udp_ping_var() > 0.0f ? sqrtf(msg.udp_ping_var()) : 0.0f, 'f', 2));
 
-	if (msg.has_from_client() && msg.has_from_server()) {
-		qgbUDP->setVisible(true);
+	bool hasTotalStats = msg.has_from_client() && msg.has_from_server();
+	bool hasRollingStats =
+		msg.has_rolling_time_window() && msg.has_rolling_from_client() && msg.has_rolling_from_server();
+
+	qgbUDP->setVisible(hasTotalStats || hasRollingStats);
+
+	if (hasTotalStats) {
 		const MumbleProto::UserStats_Stats &from = msg.from_client();
 		qlFromGood->setText(QString::number(from.good()));
 		qlFromLate->setText(QString::number(from.late()));
@@ -179,17 +184,65 @@ void UserInformation::update(const MumbleProto::UserStats &msg) {
 
 		quint32 allFromPackets = from.good() + from.late() + from.lost();
 		qlFromLatePercent->setText(
-			QString::number(allFromPackets > 0 ? from.late() * 100.0 / allFromPackets : 0., 'f', 2));
+			QString::number(allFromPackets > 0 ? from.late() * 100.0 / allFromPackets : 0., 'f', 1));
 		qlFromLostPercent->setText(
-			QString::number(allFromPackets > 0 ? from.lost() * 100.0 / allFromPackets : 0., 'f', 2));
+			QString::number(allFromPackets > 0 ? from.lost() * 100.0 / allFromPackets : 0., 'f', 1));
 
 		quint32 allToPackets = to.good() + to.late() + to.lost();
-		qlToLatePercent->setText(QString::number(allToPackets > 0 ? to.late() * 100.0 / allToPackets : 0., 'f', 2));
-		qlToLostPercent->setText(QString::number(allToPackets > 0 ? to.lost() * 100.0 / allToPackets : 0., 'f', 2));
-	} else {
-		qgbUDP->setVisible(false);
+		qlToLatePercent->setText(QString::number(allToPackets > 0 ? to.late() * 100.0 / allToPackets : 0., 'f', 1));
+		qlToLostPercent->setText(QString::number(allToPackets > 0 ? to.lost() * 100.0 / allToPackets : 0., 'f', 1));
 	}
 
+	if (hasRollingStats) {
+		const MumbleProto::UserStats_Stats &from = msg.rolling_from_client();
+		qlFromGoodRolling->setText(QString::number(from.good()));
+		qlFromLateRolling->setText(QString::number(from.late()));
+		qlFromLostRolling->setText(QString::number(from.lost()));
+		qlFromResyncRolling->setText(QString::number(from.resync()));
+
+		const MumbleProto::UserStats_Stats &to = msg.rolling_from_server();
+		qlToGoodRolling->setText(QString::number(to.good()));
+		qlToLateRolling->setText(QString::number(to.late()));
+		qlToLostRolling->setText(QString::number(to.lost()));
+		qlToResyncRolling->setText(QString::number(to.resync()));
+
+		quint32 allFromPackets = from.good() + from.late() + from.lost();
+		qlFromLatePercentRolling->setText(
+			QString::number(allFromPackets > 0 ? from.late() * 100.0 / allFromPackets : 0., 'f', 1));
+		qlFromLostPercentRolling->setText(
+			QString::number(allFromPackets > 0 ? from.lost() * 100.0 / allFromPackets : 0., 'f', 1));
+
+		quint32 allToPackets = to.good() + to.late() + to.lost();
+		qlToLatePercentRolling->setText(
+			QString::number(allToPackets > 0 ? to.late() * 100.0 / allToPackets : 0., 'f', 1));
+		qlToLostPercentRolling->setText(
+			QString::number(allToPackets > 0 ? to.lost() * 100.0 / allToPackets : 0., 'f', 1));
+
+		uint32_t rollingSeconds = msg.rolling_time_window();
+		QString rollingText     = tr("Last %1 %2:");
+		if (rollingSeconds < 120) {
+			qliRolling->setText(rollingText.arg(QString::number(rollingSeconds)).arg(tr("seconds")));
+		} else {
+			qliRolling->setText(rollingText.arg(QString::number(rollingSeconds / 60)).arg(tr("minutes")));
+		}
+	}
+
+	qlFromGoodRolling->setVisible(hasRollingStats);
+	qlFromLateRolling->setVisible(hasRollingStats);
+	qlFromLostRolling->setVisible(hasRollingStats);
+	qlFromResyncRolling->setVisible(hasRollingStats);
+	qlToGoodRolling->setVisible(hasRollingStats);
+	qlToLateRolling->setVisible(hasRollingStats);
+	qlToLostRolling->setVisible(hasRollingStats);
+	qlToResyncRolling->setVisible(hasRollingStats);
+	qlFromLatePercentRolling->setVisible(hasRollingStats);
+	qlFromLostPercentRolling->setVisible(hasRollingStats);
+	qlToLatePercentRolling->setVisible(hasRollingStats);
+	qlToLostPercentRolling->setVisible(hasRollingStats);
+	qliRolling->setVisible(hasRollingStats);
+	qliRollingFrom->setVisible(hasRollingStats);
+	qliRollingTo->setVisible(hasRollingStats);
+
 	if (msg.has_onlinesecs()) {
 		if (msg.has_idlesecs())
 			qlTime->setText(