Skip to content

Commit eb9f997

Browse files
josecelanojosecelano
committed
Merge torrust#913: Minor UDP server refactorings
f06976e docs: update some UDP server comments (Jose Celano) 89bb735 refactor: reorganize UDP server mod (Jose Celano) c121bf2 refactor: rename UDP server types (Jose Celano) 336e0e6 refactor: reorganize mod to extract new submods (Jose Celano) 61fb4b2 refactor: move active requests logic to ActiveRequest type (Jose Celano) 35b6c84 refactor: simplify UDP server receiver (Jose Celano) a5e2baf refactor: extract method (Jose Celano) b4b4515 refactor: extract const for logging targets (Jose Celano) 0388e1d refactor: extract consts for logging targets (Jose Celano) 16ae4fd refactor: rename vars and extract constructor (Jose Celano) 7ff0cd2 refactor: rename var (Jose Celano) 0e3678d refactor: rename Socket to BoundSocket and fix format errors" (Jose Celano) 9b3b75b fix: log message (Jose Celano) Pull request description: This PR only includes some minor changes I've proposed in this [PR](torrust#873), and some refactorings. ACKs for top commit: josecelano: ACK f06976e Tree-SHA512: a7a4ea14077c2ce6df0b80b222952d0c6c6588f1df50f78d01198ea0ab12ce3ca74923caed6601994e955880e2afe3d9432f87ae74f383362fae452e367ad359
2 parents b7bcd96 + f06976e commit eb9f997

29 files changed

+873
-725
lines changed

‎src/bootstrap/jobs/health_check_api.rs

+5-4
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@ use torrust_tracker_configuration::HealthCheckApi;
2020
use tracing::info;
2121

2222
use super::Started;
23-
use crate::servers::health_check_api::server;
23+
use crate::servers::health_check_api::{server, HEALTH_CHECK_API_LOG_TARGET};
24+
use crate::servers::logging::STARTED_ON;
2425
use crate::servers::registar::ServiceRegistry;
2526
use crate::servers::signals::Halted;
2627

@@ -44,18 +45,18 @@ pub async fn start_job(config: &HealthCheckApi, register: ServiceRegistry) -> Jo
4445

4546
// Run the API server
4647
let join_handle = tokio::spawn(async move {
47-
info!(target: "HEALTH CHECK API", "Starting on: {protocol}://{}", bind_addr);
48+
info!(target: HEALTH_CHECK_API_LOG_TARGET, "Starting on: {protocol}://{}", bind_addr);
4849

4950
let handle = server::start(bind_addr, tx_start, rx_halt, register);
5051

5152
if let Ok(()) = handle.await {
52-
info!(target: "HEALTH CHECK API", "Stopped server running on: {protocol}://{}", bind_addr);
53+
info!(target: HEALTH_CHECK_API_LOG_TARGET, "Stopped server running on: {protocol}://{}", bind_addr);
5354
}
5455
});
5556

5657
// Wait until the server sends the started message
5758
match rx_start.await {
58-
Ok(msg) => info!(target: "HEALTH CHECK API", "Started on: {protocol}://{}", msg.address),
59+
Ok(msg) => info!(target: HEALTH_CHECK_API_LOG_TARGET, "{STARTED_ON}: {protocol}://{}", msg.address),
5960
Err(e) => panic!("the Health Check API server was dropped: {e}"),
6061
}
6162

‎src/bootstrap/jobs/udp_tracker.rs

+7-5
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ use tracing::debug;
1414

1515
use crate::core;
1616
use crate::servers::registar::ServiceRegistrationForm;
17-
use crate::servers::udp::server::{Launcher, UdpServer};
17+
use crate::servers::udp::server::spawner::Spawner;
18+
use crate::servers::udp::server::Server;
19+
use crate::servers::udp::UDP_TRACKER_LOG_TARGET;
1820

