Skip to content

Conversation

@Ten0
Copy link
Contributor

@Ten0 Ten0 commented Jun 15, 2025

Plugs librados' logs as tracing events, so that they can be seen by users.

/// corresponding overhead.
pub(crate) fn enable_tracing_integration(rados: rados_t) -> RadosResult<()> {
let (level_str, opt_log_callback): (&[u8], rados_log_callback_t) = {
if event_enabled!(target: "librados", Level::ERROR) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, I assume you're writing this way to minimize the runtime cost of additional checks. However, the enable API itself is usually not a heavy call. How about expanding them into one place within the loop?

for example:

type RadosLogCallback = rados_log_callback_t;

fn librados_log_level() -> (&'static [u8], Option<RadosLogCallback>) {
    const CANDIDATES: &[(Level, &[u8])] = &[
        (Level::DEBUG, b"debug\0"),
        (Level::INFO,  b"info\0"),
        (Level::WARN,  b"warn\0"),
        (Level::ERROR, b"error\0"),
    ];

    for &(lvl, cstr) in CANDIDATES {
        if event_enabled!(target: "librados", lvl) {
            return (cstr, Some(log_callback));
        }
    }

    (b"\0", None)
}

Copy link
Contributor Author

@Ten0 Ten0 Jun 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that is not possible because these macros require const level, like below.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants