Skip to content
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

feat: add target filter #44

Merged
merged 3 commits into from
Aug 13, 2024
Merged

feat: add target filter #44

merged 3 commits into from
Aug 13, 2024

Conversation

andylokandy
Copy link
Contributor

@andylokandy andylokandy commented Aug 13, 2024

closes #8

@tisonkun
Copy link
Contributor

@andylokandy this should close #8

I have some questions when considering the design of this filter, you can take a look #8 (comment)

Those can be faq when using this filter.

@andylokandy
Copy link
Contributor Author

Yes. I'm following the idea in #8.

Q: If multiple LogLevelModuleFilter configured? What's the manner? Especially if the targets are overlapped
A: Simply apply by the filter order

Q: What is the priority between LogLevelModuleFilter and LevelFilter.
A: LogLevelModuleFilter is super set of LevelFilter. LevelFilter is simpler to use.

Q: Filters are applied in order, but when to short circuit the test (by Accepted/Rejected)
A: LevelFilter will reject if level is lower than levelfilter, and LogLevelModuleFilter will reject if level is lower than levelfilter and the target is matched

src/filter/level.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@tisonkun tisonkun left a comment

Choose a reason for hiding this comment

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

LGTM. One minor comment inline.

src/filter/target.rs Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LogLevelModuleFilter
2 participants