From ec2599d54533cfe90b6f2525c073d3d2dfbcb796 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 4 Jul 2025 15:51:53 +0100 Subject: [PATCH] chore(rust): simplify stream logs feature (#9780) Instead of conditionally enabling the `logs` feature in the Sentry client, we always enable it and control via the `tracing` integration, which events should get forwarded to Sentry. The feature-flag check accesses only shared-memory and is therefore really fast. We already re-evaluate feature flags on a timer which means this boolean will flip over automatically and logs will be streamed to Sentry. --- rust/Cargo.lock | 1 + rust/gateway/src/eventloop.rs | 20 +++------------ rust/gateway/src/main.rs | 3 +-- rust/gui-client/src-tauri/src/service.rs | 10 -------- rust/headless-client/src/main.rs | 7 ------ rust/logging/Cargo.toml | 1 + rust/logging/src/lib.rs | 17 ++++++++++--- rust/telemetry/src/lib.rs | 32 ++---------------------- 8 files changed, 21 insertions(+), 70 deletions(-) diff --git a/rust/Cargo.lock b/rust/Cargo.lock index bd09126d3..45c245440 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -2511,6 +2511,7 @@ name = "firezone-logging" version = "0.1.0" dependencies = [ "anyhow", + "firezone-telemetry", "nu-ansi-term 0.50.1", "output_vt100", "parking_lot", diff --git a/rust/gateway/src/eventloop.rs b/rust/gateway/src/eventloop.rs index 163eea393..08cde5ae5 100644 --- a/rust/gateway/src/eventloop.rs +++ b/rust/gateway/src/eventloop.rs @@ -48,7 +48,7 @@ enum ResolveTrigger { SetupNat(ResolveDnsRequest), } -pub struct Eventloop<'a> { +pub struct Eventloop { tunnel: GatewayTunnel, portal: PhoenixChannel<(), IngressMessages, (), PublicKeyParam>, tun_device_manager: Arc>, @@ -60,19 +60,15 @@ pub struct Eventloop<'a> { set_interface_tasks: futures_bounded::FuturesSet>, - telemetry_refresh: tokio::time::Interval, - telemetry: &'a mut Telemetry, - logged_permission_denied: bool, } -impl<'a> Eventloop<'a> { +impl Eventloop { pub(crate) fn new( tunnel: GatewayTunnel, mut portal: PhoenixChannel<(), IngressMessages, (), PublicKeyParam>, tun_device_manager: TunDeviceManager, firezone_id: String, - telemetry: &'a mut Telemetry, ) -> Self { portal.connect(PublicKeyParam(tunnel.public_key().to_bytes())); @@ -91,13 +87,11 @@ impl<'a> Eventloop<'a> { tracing::debug!(%domain, ?ips, ?cause, "DNS cache entry evicted"); }) .build(), - telemetry_refresh: tokio::time::interval(Duration::from_secs(60)), - telemetry, } } } -impl<'a> Eventloop<'a> { +impl Eventloop { pub fn poll(&mut self, cx: &mut Context<'_>) -> Poll> { loop { match self.tunnel.poll_next_event(cx) { @@ -215,14 +209,6 @@ impl<'a> Eventloop<'a> { Poll::Pending => {} } - match self.telemetry_refresh.poll_tick(cx) { - Poll::Ready(_) => { - self.telemetry.refresh_config(); - continue; - } - Poll::Pending => {} - } - return Poll::Pending; } } diff --git a/rust/gateway/src/main.rs b/rust/gateway/src/main.rs index 3281986be..7a993ad15 100644 --- a/rust/gateway/src/main.rs +++ b/rust/gateway/src/main.rs @@ -184,8 +184,7 @@ async fn try_main(cli: Cli, telemetry: &mut Telemetry) -> Result<()> { } let eventloop = future::poll_fn({ - let mut eventloop = - Eventloop::new(tunnel, portal, tun_device_manager, firezone_id, telemetry); + let mut eventloop = Eventloop::new(tunnel, portal, tun_device_manager, firezone_id); move |cx| eventloop.poll(cx) }); diff --git a/rust/gui-client/src-tauri/src/service.rs b/rust/gui-client/src-tauri/src/service.rs index 7914a3d18..70be17ae6 100644 --- a/rust/gui-client/src-tauri/src/service.rs +++ b/rust/gui-client/src-tauri/src/service.rs @@ -175,7 +175,6 @@ struct Handler<'a> { tun_device: TunDeviceManager, dns_notifier: BoxStream<'static, Result<()>>, network_notifier: BoxStream<'static, Result<()>>, - telemetry_refresh: tokio::time::Interval, } #[derive(Default, Debug)] @@ -263,7 +262,6 @@ enum Event { Terminate, NetworkChanged(Result<()>), DnsChanged(Result<()>), - RefreshTelemetry, } // Open to better names @@ -312,7 +310,6 @@ impl<'a> Handler<'a> { tun_device, dns_notifier, network_notifier, - telemetry_refresh: tokio::time::interval(Duration::from_secs(60)), }) } @@ -415,9 +412,6 @@ impl<'a> Handler<'a> { connlib.set_dns(resolvers); } - Event::RefreshTelemetry => { - self.telemetry.refresh_config(); - } } }; @@ -462,10 +456,6 @@ impl<'a> Handler<'a> { } } - if self.telemetry_refresh.poll_tick(cx).is_ready() { - return Poll::Ready(Event::RefreshTelemetry); - } - Poll::Pending } diff --git a/rust/headless-client/src/main.rs b/rust/headless-client/src/main.rs index 7d6539127..3f4fc5412 100644 --- a/rust/headless-client/src/main.rs +++ b/rust/headless-client/src/main.rs @@ -20,7 +20,6 @@ use secrecy::{Secret, SecretString}; use std::{ path::{Path, PathBuf}, sync::Arc, - time::Duration, }; use tokio::time::Instant; @@ -283,8 +282,6 @@ fn main() -> Result<()> { new_network_notifier(tokio_handle.clone(), dns_control_method).await?; drop(tokio_handle); - let mut telemetry_refresh = tokio::time::interval(Duration::from_secs(60)); - let tun = tun_device.make_tun()?; session.set_tun(tun); session.set_dns(dns_controller.system_resolvers()); @@ -314,10 +311,6 @@ fn main() -> Result<()> { session.reset(); continue; }, - _ = telemetry_refresh.tick() => { - telemetry.refresh_config(); - continue; - } event = event_stream.next() => event.context("event stream unexpectedly ran empty")?, }; diff --git a/rust/logging/Cargo.toml b/rust/logging/Cargo.toml index ea062e4c5..4e86f9ba5 100644 --- a/rust/logging/Cargo.toml +++ b/rust/logging/Cargo.toml @@ -9,6 +9,7 @@ license = { workspace = true } [dependencies] anyhow = { workspace = true } +firezone-telemetry = { workspace = true } nu-ansi-term = { workspace = true } output_vt100 = { workspace = true } parking_lot = { workspace = true } diff --git a/rust/logging/src/lib.rs b/rust/logging/src/lib.rs index f813e343b..dff679e50 100644 --- a/rust/logging/src/lib.rs +++ b/rust/logging/src/lib.rs @@ -13,6 +13,7 @@ use std::sync::Arc; use anyhow::{Context, Result}; use event_message_contains_filter::EventMessageContains; +use firezone_telemetry::feature_flags; use sentry_tracing::EventFilter; use tracing::{Subscriber, subscriber::DefaultGuard}; use tracing_log::LogTracer; @@ -205,10 +206,18 @@ where use tracing::Level; sentry_tracing::layer() - .event_filter(move |md| match *md.level() { - Level::ERROR | Level::WARN => EventFilter::Event | EventFilter::Breadcrumb | EventFilter::Log, - Level::INFO | Level::DEBUG => EventFilter::Breadcrumb | EventFilter::Log, - _ => EventFilter::Ignore, + .event_filter(move |md| { + let mut event_filter = match *md.level() { + Level::ERROR | Level::WARN => EventFilter::Event | EventFilter::Breadcrumb, + Level::INFO | Level::DEBUG => EventFilter::Breadcrumb, + _ => return EventFilter::Ignore, + }; + + if feature_flags::stream_logs() { + event_filter |= EventFilter::Log + } + + event_filter }) .span_filter(|md| { matches!( diff --git a/rust/telemetry/src/lib.rs b/rust/telemetry/src/lib.rs index 3bc6310e7..989ac05b1 100644 --- a/rust/telemetry/src/lib.rs +++ b/rust/telemetry/src/lib.rs @@ -146,9 +146,7 @@ impl Telemetry { // Important: Evaluate feature flags before checking `stream_logs` to avoid hitting the default. feature_flags::evaluate_now(firezone_id.clone(), environment).await; - let enable_logs = feature_flags::stream_logs(); - - tracing::info!(%environment, %enable_logs, "Starting telemetry"); + tracing::info!(%environment, "Starting telemetry"); let inner = sentry::init(( dsn.0, @@ -158,7 +156,7 @@ impl Telemetry { release: Some(release.to_owned().into()), max_breadcrumbs: 500, before_send: Some(event_rate_limiter(Duration::from_secs(60 * 5))), - enable_logs, + enable_logs: true, before_send_log: Some(Arc::new(append_tracing_fields_to_message)), ..Default::default() }, @@ -184,32 +182,6 @@ impl Telemetry { sentry::start_session(); } - /// Refreshes the telemetry config. - /// - /// Looks at the current values of the relevant feature flags and re-initializes the client in case they changed. - pub fn refresh_config(&mut self) { - let Some(client) = self.inner.as_ref() else { - tracing::debug!("Cannot refresh config: no client"); - return; - }; - - let enable_logs = feature_flags::stream_logs(); - - if client.options().enable_logs == enable_logs { - tracing::debug!("Config is up-to-date"); - return; - } - - let options = client.options().clone(); - - tracing::info!(%enable_logs, "Re-initializing telemetry"); - - self.inner.replace(sentry::init(sentry::ClientOptions { - enable_logs, - ..options - })); - } - /// Flushes events to sentry.io and drops the guard pub async fn stop(&mut self) { self.end_session(SessionStatus::Exited).await;