refactor(connlib): replace LogUnwrap with macros (#7138)

Using a trait means the call-site of the log message will always be the
`log_unwrap` module, despite the `#[track_caller]` annotation. That one
only works for `std::panic::Location` unfortunately which `tracing`
isn't using.

Macros will be evaluated earlier and thus the messages will show up with
the correct module name.
This commit is contained in:
Thomas Eizinger
2024-10-24 10:44:23 +11:00
committed by GitHub
parent 8b62f63065
commit 80c5b0df71
4 changed files with 76 additions and 64 deletions

View File

@@ -14,7 +14,7 @@ use bimap::BiMap;
use connlib_model::PublicKey;
use connlib_model::{GatewayId, RelayId, ResourceId, ResourceStatus, ResourceView};
use connlib_model::{Site, SiteId};
use firezone_logging::{anyhow_dyn_err, std_dyn_err, LogUnwrap as _};
use firezone_logging::{anyhow_dyn_err, std_dyn_err, unwrap_or_debug, unwrap_or_warn};
use ip_network::{IpNetwork, Ipv4Network, Ipv6Network};
use ip_network_table::IpNetworkTable;
use ip_packet::{IpPacket, UdpSlice};
@@ -384,8 +384,10 @@ impl ClientState {
dns::servfail(response.query.for_slice_ref())
});
self.try_queue_udp_dns_response(server, source, &message)
.log_unwrap_warn("Failed to queue UDP DNS response");
unwrap_or_warn!(
self.try_queue_udp_dns_response(server, source, &message),
"Failed to queue UDP DNS response"
);
}
(dns::Transport::Tcp { source }, result) => {
let message = result
@@ -398,9 +400,10 @@ impl ClientState {
dns::servfail(response.query.for_slice_ref())
});
self.tcp_dns_server
.send_message(source, message)
.log_unwrap_warn("Failed to send TCP DNS response");
unwrap_or_warn!(
self.tcp_dns_server.send_message(source, message),
"Failed to send TCP DNS response"
);
}
}
}
@@ -966,8 +969,10 @@ impl ClientState {
match self.stub_resolver.handle(message) {
dns::ResolveStrategy::LocalResponse(response) => {
self.try_queue_udp_dns_response(upstream, source, &response)
.log_unwrap_debug("Failed to queue UDP DNS response");
unwrap_or_debug!(
self.try_queue_udp_dns_response(upstream, source, &response),
"Failed to queue UDP DNS response"
);
}
dns::ResolveStrategy::Recurse => {
let query_id = message.header().id();
@@ -1010,9 +1015,10 @@ impl ClientState {
match self.stub_resolver.handle(message.for_slice_ref()) {
dns::ResolveStrategy::LocalResponse(response) => {
self.tcp_dns_server
.send_message(query.socket, response)
.log_unwrap_debug("Failed to send TCP DNS response");
unwrap_or_debug!(
self.tcp_dns_server.send_message(query.socket, response),
"Failed to send TCP DNS response"
);
}
dns::ResolveStrategy::Recurse => {
let query_id = message.header().id();
@@ -1026,9 +1032,13 @@ impl ClientState {
"Failed to send recursive TCP DNS quer"
);
self.tcp_dns_server
.send_message(query.socket, dns::servfail(message.for_slice_ref()))
.log_unwrap_debug("Failed to send TCP DNS response");
unwrap_or_debug!(
self.tcp_dns_server.send_message(
query.socket,
dns::servfail(message.for_slice_ref())
),
"Failed to send TCP DNS response"
);
return;
}
};

View File

@@ -1,7 +1,8 @@
mod dyn_err;
pub mod file;
mod format;
mod log_unwrap;
#[macro_use]
mod unwrap_or;
use sentry_tracing::EventFilter;
use tracing::{subscriber::DefaultGuard, Subscriber};
@@ -13,7 +14,6 @@ use tracing_subscriber::{
pub use dyn_err::{anyhow_dyn_err, std_dyn_err};
pub use format::Format;
pub use log_unwrap::LogUnwrap;
/// Registers a global subscriber with stdout logging and `additional_layer`
pub fn setup_global_subscriber<L>(additional_layer: L)

View File

@@ -1,48 +0,0 @@
use std::error::Error;
pub trait LogUnwrap {
#[track_caller]
fn log_unwrap_warn(&self, msg: &str);
#[track_caller]
fn log_unwrap_debug(&self, msg: &str);
#[track_caller]
fn log_unwrap_trace(&self, msg: &str);
}
impl LogUnwrap for anyhow::Result<()> {
#[track_caller]
fn log_unwrap_warn(&self, msg: &str) {
match self {
Ok(()) => {}
Err(e) => {
let error: &dyn Error = e.as_ref();
tracing::warn!(error, "{msg}")
}
}
}
#[track_caller]
fn log_unwrap_debug(&self, msg: &str) {
match self {
Ok(()) => {}
Err(e) => {
let error: &dyn Error = e.as_ref();
tracing::debug!(error, "{msg}")
}
}
}
#[track_caller]
fn log_unwrap_trace(&self, msg: &str) {
match self {
Ok(()) => {}
Err(e) => {
let error: &dyn Error = e.as_ref();
tracing::trace!(error, "{msg}")
}
}
}
}

View File

@@ -0,0 +1,50 @@
#[macro_export]
macro_rules! unwrap_or_warn {
(
$result:expr,
$($arg:tt)*
) => {
match $result {
Ok(()) => {}
Err(e) => {
let error: &dyn ::std::error::Error = e.as_ref();
::tracing::warn!(error, $($arg)*)
}
}
};
}
#[macro_export]
macro_rules! unwrap_or_debug {
(
$result:expr,
$($arg:tt)*
) => {
match $result {
Ok(()) => {}
Err(e) => {
let error: &dyn ::std::error::Error = e.as_ref();
::tracing::debug!(error, $($arg)*)
}
}
};
}
#[macro_export]
macro_rules! unwrap_or_trace {
(
$result:expr,
$($arg:tt)*
) => {
match $result {
Ok(()) => {}
Err(e) => {
let error: &dyn ::std::error::Error = e.as_ref();
::tracing::debug!(error, $($arg)*)
}
}
};
}