add log filter #339
add log filter #339
Conversation
WalkthroughThe changes introduce a new configuration option for the Changes
Sequence DiagramsequenceDiagram
participant Client
participant LoggerProxy
participant Filter
participant Logger
Client->>LoggerProxy: start_link(config)
LoggerProxy->>Filter: Configure filter function
Client->>LoggerProxy: Log message
LoggerProxy->>Filter: Evaluate message
alt Message passes filter
Filter-->>LoggerProxy: true
LoggerProxy->>Logger: Process log
else Message filtered
Filter-->>LoggerProxy: false
LoggerProxy->>Logger: Ignore log
end
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🔇 Additional comments (3)
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/logger_proxy.ex(1 hunks)test/logger_proxy_test.exs(1 hunks)
🧰 Additional context used
🪛 GitHub Check: Build and test (stable)
test/logger_proxy_test.exs
[warning] 105-105:
variable "ignore_can_consume_error" is unused (if the variable is not meant to be used, prefix it with an underscore)
[warning] 105-105:
variable "ignore_can_consume_error" is unused (if the variable is not meant to be used, prefix it with an underscore)
[warning] 105-105:
variable "ignore_can_consume_error" is unused (if the variable is not meant to be used, prefix it with an underscore)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Lint Rust (native/mediasoup_elixir/Cargo.toml)
🔇 Additional comments (2)
lib/logger_proxy.ex (2)
55-59: LGTM! Filter configuration is handled correctly.The implementation properly extracts the optional filter from config with a sensible nil default.
64-64: LGTM! State initialization is correct.The state is properly initialized with the provided configuration.
|
satoren
force-pushed
the
mshiraki/log_filter
branch
from
5a89ce8 to
f90732f
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/logger_proxy_test.exs (1)
102-132: Consider making the test more reliable.While the test effectively verifies complex filtering functionality, the use of
Process.sleep(10)could make the test flaky. Consider using a more reliable synchronization mechanism.Example approaches:
- Use
ExUnit.Callbacks.on_exit/2to ensure cleanup- Implement a wait_for_log helper that polls with timeout
- Use a message-based synchronization mechanism
# Example wait_for_log helper defp wait_for_log(pattern, timeout \\ 100) do case Process.get(:log_messages, []) do [] -> if timeout > 0 do Process.sleep(10) wait_for_log(pattern, timeout - 10) else false end messages -> Enum.any?(messages, &(Regex.match?(pattern, &1))) end end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/logger_proxy.ex(1 hunks)test/logger_proxy_test.exs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build and test (stable)
- GitHub Check: Lint Rust (native/mediasoup_elixir/Cargo.toml)
🔇 Additional comments (5)
lib/logger_proxy.ex (4)
49-52: LGTM! Type specification is well-defined.The config type specification correctly defines the structure as a keyword list with optional filter, matching the implementation.
57-61: LGTM! Initialization logic is correct.The start_link function properly extracts the filter from config with appropriate defaults and initializes the state correctly.
65-66: LGTM! State initialization is correct.The init function properly uses the provided init_arg to maintain the filter in state.
69-77: LGTM! Filter logic is correctly implemented.The handle_info function properly implements the filtering logic, only logging messages when the filter is nil or returns true.
test/logger_proxy_test.exs (1)
88-100: LGTM! Test case properly verifies basic filter functionality.The test effectively verifies that the filter correctly allows info level messages while filtering out debug messages.
satoren
force-pushed
the
mshiraki/log_filter
branch
from
38da1df to
64533d6
Compare
Summary by CodeRabbit
New Features
Tests