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

Author: josecelano

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

@@ -30,10 +30,12 @@ impl ConnectionContext {
         }
     }
 
+    #[must_use]
     pub fn client_socket_addr(&self) -> SocketAddr {
         self.client_socket_addr
     }
 
+    #[must_use]
     pub fn server_socket_addr(&self) -> SocketAddr {
         self.server_socket_addr
     }

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

@@ -16,7 +16,7 @@ use zerocopy::network_endian::I32;
 
 use crate::error::Error;
 use crate::statistics as server_statistics;
-use crate::statistics::event::UdpResponseKind;
+use crate::statistics::event::{ConnectionContext, UdpRequestKind};
 
 /// It handles the `Announce` request.
 ///
@@ -32,7 +32,7 @@ pub async fn handle_announce(
     core_config: &Arc<Core>,
     opt_udp_server_stats_event_sender: &Arc<Option<Box<dyn server_statistics::event::sender::Sender>>>,
     cookie_valid_range: Range<f64>,
-) -> Result<Response, (Error, TransactionId)> {
+) -> Result<Response, (Error, TransactionId, UdpRequestKind)> {
     tracing::Span::current()
         .record("transaction_id", request.transaction_id.0.to_string())
         .record("connection_id", request.connection_id.0.to_string())
@@ -41,28 +41,18 @@ pub async fn handle_announce(
     tracing::trace!("handle announce");
 
     if let Some(udp_server_stats_event_sender) = opt_udp_server_stats_event_sender.as_deref() {
-        match client_socket_addr.ip() {
-            IpAddr::V4(_) => {
-                udp_server_stats_event_sender
-                    .send_event(server_statistics::event::Event::Udp4Request {
-                        kind: UdpResponseKind::Announce,
-                    })
-                    .await;
-            }
-            IpAddr::V6(_) => {
-                udp_server_stats_event_sender
-                    .send_event(server_statistics::event::Event::Udp6Request {
-                        kind: UdpResponseKind::Announce,
-                    })
-                    .await;
-            }
-        }
+        udp_server_stats_event_sender
+            .send_event(server_statistics::event::Event::UdpRequestAccepted {
+                context: ConnectionContext::new(client_socket_addr, server_socket_addr),
+                kind: UdpRequestKind::Announce,
+            })
+            .await;
     }
 
     let announce_data = announce_service
         .handle_announce(client_socket_addr, server_socket_addr, request, cookie_valid_range)
         .await
-        .map_err(|e| (e.into(), request.transaction_id))?;
+        .map_err(|e| (e.into(), request.transaction_id, UdpRequestKind::Announce))?;
 
     Ok(build_response(client_socket_addr, request, core_config, &announce_data))
 }
@@ -226,7 +216,7 @@ mod tests {
                 TorrentPeerBuilder,
             };
             use crate::statistics as server_statistics;
-            use crate::statistics::event::UdpResponseKind;
+            use crate::statistics::event::UdpRequestKind;
 
             #[tokio::test]
             async fn an_announced_peer_should_be_added_to_the_tracker() {
@@ -429,11 +419,15 @@ mod tests {
 
             #[tokio::test]
             async fn should_send_the_upd4_announce_event() {
+                let client_socket_addr = sample_ipv4_socket_address();
+                let server_socket_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(203, 0, 113, 196)), 6969);
+
                 let mut udp_server_stats_event_sender_mock = MockUdpServerStatsEventSender::new();
                 udp_server_stats_event_sender_mock
                     .expect_send_event()
-                    .with(eq(server_statistics::event::Event::Udp4Request {
-                        kind: UdpResponseKind::Announce,
+                    .with(eq(server_statistics::event::Event::UdpRequestAccepted {
+                        context: server_statistics::event::ConnectionContext::new(client_socket_addr, server_socket_addr),
+                        kind: UdpRequestKind::Announce,
                     }))
                     .times(1)
                     .returning(|_| Box::pin(future::ready(Some(Ok(())))));
@@ -443,9 +437,6 @@ mod tests {
                 let (core_tracker_services, core_udp_tracker_services, _server_udp_tracker_services) =
                     initialize_core_tracker_services_for_default_tracker_configuration();
 
-                let client_socket_addr = sample_ipv4_socket_address();
-                let server_socket_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(203, 0, 113, 196)), 6969);
-
                 handle_announce(
                     &core_udp_tracker_services.announce_service,
                     client_socket_addr,
@@ -549,7 +540,7 @@ mod tests {
                 sample_issue_time, MockUdpServerStatsEventSender, TorrentPeerBuilder,
             };
             use crate::statistics as server_statistics;
-            use crate::statistics::event::UdpResponseKind;
+            use crate::statistics::event::UdpRequestKind;
 
             #[tokio::test]
             async fn an_announced_peer_should_be_added_to_the_tracker() {
@@ -771,11 +762,15 @@ mod tests {
 
             #[tokio::test]
             async fn should_send_the_upd6_announce_event() {
+                let client_socket_addr = sample_ipv6_remote_addr();
+                let server_socket_addr = SocketAddr::new(IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 203, 0, 113, 196)), 6969);
+
                 let mut udp_server_stats_event_sender_mock = MockUdpServerStatsEventSender::new();
                 udp_server_stats_event_sender_mock
                     .expect_send_event()
-                    .with(eq(server_statistics::event::Event::Udp6Request {
-                        kind: UdpResponseKind::Announce,
+                    .with(eq(server_statistics::event::Event::UdpRequestAccepted {
+                        context: server_statistics::event::ConnectionContext::new(client_socket_addr, server_socket_addr),
+                        kind: UdpRequestKind::Announce,
                     }))
                     .times(1)
                     .returning(|_| Box::pin(future::ready(Some(Ok(())))));
@@ -785,9 +780,6 @@ mod tests {
                 let (core_tracker_services, core_udp_tracker_services, _server_udp_tracker_services) =
                     initialize_core_tracker_services_for_default_tracker_configuration();
 
-                let client_socket_addr = sample_ipv6_remote_addr();
-                let server_socket_addr = SocketAddr::new(IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 203, 0, 113, 196)), 6969);
-
                 let announce_request = AnnounceRequestBuilder::default()
                     .with_connection_id(make(gen_remote_fingerprint(&client_socket_addr), sample_issue_time()).unwrap())
                     .into();
@@ -819,7 +811,6 @@ mod tests {
                 use bittorrent_tracker_core::whitelist::repository::in_memory::InMemoryWhitelist;
                 use bittorrent_udp_tracker_core::connection_cookie::{gen_remote_fingerprint, make};
                 use bittorrent_udp_tracker_core::services::announce::AnnounceService;
-                use bittorrent_udp_tracker_core::statistics::event::ConnectionContext;
                 use bittorrent_udp_tracker_core::{self, statistics as core_statistics};
                 use mockall::predicate::eq;
 
@@ -830,7 +821,7 @@ mod tests {
                     TrackerConfigurationBuilder,
                 };
                 use crate::statistics as server_statistics;
-                use crate::statistics::event::UdpResponseKind;
+                use crate::statistics::event::UdpRequestKind;
 
                 #[tokio::test]
                 async fn the_peer_ip_should_be_changed_to_the_external_ip_in_the_tracker_configuration() {
@@ -860,7 +851,7 @@ mod tests {
                     udp_core_stats_event_sender_mock
                         .expect_send_event()
                         .with(eq(core_statistics::event::Event::UdpAnnounce {
-                            context: ConnectionContext::new(client_socket_addr, server_socket_addr),
+                            context: core_statistics::event::ConnectionContext::new(client_socket_addr, server_socket_addr),
                         }))
                         .times(1)
                         .returning(|_| Box::pin(future::ready(Some(Ok(())))));
@@ -870,8 +861,9 @@ mod tests {
                     let mut udp_server_stats_event_sender_mock = MockUdpServerStatsEventSender::new();
                     udp_server_stats_event_sender_mock
                         .expect_send_event()
-                        .with(eq(server_statistics::event::Event::Udp6Request {
-                            kind: UdpResponseKind::Announce,
+                        .with(eq(server_statistics::event::Event::UdpRequestAccepted {
+                            context: server_statistics::event::ConnectionContext::new(client_socket_addr, server_socket_addr),
+                            kind: UdpRequestKind::Announce,
                         }))
                         .times(1)
                         .returning(|_| Box::pin(future::ready(Some(Ok(())))));

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

@@ -1,19 +1,19 @@
 //! UDP tracker connect handler.
-use std::net::{IpAddr, SocketAddr};
+use std::net::SocketAddr;
 use std::sync::Arc;
 
 use aquatic_udp_protocol::{ConnectRequest, ConnectResponse, ConnectionId, Response};
 use bittorrent_udp_tracker_core::services::connect::ConnectService;
 use tracing::{instrument, Level};
 
 use crate::statistics as server_statistics;
-use crate::statistics::event::UdpResponseKind;
+use crate::statistics::event::{ConnectionContext, UdpRequestKind};
 
 /// It handles the `Connect` request.
 #[instrument(fields(transaction_id), skip(connect_service, opt_udp_server_stats_event_sender), ret(level = Level::TRACE))]
 pub async fn handle_connect(
-    remote_addr: SocketAddr,
-    server_addr: SocketAddr,
+    client_socket_addr: SocketAddr,
+    server_socket_addr: SocketAddr,
     request: &ConnectRequest,
     connect_service: &Arc<ConnectService>,
     opt_udp_server_stats_event_sender: &Arc<Option<Box<dyn server_statistics::event::sender::Sender>>>,
@@ -23,26 +23,16 @@ pub async fn handle_connect(
     tracing::trace!("handle connect");
 
     if let Some(udp_server_stats_event_sender) = opt_udp_server_stats_event_sender.as_deref() {
-        match remote_addr.ip() {
-            IpAddr::V4(_) => {
-                udp_server_stats_event_sender
-                    .send_event(server_statistics::event::Event::Udp4Request {
-                        kind: UdpResponseKind::Connect,
-                    })
-                    .await;
-            }
-            IpAddr::V6(_) => {
-                udp_server_stats_event_sender
-                    .send_event(server_statistics::event::Event::Udp6Request {
-                        kind: UdpResponseKind::Connect,
-                    })
-                    .await;
-            }
-        }
+        udp_server_stats_event_sender
+            .send_event(server_statistics::event::Event::UdpRequestAccepted {
+                context: ConnectionContext::new(client_socket_addr, server_socket_addr),
+                kind: UdpRequestKind::Connect,
+            })
+            .await;
     }
 
     let connection_id = connect_service
-        .handle_connect(remote_addr, server_addr, cookie_issue_time)
+        .handle_connect(client_socket_addr, server_socket_addr, cookie_issue_time)
         .await;
 
     build_response(*request, connection_id)
@@ -70,7 +60,6 @@ mod tests {
         use bittorrent_udp_tracker_core::connection_cookie::make;
         use bittorrent_udp_tracker_core::services::connect::ConnectService;
         use bittorrent_udp_tracker_core::statistics as core_statistics;
-        use bittorrent_udp_tracker_core::statistics::event::ConnectionContext;
         use mockall::predicate::eq;
 
         use crate::handlers::handle_connect;
@@ -79,7 +68,7 @@ mod tests {
             sample_ipv6_remote_addr_fingerprint, sample_issue_time, MockUdpCoreStatsEventSender, MockUdpServerStatsEventSender,
         };
         use crate::statistics as server_statistics;
-        use crate::statistics::event::UdpResponseKind;
+        use crate::statistics::event::UdpRequestKind;
 
         fn sample_connect_request() -> ConnectRequest {
             ConnectRequest {
@@ -214,8 +203,9 @@ mod tests {
             let mut udp_server_stats_event_sender_mock = MockUdpServerStatsEventSender::new();
             udp_server_stats_event_sender_mock
                 .expect_send_event()
-                .with(eq(server_statistics::event::Event::Udp4Request {
-                    kind: UdpResponseKind::Connect,
+                .with(eq(server_statistics::event::Event::UdpRequestAccepted {
+                    context: server_statistics::event::ConnectionContext::new(client_socket_addr, server_socket_addr),
+                    kind: UdpRequestKind::Connect,
                 }))
                 .times(1)
                 .returning(|_| Box::pin(future::ready(Some(Ok(())))));
@@ -244,7 +234,7 @@ mod tests {
             udp_core_stats_event_sender_mock
                 .expect_send_event()
                 .with(eq(core_statistics::event::Event::UdpConnect {
-                    context: ConnectionContext::new(client_socket_addr, server_socket_addr),
+                    context: core_statistics::event::ConnectionContext::new(client_socket_addr, server_socket_addr),
                 }))
                 .times(1)
                 .returning(|_| Box::pin(future::ready(Some(Ok(())))));
@@ -254,8 +244,9 @@ mod tests {
             let mut udp_server_stats_event_sender_mock = MockUdpServerStatsEventSender::new();
             udp_server_stats_event_sender_mock
                 .expect_send_event()
-                .with(eq(server_statistics::event::Event::Udp6Request {
-                    kind: UdpResponseKind::Connect,
+                .with(eq(server_statistics::event::Event::UdpRequestAccepted {
+                    context: server_statistics::event::ConnectionContext::new(client_socket_addr, server_socket_addr),
+                    kind: UdpRequestKind::Connect,
                 }))
                 .times(1)
                 .returning(|_| Box::pin(future::ready(Some(Ok(())))));

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

@@ -12,12 +12,14 @@ use zerocopy::network_endian::I32;
 
 use crate::error::Error;
 use crate::statistics as server_statistics;
+use crate::statistics::event::{ConnectionContext, UdpRequestKind};
 
 #[allow(clippy::too_many_arguments)]
 #[instrument(fields(transaction_id), skip(opt_udp_server_stats_event_sender), ret(level = Level::TRACE))]
 pub async fn handle_error(
-    remote_addr: SocketAddr,
-    local_addr: SocketAddr,
+    req_kind: Option<UdpRequestKind>,
+    client_socket_addr: SocketAddr,
+    server_socket_addr: SocketAddr,
     request_id: Uuid,
     opt_udp_server_stats_event_sender: &Arc<Option<Box<dyn server_statistics::event::sender::Sender>>>,
     cookie_valid_range: Range<f64>,
@@ -29,10 +31,10 @@ pub async fn handle_error(
     match transaction_id {
         Some(transaction_id) => {
             let transaction_id = transaction_id.0.to_string();
-            tracing::error!(target: UDP_TRACKER_LOG_TARGET, error = %e, %remote_addr, %local_addr, %request_id, %transaction_id, "response error");
+            tracing::error!(target: UDP_TRACKER_LOG_TARGET, error = %e, %client_socket_addr, %server_socket_addr, %request_id, %transaction_id, "response error");
         }
         None => {
-            tracing::error!(target: UDP_TRACKER_LOG_TARGET, error = %e, %remote_addr, %local_addr, %request_id, "response error");
+            tracing::error!(target: UDP_TRACKER_LOG_TARGET, error = %e, %client_socket_addr, %server_socket_addr, %request_id, "response error");
         }
     }
 
@@ -43,7 +45,7 @@ pub async fn handle_error(
                 transaction_id,
                 err,
             } => {
-                if let Err(e) = check(connection_id, gen_remote_fingerprint(&remote_addr), cookie_valid_range) {
+                if let Err(e) = check(connection_id, gen_remote_fingerprint(&client_socket_addr), cookie_valid_range) {
                     (e.to_string(), Some(*transaction_id))
                 } else {
                     ((*err).to_string(), Some(*transaction_id))
@@ -57,18 +59,11 @@ pub async fn handle_error(
 
     if e.1.is_some() {
         if let Some(udp_server_stats_event_sender) = opt_udp_server_stats_event_sender.as_deref() {
-            match remote_addr {
-                SocketAddr::V4(_) => {
-                    udp_server_stats_event_sender
-                        .send_event(server_statistics::event::Event::Udp4Error)
-                        .await;
-                }
-                SocketAddr::V6(_) => {
-                    udp_server_stats_event_sender
-                        .send_event(server_statistics::event::Event::Udp6Error)
-                        .await;
-                }
-            }
+            udp_server_stats_event_sender
+                .send_event(server_statistics::event::Event::UdpError {
+                    context: ConnectionContext::new(client_socket_addr, server_socket_addr),
+                })
+                .await;
         }
     }