From 93a6999f014ff6f30f2a3be724efbcda4997e37b Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Fri, 26 Jan 2024 09:06:55 -0800 Subject: [PATCH 1/5] enable trace format --- dsc/Cargo.toml | 4 ++-- dsc/src/args.rs | 25 ++++++++++++++++++++---- dsc/src/main.rs | 51 +++++++++++++++++++++++++++++++++++++++---------- dsc/src/util.rs | 10 ---------- 4 files changed, 64 insertions(+), 26 deletions(-) diff --git a/dsc/Cargo.toml b/dsc/Cargo.toml index 5e136f80..7426b268 100644 --- a/dsc/Cargo.toml +++ b/dsc/Cargo.toml @@ -25,5 +25,5 @@ serde_yaml = { version = "0.9.3" } syntect = { version = "5.0", features = ["default-fancy"], default-features = false } sysinfo = { version = "0.29.10" } thiserror = "1.0" -tracing = "0.1.37" -tracing-subscriber = "0.3.17" +tracing = { version = "0.1.37" } +tracing-subscriber = { version = "0.3.17", features = ["ansi", "env-filter", "json"] } diff --git a/dsc/src/args.rs b/dsc/src/args.rs index 7172d565..295204ed 100644 --- a/dsc/src/args.rs +++ b/dsc/src/args.rs @@ -3,7 +3,6 @@ use clap::{Parser, Subcommand, ValueEnum}; use clap_complete::Shell; -use crate::util::LogLevel; #[derive(Debug, Clone, PartialEq, Eq, ValueEnum)] pub enum OutputFormat { @@ -12,6 +11,22 @@ pub enum OutputFormat { Yaml, } +#[derive(Debug, Clone, PartialEq, Eq, ValueEnum)] +pub enum TraceFormat { + Default, + Plaintext, + Json, +} + +#[derive(Debug, Clone, PartialEq, Eq, ValueEnum)] +pub enum TraceLevel { + Error, + Warning, + Info, + Debug, + Trace +} + #[derive(Debug, Parser)] #[clap(name = "dsc", version = env!("CARGO_PKG_VERSION"), about = "Apply configuration or invoke specific DSC resources", long_about = None)] pub struct Args { @@ -19,14 +34,16 @@ pub struct Args { #[clap(subcommand)] pub subcommand: SubCommand, /// The output format to use - #[clap(short = 'f', long)] + #[clap(short = 'o', long)] pub format: Option, #[clap(short = 'i', long, help = "The input to pass to the configuration or resource", conflicts_with = "input_file")] pub input: Option, #[clap(short = 'p', long, help = "The path to a file used as input to the configuration or resource")] pub input_file: Option, - #[clap(short = 'l', long = "logging-level", help = "Log level to display", value_enum, default_value = "info")] - pub logging_level: LogLevel, + #[clap(short = 'l', long, help = "Trace level to use", value_enum, default_value = "warning")] + pub trace_level: TraceLevel, + #[clap(short = 'f', long, help = "Trace format to use", value_enum, default_value = "default")] + pub trace_format: TraceFormat, } #[derive(Debug, PartialEq, Eq, Subcommand)] diff --git a/dsc/src/main.rs b/dsc/src/main.rs index 92535f3b..effb63f8 100644 --- a/dsc/src/main.rs +++ b/dsc/src/main.rs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -use args::{Args, SubCommand}; +use args::{Args, TraceLevel, TraceFormat, SubCommand}; use atty::Stream; use clap::{CommandFactory, Parser}; use clap_complete::generate; @@ -9,6 +9,7 @@ use std::io::{self, Read}; use std::process::exit; use sysinfo::{Process, ProcessExt, RefreshKind, System, SystemExt, get_current_pid, ProcessRefreshKind}; use tracing::{Level, error, info, warn, debug}; +use tracing_subscriber::{filter::EnvFilter, layer::SubscriberExt, Layer}; #[cfg(debug_assertions)] use crossterm::event; @@ -31,18 +32,48 @@ fn main() { let args = Args::parse(); - let tracing_level = match args.logging_level { - util::LogLevel::Error => Level::ERROR, - util::LogLevel::Warning => Level::WARN, - util::LogLevel::Info => Level::INFO, - util::LogLevel::Debug => Level::DEBUG, - util::LogLevel::Trace => Level::TRACE, + let tracing_level = match args.trace_level { + TraceLevel::Error => Level::ERROR, + TraceLevel::Warning => Level::WARN, + TraceLevel::Info => Level::INFO, + TraceLevel::Debug => Level::DEBUG, + TraceLevel::Trace => Level::TRACE, }; - // create subscriber that writes all events to stderr - let subscriber = tracing_subscriber::fmt().pretty().with_max_level(tracing_level).with_writer(std::io::stderr).finish(); + let filter = EnvFilter::try_from_default_env() + .or_else(|_| EnvFilter::try_new("warning")) + .unwrap() + .add_directive(tracing_level.into()); + let layer = tracing_subscriber::fmt::Layer::default().with_writer(std::io::stderr); + let fmt = match args.trace_format { + TraceFormat::Default => { + layer + .with_ansi(true) + .with_level(true) + .with_line_number(true) + .boxed() + }, + TraceFormat::Plaintext => { + layer + .with_ansi(false) + .with_level(false) + .with_line_number(false) + .boxed() + }, + TraceFormat::Json => { + layer + .with_ansi(true) + .with_level(false) + .with_line_number(false) + .json() + .boxed() + } + }; + + let subscriber = tracing_subscriber::Registry::default().with(fmt).with(filter); + if tracing::subscriber::set_global_default(subscriber).is_err() { - eprintln!("Unable to set global default subscriber"); + eprintln!("Unable to set global default logging subscriber"); } debug!("Running dsc {}", env!("CARGO_PKG_VERSION")); diff --git a/dsc/src/util.rs b/dsc/src/util.rs index ac5077f0..e93afea8 100644 --- a/dsc/src/util.rs +++ b/dsc/src/util.rs @@ -35,16 +35,6 @@ pub const EXIT_INVALID_INPUT: i32 = 4; pub const EXIT_VALIDATION_FAILED: i32 = 5; pub const EXIT_CTRL_C: i32 = 6; -#[derive(Debug)] -#[derive(clap::ValueEnum, Clone)] -pub enum LogLevel { - Error, - Warning, - Info, - Debug, - Trace -} - /// Get string representation of JSON value. /// /// # Arguments From 136be723696b3b69cd469d4dff28f984d4d8862c Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Fri, 26 Jan 2024 13:49:54 -0800 Subject: [PATCH 2/5] add tests --- dsc/src/main.rs | 8 +++--- dsc/tests/dsc_tracing.tests.ps1 | 47 +++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 4 deletions(-) create mode 100644 dsc/tests/dsc_tracing.tests.ps1 diff --git a/dsc/src/main.rs b/dsc/src/main.rs index effb63f8..6f5f9126 100644 --- a/dsc/src/main.rs +++ b/dsc/src/main.rs @@ -56,15 +56,15 @@ fn main() { TraceFormat::Plaintext => { layer .with_ansi(false) - .with_level(false) + .with_level(true) .with_line_number(false) .boxed() }, TraceFormat::Json => { layer - .with_ansi(true) - .with_level(false) - .with_line_number(false) + .with_ansi(false) + .with_level(true) + .with_line_number(true) .json() .boxed() } diff --git a/dsc/tests/dsc_tracing.tests.ps1 b/dsc/tests/dsc_tracing.tests.ps1 new file mode 100644 index 00000000..729c3d6f --- /dev/null +++ b/dsc/tests/dsc_tracing.tests.ps1 @@ -0,0 +1,47 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +Describe 'tracing tests' { + It 'trace level works' -TestCases @( + @{ level = 'error' } + # @{ level = 'WARNING' } TODO: currently no warnings are emitted + @{ level = 'info' } + @{ level = 'debug' } + # @{ level = 'trace' } TODO:L currently no trace is emitted + ) { + param($level) + + $logPath = "$TestDrive/dsc_trace.log" + $null = '{}' | dsc -l $level resource get -r 'DoesNotExist' 2> $logPath + $log = Get-Content $logPath -Raw + $log | Should -BeLikeExactly "* $($level.ToUpper()) *" + } + + It 'trace level error does not emit other levels' { + $logPath = "$TestDrive/dsc_trace.log" + $null = '{}' | dsc --trace-level error resource get -r 'DoesNotExist' 2> $logPath + $log = Get-Content $logPath -Raw + $log | Should -Not -BeLikeExactly "* WARNING *" + $log | Should -Not -BeLikeExactly "* INFO *" + $log | Should -Not -BeLikeExactly "* DEBUG *" + $log | Should -Not -BeLikeExactly "* TRACE *" + } + + It 'trace format plaintext does not emit ANSI' { + $logPath = "$TestDrive/dsc_trace.log" + $null = '{}' | dsc --trace-format plaintext resource get -r 'DoesNotExist' 2> $logPath + $log = Get-Content $logPath -Raw + $log | Should -Not -BeLikeExactly "*``[0m*" + } + + It 'trace format json emits json' { + $logPath = "$TestDrive/dsc_trace.log" + $null = '{}' | dsc --trace-format json resource get -r 'DoesNotExist' 2> $logPath + foreach ($line in (Get-Content $logPath)) { + $trace = $line | ConvertFrom-Json -Depth 10 + $trace.timestamp | Should -Not -BeNullOrEmpty + $trace.level | Should -BeIn 'ERROR', 'WARNING', 'INFO', 'DEBUG', 'TRACE' + $trace.fields.message | Should -Not -BeNullOrEmpty + } + } +} From fa87b840fa8f4d32c7d40c0bfbee4334ba78809f Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Fri, 26 Jan 2024 14:06:05 -0800 Subject: [PATCH 3/5] fix clippy --- dsc/src/main.rs | 49 +++-------------------------------------------- dsc/src/util.rs | 51 +++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 52 insertions(+), 48 deletions(-) diff --git a/dsc/src/main.rs b/dsc/src/main.rs index 6f5f9126..5eafb4e2 100644 --- a/dsc/src/main.rs +++ b/dsc/src/main.rs @@ -1,15 +1,14 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -use args::{Args, TraceLevel, TraceFormat, SubCommand}; +use args::{Args, SubCommand}; use atty::Stream; use clap::{CommandFactory, Parser}; use clap_complete::generate; use std::io::{self, Read}; use std::process::exit; use sysinfo::{Process, ProcessExt, RefreshKind, System, SystemExt, get_current_pid, ProcessRefreshKind}; -use tracing::{Level, error, info, warn, debug}; -use tracing_subscriber::{filter::EnvFilter, layer::SubscriberExt, Layer}; +use tracing::{error, info, warn, debug}; #[cfg(debug_assertions)] use crossterm::event; @@ -32,49 +31,7 @@ fn main() { let args = Args::parse(); - let tracing_level = match args.trace_level { - TraceLevel::Error => Level::ERROR, - TraceLevel::Warning => Level::WARN, - TraceLevel::Info => Level::INFO, - TraceLevel::Debug => Level::DEBUG, - TraceLevel::Trace => Level::TRACE, - }; - - let filter = EnvFilter::try_from_default_env() - .or_else(|_| EnvFilter::try_new("warning")) - .unwrap() - .add_directive(tracing_level.into()); - let layer = tracing_subscriber::fmt::Layer::default().with_writer(std::io::stderr); - let fmt = match args.trace_format { - TraceFormat::Default => { - layer - .with_ansi(true) - .with_level(true) - .with_line_number(true) - .boxed() - }, - TraceFormat::Plaintext => { - layer - .with_ansi(false) - .with_level(true) - .with_line_number(false) - .boxed() - }, - TraceFormat::Json => { - layer - .with_ansi(false) - .with_level(true) - .with_line_number(true) - .json() - .boxed() - } - }; - - let subscriber = tracing_subscriber::Registry::default().with(fmt).with(filter); - - if tracing::subscriber::set_global_default(subscriber).is_err() { - eprintln!("Unable to set global default logging subscriber"); - } + util::enable_tracing(&args.trace_level, &args.trace_format); debug!("Running dsc {}", env!("CARGO_PKG_VERSION")); diff --git a/dsc/src/util.rs b/dsc/src/util.rs index e93afea8..be9106df 100644 --- a/dsc/src/util.rs +++ b/dsc/src/util.rs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -use crate::args::{DscType, OutputFormat}; +use crate::args::{DscType, OutputFormat, TraceFormat, TraceLevel}; use atty::Stream; use dsc_lib::{ @@ -25,7 +25,8 @@ use syntect::{ parsing::SyntaxSet, util::{as_24_bit_terminal_escaped, LinesWithEndings} }; -use tracing::error; +use tracing::{Level, error}; +use tracing_subscriber::{filter::EnvFilter, layer::SubscriberExt, Layer}; pub const EXIT_SUCCESS: i32 = 0; pub const EXIT_INVALID_ARGS: i32 = 1; @@ -241,3 +242,49 @@ pub fn write_output(json: &str, format: &Option) { println!("{output}"); } } + +pub fn enable_tracing(trace_level: &TraceLevel, trace_format: &TraceFormat) { + let tracing_level = match trace_level { + TraceLevel::Error => Level::ERROR, + TraceLevel::Warning => Level::WARN, + TraceLevel::Info => Level::INFO, + TraceLevel::Debug => Level::DEBUG, + TraceLevel::Trace => Level::TRACE, + }; + + let filter = EnvFilter::try_from_default_env() + .or_else(|_| EnvFilter::try_new("warning")) + .unwrap_or_default() + .add_directive(tracing_level.into()); + let layer = tracing_subscriber::fmt::Layer::default().with_writer(std::io::stderr); + let fmt = match trace_format { + TraceFormat::Default => { + layer + .with_ansi(true) + .with_level(true) + .with_line_number(true) + .boxed() + }, + TraceFormat::Plaintext => { + layer + .with_ansi(false) + .with_level(true) + .with_line_number(false) + .boxed() + }, + TraceFormat::Json => { + layer + .with_ansi(false) + .with_level(true) + .with_line_number(true) + .json() + .boxed() + } + }; + + let subscriber = tracing_subscriber::Registry::default().with(fmt).with(filter); + + if tracing::subscriber::set_global_default(subscriber).is_err() { + eprintln!("Unable to set global default tracing subscriber. Tracing is diabled."); + } +} From c39d9c33a6cb52ca6dc8a67e877af5ed444983d6 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Fri, 26 Jan 2024 17:10:56 -0800 Subject: [PATCH 4/5] fix formatting --- dsc/src/args.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/dsc/src/args.rs b/dsc/src/args.rs index 295204ed..c3920d14 100644 --- a/dsc/src/args.rs +++ b/dsc/src/args.rs @@ -20,11 +20,11 @@ pub enum TraceFormat { #[derive(Debug, Clone, PartialEq, Eq, ValueEnum)] pub enum TraceLevel { - Error, - Warning, - Info, - Debug, - Trace + Error, + Warning, + Info, + Debug, + Trace } #[derive(Debug, Parser)] From e02d70663365023189e9d36aa527372a08bf8888 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Mon, 29 Jan 2024 18:52:02 -0800 Subject: [PATCH 5/5] Update dsc/tests/dsc_tracing.tests.ps1 Co-authored-by: Tess Gauthier --- dsc/tests/dsc_tracing.tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dsc/tests/dsc_tracing.tests.ps1 b/dsc/tests/dsc_tracing.tests.ps1 index 729c3d6f..c6601ec8 100644 --- a/dsc/tests/dsc_tracing.tests.ps1 +++ b/dsc/tests/dsc_tracing.tests.ps1 @@ -7,7 +7,7 @@ Describe 'tracing tests' { # @{ level = 'WARNING' } TODO: currently no warnings are emitted @{ level = 'info' } @{ level = 'debug' } - # @{ level = 'trace' } TODO:L currently no trace is emitted + # @{ level = 'trace' } TODO: currently no trace is emitted ) { param($level)