Skip to content

Commit 7451712

Browse files
josecelanojosecelano
committed
Merge #1383: Overhaul stats events: merge UDP server events with a different IP version
9a8a0dc refactor: [#1382] rename UDP server event enum variants (Jose Celano) 27e2db4 refactor: [#1382] include req kin in UDP error response if it's known (Jose Celano) 625d20a refactor: [#1382] rename torrust_udp_tracker_server::statistics::event::Event::UdpRequest (Jose Celano) 203a1b4 refactor: [#1382] merge UDP server stats events with different IP version (Jose Celano) e4c6000 refactor: [#1382] add connection context to UDP server events (Jose Celano) 74ffa4c refactor: [#1382] error request kind in UDP req does not make sense (Jose Celano) Pull request description: Change events in `torrust_udp_tracker_server::statistics::event::Event` from this: ```rust pub enum Event { UdpRequestAborted, UdpRequestBanned, // UDP4 Udp4IncomingRequest, Udp4Request { kind: UdpResponseKind, }, Udp4Response { kind: UdpResponseKind, req_processing_time: Duration, }, Udp4Error, // UDP6 Udp6IncomingRequest, Udp6Request { kind: UdpResponseKind, }, Udp6Response { kind: UdpResponseKind, req_processing_time: Duration, }, Udp6Error, } pub enum UdpRequestKind { Connect, Announce, Scrape, Error, } ``` To this: ```rust pub enum Event { UdpRequestReceived { context: ConnectionContext, }, UdpRequestAborted { context: ConnectionContext, }, UdpRequestBanned { context: ConnectionContext, }, UdpRequestAccepted { context: ConnectionContext, kind: UdpRequestKind, }, UdpResponseSent { context: ConnectionContext, kind: UdpResponseKind, req_processing_time: Duration, }, UdpError { context: ConnectionContext, }, } pub enum UdpRequestKind { Connect, Announce, Scrape, } pub enum UdpResponseKind { Ok { req_kind: UdpRequestKind }, Error { opt_req_kind: Option<UdpRequestKind> }, } pub struct ConnectionContext { client_socket_addr: SocketAddr, server_socket_addr: SocketAddr, } ``` ### Sub-tasks - [x] Add enum `UdpResponseKind`. `UdpRequestKind::Error` variant does not make sense. - [x] Add `ConnectionContext` to events. - [x] Merge events with the same request type (`connect`, `announce` and `scrape`). ACKs for top commit: josecelano: ACK 9a8a0dc Tree-SHA512: 3d81ebed7e0005aacb135d801ad64043bb2dccafc07476538ed372dcc256ac60faced20ded3794f0d78ee2f7295b11e6d0d45b286c896501a097aa4371227123
2 parents df507a8 + 9a8a0dc commit 7451712

File tree

11 files changed

+437
-295
lines changed

11 files changed

+437
-295
lines changed

‎packages/udp-tracker-core/src/statistics/event/mod.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,12 @@ impl ConnectionContext {
3030
}
3131
}
3232

33+
#[must_use]
3334
pub fn client_socket_addr(&self) -> SocketAddr {
3435
self.client_socket_addr
3536
}
3637

38+
#[must_use]
3739
pub fn server_socket_addr(&self) -> SocketAddr {
3840
self.server_socket_addr
3941
}

‎packages/udp-tracker-server/src/handlers/announce.rs‎

Lines changed: 28 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use zerocopy::network_endian::I32;
1616

1717
use crate::error::Error;
1818
use crate::statistics as server_statistics;
19-
use crate::statistics::event::UdpResponseKind;
19+
use crate::statistics::event::{ConnectionContext, UdpRequestKind};
2020

