-
Notifications
You must be signed in to change notification settings - Fork 0
Logs counter #2
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
base: develop
Are you sure you want to change the base?
Logs counter #2
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,29 @@ | ||||||||||||||||||||||||||||
| from datetime import datetime, timedelta | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| from multiversx_logs_parser_tools.archive_handler import ArchiveHandler | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| from .counter_checker import CounterChecker, CounterParser | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| class CounterArchiveHandler(ArchiveHandler): | ||||||||||||||||||||||||||||
| def __init__(self, checker: CounterChecker, logs_path: str): | ||||||||||||||||||||||||||||
| self.checker = checker | ||||||||||||||||||||||||||||
| # self.shard_data = ShardData() | ||||||||||||||||||||||||||||
| super().__init__(checker, logs_path) | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| def process_node_data(self): | ||||||||||||||||||||||||||||
| """Process the parsed data for a single node.""" | ||||||||||||||||||||||||||||
| # node_data = HeaderData() | ||||||||||||||||||||||||||||
| # node_data.header_dictionary = self.checker.parsed | ||||||||||||||||||||||||||||
| # self.shard_data.add_node(node_data) | ||||||||||||||||||||||||||||
|
Comment on lines
+11
to
+18
|
||||||||||||||||||||||||||||
| # self.shard_data = ShardData() | |
| super().__init__(checker, logs_path) | |
| def process_node_data(self): | |
| """Process the parsed data for a single node.""" | |
| # node_data = HeaderData() | |
| # node_data.header_dictionary = self.checker.parsed | |
| # self.shard_data.add_node(node_data) | |
| super().__init__(checker, logs_path) | |
| def process_node_data(self): | |
| """Process the parsed data for a single node.""" | |
| # TODO: implement node data processing logic if needed. |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,38 @@ | ||||||||||||||||||||
| from argparse import Namespace | ||||||||||||||||||||
| from typing import Any | ||||||||||||||||||||
|
|
||||||||||||||||||||
| from multiversx_logs_parser_tools.node_logs_checker import NodeLogsChecker | ||||||||||||||||||||
|
|
||||||||||||||||||||
| from .counter_parser import CounterData, CounterParser | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| class CounterChecker(NodeLogsChecker): | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
Comment on lines
+9
to
+10
|
||||||||||||||||||||
| class CounterChecker(NodeLogsChecker): | |
| class CounterChecker(NodeLogsChecker): | |
| """ | |
| CounterChecker coordinates log parsing and counting for a single node. | |
| It uses a CounterParser to extract and count log messages, aggregates the | |
| results into CounterData, and produces per-node JSON reports as part of | |
| the overall log counting workflow. | |
| """ |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The initialization order may be problematic. The initialize_checker method creates a CounterData instance for self.parsed, then calls super().initialize_checker(args), which may overwrite or interfere with the initialization. Consider calling super() first, then initializing self.parsed.
| self.parsed = CounterData() | |
| return super().initialize_checker(args) | |
| result = super().initialize_checker(args) | |
| self.parsed = CounterData() | |
| return result |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,39 @@ | ||||||||||||||||||||||||||||||||||
| from re import Pattern | ||||||||||||||||||||||||||||||||||
| from typing import Any | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| from multiversx_logs_parser_tools.aho_corasik_parser import AhoCorasickParser | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| from .counter_structures import CounterData | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| class CounterParser(AhoCorasickParser): | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
| class CounterParser(AhoCorasickParser): | |
| class CounterParser(AhoCorasickParser): | |
| """ | |
| Parser specialized for counting log messages by severity level. | |
| This class configures the underlying :class:`AhoCorasickParser` with a | |
| fixed set of log-level patterns (INFO, TRACE, DEBUG, WARN, ERROR) and | |
| accumulates the parsed messages in a :class:`CounterData` instance. | |
| It overrides the base parser's hook methods to: | |
| * declare the patterns of interest via :meth:`get_patterns`, | |
| * reset the internal counters in :meth:`initialize_checker`, | |
| * delegate low-level matching to the base implementation in | |
| :meth:`process_match`, and | |
| * update the message counters in :meth:`process_parsed_entry`. | |
| """ |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type annotation mismatch in return type. The method returns a list of tuples containing strings and integers, but the type hint indicates Pattern objects. The return type should be list[tuple[str, int]] instead of list[tuple[Pattern[str], int]].
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The process_match method calls super().process_match() but doesn't utilize the returned value meaningfully - it just returns it directly without any additional processing. Consider whether this override is necessary or if the implementation adds value.
| def process_match(self, line: str, end_index: int, pattern_idx: int, args: dict[str, str]) -> dict[str, Any]: | |
| parsed = super().process_match(line, end_index, pattern_idx, args) | |
| return parsed |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The should_parse_line method always returns True, making the pattern parameter unused. Consider removing the parameter if it's not needed, or implement conditional logic if different patterns should be handled differently.
| return True | |
| """ | |
| Decide whether a line associated with the given pattern should be parsed. | |
| This implementation returns True for the log level patterns that this parser | |
| is configured to handle. For the patterns currently returned by get_patterns(), | |
| this method behaves the same as the previous unconditional True. | |
| """ | |
| allowed_levels = {"INFO", "TRACE", "DEBUG", "WARN", "ERROR"} | |
| # Pattern may be a compiled regex or another string-like object. | |
| pattern_str = getattr(pattern, "pattern", str(pattern)) | |
| return any(level in pattern_str for level in allowed_levels) |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,32 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||
| from enum import Enum | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| LogLevel = Enum("LogLevel", [ | ||||||||||||||||||||||||||||||||||||||||||||||||
| 'INFO', | ||||||||||||||||||||||||||||||||||||||||||||||||
| 'TRACE', | ||||||||||||||||||||||||||||||||||||||||||||||||
| 'DEBUG', | ||||||||||||||||||||||||||||||||||||||||||||||||
| 'WARN', | ||||||||||||||||||||||||||||||||||||||||||||||||
| 'ERROR' | ||||||||||||||||||||||||||||||||||||||||||||||||
| ]) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| class CounterData: | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+12
to
+13
|
||||||||||||||||||||||||||||||||||||||||||||||||
| class CounterData: | |
| class CounterData: | |
| """ | |
| Container for counting log messages per log level. | |
| This class maintains a nested dictionary structure where the first key is | |
| the log level name (as defined in ``LogLevel``) and the second key is the | |
| log message string. The corresponding value is the number of times that | |
| message has been observed at that log level. | |
| Attributes | |
| ---------- | |
| counter_dictionary : dict[str, dict[str, int]] | |
| Maps a log level name to a dictionary of message strings and their | |
| associated occurrence counts. | |
| Usage | |
| ----- | |
| - Use :meth:`add_message` to increment the count for a single message. | |
| - Use :meth:`add_counted_messages` to merge counts from another | |
| :class:`CounterData` instance. | |
| - Use :meth:`reset` to clear all stored counts. | |
| """ |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing input validation for log_level parameter. The method should validate that the log_level exists in the counter_dictionary to prevent KeyError when an unexpected log level is passed.
| def add_message(self, log_level: str, message: str, count=1) -> None: | |
| def add_message(self, log_level: str, message: str, count=1) -> None: | |
| if log_level not in self.counter_dictionary: | |
| raise ValueError(f"Unknown log level: {log_level}") |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,3 +1,4 @@ | ||||||
|
|
||||||
|
||||||
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment says "common logic" but this could be more descriptive. Consider using a comment that better describes what this fallback logic does, such as "Parse using default separator-based or equals-sign-based logic".
| # --- common logic --- | |
| # Fallback: parse using default separator-based or equals-sign-based logic |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing newline between the two test log entries. Line 122 and 123 should be separated with a comma and newline for better readability and to match the pattern of the list.
| 'DEBUG[2025-11-12 12:01:22.747][..Start/shardchain][0/8/14409/(END_ROUND)] # EPOCH 9 BEGINS IN ROUND (14409) ##################################' | |
| 'DEBUG[2025-11-12 12:01:22.747][..Start/shardchain][0/8/14409/(END_ROUND)] # EPOCH 9 BEGINS IN ROUND (14409) ##################################', |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test or debug code should not be committed to the main codebase. This appears to be test code in the main block that should either be moved to proper unit tests or removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing docstring for the CounterArchiveHandler class. Public classes should have docstrings explaining their purpose and usage in the archive processing workflow.