Skip to content

Commit

Permalink
config: perhaps drop DynamicConfig too?
Browse files Browse the repository at this point in the history
In practice I have found that, whenever a library provides an ability to
configure through a trait generic and then provides a basic runtime
cofniguration convenience-type, the convenience type ends up being
dropped in favour of a manual implementation pretty quickly.

So it seems like a not-terrible-idea to just push users to the trait
immediately? And we don’t need to figure a good design for a
compile-time configuration type…

Might still go back on this decision, idk.
  • Loading branch information
nagisa committed Jan 6, 2024
1 parent 59b42fa commit 744afce
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 85 deletions.
108 changes: 35 additions & 73 deletions tracing-tracy/src/config.rs
Original file line number Diff line number Diff line change
@@ -1,34 +1,46 @@
use client::Client;
use tracing_subscriber::fmt::format::DefaultFields;
use tracing_subscriber::fmt::FormatFields;
#[allow(unused_imports)] // for documentation.
use super::TracyLayer;

/// Configuration of the [`TracyLayer`] behaviour.
///
/// For most users [`DynamicConfig`] is going to be a good default option, however advanced users
/// can implement this trait manually to achieve better performance through constant evaluation,
/// to override the formatter used or to otherwise modify the behaviour of `TracyLayer` in ways
/// that are not exposed via the `DynamicConfig` type.
/// For most users [`DefaultConfig`] is going to be a good default choice, however advanced users
/// can implement this trait manually to override the formatter used or to otherwise modify the
/// behaviour of the `TracyLayer`.
///
/// # Examples
///
/// ## Implementation with compile-time configuration
///
/// ```
/// #[derive(Default)]
/// struct ConstantTracyConfig {
/// formatter: tracing_subscriber::fmt::format::DefaultFields,
/// use tracing_subscriber::fmt::format::DefaultFields;
///
/// struct TracyLayerConfig {
/// fmt: DefaultFields,
/// }
/// impl tracing_tracy::Config for TracyLayerConfig {
/// type Formatter = DefaultFields;
/// fn formatter(&self) -> &Self::Formatter {
/// &self.fmt
/// }
/// // The boilerplate ends here
///
/// /// Collect 32 frames in stack traces.
/// fn stack_depth(&self) -> u16 {
/// 32
/// }
///
/// /// Do not format in fields into zone names.
/// fn format_fields_in_zone_name(&self) -> bool {
/// false
/// }
///
/// impl tracing_tracy::Config for ConstantTracyConfig {
/// type Formatter = tracing_subscriber::fmt::format::DefaultFields;
/// fn formatter(&self) -> &Self::Formatter { &self.formatter }
/// fn stack_depth(&self) -> u16 { 0 } // Same as the default trait impl.
/// fn format_fields_in_zone_name(&self) -> bool { true } // Same as the default trait impl.
/// // etc.
/// }
/// ```
///
/// With this sort of setup the compiler will be able to inline calls to `stack_depth` and
/// `format_fields_in_zone_name` and optimize accordingly.
/// With this configuration `TracyLayer` will collect some call stacks and the formatting of the
/// zone names is different from the `DefaultConfig`.
pub trait Config {
type Formatter: for<'writer> FormatFields<'writer> + 'static;

Expand Down Expand Up @@ -78,66 +90,16 @@ pub trait Config {
}
}

/// A type that implements the [`Config`] trait with runtime-adjustable values.
/// A default configuration of the [`TracyLayer`].
///
/// Ues the [`tracing_subscriber`] [`DefaultFields`] formatter. If not appropriate, consider
/// implementing the `Config` trait yourself.
pub struct DynamicConfig {
fmt: DefaultFields,
stack_depth: u16,
fields_in_zone_name: bool,
}

impl DynamicConfig {
/// Create a new implementation of `Config` that permits non-constant configuration.
#[must_use]
pub fn new() -> Self {
DynamicConfig {
fmt: DefaultFields::new(),
stack_depth: 0,
fields_in_zone_name: true,
}
}

/// Specify the maximum number of stack frames that will be collected.
///
/// Note that enabling callstack collection can and will introduce a non-trivial overhead at
/// every instrumentation point. Specifying 0 frames will disable stack trace collection.
///
/// Defaults to `0`.
#[must_use]
pub const fn with_stack_depth(mut self, stack_depth: u16) -> Self {
self.stack_depth = stack_depth;
self
}
/// This type does not allow for any adjustment of the configuration. In order to customize
/// the behaviour of the layer implement the [`Config`] trait for your own type.
#[derive(Default)]
pub struct DefaultConfig(DefaultFields);

/// Specify whether or not to include tracing span fields in the tracy zone name, or to emit
/// them as zone text.
///
/// The former enables zone analysis along unique span field invocations, while the latter
/// aggregates every invocation of a given span into a single zone, irrespective of field
/// values.
///
/// Defaults to `true`.
#[must_use]
pub const fn with_fields_in_zone_name(mut self, fields_in_zone_name: bool) -> Self {
self.fields_in_zone_name = fields_in_zone_name;
self
}
}