2121
/// It handles the `Announce` request.
2222
///
@@ -32,7 +32,7 @@ pub async fn handle_announce(
3232
core_config: &Arc<Core>,
3333
opt_udp_server_stats_event_sender: &Arc<Option<Box<dyn server_statistics::event::sender::Sender>>>,
3434
cookie_valid_range: Range<f64>,
35-
) -> Result<Response, (Error, TransactionId)> {
35+
) -> Result<Response, (Error, TransactionId, UdpRequestKind)> {
3636
tracing::Span::current()
3737
.record("transaction_id", request.transaction_id.0.to_string())
3838
.record("connection_id", request.connection_id.0.to_string())
@@ -41,28 +41,18 @@ pub async fn handle_announce(
4141
tracing::trace!("handle announce");
4242

4343
if let Some(udp_server_stats_event_sender) = opt_udp_server_stats_event_sender.as_deref() {
44-
match client_socket_addr.ip() {
45-
IpAddr::V4(_) => {
46-
udp_server_stats_event_sender
47-
.send_event(server_statistics::event::Event::Udp4Request {
48-
kind: UdpResponseKind::Announce,
49-
})
50-
.await;
51-
}
52-
IpAddr::V6(_) => {
53-
udp_server_stats_event_sender
54-
.send_event(server_statistics::event::Event::Udp6Request {
55-
kind: UdpResponseKind::Announce,
56-
})
57-
.await;
58-
}
59-
}
44+
udp_server_stats_event_sender
45+
.send_event(server_statistics::event::Event::UdpRequestAccepted {
46+
context: ConnectionContext::new(client_socket_addr, server_socket_addr),
47+
kind: UdpRequestKind::Announce,
48+
})
49+
.await;
6050
}
6151

6252
let announce_data = announce_service
6353
.handle_announce(client_socket_addr, server_socket_addr, request, cookie_valid_range)
6454
.await
65-
.map_err(|e| (e.into(), request.transaction_id))?;
55+
.map_err(|e| (e.into(), request.transaction_id, UdpRequestKind::Announce))?;
6656

6757
Ok(build_response(client_socket_addr, request, core_config, &announce_data))
6858
}
@@ -226,7 +216,7 @@ mod tests {
226216
TorrentPeerBuilder,
227217
};
228218
use crate::statistics as server_statistics;
229-
use crate::statistics::event::UdpResponseKind;
219+
use crate::statistics::event::UdpRequestKind;
230220

231221
#[tokio::test]
232222
async fn an_announced_peer_should_be_added_to_the_tracker() {
@@ -429,11 +419,15 @@ mod tests {
429419

430420
#[tokio::test]
431421
async fn should_send_the_upd4_announce_event() {
422+
let client_socket_addr = sample_ipv4_socket_address();
423+
let server_socket_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(203, 0, 113, 196)), 6969);
424+
432425
let mut udp_server_stats_event_sender_mock = MockUdpServerStatsEventSender::new();
433426
udp_server_stats_event_sender_mock
434427
.expect_send_event()
435-
.with(eq(server_statistics::event::Event::Udp4Request {
436-
kind: UdpResponseKind::Announce,
428+
.with(eq(server_statistics::event::Event::UdpRequestAccepted {
429+
context: server_statistics::event::ConnectionContext::new(client_socket_addr, server_socket_addr),
430+
kind: UdpRequestKind::Announce,
437431
}))
438432
.times(1)
439433
.returning(|_| Box::pin(future::ready(Some(Ok(())))));
@@ -443,9 +437,6 @@ mod tests {
443437
let (core_tracker_services, core_udp_tracker_services, _server_udp_tracker_services) =
444438
initialize_core_tracker_services_for_default_tracker_configuration();
445439

446-
let client_socket_addr = sample_ipv4_socket_address();
447-
let server_socket_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(203, 0, 113, 196)), 6969);
448-
449440
handle_announce(
450441
&core_udp_tracker_services.announce_service,
451442
client_socket_addr,
@@ -549,7 +540,7 @@ mod tests {
549540
sample_issue_time, MockUdpServerStatsEventSender, TorrentPeerBuilder,
550541
};
551542
use crate::statistics as server_statistics;
552-
use crate::statistics::event::UdpResponseKind;
543+
use crate::statistics::event::UdpRequestKind;
553544

554545
#[tokio::test]
555546
async fn an_announced_peer_should_be_added_to_the_tracker() {
@@ -771,11 +762,15 @@ mod tests {
771762

772763
#[tokio::test]
773764
async fn should_send_the_upd6_announce_event() {
765+
let client_socket_addr = sample_ipv6_remote_addr();
766+
let server_socket_addr = SocketAddr::new(IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 203, 0, 113, 196)), 6969);
767+
774768
let mut udp_server_stats_event_sender_mock = MockUdpServerStatsEventSender::new();
775769
udp_server_stats_event_sender_mock
776770
.expect_send_event()
777-
.with(eq(server_statistics::event::Event::Udp6Request {
778-
kind: UdpResponseKind::Announce,
771+
.with(eq(server_statistics::event::Event::UdpRequestAccepted {
772+
context: server_statistics::event::ConnectionContext::new(client_socket_addr, server_socket_addr),
773+
kind: UdpRequestKind::Announce,
779774
}))
780775
.times(1)
781776
.returning(|_| Box::pin(future::ready(Some(Ok(())))));
@@ -785,9 +780,6 @@ mod tests {
785780
let (core_tracker_services, core_udp_tracker_services, _server_udp_tracker_services) =
786781
initialize_core_tracker_services_for_default_tracker_configuration();
787782

788-
let client_socket_addr = sample_ipv6_remote_addr();
789-
let server_socket_addr = SocketAddr::new(IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 203, 0, 113, 196)), 6969);
790-
791783
let announce_request = AnnounceRequestBuilder::default()
792784
.with_connection_id(make(gen_remote_fingerprint(&client_socket_addr), sample_issue_time()).unwrap())
793785
.into();
@@ -819,7 +811,6 @@ mod tests {
819811
use bittorrent_tracker_core::whitelist::repository::in_memory::InMemoryWhitelist;
820812
use bittorrent_udp_tracker_core::connection_cookie::{gen_remote_fingerprint, make};
821813
use bittorrent_udp_tracker_core::services::announce::AnnounceService;
822-
use bittorrent_udp_tracker_core::statistics::event::ConnectionContext;
823814
use bittorrent_udp_tracker_core::{self, statistics as core_statistics};
824815
use mockall::predicate::eq;
825816

@@ -830,7 +821,7 @@ mod tests {
830821
TrackerConfigurationBuilder,
831822
};
832823
use crate::statistics as server_statistics;
833-
use crate::statistics::event::UdpResponseKind;
824+
use crate::statistics::event::UdpRequestKind;
834825

835826
#[tokio::test]
836827
async fn the_peer_ip_should_be_changed_to_the_external_ip_in_the_tracker_configuration() {
@@ -860,7 +851,7 @@ mod tests {
860851
udp_core_stats_event_sender_mock
861852
.expect_send_event()
862853
.with(eq(core_statistics::event::Event::UdpAnnounce {
863-
context: ConnectionContext::new(client_socket_addr, server_socket_addr),
854+
context: core_statistics::event::ConnectionContext::new(client_socket_addr, server_socket_addr),
864855
}))
865856
.times(1)
866857
.returning(|_| Box::pin(future::ready(Some(Ok(())))));
@@ -870,8 +861,9 @@ mod tests {
870861
let mut udp_server_stats_event_sender_mock = MockUdpServerStatsEventSender::new();
871862
udp_server_stats_event_sender_mock
872863
.expect_send_event()
873-
.with(eq(server_statistics::event::Event::Udp6Request {
874-
kind: UdpResponseKind::Announce,
864+
.with(eq(server_statistics::event::Event::UdpRequestAccepted {
865+
context: server_statistics::event::ConnectionContext::new(client_socket_addr, server_socket_addr),
866+
kind: UdpRequestKind::Announce,
875867
}))
876868
.times(1)
877869
.returning(|_| Box::pin(future::ready(Some(Ok(())))));

‎packages/udp-tracker-server/src/handlers/connect.rs‎

Lines changed: 19 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,19 @@
11
//! UDP tracker connect handler.
2-
use std::net::{IpAddr, SocketAddr};
2+
use std::net::SocketAddr;
33
use std::sync::Arc;
44

55
use aquatic_udp_protocol::{ConnectRequest, ConnectResponse, ConnectionId, Response};
66
use bittorrent_udp_tracker_core::services::connect::ConnectService;
77
use tracing::{instrument, Level};
88

99
use crate::statistics as server_statistics;
10-
use crate::statistics::event::UdpResponseKind;
10+
use crate::statistics::event::{ConnectionContext, UdpRequestKind};
1111

1212
/// It handles the `Connect` request.
1313
#[instrument(fields(transaction_id), skip(connect_service, opt_udp_server_stats_event_sender), ret(level = Level::TRACE))]
1414
pub async fn handle_connect(
15-
remote_addr: SocketAddr,
16-
server_addr: SocketAddr,
15+
client_socket_addr: SocketAddr,
16+
server_socket_addr: SocketAddr,
1717
request: &ConnectRequest,
1818
connect_service: &Arc<ConnectService>,
1919
opt_udp_server_stats_event_sender: &Arc<Option<Box<dyn server_statistics::event::sender::Sender>>>,
@@ -23,26 +23,16 @@ pub async fn handle_connect(
2323
tracing::trace!("handle connect");
2424

2525
if let Some(udp_server_stats_event_sender) = opt_udp_server_stats_event_sender.as_deref() {
26-
match remote_addr.ip() {
27-
IpAddr::V4(_) => {
28-
udp_server_stats_event_sender
29-
.send_event(server_statistics::event::Event::Udp4Request {
30-
kind: UdpResponseKind::Connect,
31-
})
32-
.await;
33-
}
34-
IpAddr::V6(_) => {
35-
udp_server_stats_event_sender
36-
.send_event(server_statistics::event::Event::Udp6Request {
37-
kind: UdpResponseKind::Connect,
38-
})
39-
.await;
40-
}
41-
}
26+
udp_server_stats_event_sender
27+
.send_event(server_statistics::event::Event::UdpRequestAccepted {
28+
context: ConnectionContext::new(client_socket_addr, server_socket_addr),
29+
kind: UdpRequestKind::Connect,
30+
})
31+
.await;
4232
}
4333

4434
let connection_id = connect_service
45-
.handle_connect(remote_addr, server_addr, cookie_issue_time)
35+
.handle_connect(client_socket_addr, server_socket_addr, cookie_issue_time)
4636
.await;
4737

4838
build_response(*request, connection_id)
@@ -70,7 +60,6 @@ mod tests {
7060
use bittorrent_udp_tracker_core::connection_cookie::make;
7161
use bittorrent_udp_tracker_core::services::connect::ConnectService;
7262
use bittorrent_udp_tracker_core::statistics as core_statistics;
73-
use bittorrent_udp_tracker_core::statistics::event::ConnectionContext;
7463
use mockall::predicate::eq;
7564

7665
use crate::handlers::handle_connect;
@@ -79,7 +68,7 @@ mod tests {
7968
sample_ipv6_remote_addr_fingerprint, sample_issue_time, MockUdpCoreStatsEventSender, MockUdpServerStatsEventSender,
8069
};
8170
use crate::statistics as server_statistics;
82-
use crate::statistics::event::UdpResponseKind;
71+
use crate::statistics::event::UdpRequestKind;
8372

8473
fn sample_connect_request() -> ConnectRequest {
8574
ConnectRequest {
@@ -214,8 +203,9 @@ mod tests {
214203
let mut udp_server_stats_event_sender_mock = MockUdpServerStatsEventSender::new();
215204
udp_server_stats_event_sender_mock
216205
.expect_send_event()
217-
.with(eq(server_statistics::event::Event::Udp4Request {
218-
kind: UdpResponseKind::Connect,
206+
.with(eq(server_statistics::event::Event::UdpRequestAccepted {
207+
context: server_statistics::event::ConnectionContext::new(client_socket_addr, server_socket_addr),
208+
kind: UdpRequestKind::Connect,
219209
}))
220210
.times(1)
221211
.returning(|_| Box::pin(future::ready(Some(Ok(())))));
@@ -244,7 +234,7 @@ mod tests {
244234
udp_core_stats_event_sender_mock
245235
.expect_send_event()
246236
.with(eq(core_statistics::event::Event::UdpConnect {
247-
context: ConnectionContext::new(client_socket_addr, server_socket_addr),
237+
context: core_statistics::event::ConnectionContext::new(client_socket_addr, server_socket_addr),
248238
}))
249239
.times(1)
250240
.returning(|_| Box::pin(future::ready(Some(Ok(())))));
@@ -254,8 +244,9 @@ mod tests {
254244
let mut udp_server_stats_event_sender_mock = MockUdpServerStatsEventSender::new();
255245
udp_server_stats_event_sender_mock
256246
.expect_send_event()
257-
.with(eq(server_statistics::event::Event::Udp6Request {
258-
kind: UdpResponseKind::Connect,
247+
.with(eq(server_statistics::event::Event::UdpRequestAccepted {
248+
context: server_statistics::event::ConnectionContext::new(client_socket_addr, server_socket_addr),
249+
kind: UdpRequestKind::Connect,
259250
}))
260251
.times(1)
261252
.returning(|_| Box::pin(future::ready(Some(Ok(())))));

‎packages/udp-tracker-server/src/handlers/error.rs‎

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,14 @@ use zerocopy::network_endian::I32;
1212

1313
use crate::error::Error;
1414
use crate::statistics as server_statistics;
15+
use crate::statistics::event::{ConnectionContext, UdpRequestKind};
1516

1617
#[allow(clippy::too_many_arguments)]
1718
#[instrument(fields(transaction_id), skip(opt_udp_server_stats_event_sender), ret(level = Level::TRACE))]
1819
pub async fn handle_error(
19-
remote_addr: SocketAddr,
20-
local_addr: SocketAddr,
20+
req_kind: Option<UdpRequestKind>,
21+
client_socket_addr: SocketAddr,
22+
server_socket_addr: SocketAddr,
2123
request_id: Uuid,
2224
opt_udp_server_stats_event_sender: &Arc<Option<Box<dyn server_statistics::event::sender::Sender>>>,
2325
cookie_valid_range: Range<f64>,
@@ -29,10 +31,10 @@ pub async fn handle_error(
2931
match transaction_id {
3032
Some(transaction_id) => {
3133
let transaction_id = transaction_id.0.to_string();
32-
tracing::error!(target: UDP_TRACKER_LOG_TARGET, error = %e, %remote_addr, %local_addr, %request_id, %transaction_id, "response error");
34+
tracing::error!(target: UDP_TRACKER_LOG_TARGET, error = %e, %client_socket_addr, %server_socket_addr, %request_id, %transaction_id, "response error");
3335
}
3436
None => {
35-
tracing::error!(target: UDP_TRACKER_LOG_TARGET, error = %e, %remote_addr, %local_addr, %request_id, "response error");
37+
tracing::error!(target: UDP_TRACKER_LOG_TARGET, error = %e, %client_socket_addr, %server_socket_addr, %request_id, "response error");
3638
}
3739
}
3840

@@ -43,7 +45,7 @@ pub async fn handle_error(
4345
transaction_id,
4446
err,
4547
} => {
46-
if let Err(e) = check(connection_id, gen_remote_fingerprint(&remote_addr), cookie_valid_range) {
48+
if let Err(e) = check(connection_id, gen_remote_fingerprint(&client_socket_addr), cookie_valid_range) {
4749
(e.to_string(), Some(*transaction_id))
4850
} else {
4951
((*err).to_string(), Some(*transaction_id))
@@ -57,18 +59,11 @@ pub async fn handle_error(
5759

5860
if e.1.is_some() {
5961
if let Some(udp_server_stats_event_sender) = opt_udp_server_stats_event_sender.as_deref() {
60-
match remote_addr {
61-
SocketAddr::V4(_) => {
62-
udp_server_stats_event_sender
63-
.send_event(server_statistics::event::Event::Udp4Error)
64-
.await;
65-
}
66-
SocketAddr::V6(_) => {
67-
udp_server_stats_event_sender
68-
.send_event(server_statistics::event::Event::Udp6Error)
69-
.await;
70-
}
71-
}
62+
udp_server_stats_event_sender
63+
.send_event(server_statistics::event::Event::UdpError {
64+
context: ConnectionContext::new(client_socket_addr, server_socket_addr),
65+
})
66+
.await;
7267
}
7368
}
7469

0 commit comments

Comments
 (0)