Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Add logger configuration hook #10440

Merged
merged 12 commits into from
Dec 16, 2021
15 changes: 11 additions & 4 deletions client/cli/src/commands/run_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,10 @@ pub struct RunCmd {
#[structopt(long = "prometheus-port", value_name = "PORT")]
pub prometheus_port: Option<u16>,

/// Default Prometheus metrics prefix.
#[structopt(skip = "substrate")]
pub prometheus_metric_prefix: &'static str,

sandreim marked this conversation as resolved.
Show resolved Hide resolved
/// Do not expose a Prometheus exporter endpoint.
///
/// Prometheus metric endpoint is enabled by default.
Expand Down Expand Up @@ -368,10 +372,13 @@ impl CliConfiguration for RunCmd {
let interface =
if self.prometheus_external { Ipv4Addr::UNSPECIFIED } else { Ipv4Addr::LOCALHOST };

Some(PrometheusConfig::new_with_default_registry(SocketAddr::new(
interface.into(),
self.prometheus_port.unwrap_or(default_listen_port),
)))
Some(PrometheusConfig::new_with_default_registry(
SocketAddr::new(
interface.into(),
self.prometheus_port.unwrap_or(default_listen_port),
),
self.prometheus_metric_prefix,
))
})
}

Expand Down
16 changes: 14 additions & 2 deletions client/cli/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -569,8 +569,17 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
/// 1. Sets the panic handler
/// 2. Initializes the logger
/// 3. Raises the FD limit
fn init<C: SubstrateCli>(&self) -> Result<()> {
sp_panic_handler::set(&C::support_url(), &C::impl_version());
fn init<F>(
&self,
support_url: &String,
impl_version: &String,
logger_hook: F,
sandreim marked this conversation as resolved.
Show resolved Hide resolved
config: &Configuration,
) -> Result<()>
where
F: FnOnce(&mut LoggerBuilder, &Configuration),
{
sp_panic_handler::set(support_url, impl_version);

let mut logger = LoggerBuilder::new(self.log_filters()?);
logger
Expand All @@ -586,6 +595,9 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
logger.with_colors(false);
}

// Call hook for custom profiling setup.
logger_hook(&mut logger, &config);

logger.init()?;

if let Some(new_limit) = fdlimit::raise_fd_limit() {
Expand Down
23 changes: 21 additions & 2 deletions client/cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,10 +224,29 @@ pub trait SubstrateCli: Sized {
/// Create a runner for the command provided in argument. This will create a Configuration and
/// a tokio runtime
fn create_runner<T: CliConfiguration>(&self, command: &T) -> error::Result<Runner<Self>> {
command.init::<Self>()?;
Runner::new(self, command)
let tokio_runtime = build_runtime()?;
let config = command.create_configuration(self, tokio_runtime.handle().clone())?;

command.init(&Self::support_url(), &Self::impl_version(), |_, _| {}, &config)?;
Runner::new(config, tokio_runtime)
}

/// Create a runner for the command provided in argument. This will create a Configuration and
/// a tokio runtime and apply the specified `logger_hook` closure.
fn create_runner_with_logger_hook<T: CliConfiguration, F>(
&self,
command: &T,
logger_hook: F,
sandreim marked this conversation as resolved.
Show resolved Hide resolved
) -> error::Result<Runner<Self>>
where
F: FnOnce(&mut LoggerBuilder, &Configuration),
{
let tokio_runtime = build_runtime()?;
let config = command.create_configuration(self, tokio_runtime.handle().clone())?;

command.init(&Self::support_url(), &Self::impl_version(), logger_hook, &config)?;
Runner::new(config, tokio_runtime)
}
/// Native runtime version.
fn native_runtime_version(chain_spec: &Box<dyn ChainSpec>) -> &'static RuntimeVersion;
}
13 changes: 3 additions & 10 deletions client/cli/src/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
// You should have received a copy of the GNU General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.

use crate::{error::Error as CliError, CliConfiguration, Result, SubstrateCli};
use crate::{error::Error as CliError, Result, SubstrateCli};
use chrono::prelude::*;
use futures::{future, future::FutureExt, pin_mut, select, Future};
use log::info;
Expand Down Expand Up @@ -112,15 +112,8 @@ pub struct Runner<C: SubstrateCli> {

impl<C: SubstrateCli> Runner<C> {
/// Create a new runtime with the command provided in argument
pub fn new<T: CliConfiguration>(cli: &C, command: &T) -> Result<Runner<C>> {
let tokio_runtime = build_runtime()?;
let runtime_handle = tokio_runtime.handle().clone();

Ok(Runner {
config: command.create_configuration(cli, runtime_handle)?,
tokio_runtime,
phantom: PhantomData,
})
pub fn new(config: Configuration, tokio_runtime: tokio::runtime::Runtime) -> Result<Runner<C>> {
Ok(Runner { config, tokio_runtime, phantom: PhantomData })
}

/// Log information about the node itself.
Expand Down
6 changes: 3 additions & 3 deletions client/executor/src/integration_tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -629,11 +629,11 @@ fn wasm_tracing_should_work(wasm_method: WasmExecutionMethod) {
struct TestTraceHandler(Arc<Mutex<Vec<SpanDatum>>>);

impl sc_tracing::TraceHandler for TestTraceHandler {
fn handle_span(&self, sd: SpanDatum) {
self.0.lock().unwrap().push(sd);
fn handle_span(&self, sd: &SpanDatum) {
self.0.lock().unwrap().push(sd.clone());
}

fn handle_event(&self, _event: TraceEvent) {}
fn handle_event(&self, _event: &TraceEvent) {}
}

let traces = Arc::new(Mutex::new(Vec::new()));
Expand Down
6 changes: 3 additions & 3 deletions client/service/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,11 +187,11 @@ pub struct PrometheusConfig {
impl PrometheusConfig {
/// Create a new config using the default registry.
///
/// The default registry prefixes metrics with `substrate`.
pub fn new_with_default_registry(port: SocketAddr) -> Self {
/// The default registry prefixes metrics with `substrate` (see struct `sc_cli::RunCmd`).
sandreim marked this conversation as resolved.
Show resolved Hide resolved
pub fn new_with_default_registry(port: SocketAddr, metric_prefix: &'static str) -> Self {
Self {
port,
registry: Registry::new_custom(Some("substrate".into()), None)
registry: Registry::new_custom(Some(metric_prefix.into()), None)
.expect("this can only fail if the prefix is empty"),
}
}
Expand Down
62 changes: 42 additions & 20 deletions client/tracing/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ const ZERO_DURATION: Duration = Duration::from_nanos(0);
/// Responsible for assigning ids to new spans, which are not re-used.
pub struct ProfilingLayer {
targets: Vec<(String, Level)>,
trace_handler: Box<dyn TraceHandler>,
trace_handlers: Vec<Box<dyn TraceHandler>>,
}

/// Used to configure how to receive the metrics
Expand All @@ -76,14 +76,14 @@ impl Default for TracingReceiver {

/// A handler for tracing `SpanDatum`
pub trait TraceHandler: Send + Sync {
/// Process a `SpanDatum`
fn handle_span(&self, span: SpanDatum);
/// Process a `TraceEvent`
fn handle_event(&self, event: TraceEvent);
/// Process a `SpanDatum`.
fn handle_span(&self, span: &SpanDatum);
/// Process a `TraceEvent`.
fn handle_event(&self, event: &TraceEvent);
}

/// Represents a tracing event, complete with values
#[derive(Debug)]
#[derive(Debug, Clone)]
pub struct TraceEvent {
/// Name of the event.
pub name: String,
Expand All @@ -98,7 +98,7 @@ pub struct TraceEvent {
}

/// Represents a single instance of a tracing span
#[derive(Debug)]
#[derive(Debug, Clone)]
pub struct SpanDatum {
/// id for this span
pub id: Id,
Expand Down Expand Up @@ -213,6 +213,15 @@ impl fmt::Display for Values {
}
}

/// Trace handler event types.
#[derive(Debug)]
pub enum TraceHandlerEvents {
/// An event.
Event(TraceEvent),
/// A span.
Span(SpanDatum),
}

impl ProfilingLayer {
/// Takes a `TracingReceiver` and a comma separated list of targets,
/// either with a level: "pallet=trace,frame=debug"
Expand All @@ -231,7 +240,12 @@ impl ProfilingLayer {
/// wasm_tracing indicates whether to enable wasm traces
pub fn new_with_handler(trace_handler: Box<dyn TraceHandler>, targets: &str) -> Self {
let targets: Vec<_> = targets.split(',').map(|s| parse_target(s)).collect();
Self { targets, trace_handler }
Self { targets, trace_handlers: vec![trace_handler] }
}

/// Attach additional handlers to allow handling of custom events/spans.
pub fn add_handler(&mut self, trace_handler: Box<dyn TraceHandler>) {
self.trace_handlers.push(trace_handler);
}

fn check_target(&self, target: &str, level: &Level) -> bool {
Expand All @@ -242,6 +256,14 @@ impl ProfilingLayer {
}
false
}

/// Sequentially dispatch a trace event to all handlers.
fn dispatch_event(&self, event: TraceHandlerEvents) {
self.trace_handlers.iter().for_each(|handler| match &event {
TraceHandlerEvents::Span(span_datum) => handler.handle_span(span_datum),
TraceHandlerEvents::Event(event) => handler.handle_event(event),
});
sandreim marked this conversation as resolved.
Show resolved Hide resolved
}
}

// Default to TRACE if no level given or unable to parse Level
Expand Down Expand Up @@ -320,7 +342,7 @@ where
values,
parent_id,
};
self.trace_handler.handle_event(trace_event);
self.dispatch_event(TraceHandlerEvents::Event(trace_event));
}

fn on_enter(&self, span: &Id, ctx: Context<S>) {
Expand Down Expand Up @@ -348,10 +370,10 @@ where
span_datum.target = t;
}
if self.check_target(&span_datum.target, &span_datum.level) {
self.trace_handler.handle_span(span_datum);
self.dispatch_event(TraceHandlerEvents::Span(span_datum));
}
} else {
self.trace_handler.handle_span(span_datum);
self.dispatch_event(TraceHandlerEvents::Span(span_datum));
}
}
}
Expand All @@ -374,7 +396,7 @@ fn log_level(level: Level) -> log::Level {
}

impl TraceHandler for LogTraceHandler {
fn handle_span(&self, span_datum: SpanDatum) {
fn handle_span(&self, span_datum: &SpanDatum) {
if span_datum.values.is_empty() {
log::log!(
log_level(span_datum.level),
Expand All @@ -383,7 +405,7 @@ impl TraceHandler for LogTraceHandler {
span_datum.name,
span_datum.overall_time.as_nanos(),
span_datum.id.into_u64(),
span_datum.parent_id.map(|s| s.into_u64()),
span_datum.parent_id.as_ref().map(|s| s.into_u64()),
);
} else {
log::log!(
Expand All @@ -393,18 +415,18 @@ impl TraceHandler for LogTraceHandler {
span_datum.name,
span_datum.overall_time.as_nanos(),
span_datum.id.into_u64(),
span_datum.parent_id.map(|s| s.into_u64()),
span_datum.parent_id.as_ref().map(|s| s.into_u64()),
span_datum.values,
);
}
}

fn handle_event(&self, event: TraceEvent) {
fn handle_event(&self, event: &TraceEvent) {
log::log!(
log_level(event.level),
"{}, parent_id: {:?}, {}",
event.target,
event.parent_id.map(|s| s.into_u64()),
event.parent_id.as_ref().map(|s| s.into_u64()),
event.values,
);
}
Expand Down Expand Up @@ -447,12 +469,12 @@ mod tests {
}

impl TraceHandler for TestTraceHandler {
fn handle_span(&self, sd: SpanDatum) {
self.spans.lock().push(sd);
fn handle_span(&self, sd: &SpanDatum) {
self.spans.lock().push(sd.clone());
}

fn handle_event(&self, event: TraceEvent) {
self.events.lock().push(event);
fn handle_event(&self, event: &TraceEvent) {
self.events.lock().push(event.clone());
}
}

Expand Down
27 changes: 25 additions & 2 deletions client/tracing/src/logging/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,10 @@ where
/// A builder that is used to initialize the global logger.
pub struct LoggerBuilder {
directives: String,
/// Requires wasm tracing feature (`with-tracing`).
sandreim marked this conversation as resolved.
Show resolved Hide resolved
profiling: Option<(crate::TracingReceiver, String)>,
/// Custom profilers built on top of wasm tracing infrastructure.
sandreim marked this conversation as resolved.
Show resolved Hide resolved
custom_profilers: Vec<Box<dyn crate::TraceHandler>>,
log_reloading: bool,
force_colors: Option<bool>,
detailed_output: bool,
Expand All @@ -204,6 +207,7 @@ impl LoggerBuilder {
Self {
directives: directives.into(),
profiling: None,
custom_profilers: Vec::new(),
log_reloading: false,
force_colors: None,
detailed_output: false,
Expand All @@ -220,6 +224,15 @@ impl LoggerBuilder {
self
}

/// Setup custom profiling.
sandreim marked this conversation as resolved.
Show resolved Hide resolved
pub fn with_custom_profiling(
&mut self,
custom_profilers: Vec<Box<dyn crate::TraceHandler>>,
) -> &mut Self {
self.custom_profilers = custom_profilers;
self
}
sandreim marked this conversation as resolved.
Show resolved Hide resolved

/// Wether or not to disable log reloading.
pub fn with_log_reloading(&mut self, enabled: bool) -> &mut Self {
self.log_reloading = enabled;
Expand Down Expand Up @@ -256,7 +269,12 @@ impl LoggerBuilder {
self.detailed_output,
|builder| enable_log_reloading!(builder),
)?;
let profiling = crate::ProfilingLayer::new(tracing_receiver, &profiling_targets);
let mut profiling =
crate::ProfilingLayer::new(tracing_receiver, &profiling_targets);

self.custom_profilers
.into_iter()
.for_each(|profiler| profiling.add_handler(profiler));

tracing::subscriber::set_global_default(subscriber.with(profiling))?;

Expand All @@ -269,7 +287,12 @@ impl LoggerBuilder {
self.detailed_output,
|builder| builder,
)?;
let profiling = crate::ProfilingLayer::new(tracing_receiver, &profiling_targets);
let mut profiling =
crate::ProfilingLayer::new(tracing_receiver, &profiling_targets);

self.custom_profilers
.into_iter()
.for_each(|profiler| profiling.add_handler(profiler));

tracing::subscriber::set_global_default(subscriber.with(profiling))?;

Expand Down