Attach more tags to feedback submissions (#8688)

Attach more tags to sentry feedback so it's easier to classify and debug
without having to scan through logs.

Formatting isn't amazing but it's a start.
<img width="1234" height="276" alt="image"
src="https://github.com/user-attachments/assets/521a349d-f627-4051-b511-9811cd5cd933"
/>

Author: pakrym-oai

codex-rs/Cargo.lock

@@ -17,13 +17,11 @@ use tokio::io::BufReader;
 use tokio::io::{self};
 use tokio::sync::mpsc;
 use toml::Value as TomlValue;
-use tracing::Level;
 use tracing::debug;
 use tracing::error;
 use tracing::info;
 use tracing_subscriber::EnvFilter;
 use tracing_subscriber::Layer;
-use tracing_subscriber::filter::Targets;
 use tracing_subscriber::layer::SubscriberExt;
 use tracing_subscriber::util::SubscriberInitExt;
 
@@ -103,11 +101,8 @@ pub async fn run_main(
         .with_span_events(tracing_subscriber::fmt::format::FmtSpan::FULL)
         .with_filter(EnvFilter::from_default_env());
 
-    let feedback_layer = tracing_subscriber::fmt::layer()
-        .with_writer(feedback.make_writer())
-        .with_ansi(false)
-        .with_target(false)
-        .with_filter(Targets::new().with_default(Level::TRACE));
+    let feedback_layer = feedback.logger_layer();
+    let feedback_metadata_layer = feedback.metadata_layer();
 
     let otel_logger_layer = otel.as_ref().and_then(|o| o.logger_layer());
 
@@ -116,6 +111,7 @@ pub async fn run_main(
     let _ = tracing_subscriber::registry()
         .with(stderr_fmt)
         .with(feedback_layer)
+        .with(feedback_metadata_layer)
         .with(otel_logger_layer)
         .with(otel_tracing_layer)
         .try_init();

codex-rs/app-server/src/lib.rs

@@ -88,6 +88,7 @@ use crate::error::Result as CodexResult;
 #[cfg(test)]
 use crate::exec::StreamOutput;
 use crate::exec_policy::ExecPolicyUpdateError;
+use crate::feedback_tags;
 use crate::mcp::auth::compute_auth_statuses;
 use crate::mcp_connection_manager::McpConnectionManager;
 use crate::model_provider_info::CHAT_WIRE_API_DEPRECATION_SUMMARY;
@@ -2539,6 +2540,15 @@ async fn try_run_turn(
         truncation_policy: Some(turn_context.truncation_policy.into()),
     });
 
+    feedback_tags!(
+        model = turn_context.client.get_model(),
+        approval_policy = turn_context.approval_policy,
+        sandbox_policy = turn_context.sandbox_policy,
+        effort = turn_context.client.get_reasoning_effort(),
+        auth_mode = sess.services.auth_manager.get_auth_mode(),
+        features = sess.features.enabled_features(),
+    );
+
     sess.persist_rollout_items(&[rollout_item]).await;
     let mut stream = turn_context
         .client

codex-rs/core/src/codex.rs

@@ -255,6 +255,10 @@ impl Features {
 
         features
     }
+
+    pub fn enabled_features(&self) -> Vec<Feature> {
+        self.enabled.iter().copied().collect()
+    }
 }
 
 /// Keys accepted in `[features]` tables.

codex-rs/core/src/features.rs

@@ -9,6 +9,31 @@ use tracing::error;
 const INITIAL_DELAY_MS: u64 = 200;
 const BACKOFF_FACTOR: f64 = 2.0;
 
+/// Emit structured feedback metadata as key/value pairs.
+///
+/// This logs a tracing event with `target: "feedback_tags"`. If
+/// `codex_feedback::CodexFeedback::metadata_layer()` is installed, these fields are captured and
+/// later attached as tags when feedback is uploaded.
+///
+/// Values are wrapped with [`tracing::field::DebugValue`], so the expression only needs to
+/// implement [`std::fmt::Debug`].
+///
+/// Example:
+///
+/// ```rust
+/// codex_core::feedback_tags!(model = "gpt-5", cached = true);
+/// codex_core::feedback_tags!(provider = provider_id, request_id = request_id);
+/// ```
+#[macro_export]
+macro_rules! feedback_tags {
+    ($( $key:ident = $value:expr ),+ $(,)?) => {
+        ::tracing::info!(
+            target: "feedback_tags",
+            $( $key = ::tracing::field::debug(&$value) ),+
+        );
+    };
+}
+
 pub(crate) fn backoff(attempt: u64) -> Duration {
     let exp = BACKOFF_FACTOR.powi(attempt.saturating_sub(1) as i32);
     let base = (INITIAL_DELAY_MS as f64 * exp) as u64;
@@ -74,4 +99,12 @@ mod tests {
         let message = try_parse_error_message(text);
         assert_eq!(message, r#"{"message": "test"}"#);
     }
+
+    #[test]
+    fn feedback_tags_macro_compiles() {
+        #[derive(Debug)]
+        struct OnlyDebug;
+
+        feedback_tags!(model = "gpt-5", cached = true, debug_only = OnlyDebug);
+    }
 }

codex-rs/core/src/util.rs

@@ -8,6 +8,7 @@ license.workspace = true
 anyhow = { workspace = true }
 codex-protocol = { workspace = true }
 sentry = { version = "0.46" }
+tracing = { workspace = true }
 tracing-subscriber = { workspace = true }
 
 [dev-dependencies]

codex-rs/feedback/Cargo.toml

@@ -1,4 +1,6 @@
+use std::collections::BTreeMap;
 use std::collections::VecDeque;
+use std::collections::btree_map::Entry;
 use std::fs;
 use std::io::Write;
 use std::io::{self};
@@ -11,12 +13,20 @@ use anyhow::Result;
 use anyhow::anyhow;
 use codex_protocol::ConversationId;
 use codex_protocol::protocol::SessionSource;
+use tracing::Event;
+use tracing::Level;
+use tracing::field::Visit;
+use tracing_subscriber::Layer;
+use tracing_subscriber::filter::Targets;
 use tracing_subscriber::fmt::writer::MakeWriter;
+use tracing_subscriber::registry::LookupSpan;
 
 const DEFAULT_MAX_BYTES: usize = 4 * 1024 * 1024; // 4 MiB
 const SENTRY_DSN: &str =
     "https://[email protected]/4510195390611458";
 const UPLOAD_TIMEOUT_SECS: u64 = 10;
+const FEEDBACK_TAGS_TARGET: &str = "feedback_tags";
+const MAX_FEEDBACK_TAGS: usize = 64;
 
 #[derive(Clone)]
 pub struct CodexFeedback {
@@ -46,13 +56,50 @@ impl CodexFeedback {
         }
     }
 
+    /// Returns a [`tracing_subscriber`] layer that captures full-fidelity logs into this feedback
+    /// ring buffer.
+    ///
+    /// This is intended for initialization code so call sites don't have to duplicate the exact
+    /// `fmt::layer()` configuration and filter logic.
+    pub fn logger_layer<S>(&self) -> impl Layer<S> + Send + Sync + 'static
+    where
+        S: tracing::Subscriber + for<'a> LookupSpan<'a>,
+    {
+        tracing_subscriber::fmt::layer()
+            .with_writer(self.make_writer())
+            .with_ansi(false)
+            .with_target(false)
+            // Capture everything, regardless of the caller's `RUST_LOG`, so feedback includes the
+            // full trace when the user uploads a report.
+            .with_filter(Targets::new().with_default(Level::TRACE))
+    }
+
+    /// Returns a [`tracing_subscriber`] layer that collects structured metadata for feedback.
+    ///
+    /// Events with `target: "feedback_tags"` are treated as key/value tags to attach to feedback
+    /// uploads later.
+    pub fn metadata_layer<S>(&self) -> impl Layer<S> + Send + Sync + 'static
+    where
+        S: tracing::Subscriber + for<'a> LookupSpan<'a>,
+    {
+        FeedbackMetadataLayer {
+            inner: self.inner.clone(),
+        }
+        .with_filter(Targets::new().with_target(FEEDBACK_TAGS_TARGET, Level::TRACE))
+    }
+
     pub fn snapshot(&self, session_id: Option<ConversationId>) -> CodexLogSnapshot {
         let bytes = {
             let guard = self.inner.ring.lock().expect("mutex poisoned");
             guard.snapshot_bytes()
         };
+        let tags = {
+            let guard = self.inner.tags.lock().expect("mutex poisoned");
+            guard.clone()
+        };
         CodexLogSnapshot {
             bytes,
+            tags,
             thread_id: session_id
                 .map(|id| id.to_string())
                 .unwrap_or("no-active-thread-".to_string() + &ConversationId::new().to_string()),
@@ -62,12 +109,14 @@ impl CodexFeedback {
 
 struct FeedbackInner {
     ring: Mutex<RingBuffer>,
+    tags: Mutex<BTreeMap<String, String>>,
 }
 
 impl FeedbackInner {
     fn new(max_bytes: usize) -> Self {
         Self {
             ring: Mutex::new(RingBuffer::new(max_bytes)),
+            tags: Mutex::new(BTreeMap::new()),
         }
     }
 }
@@ -152,6 +201,7 @@ impl RingBuffer {
 
 pub struct CodexLogSnapshot {
     bytes: Vec<u8>,
+    tags: BTreeMap<String, String>,
     pub thread_id: String,
 }
 
@@ -212,6 +262,22 @@ impl CodexLogSnapshot {
             tags.insert(String::from("reason"), r.to_string());
         }
 
+        let reserved = [
+            "thread_id",
+            "classification",
+            "cli_version",
+            "session_source",
+            "reason",
+        ];
+        for (key, value) in &self.tags {
+            if reserved.contains(&key.as_str()) {
+                continue;
+            }
+            if let Entry::Vacant(entry) = tags.entry(key.clone()) {
+                entry.insert(value.clone());
+            }
+        }
+
         let level = match classification {
             "bug" | "bad_result" => Level::Error,
             _ => Level::Info,
@@ -280,9 +346,80 @@ fn display_classification(classification: &str) -> String {
     }
 }
 
+#[derive(Clone)]
+struct FeedbackMetadataLayer {
+    inner: Arc<FeedbackInner>,
+}
+
+impl<S> Layer<S> for FeedbackMetadataLayer
+where
+    S: tracing::Subscriber + for<'a> LookupSpan<'a>,
+{
+    fn on_event(&self, event: &Event<'_>, _ctx: tracing_subscriber::layer::Context<'_, S>) {
+        // This layer is filtered by `Targets`, but keep the guard anyway in case it is used without
+        // the filter.
+        if event.metadata().target() != FEEDBACK_TAGS_TARGET {
+            return;
+        }
+
+        let mut visitor = FeedbackTagsVisitor::default();
+        event.record(&mut visitor);
+        if visitor.tags.is_empty() {
+            return;
+        }
+
+        let mut guard = self.inner.tags.lock().expect("mutex poisoned");
+        for (key, value) in visitor.tags {
+            if guard.len() >= MAX_FEEDBACK_TAGS && !guard.contains_key(&key) {
+                continue;
+            }
+            guard.insert(key, value);
+        }
+    }
+}
+
+#[derive(Default)]
+struct FeedbackTagsVisitor {
+    tags: BTreeMap<String, String>,
+}
+
+impl Visit for FeedbackTagsVisitor {
+    fn record_i64(&mut self, field: &tracing::field::Field, value: i64) {
+        self.tags
+            .insert(field.name().to_string(), value.to_string());
+    }
+
+    fn record_u64(&mut self, field: &tracing::field::Field, value: u64) {
+        self.tags
+            .insert(field.name().to_string(), value.to_string());
+    }
+
+    fn record_bool(&mut self, field: &tracing::field::Field, value: bool) {
+        self.tags
+            .insert(field.name().to_string(), value.to_string());
+    }
+
+    fn record_f64(&mut self, field: &tracing::field::Field, value: f64) {
+        self.tags
+            .insert(field.name().to_string(), value.to_string());
+    }
+
+    fn record_str(&mut self, field: &tracing::field::Field, value: &str) {
+        self.tags
+            .insert(field.name().to_string(), value.to_string());
+    }
+
+    fn record_debug(&mut self, field: &tracing::field::Field, value: &dyn std::fmt::Debug) {
+        self.tags
+            .insert(field.name().to_string(), format!("{value:?}"));
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
+    use tracing_subscriber::layer::SubscriberExt;
+    use tracing_subscriber::util::SubscriberInitExt;
 
     #[test]
     fn ring_buffer_drops_front_when_full() {
@@ -296,4 +433,18 @@ mod tests {
         // Capacity 8: after writing 10 bytes, we should keep the last 8.
         pretty_assertions::assert_eq!(std::str::from_utf8(snap.as_bytes()).unwrap(), "cdefghij");
     }
+
+    #[test]
+    fn metadata_layer_records_tags_from_feedback_target() {
+        let fb = CodexFeedback::new();
+        let _guard = tracing_subscriber::registry()
+            .with(fb.metadata_layer())
+            .set_default();
+
+        tracing::info!(target: FEEDBACK_TAGS_TARGET, model = "gpt-5", cached = true, "tags");
+
+        let snap = fb.snapshot(None);
+        pretty_assertions::assert_eq!(snap.tags.get("model").map(String::as_str), Some("gpt-5"));
+        pretty_assertions::assert_eq!(snap.tags.get("cached").map(String::as_str), Some("true"));
+    }
 }

codex-rs/feedback/src/lib.rs

@@ -29,7 +29,6 @@ use std::path::PathBuf;
 use tracing::error;
 use tracing_appender::non_blocking;
 use tracing_subscriber::EnvFilter;
-use tracing_subscriber::filter::Targets;
 use tracing_subscriber::prelude::*;
 
 mod additional_dirs;
@@ -282,13 +281,8 @@ pub async fn run_main(
         .with_filter(env_filter());
 
     let feedback = codex_feedback::CodexFeedback::new();
-    let targets = Targets::new().with_default(tracing::Level::TRACE);
-
-    let feedback_layer = tracing_subscriber::fmt::layer()
-        .with_writer(feedback.make_writer())
-        .with_ansi(false)
-        .with_target(false)
-        .with_filter(targets);
+    let feedback_layer = feedback.logger_layer();
+    let feedback_metadata_layer = feedback.metadata_layer();
 
     if cli.oss && model_provider_override.is_some() {
         // We're in the oss section, so provider_id should be Some
@@ -323,6 +317,7 @@ pub async fn run_main(
     let _ = tracing_subscriber::registry()
         .with(file_layer)
         .with(feedback_layer)
+        .with(feedback_metadata_layer)
         .with(otel_logger_layer)
         .with(otel_tracing_layer)
         .try_init();