From c46eaed0e04a6c621fa93822fe9cc1fec07aed13 Mon Sep 17 00:00:00 2001
From: Charles Hall <charles@computer.surgery>
Date: Fri, 7 Jun 2024 13:11:46 -0700
Subject: [PATCH] parse configured EnvFilter once

This allows the error handling to be done upfront instead of for each
use. In particular, the `toml` error now points to the span of text in
the config file where the misconfigured EnvFilter value is. This is much
better than the previous error that did not indicate what was actually
causing it to happen.
---
 src/config.rs                  | 10 +++++---
 src/config/env_filter_clone.rs | 44 ++++++++++++++++++++++++++++++++++
 src/error.rs                   |  3 ---
 src/observability.rs           |  8 +++----
 4 files changed, 54 insertions(+), 11 deletions(-)
 create mode 100644 src/config/env_filter_clone.rs

diff --git a/src/config.rs b/src/config.rs
index 04e723f4..c1ffde91 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -8,8 +8,10 @@ use serde::Deserialize;
 
 use crate::error;
 
+mod env_filter_clone;
 mod proxy;
 
+use env_filter_clone::EnvFilterClone;
 use proxy::ProxyConfig;
 
 #[allow(clippy::struct_excessive_bools)]
@@ -69,7 +71,7 @@ pub(crate) struct Config {
     #[serde(default = "default_trusted_servers")]
     pub(crate) trusted_servers: Vec<OwnedServerName>,
     #[serde(default = "default_log")]
-    pub(crate) log: String,
+    pub(crate) log: EnvFilterClone,
     #[serde(default)]
     pub(crate) turn_username: String,
     #[serde(default)]
@@ -146,8 +148,10 @@ fn default_trusted_servers() -> Vec<OwnedServerName> {
     vec![OwnedServerName::try_from("matrix.org").unwrap()]
 }
 
-fn default_log() -> String {
-    "warn,state_res=warn,_=off".to_owned()
+fn default_log() -> EnvFilterClone {
+    "warn,state_res=warn,_=off"
+        .parse()
+        .expect("hardcoded env filter should be valid")
 }
 
 fn default_turn_ttl() -> u64 {
diff --git a/src/config/env_filter_clone.rs b/src/config/env_filter_clone.rs
new file mode 100644
index 00000000..183c827f
--- /dev/null
+++ b/src/config/env_filter_clone.rs
@@ -0,0 +1,44 @@
+//! A workaround for [`EnvFilter`] not directly implementing [`Clone`]
+//!
+//! This will be unnecessary after [tokio-rs/tracing#2956][0] is merged.
+//!
+//! [0]: https://github.com/tokio-rs/tracing/pull/2956
+#![warn(missing_docs, clippy::missing_docs_in_private_items)]
+
+use std::str::FromStr;
+
+use serde::{de, Deserialize, Deserializer};
+use tracing_subscriber::EnvFilter;
+
+/// A workaround for [`EnvFilter`] not directly implementing [`Clone`]
+///
+/// Use [`FromStr`] or [`Deserialize`] to construct this type, then [`From`] or
+/// [`Into`] to convert it into an [`EnvFilter`] when needed.
+#[derive(Debug)]
+pub(crate) struct EnvFilterClone(String);
+
+impl FromStr for EnvFilterClone {
+    type Err = <EnvFilter as FromStr>::Err;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        EnvFilter::from_str(s)?;
+        Ok(Self(s.to_owned()))
+    }
+}
+
+impl From<&EnvFilterClone> for EnvFilter {
+    fn from(other: &EnvFilterClone) -> Self {
+        EnvFilter::from_str(&other.0)
+            .expect("env filter syntax should have been validated already")
+    }
+}
+
+impl<'de> Deserialize<'de> for EnvFilterClone {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        let s = String::deserialize(deserializer)?;
+        Self::from_str(&s).map_err(de::Error::custom)
+    }
+}
diff --git a/src/error.rs b/src/error.rs
index 94d9a847..c8e9d8e2 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -61,9 +61,6 @@ pub(crate) enum Observability {
     #[error("opentelemetry error")]
     Otel(#[from] opentelemetry::trace::TraceError),
 
-    #[error("invalid log filter syntax")]
-    EnvFilter(#[from] tracing_subscriber::filter::ParseError),
-
     #[error("failed to install global default tracing subscriber")]
     SetSubscriber(#[from] tracing::subscriber::SetGlobalDefaultError),
 
diff --git a/src/observability.rs b/src/observability.rs
index f182219f..b6ded983 100644
--- a/src/observability.rs
+++ b/src/observability.rs
@@ -82,8 +82,6 @@ pub(crate) enum FoundIn {
 
 /// Initialize observability
 pub(crate) fn init(config: &Config) -> Result<Guard, error::Observability> {
-    let config_filter_layer = || EnvFilter::try_new(&config.log);
-
     let jaeger_layer = config
         .allow_jaeger
         .then(|| {
@@ -101,7 +99,7 @@ pub(crate) fn init(config: &Config) -> Result<Guard, error::Observability> {
             let telemetry = tracing_opentelemetry::layer().with_tracer(tracer);
 
             Ok::<_, error::Observability>(
-                telemetry.with_filter(config_filter_layer()?),
+                telemetry.with_filter(EnvFilter::from(&config.log)),
             )
         })
         .transpose()?;
@@ -114,7 +112,7 @@ pub(crate) fn init(config: &Config) -> Result<Guard, error::Observability> {
             let flame_layer = flame_layer.with_empty_samples(false);
 
             Ok::<_, error::Observability>((
-                flame_layer.with_filter(config_filter_layer()?),
+                flame_layer.with_filter(EnvFilter::from(&config.log)),
                 guard,
             ))
         })
@@ -122,7 +120,7 @@ pub(crate) fn init(config: &Config) -> Result<Guard, error::Observability> {
         .unzip();
 
     let fmt_layer = tracing_subscriber::fmt::Layer::new()
-        .with_filter(config_filter_layer()?);
+        .with_filter(EnvFilter::from(&config.log));
 
     let subscriber = Registry::default()
         .with(jaeger_layer)