Skip to content

Commit cd57f7a

Browse files
committed
fix: [#1589] use average aggregation for UDP processing time metrics
When calculating aggregated values for processing time metrics across multiple servers, we need to use the average (.avg()) instead of sum (.sum()) because the metric samples are already averages per server. Using sum() on pre-averaged values would produce incorrect results, as it would add up the averages rather than computing the true average across all servers. Changes: - Add new *_averaged() methods that use .avg() for proper aggregation - Update services.rs to use the corrected averaging methods - Import Avg trait for metric collection averaging functionality Fixes incorrect metric aggregation for: - udp_avg_connect_processing_time_ns - udp_avg_announce_processing_time_ns - udp_avg_scrape_processing_time_ns"
1 parent ba3d8a9 commit cd57f7a

File tree

2 files changed

+270
-3
lines changed

2 files changed

+270
-3
lines changed

packages/rest-tracker-api-core/src/statistics/services.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,9 @@ async fn get_protocol_metrics(
7474
let udp_requests_aborted = udp_server_stats.udp_requests_aborted();
7575
let udp_requests_banned = udp_server_stats.udp_requests_banned();
7676
let udp_banned_ips_total = udp_server_stats.udp_banned_ips_total();
77-
let udp_avg_connect_processing_time_ns = udp_server_stats.udp_avg_connect_processing_time_ns();
78-
let udp_avg_announce_processing_time_ns = udp_server_stats.udp_avg_announce_processing_time_ns();
79-
let udp_avg_scrape_processing_time_ns = udp_server_stats.udp_avg_scrape_processing_time_ns();
77+
let udp_avg_connect_processing_time_ns = udp_server_stats.udp_avg_connect_processing_time_ns_averaged();
78+
let udp_avg_announce_processing_time_ns = udp_server_stats.udp_avg_announce_processing_time_ns_averaged();
79+
let udp_avg_scrape_processing_time_ns = udp_server_stats.udp_avg_scrape_processing_time_ns_averaged();
8080

8181
// UDPv4
8282

packages/udp-tracker-server/src/statistics/metrics.rs

Lines changed: 267 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use std::time::Duration;
33
use serde::Serialize;
44
use torrust_tracker_metrics::label::LabelSet;
55
use torrust_tracker_metrics::metric::MetricName;
6+
use torrust_tracker_metrics::metric_collection::aggregate::avg::Avg;
67
use torrust_tracker_metrics::metric_collection::aggregate::sum::Sum;
78
use torrust_tracker_metrics::metric_collection::{Error, MetricCollection};
89
use torrust_tracker_metrics::metric_name;
@@ -215,6 +216,48 @@ impl Metrics {
215216
.unwrap_or_default() as u64
216217
}
217218

219+
/// Average processing time for UDP connect requests across all servers (in nanoseconds).
220+
/// This calculates the average of all gauge samples for connect requests.
221+
#[must_use]
222+
#[allow(clippy::cast_sign_loss)]
223+
#[allow(clippy::cast_possible_truncation)]
224+
pub fn udp_avg_connect_processing_time_ns_averaged(&self) -> u64 {
225+
self.metric_collection
226+
.avg(
227+
&metric_name!(UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS),
228+
&[("request_kind", "connect")].into(),
229+
)
230+
.unwrap_or(0.0) as u64
231+
}
232+
233+
/// Average processing time for UDP announce requests across all servers (in nanoseconds).
234+
/// This calculates the average of all gauge samples for announce requests.
235+
#[must_use]
236+
#[allow(clippy::cast_sign_loss)]
237+
#[allow(clippy::cast_possible_truncation)]
238+
pub fn udp_avg_announce_processing_time_ns_averaged(&self) -> u64 {
239+
self.metric_collection
240+
.avg(
241+
&metric_name!(UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS),
242+
&[("request_kind", "announce")].into(),
243+
)
244+
.unwrap_or(0.0) as u64
245+
}
246+
247+
/// Average processing time for UDP scrape requests across all servers (in nanoseconds).
248+
/// This calculates the average of all gauge samples for scrape requests.
249+
#[must_use]
250+
#[allow(clippy::cast_sign_loss)]
251+
#[allow(clippy::cast_possible_truncation)]
252+
pub fn udp_avg_scrape_processing_time_ns_averaged(&self) -> u64 {
253+
self.metric_collection
254+
.avg(
255+
&metric_name!(UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS),
256+
&[("request_kind", "scrape")].into(),
257+
)
258+
.unwrap_or(0.0) as u64
259+
}
260+
218261
// UDPv4
219262
/// Total number of UDP (UDP tracker) requests from IPv4 peers.
220263
#[must_use]
@@ -1179,4 +1222,228 @@ mod tests {
11791222
assert!(result.is_ok());
11801223
}
11811224
}
1225+
1226+
mod averaged_processing_time_metrics {
1227+
use super::*;
1228+
1229+
#[test]
1230+
fn it_should_return_zero_for_udp_avg_connect_processing_time_ns_averaged_when_no_data() {
1231+
let metrics = Metrics::default();
1232+
assert_eq!(metrics.udp_avg_connect_processing_time_ns_averaged(), 0);
1233+
}
1234+
1235+
#[test]
1236+
fn it_should_return_averaged_value_for_udp_avg_connect_processing_time_ns_averaged() {
1237+
let mut metrics = Metrics::default();
1238+
let now = CurrentClock::now();
1239+
let labels1 = LabelSet::from([("request_kind", "connect"), ("server_id", "server1")]);
1240+
let labels2 = LabelSet::from([("request_kind", "connect"), ("server_id", "server2")]);
1241+
1242+
// Set different gauge values for connect requests from different servers
1243+
metrics
1244+
.set_gauge(
1245+
&metric_name!(UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS),
1246+
&labels1,
1247+
1000.0,
1248+
now,
1249+
)
1250+
.unwrap();
1251+
1252+
metrics
1253+
.set_gauge(
1254+
&metric_name!(UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS),
1255+
&labels2,
1256+
2000.0,
1257+
now,
1258+
)
1259+
.unwrap();
1260+
1261+
// Should return the average: (1000 + 2000) / 2 = 1500
1262+
assert_eq!(metrics.udp_avg_connect_processing_time_ns_averaged(), 1500);
1263+
}
1264+
1265+
#[test]
1266+
fn it_should_return_zero_for_udp_avg_announce_processing_time_ns_averaged_when_no_data() {
1267+
let metrics = Metrics::default();
1268+
assert_eq!(metrics.udp_avg_announce_processing_time_ns_averaged(), 0);
1269+
}
1270+
1271+
#[test]
1272+
fn it_should_return_averaged_value_for_udp_avg_announce_processing_time_ns_averaged() {
1273+
let mut metrics = Metrics::default();
1274+
let now = CurrentClock::now();
1275+
let labels1 = LabelSet::from([("request_kind", "announce"), ("server_id", "server1")]);
1276+
let labels2 = LabelSet::from([("request_kind", "announce"), ("server_id", "server2")]);
1277+
let labels3 = LabelSet::from([("request_kind", "announce"), ("server_id", "server3")]);
1278+
1279+
// Set different gauge values for announce requests from different servers
1280+
metrics
1281+
.set_gauge(
1282+
&metric_name!(UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS),
1283+
&labels1,
1284+
1500.0,
1285+
now,
1286+
)
1287+
.unwrap();
1288+
1289+
metrics
1290+
.set_gauge(
1291+
&metric_name!(UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS),
1292+
&labels2,
1293+
2500.0,
1294+
now,
1295+
)
1296+
.unwrap();
1297+
1298+
metrics
1299+
.set_gauge(
1300+
&metric_name!(UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS),
1301+
&labels3,
1302+
3000.0,
1303+
now,
1304+
)
1305+
.unwrap();
1306+
1307+
// Should return the average: (1500 + 2500 + 3000) / 3 = 2333 (truncated)
1308+
assert_eq!(metrics.udp_avg_announce_processing_time_ns_averaged(), 2333);
1309+
}
1310+
1311+
#[test]
1312+
fn it_should_return_zero_for_udp_avg_scrape_processing_time_ns_averaged_when_no_data() {
1313+
let metrics = Metrics::default();
1314+
assert_eq!(metrics.udp_avg_scrape_processing_time_ns_averaged(), 0);
1315+
}
1316+
1317+
#[test]
1318+
fn it_should_return_averaged_value_for_udp_avg_scrape_processing_time_ns_averaged() {
1319+
let mut metrics = Metrics::default();
1320+
let now = CurrentClock::now();
1321+
let labels1 = LabelSet::from([("request_kind", "scrape"), ("server_id", "server1")]);
1322+
let labels2 = LabelSet::from([("request_kind", "scrape"), ("server_id", "server2")]);
1323+
1324+
// Set different gauge values for scrape requests from different servers
1325+
metrics
1326+
.set_gauge(
1327+
&metric_name!(UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS),
1328+
&labels1,
1329+
500.0,
1330+
now,
1331+
)
1332+
.unwrap();
1333+
1334+
metrics
1335+
.set_gauge(
1336+
&metric_name!(UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS),
1337+
&labels2,
1338+
1500.0,
1339+
now,
1340+
)
1341+
.unwrap();
1342+
1343+
// Should return the average: (500 + 1500) / 2 = 1000
1344+
assert_eq!(metrics.udp_avg_scrape_processing_time_ns_averaged(), 1000);
1345+
}
1346+
1347+
#[test]
1348+
fn it_should_handle_fractional_averages_with_truncation() {
1349+
let mut metrics = Metrics::default();
1350+
let now = CurrentClock::now();
1351+
let labels1 = LabelSet::from([("request_kind", "connect"), ("server_id", "server1")]);
1352+
let labels2 = LabelSet::from([("request_kind", "connect"), ("server_id", "server2")]);
1353+
let labels3 = LabelSet::from([("request_kind", "connect"), ("server_id", "server3")]);
1354+
1355+
// Set values that will result in a fractional average
1356+
metrics
1357+
.set_gauge(
1358+
&metric_name!(UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS),
1359+
&labels1,
1360+
1000.0,
1361+
now,
1362+
)
1363+
.unwrap();
1364+
1365+
metrics
1366+
.set_gauge(
1367+
&metric_name!(UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS),
1368+
&labels2,
1369+
1001.0,
1370+
now,
1371+
)
1372+
.unwrap();
1373+
1374+
metrics
1375+
.set_gauge(
1376+
&metric_name!(UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS),
1377+
&labels3,
1378+
1001.0,
1379+
now,
1380+
)
1381+
.unwrap();
1382+
1383+
// Should return the average: (1000 + 1001 + 1001) / 3 = 1000.666... → 1000 (truncated)
1384+
assert_eq!(metrics.udp_avg_connect_processing_time_ns_averaged(), 1000);
1385+
}
1386+
1387+
#[test]
1388+
fn it_should_only_average_matching_request_kinds() {
1389+
let mut metrics = Metrics::default();
1390+
let now = CurrentClock::now();
1391+
1392+
// Set values for different request kinds with the same server_id
1393+
let connect_labels = LabelSet::from([("request_kind", "connect"), ("server_id", "server1")]);
1394+
let announce_labels = LabelSet::from([("request_kind", "announce"), ("server_id", "server1")]);
1395+
let scrape_labels = LabelSet::from([("request_kind", "scrape"), ("server_id", "server1")]);
1396+
1397+
metrics
1398+
.set_gauge(
1399+
&metric_name!(UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS),
1400+
&connect_labels,
1401+
1000.0,
1402+
now,
1403+
)
1404+
.unwrap();
1405+
1406+
metrics
1407+
.set_gauge(
1408+
&metric_name!(UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS),
1409+
&announce_labels,
1410+
2000.0,
1411+
now,
1412+
)
1413+
.unwrap();
1414+
1415+
metrics
1416+
.set_gauge(
1417+
&metric_name!(UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS),
1418+
&scrape_labels,
1419+
3000.0,
1420+
now,
1421+
)
1422+
.unwrap();
1423+
1424+
// Each function should only return the value for its specific request kind
1425+
assert_eq!(metrics.udp_avg_connect_processing_time_ns_averaged(), 1000);
1426+
assert_eq!(metrics.udp_avg_announce_processing_time_ns_averaged(), 2000);
1427+
assert_eq!(metrics.udp_avg_scrape_processing_time_ns_averaged(), 3000);
1428+
}
1429+
1430+
#[test]
1431+
fn it_should_handle_single_server_averaged_metrics() {
1432+
let mut metrics = Metrics::default();
1433+
let now = CurrentClock::now();
1434+
let labels = LabelSet::from([("request_kind", "connect"), ("server_id", "single_server")]);
1435+
1436+
metrics
1437+
.set_gauge(
1438+
&metric_name!(UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS),
1439+
&labels,
1440+
1234.0,
1441+
now,
1442+
)
1443+
.unwrap();
1444+
1445+
// With only one server, the average should be the same as the single value
1446+
assert_eq!(metrics.udp_avg_connect_processing_time_ns_averaged(), 1234);
1447+
}
1448+
}
11821449
}

0 commit comments

Comments
 (0)