search

picodata / tarantool-module

Tarantool Rust SDK
https://docs.rs/tarantool/
BSD 2-Clause "Simplified" License
58 stars 11 forks source link

[discussion] Logging levels #17

Closed YaZasnyal closed 1 year ago

YaZasnyal commented 2 years ago

Hi, I am experimenting with stored procedures written in rust and noticed that there are much fewer logging levels than in Tarantool itself. The main reason is a mismatch of levels in rust and Tarantool. I think that it is a good idea to avoid ambiguity but I still want to discuss the possibility of changing Rust's Debug to Tarantool's Verbose. It will make more sense for Rust programmers.

https://github.com/picodata/tarantool-module/blob/8a1006460de486b908ddd8dfeb00c0ba149a860f/tarantool/src/log.rs#L78-L88

gmoshkin commented 2 years ago

Sounds like a good idea. Considering the amount of debugging info tarantool spurts out on "debug" level we might as well call it "trace". And leaving out "verbose" looks like a bug to me

YaZasnyal commented 1 year ago

Mapping can be adjusted using with_mapping constructor. I think both this issue and #19 can be closed.

https://github.com/picodata/tarantool-module/blob/7dbfdf17cecfa0b6a395798e37b621416078fc66/tests/src/log.rs#L15-L28

YaZasnyal commented 1 year ago

It seems that I closed this issue too early.

https://github.com/picodata/tarantool-module/blob/9326a296ec5add45d883341c17ae34e8ebca8c25/tarantool/src/log.rs#L45-L48

This function does not take into account custom mapping and no logging is performed. I think it should be changed like that

        let level: SayLevel = (self.0)(metadata.level());
        level <= SayLevel::from_i32(unsafe { ffi::LOG_LEVEL }).unwrap()
gmoshkin commented 1 year ago

It seems that I closed this issue too early.

https://github.com/picodata/tarantool-module/blob/9326a296ec5add45d883341c17ae34e8ebca8c25/tarantool/src/log.rs#L45-L48

This function does not take into account custom mapping and no logging is performed. I think it should be changed like that

        let level: SayLevel = (self.0)(metadata.level());
        level <= SayLevel::from_i32(unsafe { ffi::LOG_LEVEL }).unwrap()

Thanks for the bug report, this has been fixed