From 89607cc8025fe048a3b9ee41c8b8082b1a8d8321 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Fri, 7 Mar 2025 11:48:50 +0000 Subject: [PATCH] refactor: [#1342] remove counter for HTTP connetions internally The number of HTTP tracker connections don't make sense. There are connection requests only in the UDP tracker. That code is removed but, in order to keep backward compatibility, the API still exposes that value which is the: number of announce requests + number of scrape requests --- .../tests/server/v1/contract.rs | 85 ------------------- .../src/v1/context/stats/resources.rs | 2 + .../src/v1/context/stats/responses.rs | 1 + .../src/services/announce.rs | 6 +- .../http-tracker-core/src/services/scrape.rs | 6 +- .../src/statistics/event/handler.rs | 48 ----------- .../src/statistics/metrics.rs | 8 -- .../src/statistics/repository.rs | 12 --- .../src/statistics/services.rs | 2 - .../src/statistics/metrics.rs | 2 + .../src/statistics/services.rs | 10 ++- 11 files changed, 15 insertions(+), 167 deletions(-) diff --git a/packages/axum-http-tracker-server/tests/server/v1/contract.rs b/packages/axum-http-tracker-server/tests/server/v1/contract.rs index 992793022..ad5b5a482 100644 --- a/packages/axum-http-tracker-server/tests/server/v1/contract.rs +++ b/packages/axum-http-tracker-server/tests/server/v1/contract.rs @@ -666,91 +666,6 @@ mod for_all_config_modes { compact_announce.is_ok() } - #[tokio::test] - async fn should_increase_the_number_of_tcp4_connections_handled_in_statistics() { - logging::setup(); - - let env = Started::new(&configuration::ephemeral_public().into()).await; - - Client::new(*env.bind_address()) - .announce(&QueryBuilder::default().query()) - .await; - - let stats = env - .container - .http_tracker_core_container - .http_stats_repository - .get_stats() - .await; - - assert_eq!(stats.tcp4_connections_handled, 1); - - drop(stats); - - env.stop().await; - } - - #[tokio::test] - async fn should_increase_the_number_of_tcp6_connections_handled_in_statistics() { - logging::setup(); - - if TcpListener::bind(SocketAddrV6::new(Ipv6Addr::LOCALHOST, 0, 0, 0)) - .await - .is_err() - { - return; // we cannot bind to a ipv6 socket, so we will skip this test - } - - let env = Started::new(&configuration::ephemeral_ipv6().into()).await; - - Client::bind(*env.bind_address(), IpAddr::from_str("::1").unwrap()) - .announce(&QueryBuilder::default().query()) - .await; - - let stats = env - .container - .http_tracker_core_container - .http_stats_repository - .get_stats() - .await; - - assert_eq!(stats.tcp6_connections_handled, 1); - - drop(stats); - - env.stop().await; - } - - #[tokio::test] - async fn should_not_increase_the_number_of_tcp6_connections_handled_if_the_client_is_not_using_an_ipv6_ip() { - logging::setup(); - - // The tracker ignores the peer address in the request param. It uses the client remote ip address. - - let env = Started::new(&configuration::ephemeral_public().into()).await; - - Client::new(*env.bind_address()) - .announce( - &QueryBuilder::default() - .with_peer_addr(&IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 1))) - .query(), - ) - .await; - - let stats = env - .container - .http_tracker_core_container - .http_stats_repository - .get_stats() - .await; - - assert_eq!(stats.tcp6_connections_handled, 0); - - drop(stats); - - env.stop().await; - } - #[tokio::test] async fn should_increase_the_number_of_tcp4_announce_requests_handled_in_statistics() { logging::setup(); diff --git a/packages/axum-rest-tracker-api-server/src/v1/context/stats/resources.rs b/packages/axum-rest-tracker-api-server/src/v1/context/stats/resources.rs index 9ed61cc6b..d9480259e 100644 --- a/packages/axum-rest-tracker-api-server/src/v1/context/stats/resources.rs +++ b/packages/axum-rest-tracker-api-server/src/v1/context/stats/resources.rs @@ -77,6 +77,7 @@ pub struct Stats { } impl From for Stats { + #[allow(deprecated)] fn from(metrics: TrackerMetrics) -> Self { Self { torrents: metrics.torrents_metrics.total_torrents, @@ -124,6 +125,7 @@ mod tests { use super::Stats; #[test] + #[allow(deprecated)] fn stats_resource_should_be_converted_from_tracker_metrics() { assert_eq!( Stats::from(TrackerMetrics { diff --git a/packages/axum-rest-tracker-api-server/src/v1/context/stats/responses.rs b/packages/axum-rest-tracker-api-server/src/v1/context/stats/responses.rs index 6d279726c..853fdd2e2 100644 --- a/packages/axum-rest-tracker-api-server/src/v1/context/stats/responses.rs +++ b/packages/axum-rest-tracker-api-server/src/v1/context/stats/responses.rs @@ -12,6 +12,7 @@ pub fn stats_response(tracker_metrics: TrackerMetrics) -> Response { } /// `200` response that contains the [`Stats`] resource in Prometheus Text Exposition Format . +#[allow(deprecated)] #[must_use] pub fn metrics_response(tracker_metrics: &TrackerMetrics) -> Response { let mut lines = vec![]; diff --git a/packages/http-tracker-core/src/services/announce.rs b/packages/http-tracker-core/src/services/announce.rs index 959dcc615..896387b28 100644 --- a/packages/http-tracker-core/src/services/announce.rs +++ b/packages/http-tracker-core/src/services/announce.rs @@ -28,12 +28,8 @@ use crate::statistics; /// /// The service sends an statistics event that increments: /// -/// - The number of TCP connections handled by the HTTP tracker. /// - The number of TCP `announce` requests handled by the HTTP tracker. -/// -/// > **NOTICE**: as the HTTP tracker does not requires a connection request -/// > like the UDP tracker, the number of TCP connections is incremented for -/// > each `announce` request. +/// - The number of TCP `scrape` requests handled by the HTTP tracker. pub struct AnnounceService { core_config: Arc, announce_handler: Arc, diff --git a/packages/http-tracker-core/src/services/scrape.rs b/packages/http-tracker-core/src/services/scrape.rs index dcb88508c..53eed0361 100644 --- a/packages/http-tracker-core/src/services/scrape.rs +++ b/packages/http-tracker-core/src/services/scrape.rs @@ -25,13 +25,9 @@ use crate::statistics; /// /// The service sends an statistics event that increments: /// -/// - The number of TCP connections handled by the HTTP tracker. +/// - The number of TCP `announce` requests handled by the HTTP tracker. /// - The number of TCP `scrape` requests handled by the HTTP tracker. /// -/// > **NOTICE**: as the HTTP tracker does not requires a connection request -/// > like the UDP tracker, the number of TCP connections is incremented for -/// > each `scrape` request. -/// /// # Errors /// /// This function will return an error if: diff --git a/packages/http-tracker-core/src/statistics/event/handler.rs b/packages/http-tracker-core/src/statistics/event/handler.rs index af323d06b..b0a0c186f 100644 --- a/packages/http-tracker-core/src/statistics/event/handler.rs +++ b/packages/http-tracker-core/src/statistics/event/handler.rs @@ -6,21 +6,17 @@ pub async fn handle_event(event: Event, stats_repository: &Repository) { // TCP4 Event::Tcp4Announce => { stats_repository.increase_tcp4_announces().await; - stats_repository.increase_tcp4_connections().await; } Event::Tcp4Scrape => { stats_repository.increase_tcp4_scrapes().await; - stats_repository.increase_tcp4_connections().await; } // TCP6 Event::Tcp6Announce => { stats_repository.increase_tcp6_announces().await; - stats_repository.increase_tcp6_connections().await; } Event::Tcp6Scrape => { stats_repository.increase_tcp6_scrapes().await; - stats_repository.increase_tcp6_connections().await; } } @@ -44,17 +40,6 @@ mod tests { assert_eq!(stats.tcp4_announces_handled, 1); } - #[tokio::test] - async fn should_increase_the_tcp4_connections_counter_when_it_receives_a_tcp4_announce_event() { - let stats_repository = Repository::new(); - - handle_event(Event::Tcp4Announce, &stats_repository).await; - - let stats = stats_repository.get_stats().await; - - assert_eq!(stats.tcp4_connections_handled, 1); - } - #[tokio::test] async fn should_increase_the_tcp4_scrapes_counter_when_it_receives_a_tcp4_scrape_event() { let stats_repository = Repository::new(); @@ -66,17 +51,6 @@ mod tests { assert_eq!(stats.tcp4_scrapes_handled, 1); } - #[tokio::test] - async fn should_increase_the_tcp4_connections_counter_when_it_receives_a_tcp4_scrape_event() { - let stats_repository = Repository::new(); - - handle_event(Event::Tcp4Scrape, &stats_repository).await; - - let stats = stats_repository.get_stats().await; - - assert_eq!(stats.tcp4_connections_handled, 1); - } - #[tokio::test] async fn should_increase_the_tcp6_announces_counter_when_it_receives_a_tcp6_announce_event() { let stats_repository = Repository::new(); @@ -88,17 +62,6 @@ mod tests { assert_eq!(stats.tcp6_announces_handled, 1); } - #[tokio::test] - async fn should_increase_the_tcp6_connections_counter_when_it_receives_a_tcp6_announce_event() { - let stats_repository = Repository::new(); - - handle_event(Event::Tcp6Announce, &stats_repository).await; - - let stats = stats_repository.get_stats().await; - - assert_eq!(stats.tcp6_connections_handled, 1); - } - #[tokio::test] async fn should_increase_the_tcp6_scrapes_counter_when_it_receives_a_tcp6_scrape_event() { let stats_repository = Repository::new(); @@ -109,15 +72,4 @@ mod tests { assert_eq!(stats.tcp6_scrapes_handled, 1); } - - #[tokio::test] - async fn should_increase_the_tcp6_connections_counter_when_it_receives_a_tcp6_scrape_event() { - let stats_repository = Repository::new(); - - handle_event(Event::Tcp6Scrape, &stats_repository).await; - - let stats = stats_repository.get_stats().await; - - assert_eq!(stats.tcp6_connections_handled, 1); - } } diff --git a/packages/http-tracker-core/src/statistics/metrics.rs b/packages/http-tracker-core/src/statistics/metrics.rs index ae4db9704..6c102770b 100644 --- a/packages/http-tracker-core/src/statistics/metrics.rs +++ b/packages/http-tracker-core/src/statistics/metrics.rs @@ -8,20 +8,12 @@ /// and also for each IP version used by the peers: IPv4 and IPv6. #[derive(Debug, PartialEq, Default)] pub struct Metrics { - /// Total number of TCP (HTTP tracker) connections from IPv4 peers. - /// Since the HTTP tracker spec does not require a handshake, this metric - /// increases for every HTTP request. - pub tcp4_connections_handled: u64, - /// Total number of TCP (HTTP tracker) `announce` requests from IPv4 peers. pub tcp4_announces_handled: u64, /// Total number of TCP (HTTP tracker) `scrape` requests from IPv4 peers. pub tcp4_scrapes_handled: u64, - /// Total number of TCP (HTTP tracker) connections from IPv6 peers. - pub tcp6_connections_handled: u64, - /// Total number of TCP (HTTP tracker) `announce` requests from IPv6 peers. pub tcp6_announces_handled: u64, diff --git a/packages/http-tracker-core/src/statistics/repository.rs b/packages/http-tracker-core/src/statistics/repository.rs index 41f048e29..5e15fc298 100644 --- a/packages/http-tracker-core/src/statistics/repository.rs +++ b/packages/http-tracker-core/src/statistics/repository.rs @@ -34,12 +34,6 @@ impl Repository { drop(stats_lock); } - pub async fn increase_tcp4_connections(&self) { - let mut stats_lock = self.stats.write().await; - stats_lock.tcp4_connections_handled += 1; - drop(stats_lock); - } - pub async fn increase_tcp4_scrapes(&self) { let mut stats_lock = self.stats.write().await; stats_lock.tcp4_scrapes_handled += 1; @@ -52,12 +46,6 @@ impl Repository { drop(stats_lock); } - pub async fn increase_tcp6_connections(&self) { - let mut stats_lock = self.stats.write().await; - stats_lock.tcp6_connections_handled += 1; - drop(stats_lock); - } - pub async fn increase_tcp6_scrapes(&self) { let mut stats_lock = self.stats.write().await; stats_lock.tcp6_scrapes_handled += 1; diff --git a/packages/http-tracker-core/src/statistics/services.rs b/packages/http-tracker-core/src/statistics/services.rs index f7808440a..dce7098b9 100644 --- a/packages/http-tracker-core/src/statistics/services.rs +++ b/packages/http-tracker-core/src/statistics/services.rs @@ -54,11 +54,9 @@ pub async fn get_metrics( torrents_metrics, protocol_metrics: Metrics { // TCPv4 - tcp4_connections_handled: stats.tcp4_connections_handled, tcp4_announces_handled: stats.tcp4_announces_handled, tcp4_scrapes_handled: stats.tcp4_scrapes_handled, // TCPv6 - tcp6_connections_handled: stats.tcp6_connections_handled, tcp6_announces_handled: stats.tcp6_announces_handled, tcp6_scrapes_handled: stats.tcp6_scrapes_handled, }, diff --git a/packages/rest-tracker-api-core/src/statistics/metrics.rs b/packages/rest-tracker-api-core/src/statistics/metrics.rs index 40262efd6..7e41cf713 100644 --- a/packages/rest-tracker-api-core/src/statistics/metrics.rs +++ b/packages/rest-tracker-api-core/src/statistics/metrics.rs @@ -11,6 +11,7 @@ pub struct Metrics { /// Total number of TCP (HTTP tracker) connections from IPv4 peers. /// Since the HTTP tracker spec does not require a handshake, this metric /// increases for every HTTP request. + #[deprecated(since = "3.1.0")] pub tcp4_connections_handled: u64, /// Total number of TCP (HTTP tracker) `announce` requests from IPv4 peers. @@ -20,6 +21,7 @@ pub struct Metrics { pub tcp4_scrapes_handled: u64, /// Total number of TCP (HTTP tracker) connections from IPv6 peers. + #[deprecated(since = "3.1.0")] pub tcp6_connections_handled: u64, /// Total number of TCP (HTTP tracker) `announce` requests from IPv6 peers. diff --git a/packages/rest-tracker-api-core/src/statistics/services.rs b/packages/rest-tracker-api-core/src/statistics/services.rs index ea4e159b6..5d7629443 100644 --- a/packages/rest-tracker-api-core/src/statistics/services.rs +++ b/packages/rest-tracker-api-core/src/statistics/services.rs @@ -24,6 +24,7 @@ pub struct TrackerMetrics { } /// It returns all the [`TrackerMetrics`] +#[allow(deprecated)] pub async fn get_metrics( in_memory_torrent_repository: Arc, ban_service: Arc>, @@ -37,15 +38,20 @@ pub async fn get_metrics( let udp_core_stats = udp_core_stats_repository.get_stats().await; let udp_server_stats = udp_server_stats_repository.get_stats().await; + // For backward compatibility we keep the `tcp4_connections_handled` and + // `tcp6_connections_handled` metrics. They don't make sense for the HTTP + // tracker, but we keep them for now. In new major versions we should remove + // them. + TrackerMetrics { torrents_metrics, protocol_metrics: Metrics { // TCPv4 - tcp4_connections_handled: http_stats.tcp4_connections_handled, + tcp4_connections_handled: http_stats.tcp4_announces_handled + http_stats.tcp4_scrapes_handled, tcp4_announces_handled: http_stats.tcp4_announces_handled, tcp4_scrapes_handled: http_stats.tcp4_scrapes_handled, // TCPv6 - tcp6_connections_handled: http_stats.tcp6_connections_handled, + tcp6_connections_handled: http_stats.tcp6_announces_handled + http_stats.tcp6_scrapes_handled, tcp6_announces_handled: http_stats.tcp6_announces_handled, tcp6_scrapes_handled: http_stats.tcp6_scrapes_handled, // UDP