1921
/// It starts a new UDP server with the provided configuration.
2022
///
@@ -29,14 +31,14 @@ use crate::servers::udp::server::{Launcher, UdpServer};
2931
pub async fn start_job(config: &UdpTracker, tracker: Arc<core::Tracker>, form: ServiceRegistrationForm) -> JoinHandle<()> {
3032
let bind_to = config.bind_address;
3133

32-
let server = UdpServer::new(Launcher::new(bind_to))
34+
let server = Server::new(Spawner::new(bind_to))
3335
.start(tracker, form)
3436
.await
3537
.expect("it should be able to start the udp tracker");
3638

3739
tokio::spawn(async move {
38-
debug!(target: "UDP TRACKER", "Wait for launcher (UDP service) to finish ...");
39-
debug!(target: "UDP TRACKER", "Is halt channel closed before waiting?: {}", server.state.halt_task.is_closed());
40+
debug!(target: UDP_TRACKER_LOG_TARGET, "Wait for launcher (UDP service) to finish ...");
41+
debug!(target: UDP_TRACKER_LOG_TARGET, "Is halt channel closed before waiting?: {}", server.state.halt_task.is_closed());
4042

4143
assert!(
4244
!server.state.halt_task.is_closed(),
@@ -49,6 +51,6 @@ pub async fn start_job(config: &UdpTracker, tracker: Arc<core::Tracker>, form: S
4951
.await
5052
.expect("it should be able to join to the udp tracker task");
5153

52-
debug!(target: "UDP TRACKER", "Is halt channel closed after finishing the server?: {}", server.state.halt_task.is_closed());
54+
debug!(target: UDP_TRACKER_LOG_TARGET, "Is halt channel closed after finishing the server?: {}", server.state.halt_task.is_closed());
5355
})
5456
}

‎src/console/ci/e2e/logs_parser.rs

+8-6
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@
22
use regex::Regex;
33
use serde::{Deserialize, Serialize};
44

5+
use crate::servers::health_check_api::HEALTH_CHECK_API_LOG_TARGET;
6+
use crate::servers::http::HTTP_TRACKER_LOG_TARGET;
7+
use crate::servers::logging::STARTED_ON;
8+
use crate::servers::udp::UDP_TRACKER_LOG_TARGET;
9+
510
const INFO_LOG_LEVEL: &str = "INFO";
6-
const UDP_TRACKER_LOG_TARGET: &str = "UDP TRACKER";
7-
const HTTP_TRACKER_LOG_TARGET: &str = "HTTP TRACKER";
8-
const HEALTH_CHECK_API_LOG_TARGET: &str = "HEALTH CHECK API";
911

1012
#[derive(Serialize, Deserialize, Debug, Default)]
1113
pub struct RunningServices {
@@ -64,9 +66,9 @@ impl RunningServices {
6466
let mut http_trackers: Vec<String> = Vec::new();
6567
let mut health_checks: Vec<String> = Vec::new();
6668

67-
let udp_re = Regex::new(r"Started on: udp://([0-9.]+:[0-9]+)").unwrap();
68-
let http_re = Regex::new(r"Started on: (https?://[0-9.]+:[0-9]+)").unwrap(); // DevSkim: ignore DS137138
69-
let health_re = Regex::new(r"Started on: (https?://[0-9.]+:[0-9]+)").unwrap(); // DevSkim: ignore DS137138
69+
let udp_re = Regex::new(&format!("{STARTED_ON}: {}", r"udp://([0-9.]+:[0-9]+)")).unwrap();
70+
let http_re = Regex::new(&format!("{STARTED_ON}: {}", r"(https?://[0-9.]+:[0-9]+)")).unwrap(); // DevSkim: ignore DS137138
71+
let health_re = Regex::new(&format!("{STARTED_ON}: {}", r"(https?://[0-9.]+:[0-9]+)")).unwrap(); // DevSkim: ignore DS137138
7072
let ansi_escape_re = Regex::new(r"\x1b\[[0-9;]*m").unwrap();
7173

7274
for line in logs.lines() {

‎src/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,7 @@ pub mod bootstrap;
494494
pub mod console;
495495
pub mod core;
496496
pub mod servers;
497-
pub mod shared;
497+
pub mod shared;
498498

499499
#[macro_use]
500500
extern crate lazy_static;

‎src/servers/apis/mod.rs

+4-2
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,10 @@ pub mod routes;
157157
pub mod server;
158158
pub mod v1;
159159

160+
use serde::{Deserialize, Serialize};
161+
162+
pub const API_LOG_TARGET: &str = "API";
163+
160164
/// The info hash URL path parameter.
161165
///
162166
/// Some API endpoints require an info hash as a path parameter.
@@ -169,8 +173,6 @@ pub mod v1;
169173
#[derive(Deserialize)]
170174
pub struct InfoHashParam(pub String);
171175

172-
use serde::{Deserialize, Serialize};
173-
174176
/// The version of the HTTP Api.
175177
#[derive(Serialize, Deserialize, Copy, Clone, PartialEq, Eq, Debug)]
176178
pub enum Version {

‎src/servers/apis/routes.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ use super::v1;
2727
use super::v1::context::health_check::handlers::health_check_handler;
2828
use super::v1::middlewares::auth::State;
2929
use crate::core::Tracker;
30+
use crate::servers::apis::API_LOG_TARGET;
3031

3132
const TIMEOUT: Duration = Duration::from_secs(5);
3233

@@ -60,7 +61,7 @@ pub fn router(tracker: Arc<Tracker>, access_tokens: Arc<AccessTokens>) -> Router
6061
.unwrap_or_default();
6162

6263
tracing::span!(
63-
target: "API",
64+
target: API_LOG_TARGET,
6465
tracing::Level::INFO, "request", method = %method, uri = %uri, request_id = %request_id);
6566
})
6667
.on_response(|response: &Response, latency: Duration, _span: &Span| {
@@ -73,7 +74,7 @@ pub fn router(tracker: Arc<Tracker>, access_tokens: Arc<AccessTokens>) -> Router
7374
let latency_ms = latency.as_millis();
7475

7576
tracing::span!(
76-
target: "API",
77+
target: API_LOG_TARGET,
7778
tracing::Level::INFO, "response", latency = %latency_ms, status = %status_code, request_id = %request_id);
7879
}),
7980
)

‎src/servers/apis/server.rs

+6-4
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,9 @@ use tracing::{debug, error, info};
3737
use super::routes::router;
3838
use crate::bootstrap::jobs::Started;
3939
use crate::core::Tracker;
40+
use crate::servers::apis::API_LOG_TARGET;
4041
use crate::servers::custom_axum_server::{self, TimeoutAcceptor};
42+
use crate::servers::logging::STARTED_ON;
4143
use crate::servers::registar::{ServiceHealthCheckJob, ServiceRegistration, ServiceRegistrationForm};
4244
use crate::servers::signals::{graceful_shutdown, Halted};
4345

@@ -121,11 +123,11 @@ impl ApiServer<Stopped> {
121123
let launcher = self.state.launcher;
122124

123125
let task = tokio::spawn(async move {
124-
debug!(target: "API", "Starting with launcher in spawned task ...");
126+
debug!(target: API_LOG_TARGET, "Starting with launcher in spawned task ...");
125127

126128
let _task = launcher.start(tracker, access_tokens, tx_start, rx_halt).await;
127129

128-
debug!(target: "API", "Started with launcher in spawned task");
130+
debug!(target: API_LOG_TARGET, "Started with launcher in spawned task");
129131

130132
launcher
131133
});
@@ -231,7 +233,7 @@ impl Launcher {
231233
let tls = self.tls.clone();
232234
let protocol = if tls.is_some() { "https" } else { "http" };
233235

234-
info!(target: "API", "Starting on {protocol}://{}", address);
236+
info!(target: API_LOG_TARGET, "Starting on {protocol}://{}", address);
235237

236238
let running = Box::pin(async {
237239
match tls {
@@ -250,7 +252,7 @@ impl Launcher {
250252
}
251253
});
252254

253-
info!(target: "API", "Started on {protocol}://{}", address);
255+
info!(target: API_LOG_TARGET, "{STARTED_ON} {protocol}://{}", address);
254256

255257
tx_start
256258
.send(Started { address })

‎src/servers/health_check_api/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,5 @@ pub mod handlers;
22
pub mod resources;
33
pub mod responses;
44
pub mod server;
5+
6+
pub const HEALTH_CHECK_API_LOG_TARGET: &str = "HEALTH CHECK API";

‎src/servers/health_check_api/server.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ use tracing::{debug, Level, Span};
2222

2323
use crate::bootstrap::jobs::Started;
2424
use crate::servers::health_check_api::handlers::health_check_handler;
25+
use crate::servers::health_check_api::HEALTH_CHECK_API_LOG_TARGET;
2526
use crate::servers::registar::ServiceRegistry;
2627
use crate::servers::signals::{graceful_shutdown, Halted};
2728

@@ -56,7 +57,7 @@ pub fn start(
5657
.unwrap_or_default();
5758

5859
tracing::span!(
59-
target: "HEALTH CHECK API",
60+
target: HEALTH_CHECK_API_LOG_TARGET,
6061
tracing::Level::INFO, "request", method = %method, uri = %uri, request_id = %request_id);
6162
})
6263
.on_response(|response: &Response, latency: Duration, _span: &Span| {
@@ -69,7 +70,7 @@ pub fn start(
6970
let latency_ms = latency.as_millis();
7071

7172
tracing::span!(
72-
target: "HEALTH CHECK API",
73+
target: HEALTH_CHECK_API_LOG_TARGET,
7374
tracing::Level::INFO, "response", latency = %latency_ms, status = %status_code, request_id = %request_id);
7475
}),
7576
)
@@ -80,7 +81,7 @@ pub fn start(
8081

8182
let handle = Handle::new();
8283

83-
debug!(target: "HEALTH CHECK API", "Starting service with graceful shutdown in a spawned task ...");
84+
debug!(target: HEALTH_CHECK_API_LOG_TARGET, "Starting service with graceful shutdown in a spawned task ...");
8485

8586
tokio::task::spawn(graceful_shutdown(
8687
handle.clone(),

‎src/servers/http/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,8 @@ pub mod percent_encoding;
309309
pub mod server;
310310
pub mod v1;
311311

312+
pub const HTTP_TRACKER_LOG_TARGET: &str = "HTTP TRACKER";
313+
312314
/// The version of the HTTP tracker.
313315
#[derive(Serialize, Deserialize, Copy, Clone, PartialEq, Eq, Debug)]
314316
pub enum Version {

‎src/servers/http/server.rs

+4-2
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ use super::v1::routes::router;
1313
use crate::bootstrap::jobs::Started;
1414
use crate::core::Tracker;
1515
use crate::servers::custom_axum_server::{self, TimeoutAcceptor};
16+
use crate::servers::http::HTTP_TRACKER_LOG_TARGET;
17+
use crate::servers::logging::STARTED_ON;
1618
use crate::servers::registar::{ServiceHealthCheckJob, ServiceRegistration, ServiceRegistrationForm};
1719
use crate::servers::signals::{graceful_shutdown, Halted};
1820

@@ -55,7 +57,7 @@ impl Launcher {
5557
let tls = self.tls.clone();
5658
let protocol = if tls.is_some() { "https" } else { "http" };
5759

58-
info!(target: "HTTP TRACKER", "Starting on: {protocol}://{}", address);
60+
info!(target: HTTP_TRACKER_LOG_TARGET, "Starting on: {protocol}://{}", address);
5961

6062
let app = router(tracker, address);
6163

@@ -76,7 +78,7 @@ impl Launcher {
7678
}
7779
});
7880

79-
info!(target: "HTTP TRACKER", "Started on: {protocol}://{}", address);
81+
info!(target: HTTP_TRACKER_LOG_TARGET, "{STARTED_ON}: {protocol}://{}", address);
8082

8183
tx_start
8284
.send(Started { address })

‎src/servers/http/v1/routes.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ use tracing::{Level, Span};
2020

2121
use super::handlers::{announce, health_check, scrape};
2222
use crate::core::Tracker;
23+
use crate::servers::http::HTTP_TRACKER_LOG_TARGET;
2324

2425
const TIMEOUT: Duration = Duration::from_secs(5);
2526

@@ -56,7 +57,7 @@ pub fn router(tracker: Arc<Tracker>, server_socket_addr: SocketAddr) -> Router {
5657
.unwrap_or_default();
5758

5859
tracing::span!(
59-
target:"HTTP TRACKER",
60+
target: HTTP_TRACKER_LOG_TARGET,
6061
tracing::Level::INFO, "request", server_socket_addr= %server_socket_addr, method = %method, uri = %uri, request_id = %request_id);
6162
})
6263
.on_response(move |response: &Response, latency: Duration, _span: &Span| {
@@ -69,7 +70,7 @@ pub fn router(tracker: Arc<Tracker>, server_socket_addr: SocketAddr) -> Router {
6970
let latency_ms = latency.as_millis();
7071

7172
tracing::span!(
72-
target: "HTTP TRACKER",
73+
target: HTTP_TRACKER_LOG_TARGET,
7374
tracing::Level::INFO, "response", server_socket_addr= %server_socket_addr, latency = %latency_ms, status = %status_code, request_id = %request_id);
7475
}),
7576
)

‎src/servers/logging.rs

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/// This is the prefix used in logs to identify a started service.
2+
///
3+
/// For example:
4+
///
5+
/// ```text
6+
/// 2024-06-25T12:36:25.025312Z INFO UDP TRACKER: Started on: udp://0.0.0.0:6969
7+
/// 2024-06-25T12:36:25.025445Z INFO HTTP TRACKER: Started on: http://0.0.0.0:7070
8+
/// 2024-06-25T12:36:25.025527Z INFO API: Started on http://0.0.0.0:1212
9+
/// 2024-06-25T12:36:25.025580Z INFO HEALTH CHECK API: Started on: http://127.0.0.1:1313
10+
/// ```
11+
pub const STARTED_ON: &str = "Started on";
12+
13+
/*
14+
15+
todo: we should use a field fot the URL.
16+
17+
For example, instead of:
18+
19+
```
20+
2024-06-25T12:36:25.025312Z INFO UDP TRACKER: Started on: udp://0.0.0.0:6969
21+
```
22+
23+
We should use something like:
24+
25+
```
26+
2024-06-25T12:36:25.025312Z INFO UDP TRACKER started_at_url=udp://0.0.0.0:6969
27+
```
28+
29+
*/

‎src/servers/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ pub mod apis;
33
pub mod custom_axum_server;
44
pub mod health_check_api;
55
pub mod http;
6+
pub mod logging;
67
pub mod registar;
78
pub mod signals;
89
pub mod udp;

‎src/servers/udp/handlers.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use uuid::Uuid;
1717
use zerocopy::network_endian::I32;
1818

1919
use super::connection_cookie::{check, from_connection_id, into_connection_id, make};
20-
use super::UdpRequest;
20+
use super::RawRequest;
2121
use crate::core::{statistics, ScrapeData, Tracker};
2222
use crate::servers::udp::error::Error;
2323
use crate::servers::udp::logging::{log_bad_request, log_error_response, log_request, log_response};
@@ -33,7 +33,7 @@ use crate::shared::bit_torrent::common::MAX_SCRAPE_TORRENTS;
3333
/// - Delegating the request to the correct handler depending on the request type.
3434
///
3535
/// It will return an `Error` response if the request is invalid.
36-
pub(crate) async fn handle_packet(udp_request: UdpRequest, tracker: &Arc<Tracker>, addr: SocketAddr) -> Response {
36+
pub(crate) async fn handle_packet(udp_request: RawRequest, tracker: &Arc<Tracker>, addr: SocketAddr) -> Response {
3737
debug!("Handling Packets: {udp_request:?}");
3838

3939
let start_time = Instant::now();
@@ -304,7 +304,7 @@ fn handle_error(e: &Error, transaction_id: TransactionId) -> Response {
304304
pub struct RequestId(Uuid);
305305

306306
impl RequestId {
307-
fn make(_request: &UdpRequest) -> RequestId {
307+
fn make(_request: &RawRequest) -> RequestId {
308308
RequestId(Uuid::new_v4())
309309
}
310310
}

0 commit comments

Comments
 (0)