feat: add logging macro #196

TropicalDog17 · 2024-09-16T08:10:29Z

resolves [feat] create a log! macro #187
follows contribution guide
code change includes tests

Guide to be added in README.md of the logging package

Prequiste for using log! macro for a package:

import this as a dependency in Scarb.toml

[dependencies]
`raito_macros = { path = "../macros" }`

add the features in Scarb.toml

[features]
default = [log_level_none]
log_level_none = []
log_level_trace = []
log_level_debug = []

add the logging.cairo(must be this name) file to the package root, like the package/consensus

# packages/consensus/src/logging.cairo
#[cfg(feature: 'log_level_trace')]
pub const LOG_LEVEL_TRACE: bool = true;

#[cfg(feature: 'log_level_trace')]
pub const LOG_LEVEL_DEBUG: bool = true;

#[cfg(feature: 'log_level_debug')]
pub const LOG_LEVEL_TRACE: bool = false;

#[cfg(feature: 'log_level_debug')]
pub const LOG_LEVEL_DEBUG: bool = true;

#[cfg(feature: 'log_level_none')]
pub const LOG_LEVEL_TRACE: bool = false;

#[cfg(feature: 'log_level_none')]
pub const LOG_LEVEL_DEBUG: bool = false;

in lib.cairo

pub mod logging;

vercel · 2024-09-16T08:10:35Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
raito	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 25, 2024 2:36pm

packages/macros/src/logging.rs

maciejka · 2024-09-17T10:30:12Z

packages/macros/src/logging.rs

+        .collect();
+
+       // Generate the log statement with feature checks
+       let log_statement = match level.as_str() {


Is it really necessary to have separate versions for each log level?

packages/macros/src/logging.rs

maciejka · 2024-09-19T19:49:25Z

Scarb.toml

@@ -11,3 +11,4 @@ license-file = "LICENSE"

 [workspace.dependencies]
 cairo_test = "2.8.0"
+


maciejka · 2024-09-19T19:51:04Z

packages/consensus/src/validation/timestamp.cairo

@@ -44,6 +43,18 @@ pub fn validate_timestamp(prev_timestamps: Span<u32>, block_time: u32) -> Result
    }
 }

+#[cfg(feature: 'log_level_debug')]


What are we testing here? No conditions are checked.

maciejka · 2024-09-19T19:53:02Z

packages/macros/src/logging.rs

+    // Generate the log statement
+    let log_statement = generate_log_statement(&level, &format_string, &log_args);
+
+    println!("Log statement: {}", log_statement);


Not necessary?

maciejka · 2024-09-19T19:53:41Z

packages/macros/src/logging.rs

