Repository: bitcoin/bitcoin
File: src/logging.cpp (C++)

Code before:

    FormatLogStrInPlace(str_prefixed, category, level, source_loc, util::ThreadGetInternalName(), SystemClock::now(), GetMockTime());
    bool ratelimit{false};
    if (should_ratelimit && m_limiter) {
        auto status{m_limiter->Consume(source_loc, str_prefixed)};
        if (status == LogRateLimiter::Status::NEWLY_SUPPRESSED) {
            // NOLINTNEXTLINE(misc-no-recursion)
            LogPrintStr_(strprintf(
                             "Excessive logging detected from %s:%d (%s): >%d bytes logged during "
                             "the last time window of %is. Suppressing logging to disk from this "
                             "source location until time window resets. Console logging "
                             "unaffected. Last log entry.",
                             source_loc.file_name(), source_loc.line(), source_loc.function_name(),
                             m_limiter->m_max_bytes,
                             Ticks<std::chrono::seconds>(m_limiter->m_reset_window)),
                         std::source_location::current(), LogFlags::ALL, Level::Warning, /*should_ratelimit=*/false); // with should_ratelimit=false, this cannot lead to infinite recursion
        } else if (status == LogRateLimiter::Status::STILL_SUPPRESSED) {
            ratelimit = true;
        }
    }

    // To avoid confusion caused by dropped log messages when debugging an issue,
    // we prefix log lines with "[*]" when there are any suppressed source locations.
    if (m_limiter && m_limiter->SuppressionsActive()) {
        str_prefixed.insert(0, "[*] ");
    }

    if (m_print_to_console) {
        // print to console
        fwrite(str_prefixed.data(), 1, str_prefixed.size(), stdout);
        fflush(stdout);
    }
    for (const auto& cb : m_print_callbacks) {
        cb(str_prefixed);
    }
    if (m_print_to_file && !ratelimit) {
        assert(m_fileout != nullptr);

        // reopen the log file, if requested
        if (m_reopen_file) {
            m_reopen_file = false;
            FILE* new_fileout = fsbridge::fopen(m_file_path, "a");
            if (new_fileout) {
                setbuf(new_fileout, nullptr); // unbuffered
                fclose(m_fileout);
                m_fileout = new_fileout;
            }
        }
        FileWriteStr(str_prefixed, m_fileout);
    }
}

Review comment (suggestion):
We already have a `should_ratelimit` and we're calculating the `ratelimit` again. The distinction isn't immediately obvious.
Maybe the `should_ratelimit` parameter could be called `rate_limit_enabled`, while the line limiting could maybe be `is_rate_limited` though that still isn't as clear as I'd like it to be.

----

Also, the `else if` makew it more complicated in my opinion (we have to look what the value is if we entered the `NEWLY_SUPPRESSED` branch). We can just assign it if we entered here:
```suggestion
        is_rate_limited = (status == LogRateLimiter::Status::STILL_SUPPRESSED);
```

Code after:
            m_cur_buffer_memusage -= MemUsage(m_msgs_before_open.front());
            m_msgs_before_open.pop_front();
            ++m_buffer_lines_discarded;
        }

        return;
    }

    FormatLogStrInPlace(str_prefixed, category, level, source_loc, util::ThreadGetInternalName(), SystemClock::now(), GetMockTime());
    bool ratelimit{false};
    if (should_ratelimit && m_limiter) {
        auto status{m_limiter->Consume(source_loc, str_prefixed)};
        if (status == LogRateLimiter::Status::NEWLY_SUPPRESSED) {
            // NOLINTNEXTLINE(misc-no-recursion)
            LogPrintStr_(strprintf(
                             "Excessive logging detected from %s:%d (%s): >%d bytes logged during "
                             "the last time window of %is. Suppressing logging to disk from this "
                             "source location until time window resets. Console logging "
                             "unaffected. Last log entry.",
                             source_loc.file_name(), source_loc.line(), source_loc.function_name(),
                             m_limiter->m_max_bytes,
                             Ticks<std::chrono::seconds>(m_limiter->m_reset_window)),
                         std::source_location::current(), LogFlags::ALL, Level::Warning, /*should_ratelimit=*/false); // with should_ratelimit=false, this cannot lead to infinite recursion
        } else if (status == LogRateLimiter::Status::STILL_SUPPRESSED) {
            ratelimit = true;
        }
    }

    // To avoid confusion caused by dropped log messages when debugging an issue,
    // we prefix log lines with "[*]" when there are any suppressed source locations.
    if (m_limiter && m_limiter->SuppressionsActive()) {
        str_prefixed.insert(0, "[*] ");
    }

    if (m_print_to_console) {
        // print to console
        fwrite(str_prefixed.data(), 1, str_prefixed.size(), stdout);
        fflush(stdout);
    }
    for (const auto& cb : m_print_callbacks) {
        cb(str_prefixed);
    }
    if (m_print_to_file && !ratelimit) {
        assert(m_fileout != nullptr);

        // reopen the log file, if requested
        if (m_reopen_file) {
            m_reopen_file = false;
            FILE* new_fileout = fsbridge::fopen(m_file_path, "a");
            if (new_fileout) {
                setbuf(new_fileout, nullptr); // unbuffered