Skip to content

feat: Add KQL query support for filtering structured IR log events. #104

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Aug 22, 2025
Merged

feat: Add KQL query support for filtering structured IR log events. #104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
dedb758
feat!: Add KQL filtering support.
hoophalab Aug 11, 2025
002e055
fix comments
hoophalab Aug 20, 2025
927dabd
fix coderabbit comments
hoophalab Aug 20, 2025
56903d6
fix coderabbit comments
hoophalab Aug 20, 2025
d2eef8d
address comments
hoophalab Aug 21, 2025
587c25f
rename indices
hoophalab Aug 21, 2025
994a77e
address comments
hoophalab Aug 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 40 additions & 2 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ execute_process(

set(CLP_FFI_JS_SRC_MAIN
src/clp_ffi_js/ir/decoding_methods.cpp
src/clp_ffi_js/ir/query_methods.cpp
src/clp_ffi_js/ir/StreamReader.cpp
src/clp_ffi_js/ir/StructuredIrStreamReader.cpp
src/clp_ffi_js/ir/StructuredIrUnitHandler.cpp
Expand All @@ -113,6 +114,8 @@ set(CLP_FFI_JS_SRC_CLP_CORE
${CLP_FFI_JS_CLP_SOURCE_DIRECTORY}/components/core/src/clp/ffi/ir_stream/decoding_methods.cpp
${CLP_FFI_JS_CLP_SOURCE_DIRECTORY}/components/core/src/clp/ffi/ir_stream/ir_unit_deserialization_methods.cpp
${CLP_FFI_JS_CLP_SOURCE_DIRECTORY}/components/core/src/clp/ffi/ir_stream/search/ErrorCode.cpp
${CLP_FFI_JS_CLP_SOURCE_DIRECTORY}/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp
${CLP_FFI_JS_CLP_SOURCE_DIRECTORY}/components/core/src/clp/ffi/ir_stream/search/utils.cpp
${CLP_FFI_JS_CLP_SOURCE_DIRECTORY}/components/core/src/clp/ffi/ir_stream/utils.cpp
${CLP_FFI_JS_CLP_SOURCE_DIRECTORY}/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp
${CLP_FFI_JS_CLP_SOURCE_DIRECTORY}/components/core/src/clp/ffi/SchemaTree.cpp
Expand Down Expand Up @@ -144,6 +147,36 @@ set(CLP_FFI_JS_SUPPORTED_ENVIRONMENTS
"List of supported environments."
)

set(CLP_BUILD_CLP_STRING_UTILS ON)
add_subdirectory(${CLP_FFI_JS_CLP_SOURCE_DIRECTORY}/components/core/src/clp/string_utils)

# The `clp_s` component is linked because the IR search feature uses the same KQL syntax as `clp_s`,
# by reusing `clp_s`'s AST and KQL libraries.
#
# However, 'clp-ffi-js' doesn't currently require other 'clp_s' build targets (e.g., executables,
# archive readers, etc.). Disabling them prevents an increase in the binary size and reduces the
# number of dependencies needed during the build process.
set(CLP_BUILD_EXECUTABLES OFF)
set(CLP_BUILD_CLP_S_ARCHIVEREADER OFF)
set(CLP_BUILD_CLP_S_ARCHIVEWRITER OFF)
set(CLP_BUILD_CLP_S_CLP_DEPENDENCIES OFF)
set(CLP_BUILD_CLP_S_IO OFF)
set(CLP_BUILD_CLP_S_JSONCONSTRUCTOR OFF)
set(CLP_BUILD_CLP_S_REDUCER_DEPENDENCIES OFF)
set(CLP_BUILD_CLP_S_SEARCH OFF)
set(CLP_BUILD_CLP_S_SEARCH_AST ON)
set(CLP_BUILD_CLP_S_SEARCH_KQL ON)
set(CLP_BUILD_CLP_S_TIMESTAMPPATTERN ON)
add_subdirectory(${CLP_FFI_JS_CLP_SOURCE_DIRECTORY}/components/core/src/clp_s)

set(ANTLR_BUILD_SHARED OFF)
set(ANTLR_BUILD_STATIC ON)
add_subdirectory(${CLP_FFI_JS_ANTLR_RUNTIME_SOURCE_DIRECTORY})

add_subdirectory(${CLP_FFI_JS_DATE_SOURCE_DIRECTORY})
add_subdirectory(${CLP_FFI_JS_SIMDJSON_SOURCE_DIRECTORY})
add_subdirectory(${CLP_FFI_JS_SPDLOG_SOURCE_DIRECTORY})

foreach(env ${CLP_FFI_JS_SUPPORTED_ENVIRONMENTS})
set(CLP_FFI_JS_BIN_NAME "ClpFfiJs-${env}")
add_executable(${CLP_FFI_JS_BIN_NAME})
Expand All @@ -154,7 +187,13 @@ foreach(env ${CLP_FFI_JS_SUPPORTED_ENVIRONMENTS})
target_compile_options(${CLP_FFI_JS_BIN_NAME} PRIVATE ${CLP_FFI_JS_COMMON_COMPILE_OPTIONS})

# Set up link options
target_link_libraries(${CLP_FFI_JS_BIN_NAME} PRIVATE embind)
target_link_libraries(${CLP_FFI_JS_BIN_NAME}
PRIVATE
clp_s::search::ast
clp_s::search::kql
embind
spdlog::spdlog
)
set(CLP_FFI_JS_LINK_OPTIONS
${CLP_FFI_JS_COMMON_LINK_OPTIONS}
--emit-tsd=${CLP_FFI_JS_BIN_NAME}.d.ts
Expand Down Expand Up @@ -184,7 +223,6 @@ Link options: ${CLP_FFI_JS_LINK_OPTIONS}."
${CLP_FFI_JS_CLP_SOURCE_DIRECTORY}/components/core/src/clp
${CLP_FFI_JS_FMT_SOURCE_DIRECTORY}/include
${CLP_FFI_JS_NLOHMANN_JSON_SOURCE_DIRECTORY}/include
${CLP_FFI_JS_SPDLOG_SOURCE_DIRECTORY}/include
${CLP_FFI_JS_YSTDLIB_SOURCE_DIRECTORY}/src
${CLP_FFI_JS_ZSTD_SOURCE_DIRECTORY}/lib
)
Expand Down
14 changes: 13 additions & 1 deletion src/clp_ffi_js/ir/StreamReader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,19 @@ EMSCRIPTEN_BINDINGS(ClpStreamReader) {
"getFilteredLogEventMap",
&clp_ffi_js::ir::StreamReader::get_filtered_log_event_map
)
.function("filterLogEvents", &clp_ffi_js::ir::StreamReader::filter_log_events)
.function(
"filterLogEvents",
emscripten::select_overload<void(clp_ffi_js::ir::LogLevelFilterTsType const&)>(
&clp_ffi_js::ir::StreamReader::filter_log_events
)
)
.function(
"filterLogEvents",
emscripten::select_overload<
void(clp_ffi_js::ir::LogLevelFilterTsType const&, std::string const&)>(
&clp_ffi_js::ir::StreamReader::filter_log_events
)
)
.function("deserializeStream", &clp_ffi_js::ir::StreamReader::deserialize_stream)
.function("decodeRange", &clp_ffi_js::ir::StreamReader::decode_range)
.function(
Expand Down
20 changes: 18 additions & 2 deletions src/clp_ffi_js/ir/StreamReader.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,15 +96,31 @@ class StreamReader {

/**
* @return The filtered log events map.
* This is a sorted list of log event indices that match the filter.
* If all log events match the filter, it returns `null` or a vector of all log events.
*/
Comment on lines +99 to 101
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Clarify “null vs full-vector” invariant for callers

The API can return either null or a full vector when everything matches. Make the contract explicit so consumers don’t branch on both if unnecessary.

  /**
   * @return The filtered log events map.
- * This is a sorted list of log event indices that match the filter.
- * If all log events match the filter, it returns `null` or a vector of all log events.
+ * This is a sorted list of log event indices that match the filter.
+ * If all log events match the filter, this returns `null` (preferred) to signal
+ * “no-op filtering”. Some internal paths may return a full vector; callers should
+ * treat `null` and “full coverage” equivalently.
  */
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
* This is a sorted list of log event indices that match the filter.
* If all log events match the filter, it returns `null` or a vector of all log events.
*/
/**
* @return The filtered log events map.
* This is a sorted list of log event indices that match the filter.
* If all log events match the filter, this returns `null` (preferred) to signal
* “no-op filtering”. Some internal paths may return a full vector; callers should
* treat `null` and “full coverage” equivalently.
*/
🤖 Prompt for AI Agents 
In src/clp_ffi_js/ir/StreamReader.hpp around lines 99–101, the comment is
ambiguous about whether the function returns null or a full vector when every
log event matches; make the contract explicit by deciding on one behavior
(recommended: return nullptr to signal "all match" to avoid allocating a full
vector), update the doc comment to state that explicitly, and update the
function signature/type if needed (e.g., use a pointer/optional to express
nullable result) and adjust callers to treat nullptr as "all events match"
instead of also handling a full-vector case.

[[nodiscard]] virtual auto get_filtered_log_event_map() const -> FilteredLogEventMapTsType = 0;

/**
* Generates a filtered collection from all log events.
*
* @param log_level_filter Array of selected log levels
* @param log_level_filter Array of selected log levels.
* @param kql_filter: A KQL expression used to filter kv-pairs.
* - For structured IR: the filter is applied when non-empty.
* - For unstructured IR: the filter is always ignored, and a warning is logged when non-empty.
*/
virtual void filter_log_events(LogLevelFilterTsType const& log_level_filter) = 0;
virtual void
filter_log_events(LogLevelFilterTsType const& log_level_filter, std::string const& kql_filter)
= 0;

/**
* Generates a filtered collection from all log events.
*
* @param log_level_filter Array of selected log levels.
*/
void filter_log_events(LogLevelFilterTsType const& log_level_filter) {
filter_log_events(log_level_filter, "");
}
Comment on lines +116 to +123
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Minor: document the single-arg wrapper’s behaviour

Clarify that this overload disables KQL by design.

-/**
- * Generates a filtered collection from all log events.
- *
- * @param log_level_filter Array of selected log levels.
- */
+/**
+ * Convenience wrapper: filters only by `log_level_filter` (KQL disabled).
+ *
+ * @param log_level_filter Array of selected log levels.
+ */
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* Generates a filtered collection from all log events.
*
* @param log_level_filter Array of selected log levels.
*/
void filter_log_events(LogLevelFilterTsType const& log_level_filter) {
filter_log_events(log_level_filter, "");
}
/**
* Convenience wrapper: filters only by `log_level_filter` (KQL disabled).
*
* @param log_level_filter Array of selected log levels.
*/
void filter_log_events(LogLevelFilterTsType const& log_level_filter) {
filter_log_events(log_level_filter, "");
}
🤖 Prompt for AI Agents 
In src/clp_ffi_js/ir/StreamReader.hpp around lines 116 to 123, the doc comment
for the single-argument filter_log_events overload should explicitly state that
this wrapper disables KQL by design; update the comment to mention that calling
filter_log_events(LogLevelFilterTsType const& log_level_filter) invokes the
two-argument overload with an empty KQL string and therefore performs only
log-level filtering (KQL is not applied), so readers understand the intended
behavior.


/**
* Deserializes all log events in the stream.
Expand Down
89 changes: 60 additions & 29 deletions src/clp_ffi_js/ir/StructuredIrStreamReader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,28 +1,33 @@
#include "StructuredIrStreamReader.hpp"

#include <algorithm>
#include <cstddef>
#include <format>
#include <memory>
#include <optional>
#include <string>
#include <string_view>
#include <system_error>
#include <type_traits>
#include <utility>
#include <vector>

#include <clp/ErrorCode.hpp>
#include <clp/ffi/ir_stream/Deserializer.hpp>
#include <clp/ffi/SchemaTree.hpp>
#include <clp/ir/types.hpp>
#include <clp/TraceableException.hpp>
#include <clp/type_utils.hpp>
#include <emscripten/bind.h>
#include <emscripten/val.h>
#include <nlohmann/json.hpp>
#include <spdlog/spdlog.h>
#include <ystdlib/containers/Array.hpp>

#include <clp_ffi_js/ClpFfiJsException.hpp>
#include <clp_ffi_js/constants.hpp>
#include <clp_ffi_js/ir/decoding_methods.hpp>
#include <clp_ffi_js/ir/LogEventWithFilterData.hpp>
#include <clp_ffi_js/ir/query_methods.hpp>
#include <clp_ffi_js/ir/StreamReader.hpp>
#include <clp_ffi_js/ir/StreamReaderDataContext.hpp>
#include <clp_ffi_js/ir/StructuredIrUnitHandler.hpp>
Expand Down Expand Up @@ -139,12 +144,57 @@ auto StructuredIrStreamReader::get_filtered_log_event_map() const -> FilteredLog
return FilteredLogEventMapTsType{emscripten::val::array(m_filtered_log_event_map.value())};
}

void StructuredIrStreamReader::filter_log_events(LogLevelFilterTsType const& log_level_filter) {
generic_filter_log_events(
m_filtered_log_event_map,
log_level_filter,
*m_deserialized_log_events
);
void StructuredIrStreamReader::filter_log_events(
LogLevelFilterTsType const& log_level_filter,
std::string const& kql_filter
) {
Comment on lines +147 to +150
Copy link
Member

Choose a reason for hiding this comment

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

I'm ok with the current implementation. There can be a more efficient way: we can embed this log-level filtering using a lambda into the query method. But that might be over-engineering at this point.

Copy link
Contributor Author

@hoophalab hoophalab Aug 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

we are probably going to extend the kql ast by ourself
(user_kql) AND (loglevelkey: ERROR OR loglevelkey: WARNING OR ...)
This is tracked in #105 #106
*the log level code is actually copied from generic_filter_log_events

m_filtered_log_event_map.reset();

if (false == kql_filter.empty()) {
auto& reader{m_stream_reader_data_context->get_reader()};
reader.seek_from_begin(0);
m_filtered_log_event_map.emplace(collect_matched_log_event_indices(reader, kql_filter));
}

if (false == log_level_filter.isNull()) {
std::vector<size_t> filtered_log_event_map;

auto const filter_levels{
emscripten::vecFromJSArray<std::underlying_type_t<LogLevel>>(log_level_filter)
};

auto filter_and_collect_idx = [&](size_t const log_event_idx) {
auto const& log_event{m_deserialized_log_events->at(log_event_idx)};
if (std::ranges::find(
filter_levels,
clp::enum_to_underlying_type(log_event.get_log_level())
)
!= filter_levels.end())
{
filtered_log_event_map.emplace_back(log_event_idx);
}
};

if (m_filtered_log_event_map.has_value()) {
for (auto const log_event_idx : m_filtered_log_event_map.value()) {
filter_and_collect_idx(log_event_idx);
}
} else {
for (size_t log_event_idx{0}; log_event_idx < m_deserialized_log_events->size();
++log_event_idx)
{
filter_and_collect_idx(log_event_idx);
}
}

m_filtered_log_event_map = std::move(filtered_log_event_map);
}

if (m_filtered_log_event_map.has_value()
&& m_filtered_log_event_map->size() == m_deserialized_log_events->size())
{
m_filtered_log_event_map = std::nullopt;
}
Comment on lines +193 to +197
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Document “null = all events match” behaviour in the public API

This normalisation is fine; ensure the header/TS typings and README explicitly state that get_filtered_log_event_map returns null when the filtered set equals the full set.

I can update the doxygen in StructuredIrStreamReader.hpp and the TS wrapper comments to make this explicit; let me know if you want a follow-up PR.

🤖 Prompt for AI Agents 
In src/clp_ffi_js/ir/StructuredIrStreamReader.cpp around lines 195-199, the code
normalizes m_filtered_log_event_map to null when the filtered set equals the
full set; update the public API docs to document this behavior: add a doxygen
note to StructuredIrStreamReader.hpp on get_filtered_log_event_map stating it
returns null/optional empty when filtered == full (i.e., "null = all events
match"), update the TypeScript wrapper typings and JSDoc/comments to match that
contract, and add a short note to the README (or relevant API docs) describing
the same semantics so callers know to treat null as "no filtering applied."

}

auto StructuredIrStreamReader::deserialize_stream() -> size_t {
Expand All @@ -157,28 +207,7 @@ auto StructuredIrStreamReader::deserialize_stream() -> size_t {
auto& reader{m_stream_reader_data_context->get_reader()};
auto& deserializer = m_stream_reader_data_context->get_deserializer();

while (false == deserializer.is_stream_completed()) {
auto result{deserializer.deserialize_next_ir_unit(reader)};
if (false == result.has_error()) {
continue;
}
auto const error{result.error()};
if (std::errc::result_out_of_range == error) {
SPDLOG_ERROR("File contains an incomplete IR stream");
break;
}
throw ClpFfiJsException{
clp::ErrorCode::ErrorCode_Corrupt,
__FILENAME__,
__LINE__,
std::format(
"Failed to deserialize IR unit: {}:{}",
error.category().name(),
error.message()
)
};
}
m_stream_reader_data_context.reset(nullptr);
deserialize_log_events(deserializer, reader);
return m_deserialized_log_events->size();
}

Expand All @@ -188,11 +217,13 @@ auto StructuredIrStreamReader::decode_range(size_t begin_idx, size_t end_idx, bo
auto json_pair_result{log_event.serialize_to_json()};
if (json_pair_result.has_error()) {
auto const error_code{json_pair_result.error()};
// NOLINTBEGIN(bugprone-lambda-function-name)
SPDLOG_ERROR(
"Failed to deserialize log event to JSON: {}:{}",
error_code.category().name(),
error_code.message()
);
// NOLINTEND(bugprone-lambda-function-name)
return std::string{cEmptyJsonStr};
Comment on lines +220 to 227
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Localise the lint suppression

Use NOLINTNEXTLINE to target the SPDLOG call, keeping the suppression as narrow as possible.

-            // NOLINTBEGIN(bugprone-lambda-function-name)
-            SPDLOG_ERROR(
+            // NOLINTNEXTLINE(bugprone-lambda-function-name)
+            SPDLOG_ERROR(
                     "Failed to deserialize log event to JSON: {}:{}",
                     error_code.category().name(),
                     error_code.message()
             );
-            // NOLINTEND(bugprone-lambda-function-name)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// NOLINTBEGIN(bugprone-lambda-function-name)
SPDLOG_ERROR(
"Failed to deserialize log event to JSON: {}:{}",
error_code.category().name(),
error_code.message()
);
// NOLINTEND(bugprone-lambda-function-name)
return std::string{cEmptyJsonStr};
// NOLINTNEXTLINE(bugprone-lambda-function-name)
SPDLOG_ERROR(
"Failed to deserialize log event to JSON: {}:{}",
error_code.category().name(),
error_code.message()
);
return std::string{cEmptyJsonStr};
🤖 Prompt for AI Agents 
In src/clp_ffi_js/ir/StructuredIrStreamReader.cpp around lines 242 to 249, the
current NOLINTBEGIN/END block is too broad; replace it by removing the
NOLINTBEGIN(bugprone-lambda-function-name) and
NOLINTEND(bugprone-lambda-function-name) and place a single
NOLINTNEXTLINE(bugprone-lambda-function-name) immediately above the SPDLOG_ERROR
call so the lint suppression only applies to that call; keep the SPDLOG_ERROR
invocation and the subsequent return unchanged.

}

Expand Down
5 changes: 4 additions & 1 deletion src/clp_ffi_js/ir/StructuredIrStreamReader.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,10 @@ class StructuredIrStreamReader : public StreamReader {

[[nodiscard]] auto get_filtered_log_event_map() const -> FilteredLogEventMapTsType override;

void filter_log_events(LogLevelFilterTsType const& log_level_filter) override;
void filter_log_events(
LogLevelFilterTsType const& log_level_filter,
std::string const& kql_filter
) override;

/**
* @see StreamReader::deserialize_stream
Expand Down
6 changes: 4 additions & 2 deletions src/clp_ffi_js/ir/StructuredIrUnitHandler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,10 @@ auto StructuredIrUnitHandler::SchemaTreeFullBranch::match(
return true;
}

auto StructuredIrUnitHandler::handle_log_event(StructuredLogEvent&& log_event)
-> clp::ffi::ir_stream::IRErrorCode {
auto StructuredIrUnitHandler::handle_log_event(
StructuredLogEvent&& log_event,
[[maybe_unused]] size_t log_event_idx
) -> clp::ffi::ir_stream::IRErrorCode {
auto const timestamp = get_timestamp(log_event);
auto const log_level = get_log_level(log_event);
auto const utc_offset = get_utc_offset(log_event);
Expand Down
3 changes: 2 additions & 1 deletion src/clp_ffi_js/ir/StructuredIrUnitHandler.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ class StructuredIrUnitHandler {
* @param log_event
* @return IRErrorCode::IRErrorCode_Success
*/
[[nodiscard]] auto handle_log_event(StructuredLogEvent&& log_event)
[[nodiscard]] auto
handle_log_event(StructuredLogEvent&& log_event, [[maybe_unused]] size_t log_event_idx)
-> clp::ffi::ir_stream::IRErrorCode;

/**
Expand Down
11 changes: 10 additions & 1 deletion src/clp_ffi_js/ir/UnstructuredIrStreamReader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,16 @@ auto UnstructuredIrStreamReader::get_filtered_log_event_map() const -> FilteredL
return FilteredLogEventMapTsType{emscripten::val::array(m_filtered_log_event_map.value())};
}

void UnstructuredIrStreamReader::filter_log_events(LogLevelFilterTsType const& log_level_filter) {
void UnstructuredIrStreamReader::filter_log_events(
LogLevelFilterTsType const& log_level_filter,
[[maybe_unused]] std::string const& kql_filter
) {
if (false == kql_filter.empty()) {
SPDLOG_WARN(
"KQL filters aren't supported for unstructured IR streams, so they're being "
"ignored."
);
Comment on lines +83 to +91
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Remove [[maybe_unused]] and keep warning conditional

You’re using kql_filter in the condition; the [[maybe_unused]] attribute is now incorrect. The conditional warning is good and matches team guidance.

 void UnstructuredIrStreamReader::filter_log_events(
         LogLevelFilterTsType const& log_level_filter,
-        [[maybe_unused]] std::string const& kql_filter
+        std::string const& kql_filter
 ) {
     if (false == kql_filter.empty()) {
         SPDLOG_WARN(
                 "KQL filters aren't supported for unstructured IR streams, so they're being "
                 "ignored."
         );
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
void UnstructuredIrStreamReader::filter_log_events(
LogLevelFilterTsType const& log_level_filter,
[[maybe_unused]] std::string const& kql_filter
) {
if (false == kql_filter.empty()) {
SPDLOG_WARN(
"KQL filters aren't supported for unstructured IR streams, so they're being "
"ignored."
);
void UnstructuredIrStreamReader::filter_log_events(
LogLevelFilterTsType const& log_level_filter,
std::string const& kql_filter
) {
if (false == kql_filter.empty()) {
SPDLOG_WARN(
"KQL filters aren't supported for unstructured IR streams, so they're being "
"ignored."
);
}
generic_filter_log_events(m_filtered_log_event_map, log_level_filter, m_encoded_log_events);
}
🤖 Prompt for AI Agents 
In src/clp_ffi_js/ir/UnstructuredIrStreamReader.cpp around lines 83 to 91, the
parameter kql_filter is referenced in the function body so the [[maybe_unused]]
attribute is incorrect; remove the [[maybe_unused]] attribute from the function
signature (change to std::string const& kql_filter) so the declaration matches
usage, leave the existing if (false == kql_filter.empty()) conditional and
SPDLOG_WARN call as-is, and rebuild to ensure no unused-parameter diagnostics
remain.

}
generic_filter_log_events(m_filtered_log_event_map, log_level_filter, m_encoded_log_events);
}

Expand Down
5 changes: 4 additions & 1 deletion src/clp_ffi_js/ir/UnstructuredIrStreamReader.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,10 @@ class UnstructuredIrStreamReader : public StreamReader {

[[nodiscard]] auto get_filtered_log_event_map() const -> FilteredLogEventMapTsType override;

void filter_log_events(LogLevelFilterTsType const& log_level_filter) override;
void filter_log_events(
LogLevelFilterTsType const& log_level_filter,
[[maybe_unused]] std::string const& kql_filter
) override;

/**
* @see StreamReader::deserialize_stream
Expand Down
35 changes: 35 additions & 0 deletions src/clp_ffi_js/ir/decoding_methods.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
#ifndef CLP_FFI_JS_IR_DECODING_METHODS_HPP
#define CLP_FFI_JS_IR_DECODING_METHODS_HPP

#include <clp/ffi/ir_stream/Deserializer.hpp>
#include <clp/ffi/ir_stream/IrUnitHandlerReq.hpp>
#include <clp/ffi/ir_stream/search/QueryHandlerReq.hpp>
#include <clp/ReaderInterface.hpp>
#include <nlohmann/json.hpp>

#include <clp_ffi_js/ClpFfiJsException.hpp>
#include <clp_ffi_js/ir/StreamReader.hpp>
#include <clp_ffi_js/ir/StructuredIrStreamReader.hpp>
Comment on lines 11 to +12
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Decouple: remove unnecessary include of StructuredIrStreamReader.hpp

This header doesn’t reference StructuredIrStreamReader; drop the include to reduce compile-time and coupling.

-#include <clp_ffi_js/ir/StreamReader.hpp>
-#include <clp_ffi_js/ir/StructuredIrStreamReader.hpp>
+#include <clp_ffi_js/ir/StreamReader.hpp>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#include <clp_ffi_js/ir/StreamReader.hpp>
#include <clp_ffi_js/ir/StructuredIrStreamReader.hpp>
#include <clp_ffi_js/ir/StreamReader.hpp>
🤖 Prompt for AI Agents 
In src/clp_ffi_js/ir/decoding_methods.hpp around lines 11 to 12, the file
unnecessarily includes StructuredIrStreamReader.hpp even though it does not
reference that type; remove the line including
<clp_ffi_js/ir/StructuredIrStreamReader.hpp> to reduce coupling and compile
time, leaving only the required #include <clp_ffi_js/ir/StreamReader.hpp>, and
run a quick rebuild to confirm no missing symbols.


Comment on lines +4 to 13
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add required headers for std::format and logging; avoid relying on transitive includes

This TU uses std::format and SPDLOG_WARN; include their headers explicitly. Also include <system_error> for std::errc.

 #include <clp/ffi/ir_stream/Deserializer.hpp>
 #include <clp/ffi/ir_stream/IrUnitHandlerReq.hpp>
 #include <clp/ffi/ir_stream/search/QueryHandlerReq.hpp>
 #include <clp/ReaderInterface.hpp>
 
 #include <nlohmann/json.hpp>
 
 #include <clp_ffi_js/ClpFfiJsException.hpp>
 #include <clp_ffi_js/ir/StreamReader.hpp>
 #include <clp_ffi_js/ir/StructuredIrStreamReader.hpp>
+#include <format>
+#include <spdlog/spdlog.h>
+#include <system_error>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#include <clp/ffi/ir_stream/Deserializer.hpp>
#include <clp/ffi/ir_stream/IrUnitHandlerReq.hpp>
#include <clp/ffi/ir_stream/search/QueryHandlerReq.hpp>
#include <clp/ReaderInterface.hpp>
#include <nlohmann/json.hpp>
#include <clp_ffi_js/ClpFfiJsException.hpp>
#include <clp_ffi_js/ir/StreamReader.hpp>
#include <clp_ffi_js/ir/StructuredIrStreamReader.hpp>
#include <clp/ffi/ir_stream/Deserializer.hpp>
#include <clp/ffi/ir_stream/IrUnitHandlerReq.hpp>
#include <clp/ffi/ir_stream/search/QueryHandlerReq.hpp>
#include <clp/ReaderInterface.hpp>
#include <nlohmann/json.hpp>
#include <clp_ffi_js/ClpFfiJsException.hpp>
#include <clp_ffi_js/ir/StreamReader.hpp>
#include <clp_ffi_js/ir/StructuredIrStreamReader.hpp>
#include <format>
#include <spdlog/spdlog.h>
#include <system_error>
🤖 Prompt for AI Agents 
In src/clp_ffi_js/ir/decoding_methods.hpp around lines 4 to 13, this translation
unit uses std::format, SPDLOG_WARN and std::errc but relies on transitive
includes; add explicit includes for <format>, <spdlog/spdlog.h> and
<system_error> near the other headers to ensure std::format, spdlog macros and
std::errc are available and avoid depending on transitive headers.

namespace clp_ffi_js::ir {
/**
Expand Down Expand Up @@ -32,6 +37,36 @@ auto rewind_reader_and_validate_encoding_type(clp::ReaderInterface& reader) -> v
* @return The converted JavaScript object.
*/
[[nodiscard]] auto convert_metadata_to_js_object(nlohmann::json const& metadata) -> MetadataTsType;

template <
clp::ffi::ir_stream::IrUnitHandlerReq IrUnitHandlerType,
clp::ffi::ir_stream::search::QueryHandlerReq QueryHandlerType>
auto deserialize_log_events(
clp::ffi::ir_stream::Deserializer<IrUnitHandlerType, QueryHandlerType>& deserializer,
clp::ReaderInterface& reader
) -> void {
while (false == deserializer.is_stream_completed()) {
auto const result{deserializer.deserialize_next_ir_unit(reader)};
if (false == result.has_error()) {
continue;
}
auto const error{result.error()};
if (std::errc::result_out_of_range == error) {
SPDLOG_WARN("File contains an incomplete IR stream");
break;
}
throw ClpFfiJsException{
clp::ErrorCode::ErrorCode_Corrupt,
__FILENAME__,
__LINE__,
std::format(
"Failed to deserialize IR unit: {}:{}",
error.category().name(),
error.message()
)
};
}
}
} // namespace clp_ffi_js::ir

#endif // CLP_FFI_JS_IR_DECODING_METHODS_HPP
Loading
Loading