impl Config for DynamicConfig {
impl Config for DefaultConfig {
type Formatter = DefaultFields;

fn formatter(&self) -> &Self::Formatter {
&self.fmt
}

fn stack_depth(&self) -> u16 {
self.stack_depth
}

fn format_fields_in_zone_name(&self) -> bool {
self.fields_in_zone_name
&self.0
}
}
6 changes: 3 additions & 3 deletions tracing-tracy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
#![cfg_attr(tracing_tracy_docs, feature(doc_auto_cfg))]

use client::{Client, Span};
pub use config::{Config, DynamicConfig};
pub use config::{Config, DefaultConfig};
use std::sync::atomic::{AtomicUsize, Ordering};
use std::{fmt::Write, mem};
use tracing_core::{
Expand Down Expand Up @@ -85,7 +85,7 @@ thread_local! {
/// ).expect("setup tracy layer");
/// ```
#[derive(Clone)]
pub struct TracyLayer<C = DynamicConfig> {
pub struct TracyLayer<C = DefaultConfig> {
config: C,
client: Client,
}
Expand Down Expand Up @@ -139,7 +139,7 @@ impl<C: Config> TracyLayer<C> {

impl Default for TracyLayer {
fn default() -> Self {
Self::new(DynamicConfig::new())
Self::new(DefaultConfig::default())
}
}

Expand Down
30 changes: 21 additions & 9 deletions tracing-tracy/src/tests.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use super::{DynamicConfig, TracyLayer};
use crate::{Config, DefaultConfig};

use super::TracyLayer;
use criterion::Criterion;
use futures::future::join_all;
use tracing::{debug, event, info, info_span, span, Level};
Expand Down Expand Up @@ -122,10 +124,22 @@ pub(crate) fn test() {
runtime.block_on(async_futures());
}

#[derive(Default)]
struct CallstackConfig(DefaultConfig);
impl Config for CallstackConfig {
type Formatter = <DefaultConfig as Config>::Formatter;
fn formatter(&self) -> &Self::Formatter {
self.0.formatter()
}
fn stack_depth(&self) -> u16 {
100
}
}

fn benchmark_span(c: &mut Criterion) {
c.bench_function("span/callstack", |bencher| {
let layer = tracing_subscriber::registry()
.with(TracyLayer::new(DynamicConfig::new().with_stack_depth(100)));
let layer =
tracing_subscriber::registry().with(TracyLayer::new(CallstackConfig::default()));
tracing::subscriber::with_default(layer, || {
bencher.iter(|| {
let _span =
Expand All @@ -135,8 +149,7 @@ fn benchmark_span(c: &mut Criterion) {
});

c.bench_function("span/no_callstack", |bencher| {
let layer = tracing_subscriber::registry()
.with(TracyLayer::new(DynamicConfig::new().with_stack_depth(0)));
let layer = tracing_subscriber::registry().with(TracyLayer::default());
tracing::subscriber::with_default(layer, || {
bencher.iter(|| {
let _span =
Expand All @@ -148,8 +161,8 @@ fn benchmark_span(c: &mut Criterion) {

fn benchmark_message(c: &mut Criterion) {
c.bench_function("event/callstack", |bencher| {
let layer = tracing_subscriber::registry()
.with(TracyLayer::new(DynamicConfig::new().with_stack_depth(100)));
let layer =
tracing_subscriber::registry().with(TracyLayer::new(CallstackConfig::default()));
tracing::subscriber::with_default(layer, || {
bencher.iter(|| {
tracing::error!(field1 = "first", field2 = "second", "message");
Expand All @@ -158,8 +171,7 @@ fn benchmark_message(c: &mut Criterion) {
});

c.bench_function("event/no_callstack", |bencher| {
let layer = tracing_subscriber::registry()
.with(TracyLayer::new(DynamicConfig::new().with_stack_depth(0)));
let layer = tracing_subscriber::registry().with(TracyLayer::default());
tracing::subscriber::with_default(layer, || {
bencher.iter(|| {
tracing::error!(field1 = "first", field2 = "second", "message");
Expand Down

0 comments on commit 744afce

Please sign in to comment.