+            if matches!(node.kind(&db), SyntaxKind::Arg) {
+                Some(node.get_text(&db))
+            } else {
+                println!("Invalid argument: {:?} {:?}", node.get_text(&db), node.kind(&db));


This should return an error?

maciejka · 2024-09-19T19:56:14Z

packages/macros/Cargo.toml

@@ -0,0 +1,14 @@
+# Cargo.toml


Package should be called logging not macros.

maciejka · 2024-09-19T19:56:56Z

packages/consensus/src/logging.cairo

@@ -0,0 +1,17 @@
+#[cfg(feature: 'log_level_trace')]


This file should go to macros(or rather logging) package.

I can't find a way to import the logging constant to the desired package to use the constants. seems that the procedural macros package(logging package) behaves differently.

Maybe try to make package with macros a dependency of loggin package(which would be cairo only)?

very weird even the compiler is happy after the feature is enabled, maybe this is cairo bug @maciejka

error: Identifier not found. --> //raito/packages/consensus/src/codec.cairo:14:30 if logging::logging::LOG_LEVEL_DEBUG { println!("{}: {}", format!("DEBUG"), format!("tx_encoded: {}", dest)) } ^*************^ error: could not compile `consensus` due to previous error

It should be logging::log::LOG_LEVEL_DEBUG not logging::logging::LOG_LEVEL_DEBUG:

@maciejka i previously rename the file then logging::logging::LOG_LEVEL_DEBUG. also logging::log::LOG_LEVEL_DEBUG also not work even though the language server recognize when the feature is enabled.

You are right:

I asked in scarb group: https://t.me/c/1788692421/1887

maciejka · 2024-09-22T18:41:04Z

packages/logging/src/logging.rs

+    // Generate the log statement
+    let log_statement = generate_log_statement(&level, &format_string, &log_args);
+
+    println!("Log statement: {}", log_statement);


Remove the trace

maciejka · 2024-09-24T07:28:49Z

packages/logging/Cargo.toml

+[dependencies]
+bigdecimal = "0.4.5"
+cairo-lang-macro = "0.1"
+cairo-lang-parser = "2.7.0"


don't we want 2.8.0?

maciejka · 2024-09-24T07:44:17Z

packages/consensus/src/logging.cairo

@@ -0,0 +1,17 @@
+#[cfg(feature: 'log_level_trace')]


It should be logging::log::LOG_LEVEL_DEBUG not logging::logging::LOG_LEVEL_DEBUG:

maciejka · 2024-09-25T12:24:57Z

How is it going @TropicalDog17? It seems like this one is close to be finished.

TropicalDog17 · 2024-09-25T13:12:56Z

How is it going @TropicalDog17? It seems like this one is close to be finished.

Yeah @maciejka I think these feature-based constants cannot be imported from an external package, I tried some workaround but it's no luck, maybe the language hasn't supported that feature yet. It appears that for the logging feature to work correctly, the constant file needs to be placed within the specific package where we want to enable logging.

maciejka · 2024-09-25T15:40:31Z

Please check: #224. I had to try it myself. Indeed there is a problem with conditional compilation in a multipackage project. Waiting for scarb developers response.

maciejka · 2024-09-26T14:16:27Z

Please check: #224. I had to try it myself. Indeed there is a problem with conditional compilation in a multipackage project. Waiting for scarb developers response.

maciejka · 2024-09-26T14:30:37Z

Please check: #224. I had to try it myself. Indeed there is a problem with conditional compilation in a multipackage project. Waiting for scarb developers response.

Which means we might need to wait until they implement it.

TropicalDog17 · 2024-09-26T15:40:20Z

Please check: #224. I had to try it myself. Indeed there is a problem with conditional compilation in a multipackage project. Waiting for scarb developers response.

Which means we might need to wait until they implement it.

yeah I see, and very weird that the language server still recognize that feature.

maciejka · 2024-09-26T15:55:51Z

yeah I see, and very weird that the language server still recognize that feature.

true

maciejka · 2024-09-26T15:57:30Z

I am closing it for now. We will need to revisit this issue once feature support in Scarb is more mature. The work with my small fixes is parked in branch: feat/logging-macro

maciejka · 2024-09-26T15:57:58Z

@TropicalDog17 please contact me on tg: @aundumla

add macros

009358a

vercel bot deployed to Preview September 16, 2024 08:11 View deployment

maciejka requested changes Sep 17, 2024

View reviewed changes

TropicalDog17 requested a review from maciejka September 17, 2024 15:25

impl logging macros

4509808

vercel bot deployed to Preview September 19, 2024 07:10 View deployment

Merge branch 'main' into feat/log-macro

c69d376

vercel bot deployed to Preview September 19, 2024 07:10 View deployment

lint

0c78897

vercel bot deployed to Preview September 19, 2024 07:11 View deployment

TropicalDog17 marked this pull request as ready for review September 19, 2024 07:19

TropicalDog17 requested a review from m-kus as a code owner September 19, 2024 07:19

TropicalDog17 changed the title ~~wip: add logging macro~~ feat: add logging macro Sep 19, 2024

maciejka requested changes Sep 19, 2024

View reviewed changes

TropicalDog17 added 2 commits September 22, 2024 16:51

refactor

5cabfd8

refactor, add cairo test

5e061ed

vercel bot deployed to Preview September 22, 2024 09:55 View deployment

fmt

ce7ff20

vercel bot deployed to Preview September 22, 2024 12:52 View deployment

maciejka reviewed Sep 22, 2024

View reviewed changes

TropicalDog17 mentioned this pull request Sep 23, 2024

[CONTRACTS] implement register and is_registered tests keep-starknet-strange/zkramp#93

Closed

maciejka reviewed Sep 24, 2024

View reviewed changes

update

4b4809f

vercel bot deployed to Preview September 25, 2024 14:36 View deployment

maciejka closed this Sep 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add logging macro #196

feat: add logging macro #196

TropicalDog17 commented Sep 16, 2024 •

edited

Loading

vercel bot commented Sep 16, 2024 •

edited

Loading

maciejka Sep 17, 2024

maciejka Sep 19, 2024

maciejka Sep 19, 2024

maciejka Sep 19, 2024

maciejka Sep 19, 2024

maciejka Sep 19, 2024

maciejka Sep 19, 2024

TropicalDog17 Sep 22, 2024 •

edited

Loading

maciejka Sep 22, 2024

TropicalDog17 Sep 23, 2024 •

edited

Loading

maciejka Sep 24, 2024

TropicalDog17 Sep 25, 2024

maciejka Sep 25, 2024

maciejka Sep 25, 2024

maciejka Sep 22, 2024

maciejka Sep 24, 2024

maciejka Sep 24, 2024

maciejka commented Sep 25, 2024

TropicalDog17 commented Sep 25, 2024

maciejka commented Sep 25, 2024

maciejka commented Sep 26, 2024

maciejka commented Sep 26, 2024

TropicalDog17 commented Sep 26, 2024

maciejka commented Sep 26, 2024

maciejka commented Sep 26, 2024

maciejka commented Sep 26, 2024

		@@ -11,3 +11,4 @@ license-file = "LICENSE"

		[workspace.dependencies]
		cairo_test = "2.8.0"

feat: add logging macro #196

feat: add logging macro #196

Conversation

TropicalDog17 commented Sep 16, 2024 • edited Loading

Guide to be added in README.md of the logging package

vercel bot commented Sep 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TropicalDog17 Sep 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TropicalDog17 Sep 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maciejka commented Sep 25, 2024

TropicalDog17 commented Sep 25, 2024

maciejka commented Sep 25, 2024

maciejka commented Sep 26, 2024

maciejka commented Sep 26, 2024

TropicalDog17 commented Sep 26, 2024

maciejka commented Sep 26, 2024

maciejka commented Sep 26, 2024

maciejka commented Sep 26, 2024

TropicalDog17 commented Sep 16, 2024 •

edited

Loading

vercel bot commented Sep 16, 2024 •

edited

Loading

TropicalDog17 Sep 22, 2024 •

edited

Loading

TropicalDog17 Sep 23, 2024 •

edited

Loading