Skip to content

[k2] refactor logs#1548

Open
Shamzik wants to merge 6 commits intomasterfrom
kshamazov/logs_refactoring
Open

[k2] refactor logs#1548
Shamzik wants to merge 6 commits intomasterfrom
kshamazov/logs_refactoring

Conversation

@Shamzik
Copy link
Contributor

@Shamzik Shamzik commented Mar 10, 2026

This PR rewrites the logging functions into a single function. This eliminates the need for duplicate logic in different places, making it easier to add new logging changes.

@Shamzik Shamzik added this to the next milestone Mar 10, 2026
@Shamzik Shamzik requested a review from astrophysik March 10, 2026 11:36
@Shamzik Shamzik self-assigned this Mar 10, 2026
@Shamzik Shamzik added refactoring Logic and code style improvements k2 k2 related labels Mar 10, 2026
return extra_tags.end();
}

void add_extra_tag(std::string_view key, std::string_view value) noexcept;
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like this methods aren't implemented

std::array<char, BACKTRACE_BUFFER_SIZE> backtrace_buffer; // NOLINT
size_t backtrace_size{impl::resolve_log_trace(backtrace_buffer, *trace)};
tagged_entries.push_back(
k2::LogTaggedEntry{.key = BACKTRACE_KEY.data(), .value = backtrace_buffer.data(), .key_len = BACKTRACE_KEY.size(), .value_len = backtrace_size});
Copy link
Contributor

Choose a reason for hiding this comment

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

Here you get pointer to backtrace_buffer witch lifetime expires after if scope ends

}
};

static constexpr std::string_view BACKTRACE_KEY = "trace";
Copy link
Contributor

Choose a reason for hiding this comment

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

It's unused

return extra_tags.size();
}

auto begin() const noexcept {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe to it like? Instead of pair begin/end

auto values() const noexcept {
  return std::views::all()
}

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

Labels

k2 k2 related refactoring Logic and code style improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants