chore(starknet_sequencer_infra): add dynamic logging util fn #2328
Conversation
|
This change is |
|
Benchmark movements: full_committer_flow performance improved 😺 |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2328 +/- ##
===========================================
+ Coverage 40.10% 77.55% +37.44%
===========================================
Files 26 388 +362
Lines 1895 41126 +39231
Branches 1895 41126 +39231
===========================================
+ Hits 760 31895 +31135
- Misses 1100 6978 +5878
- Partials 35 2253 +2218 ☔ View full report in Codecov by Sentry. |
Itay-Tsabary-Starkware
changed the base branch from
spr/main/4d0abfb5
to
main
Itay-Tsabary-Starkware
force-pushed
the
spr/main/9ffe9fbe
branch
from
58d68f8 to
f07b584
Compare
Itay-Tsabary-Starkware
changed the base branch from
main
to
spr/main/4d0abfb5
|
Benchmark movements: full_committer_flow performance improved 😺 |
Itay-Tsabary-Starkware
changed the base branch from
spr/main/4d0abfb5
to
main
Itay-Tsabary-Starkware
force-pushed
the
spr/main/9ffe9fbe
branch
from
f07b584 to
8f77d9b
Compare
|
Benchmark movements: |
Itay-Tsabary-Starkware
force-pushed
the
spr/main/9ffe9fbe
branch
2 times, most recently
from
3074675 to
e3aeefe
Compare
Itay-Tsabary-Starkware
changed the title
Itay-Tsabary-Starkware
force-pushed
the
spr/main/9ffe9fbe
branch
2 times, most recently
from
cfe3da9 to
1c04622
Compare
Itay-Tsabary-Starkware
force-pushed
the
spr/main/9ffe9fbe
branch
2 times, most recently
from
f6705a7 to
397d1e8
Compare
There was a problem hiding this comment.
Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on @Itay-Tsabary-Starkware)
crates/infra_utils/src/tracing.rs line 4 at r1 (raw file):
/// Dynamically set tracing level of a message. pub struct DynamicLogger {
Why this is DynamicLogger?
Your previous version, As I remember, could log every message with a different TraceLevel
crates/infra_utils/src/tracing.rs line 23 at r1 (raw file):
}; match self.level {
I understand that 'match' is quite effective, but it doesn't seem pretty.
I would suggest another solution but its up to you
Code snippet:
use tracing::{debug, error, info, trace, warn};
pub struct DynamicLogger {
base_msg: Option<String>,
log_message_fn: Box<dyn Fn(&str)>,
}
impl DynamicLogger {
fn new(base_msg: Option<String>, log_message_fn: Box<dyn Fn(&str)>) -> Self {
Self { base_msg, log_message_fn }
}
fn log_message(&self, msg: &str) {
let message = match &self.base_msg {
Some(base_msg) => format!("{}: {}", base_msg, msg),
None => msg.to_string(),
};
(self.log_message_fn)(&message);
}
}
pub enum TraceLevel {
Trace,
Debug,
Info,
Warn,
Error,
}
pub fn create_logger(base_msg: Option<String>, level: TraceLevel) -> DynamicLogger {
let log_fn: Box<dyn Fn(&str)> = match level {
TraceLevel::Trace => Box::new(|msg| trace!("{}", msg)),
TraceLevel::Debug => Box::new(|msg| debug!("{}", msg)),
TraceLevel::Info => Box::new(|msg| info!("{}", msg)),
TraceLevel::Warn => Box::new(|msg| warn!("{}", msg)),
TraceLevel::Error => Box::new(|msg| error!("{}", msg)),
};
DynamicLogger::new(base_msg, log_fn)
}There was a problem hiding this comment.
Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion (waiting on @lev-starkware)
crates/infra_utils/src/tracing.rs line 4 at r1 (raw file):
Previously, lev-starkware wrote…
Why this is DynamicLogger?
Your previous version, As I remember, could log every message with a different TraceLevel
It determines the logging level at runtime and not at compile time.
Happy to take name suggestions.
Itay-Tsabary-Starkware
force-pushed
the
spr/main/9ffe9fbe
branch
from
397d1e8 to
8cd1e86
Compare
There was a problem hiding this comment.
Reviewed 3 of 4 files at r1, 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Itay-Tsabary-Starkware)
crates/infra_utils/src/tracing.rs line 4 at r1 (raw file):
Previously, Itay-Tsabary-Starkware wrote…
It determines the logging level at runtime and not at compile time.
Happy to take name suggestions.
AdaptiveLogger, FlexibleLogger, and my favorite CustomLogger?
commit-id:9ffe9fbe
Itay-Tsabary-Starkware
force-pushed
the
spr/main/9ffe9fbe
branch
from
8cd1e86 to
7967122
Compare
There was a problem hiding this comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @lev-starkware)
crates/infra_utils/src/tracing.rs line 4 at r1 (raw file):
Previously, lev-starkware wrote…
AdaptiveLogger, FlexibleLogger, and my favorite CustomLogger?
Added a refactor in #2424
There was a problem hiding this comment.
Reviewable status:
complete! all files reviewed, all discussions resolved (waiting on @Itay-Tsabary-Starkware)
|
✓ Commit merged in pull request #2424 |
Stack: