Skip to content

Commit

Permalink
Implement dynamic metadata injection
Browse files Browse the repository at this point in the history
Now that places are using non-'static metadata, we can overlay a patched
view of the metadata. Using the strategy seen in tracing-log, we
recognize specifically named metadata/fields and use the values they
provide to create metadata with the resolved runtime values.

Importantly, we also strive to only expose dynamic injection fields when
explicitly asked for. This means that most code can continue on without
knowing that these fields even exist.

Applying these patches does have a cost; notably, things which used to
deal in references like &Metadata or &FieldSet now often use an owned,
patched version instead. These types are primarily bags of references
themselves, and no copying of actual data is involved, but the fact that
the bags of references (which aren't exactly tiny) are copied does have
an impact.

More places which don't have to deal with metadata patching (e.g. the
macro support code) should be audited for what can use the _prenormal
versions of methods.

Additionally, there are likely places where the magic fields leak out
without being explicitly asked for. Ideally, these are all bugs which
can be fixed. There aren't really any tests that effectively check for
this, so new tests will need to be written.

Finaly, there's the question of actually dispatching such a dynamic
event. tracing-log is an example of doing so (but uses touchy, hidden
API to construct the ValueSet) (implemented in the next commit), but a
generally applicable is likely desirable. This will probably take the
shape of dynamic_span! and dynamic_event! macros in tracing proper.
  • Loading branch information
CAD97 committed Jun 14, 2022
1 parent cdfbf8e commit 344539a
Show file tree
Hide file tree
Showing 20 changed files with 848 additions and 92 deletions.
606 changes: 606 additions & 0 deletions tracing-core/src/dynamic.rs

Large diffs are not rendered by default.

