Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bug][nit pick] dup_filter_sink logs with the current log call's level, not the filtered log call's level #1710

Closed
Tridacnid opened this issue Oct 19, 2020 · 1 comment

Comments

@Tridacnid
Copy link
Contributor

// dup_filter_sink.h
        // log the "skipped.." message
        if (skip_counter_ > 0)
        {
            memory_buf_t buf;
            fmt::format_to(buf, "Skipped {} duplicate messages..", skip_counter_);
            details::log_msg skipped_msg{msg.logger_name, msg.level, string_view_t{buf.data(), buf.size()}};
            dist_sink<Mutex>::sink_it_(skipped_msg);
        }

msg.level can be different from the level that was used for the filtered logs. I suppose this is a bit of a catch22, because the duplicate detection could trigger on identical log messages from different log calls that each use a different level, but I think the expectation is that the dup_filter_sink will be used to catch duplicates from log messages inside a loop.

pseudo-code:

// set up a dup_filter_sink logger
logger->set_pattern("%l - %v");
logger->log(trace, "hello, world!");
// info - hello, world!
logger->log(info, "hello, world!"); // filtered, even though different log level (fine by me)
logger->log(err, "goodbye, cruel world!");
// err - Skipped 1 duplicate messages..
// err - goodbye, cruel world!

I'd have expected the 2nd to last log message to have an info or trace level, not an err level.

@gabime
Copy link
Owner

gabime commented Nov 5, 2020

I agree it can be confusing.
I think msg.level should be always “info” for consistency sake.

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

No branches or pull requests

2 participants