Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat[tracing-subscriber]: extra fields #2733

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat[tracing-subscriber]: extra fields #2733

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8537bb7
feat[tracing-subscriber]: extra fields
hseeberger Oct 2, 2023
7dc4917
Update tracing-subscriber/src/fmt/format/mod.rs
hseeberger Feb 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions tracing-subscriber/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ json = ["tracing-serde", "serde", "serde_json"]
# Enables support for local time when using the `time` crate timestamp
# formatters.
local-time = ["time/local-offset"]
extra-fields = []

[dependencies]
tracing-core = { path = "../tracing-core", version = "0.2", default-features = false }
Expand Down
9 changes: 9 additions & 0 deletions tracing-subscriber/src/fmt/fmt_subscriber.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,15 @@ where
_inner: self._inner,
}
}

/// Set the function to make extra fields.
#[cfg(feature = "extra-fields")]
pub fn with_extra_fields(self, make_extra_fields: Box<dyn format::MakeExtraFields>) -> Self {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit, the other methods are named with_make_extra_fields, maybe it would be better to use just one of the two names.

Subscriber {
fmt_event: self.fmt_event.with_make_extra_fields(make_extra_fields),
..self
}
}
}

#[cfg(feature = "json")]
Expand Down
7 changes: 7 additions & 0 deletions tracing-subscriber/src/fmt/format/json.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,13 @@ where
.serialize_entry("threadId", &format!("{:?}", std::thread::current().id()))?;
}

#[cfg(feature = "extra-fields")]
if let Some((make_extra_fields, current_span)) = self.make_extra_fields.as_ref().zip(current_span) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should also add an option to line 236 so that the current span is present even if it is not to be logged.

for (k, v) in make_extra_fields(current_span.extensions()) {
serializer.serialize_entry(&k, &v)?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs to be moved to where fields serialization happens. Currently, the output is this for me even though I did not want to flatten it (lorem and foo are extra fields):

{
  "timestamp": "fake time",
  "level": "INFO",
  "fields": {
    "message": "hello"
  },
  "target": "tracing_subscriber::fmt::format::test",
  "span": {
    "name": "span"
  },
  "spans": [
    {
      "name": "span"
    }
  ],
  "lorem": "ipsum",
  "foo": "bar"
}

This can be used when the fields are flattened but when not, this will most likely need to be merged with event.field_map().

I tried to simplify this by calling serializer.serialize_entry("fields", ...) twice, but it will create two entries in the JSON map called fields so I don't think there is any way except for merging (or maybe just chaining will work if this takes an iterator).

}
}

serializer.end()
};

Expand Down
63 changes: 63 additions & 0 deletions tracing-subscriber/src/fmt/format/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ mod pretty;
#[cfg_attr(docsrs, doc(cfg(feature = "ansi")))]
pub use pretty::*;

#[cfg(feature = "extra-fields")]
use crate::registry::Extensions;
#[cfg(feature = "extra-fields")]
use std::collections::HashMap;

use fmt::{Debug, Display};

/// A type that can format a tracing [`Event`] to a [`Writer`].
Expand Down Expand Up @@ -413,6 +418,43 @@ pub struct Format<F = Full, T = SystemTime> {
pub(crate) display_thread_name: bool,
pub(crate) display_filename: bool,
pub(crate) display_line_number: bool,
#[cfg(feature = "extra-fields")]
pub(crate) make_extra_fields: Option<Box<dyn MakeExtraFields>>,
}

/// Make extra fields.
#[cfg(feature = "extra-fields")]
pub trait MakeExtraFields: Fn(Extensions<'_>) -> HashMap<String, String> + Send + Sync {
/// Clone self into a boxed [MakeTraceId].
fn clone_box<'a>(&self) -> Box<dyn 'a + MakeExtraFields>
where
Self: 'a;
}

#[cfg(feature = "extra-fields")]
impl Debug for Box<dyn MakeExtraFields> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "MakeExtraFields")
}
}

#[cfg(feature = "extra-fields")]
impl<F> MakeExtraFields for F
where
F: Fn(Extensions<'_>) -> HashMap<String, String> + Clone + Send + Sync {
fn clone_box<'a>(&self) -> Box<dyn 'a + MakeExtraFields>
where
Self: 'a,
{
Box::new(self.clone())
}
}

#[cfg(feature = "extra-fields")]
impl<'a> Clone for Box<dyn 'a + MakeExtraFields> {
fn clone(&self) -> Self {
self.clone_box()
}
}

// === impl Writer ===
Expand Down Expand Up @@ -603,6 +645,8 @@ impl Default for Format<Full, SystemTime> {
display_thread_name: false,
display_filename: false,
display_line_number: false,
#[cfg(feature = "extra-fields")]
make_extra_fields: None,
}
}
}
Expand All @@ -623,6 +667,8 @@ impl<F, T> Format<F, T> {
display_thread_name: self.display_thread_name,
display_filename: self.display_filename,
display_line_number: self.display_line_number,
#[cfg(feature = "extra-fields")]
make_extra_fields: None,
}
}

Expand Down Expand Up @@ -662,6 +708,8 @@ impl<F, T> Format<F, T> {
display_thread_name: self.display_thread_name,
display_filename: true,
display_line_number: true,
#[cfg(feature = "extra-fields")]
make_extra_fields: None,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please take the extra fields from self. Here and in all other instances of this.

If this sets it to None, we cannot do some chains such as builder.with_make_extra_fields(...).with_timer(...) because you'll lose this.

}
}

Expand Down Expand Up @@ -694,6 +742,8 @@ impl<F, T> Format<F, T> {
display_thread_name: self.display_thread_name,
display_filename: self.display_filename,
display_line_number: self.display_line_number,
#[cfg(feature = "extra-fields")]
make_extra_fields: None,
}
}

Expand Down Expand Up @@ -723,6 +773,8 @@ impl<F, T> Format<F, T> {
display_thread_name: self.display_thread_name,
display_filename: self.display_filename,
display_line_number: self.display_line_number,
#[cfg(feature = "extra-fields")]
make_extra_fields: None,
}
}

Expand All @@ -739,6 +791,8 @@ impl<F, T> Format<F, T> {
display_thread_name: self.display_thread_name,
display_filename: self.display_filename,
display_line_number: self.display_line_number,
#[cfg(feature = "extra-fields")]
make_extra_fields: None,
}
}

Expand Down Expand Up @@ -841,6 +895,15 @@ impl<F, T> Format<F, T> {
Ok(())
}

/// Set the function to make extra fields.
#[cfg(feature = "extra-fields")]
pub fn with_make_extra_fields(self, make_trace_id: Box<dyn MakeExtraFields>) -> Self {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this function (and the field on the struct) should be moved to the Json struct similarly to the options such as with_current_span.

If it were here, it's too easy to misuse because it does nothing for all formatters except for json.

Format {
make_extra_fields: Some(make_trace_id),
hseeberger marked this conversation as resolved.
Show resolved Hide resolved
..self
}
}

#[inline]
fn format_timestamp(&self, writer: &mut Writer<'_>) -> fmt::Result
where
Expand Down