8 changes: 7 additions & 1 deletion tracing-core/src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,10 @@ impl<'a> Event<'a> {
/// Returns [metadata] describing this `Event`.
///
/// [metadata]: super::Metadata
pub fn metadata(&self) -> &'a Metadata<'a> {
pub fn metadata(&self) -> Metadata<'a> {
self.metadata
.normalized(self)
.unwrap_or_else(|| self.metadata.clone())
}

/// Returns true if the new event should be a root.
Expand Down Expand Up @@ -131,4 +133,8 @@ impl<'a> field::RecordFields<'a> for Event<'a> {
fn record(&self, visitor: &mut dyn field::Visit<'a>) {
Event::record(self, visitor)
}

fn record_prenormal(&self, visitor: &mut dyn field::Visit<'a>) {
self.fields.record_prenormal(visitor)
}
}
68 changes: 58 additions & 10 deletions tracing-core/src/field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ pub struct Field {
pub struct Empty;

/// Describes the fields present on a span.
#[derive(Clone)]
pub struct FieldSet {
/// The names of each field on the described span.
names: &'static [&'static str],
Expand Down Expand Up @@ -254,8 +255,13 @@ pub trait Visit<'a> {
/// [`span::Attributes`]: crate::span::Attributes
/// [`span::Record`]: crate::span::Record
pub trait RecordFields<'a> {
/// Record all the fields in `self` with the provided `visitor`.
/// Record all the fields in `self` with the provided `visitor`, excluding
/// those used for [dynamic metadata][crate::dynamic].
fn record(&self, visitor: &mut dyn Visit<'a>);

/// Record all the fields in `self` with the provided `visitor`, *including*
/// those used for [dynamic metadata][crate::dynamic].
fn record_prenormal(&self, visitor: &mut dyn Visit<'a>);
}

/// A field value of an erased type.
Expand Down Expand Up @@ -331,6 +337,10 @@ where
fn record(&self, visitor: &mut dyn Visit<'a>) {
F::record(*self, visitor)
}

fn record_prenormal(&self, visitor: &mut dyn Visit<'a>) {
F::record_prenormal(*self, visitor)
}
}

// ===== impl Value =====
Expand Down Expand Up @@ -751,7 +761,17 @@ impl FieldSet {
/// of those callsites are not equivalent.
/// </pre></div>
pub fn contains(&self, field: &Field) -> bool {
field.callsite() == self.callsite() && field.i <= self.len()
if field.callsite() != self.callsite() {
return false;
}

// Example:
// Sliced Fieldset: [ "carol", "david" ]
// Field, Unsliced: [ "alice", "bob", "carol", "david" ], 3
let slice_offset = field.fields.names.len() as isize - self.len() as isize;
let field_index_in_me = field.i as isize - slice_offset;

0 <= field_index_in_me && field_index_in_me <= self.len() as isize
}

/// Returns an iterator over the `Field`s in this `FieldSet`.
Expand Down Expand Up @@ -792,6 +812,22 @@ impl FieldSet {
pub fn is_empty(&self) -> bool {
self.names.is_empty()
}

pub(crate) fn slice(&self, range: core::ops::RangeFrom<usize>) -> Self {
Self {
names: &self.names[range],
callsite: self.callsite.clone(),
}
}
}

impl IntoIterator for FieldSet {
type IntoIter = Iter;
type Item = Field;
#[inline]
fn into_iter(self) -> Self::IntoIter {
self.iter()
}
}

impl<'a> IntoIterator for &'a FieldSet {
Expand Down Expand Up @@ -828,10 +864,7 @@ impl Iterator for Iter {
let i = self.idxs.next()?;
Some(Field {
i,
fields: FieldSet {
names: self.fields.names,
callsite: self.fields.callsite(),
},
fields: self.fields.clone(),
})
}
}
Expand All @@ -853,6 +886,17 @@ impl<'a> ValueSet<'a> {
///
/// [visitor]: Visit
pub fn record(&self, visitor: &mut dyn Visit<'a>) {
for (field, value) in self.values {
if !self.fields.contains(field) {
continue;
}
if let Some(value) = value {
value.record(field, visitor);
}
}
}

fn record_unsliced(&self, visitor: &mut dyn Visit<'a>) {
let my_callsite = self.callsite();
for (field, value) in self.values {
if field.callsite() != my_callsite {
Expand All @@ -866,7 +910,7 @@ impl<'a> ValueSet<'a> {

/// Returns `true` if this `ValueSet` contains a value for the given `Field`.
pub(crate) fn contains(&self, field: &Field) -> bool {
field.callsite() == self.callsite()
self.fields.contains(field)
&& self
.values
.iter()
Expand All @@ -875,10 +919,9 @@ impl<'a> ValueSet<'a> {

/// Returns true if this `ValueSet` contains _no_ values.
pub(crate) fn is_empty(&self) -> bool {
let my_callsite = self.callsite();
self.values
.iter()
.all(|(key, val)| val.is_none() || key.callsite() != my_callsite)
.all(|(key, val)| val.is_none() || !self.fields.contains(key))
}

pub(crate) fn field_set(&self) -> &FieldSet {
Expand All @@ -890,6 +933,10 @@ impl<'a> RecordFields<'a> for ValueSet<'a> {
fn record(&self, visitor: &mut dyn Visit<'a>) {
ValueSet::record(self, visitor)
}

fn record_prenormal(&self, visitor: &mut dyn Visit<'a>) {
ValueSet::record_unsliced(self, visitor)
}
}

impl<'a> fmt::Debug for ValueSet<'a> {
Expand Down Expand Up @@ -1024,7 +1071,8 @@ mod test {
(&fields.field("bar").unwrap(), Some(&2 as &dyn Value)),
(&fields.field("baz").unwrap(), Some(&3 as &dyn Value)),
];
let valueset = TEST_META_2.fields().value_set(values);
let fields = TEST_META_2.fields();
let valueset = fields.value_set(values);
assert!(valueset.is_empty())
}

Expand Down
1 change: 1 addition & 0 deletions tracing-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ pub(crate) mod spin;
pub mod callsite;
pub mod collect;
pub mod dispatch;
pub mod dynamic;
pub mod event;
pub mod field;
pub mod metadata;
Expand Down
28 changes: 26 additions & 2 deletions tracing-core/src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ use core::{
/// [module path]: Self::module_path
/// [collector]: super::collect::Collect
/// [callsite identifier]: super::callsite::Identifier
#[derive(Clone)]
pub struct Metadata<'a> {
/// The name of the span described by this metadata.
name: &'a str,
Expand Down Expand Up @@ -276,11 +277,34 @@ impl<'a> Metadata<'a> {
}
}

/// Returns the names of the fields on the described span or event.
pub fn fields(&self) -> &field::FieldSet {
/// Returns the kind of this callsite.
pub fn kind(&self) -> &Kind {
&self.kind
}

/// Returns the names of the fields on the described span or event,
/// excluding those used for [dynamic metadata][crate::dynamic].
pub fn fields(&self) -> field::FieldSet {
if self.is_dynamic() {
let magic = self.magic_fields();
self.fields.slice(magic.count()..)
} else {
self.fields.clone()
}
}

/// Returns the names of the fields on the described span or event,
/// *including* those used for [dynamic metadata][crate::dynamic].
pub fn fields_prenormal(&self) -> &field::FieldSet {
&self.fields
}

/// Returns the names of the fields on the described span or event
/// used for [dynamic metadata][crate::dynamic].
pub(crate) fn magic_fields(&self) -> crate::dynamic::MagicFields {
crate::dynamic::MagicFields::new(&self.fields)
}

/// Returns the level of verbosity of the described span or event.
pub fn level(&self) -> &Level {
&self.level
Expand Down
8 changes: 8 additions & 0 deletions tracing-core/src/span.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,10 @@ impl<'a> field::RecordFields<'a> for Attributes<'a> {
fn record(&self, visitor: &mut dyn field::Visit<'a>) {
Attributes::record(self, visitor)
}

fn record_prenormal(&self, visitor: &mut dyn field::Visit<'a>) {
self.values.record_prenormal(visitor)
}
}

// ===== impl Record =====
Expand Down Expand Up @@ -247,6 +251,10 @@ impl<'a> field::RecordFields<'a> for Record<'a> {
fn record(&self, visitor: &mut dyn field::Visit<'a>) {
Record::record(self, visitor)
}

fn record_prenormal(&self, visitor: &mut dyn field::Visit<'a>) {
self.values.record_prenormal(visitor)
}
}

// ===== impl Current =====
Expand Down
4 changes: 2 additions & 2 deletions tracing-error/src/subscriber.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,9 @@ where
.expect("registry should have a span for the current ID");
for span in span.scope() {
let cont = if let Some(fields) = span.extensions().get::<FormattedFields<F>>() {
f(span.metadata(), fields.fields.as_str())
f(&span.metadata(), fields.fields.as_str())
} else {
f(span.metadata(), "")
f(&span.metadata(), "")
};
if !cont {
break;
Expand Down
6 changes: 3 additions & 3 deletions tracing-journald/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ where

writeln!(buf, "SPAN_NAME").unwrap();
put_value(&mut buf, span.name().as_bytes());
put_metadata(&mut buf, span.metadata(), Some("SPAN_"));
put_metadata(&mut buf, &span.metadata(), Some("SPAN_"));

attrs.record(&mut SpanVisitor {
buf: &mut buf,
Expand Down Expand Up @@ -252,8 +252,8 @@ where
}

// Record event fields
put_priority(&mut buf, event.metadata());
put_metadata(&mut buf, event.metadata(), None);
put_priority(&mut buf, &event.metadata());
put_metadata(&mut buf, &event.metadata(), None);
put_field_length_encoded(&mut buf, "SYSLOG_IDENTIFIER", |buf| {
write!(buf, "{}", self.syslog_identifier).unwrap()
});
Expand Down
2 changes: 1 addition & 1 deletion tracing-mock/src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ impl MockEvent {
let meta = event.metadata();
let name = meta.name();
self.metadata
.check(meta, format_args!("event \"{}\"", name), collector_name);
.check(&meta, format_args!("event \"{}\"", name), collector_name);
assert!(
meta.is_event(),
"[{}] expected {}, but got {:?}",
Expand Down
20 changes: 3 additions & 17 deletions tracing-opentelemetry/src/subscriber.rs
Original file line number Diff line number Diff line change
Expand Up @@ -627,7 +627,7 @@ where
#[cfg(feature = "tracing-log")]
let normalized_meta = event.normalized_metadata();
#[cfg(feature = "tracing-log")]
let meta = normalized_meta.as_ref().unwrap_or_else(|| event.metadata());
let meta = normalized_meta.unwrap_or_else(|| event.metadata());
#[cfg(not(feature = "tracing-log"))]
let meta = event.metadata();

Expand All @@ -648,22 +648,8 @@ where
}

if self.location {
#[cfg(not(feature = "tracing-log"))]
let normalized_meta: Option<tracing_core::Metadata<'_>> = None;
let (file, module) = match &normalized_meta {
Some(meta) => (
meta.file().map(|s| Value::from(s.to_owned())),
meta.module_path().map(|s| Value::from(s.to_owned())),
),
None => (
event.metadata().file().map(String::from).map(Value::from),
event
.metadata()
.module_path()
.map(String::from)
.map(Value::from),
),
};
let file = meta.file().map(String::from).map(Value::from);
let module = meta.module_path().map(String::from).map(Value::from);

if let Some(file) = file {
otel_event
Expand Down
4 changes: 2 additions & 2 deletions tracing-serde/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ impl<'a> Serialize for SerializeMetadata<'a> {
state.serialize_field("module_path", &self.0.module_path())?;
state.serialize_field("file", &self.0.file())?;
state.serialize_field("line", &self.0.line())?;
state.serialize_field("fields", &SerializeFieldSet(self.0.fields()))?;
state.serialize_field("fields", &SerializeFieldSet(&self.0.fields()))?;
state.serialize_field("is_span", &self.0.is_span())?;
state.serialize_field("is_event", &self.0.is_event())?;
state.end()
Expand All @@ -286,7 +286,7 @@ impl<'a> Serialize for SerializeEvent<'a> {
S: Serializer,
{
let mut serializer = serializer.serialize_struct("Event", 2)?;
serializer.serialize_field("metadata", &SerializeMetadata(self.0.metadata()))?;
serializer.serialize_field("metadata", &SerializeMetadata(&self.0.metadata()))?;
let mut visitor = SerdeStructVisitor {
serializer,
state: Ok(()),
Expand Down
6 changes: 4 additions & 2 deletions tracing-subscriber/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ std = ["alloc", "tracing-core/std"]
env-filter = ["matchers", "regex", "once_cell", "tracing", "std", "thread_local"]
fmt = ["registry", "std"]
ansi = ["fmt", "ansi_term"]
registry = ["sharded-slab", "thread_local", "std"]
registry = ["sharded-slab", "thread_local", "std", "once_cell"]
json = ["tracing-serde", "serde", "serde_json"]
# Enables support for local time when using the `time` crate timestamp
# formatters.
Expand All @@ -44,7 +44,6 @@ tracing = { optional = true, path = "../tracing", version = "0.2", default-featu
matchers = { optional = true, version = "0.1.0" }
regex = { optional = true, version = "1", default-features = false, features = ["std"] }
smallvec = { optional = true, version = "1" }
once_cell = { optional = true, version = "1.12" }

# fmt
tracing-log = { path = "../tracing-log", version = "0.2", optional = true, default-features = false, features = ["log-tracer", "std"] }
Expand All @@ -63,6 +62,9 @@ parking_lot = { version = ">= 0.9.0, <= 0.13", optional = true }
sharded-slab = { version = "0.1.0", optional = true }
thread_local = { version = "1.1.4", optional = true }

# registry OR env-filter
once_cell = { version = "1.12", optional = true }

[dev-dependencies]
tracing = { path = "../tracing", version = "0.2" }
tracing-mock = { path = "../tracing-mock" }
Expand Down
Loading

0 comments on commit 344539a

Please sign in to comment.