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

Feature #612 - implement LogFilter glob-like wildcard support #613

Merged

Conversation

ThijsBroersen
Copy link
Contributor

@ThijsBroersen ThijsBroersen commented Jan 8, 2023

#612

I implemented ** (any number of paths) and * (any (partial) path) within the existing LogFilter path structure List[String].

I think this could be optimized if the LogFilters were used in a nested map-like structure.

Another way would be the use regex.

@ThijsBroersen ThijsBroersen requested a review from a team as a code owner January 8, 2023 20:50
@ThijsBroersen
Copy link
Contributor Author

ThijsBroersen commented Jan 8, 2023

Did some benchmarks to see if there is much performance loss. I did not extend the benchmark tests to include wildcard paths, only tested for regression for current supported structures.

Feature #612

[info] Benchmark                                           Mode  Cnt     Score       Error  Units
[info] FilterBenchmarks.cachedFilterByLogLevelAndNameLog  thrpt    3  5668.719 ± 39326.263  ops/s
[info] FilterBenchmarks.filterByLogLevelAndNameLog        thrpt    3  4649.646 ± 12339.049  ops/s
[info] FilterBenchmarks.handWrittenFilterLog              thrpt    3  3109.203 ±  2338.471  ops/s
[info] FilterBenchmarks.noFilteringLog                    thrpt    3  2270.438 ±  8116.408  ops/s```

Master
```[info] Benchmark                                           Mode  Cnt     Score       Error  Units
[info] FilterBenchmarks.cachedFilterByLogLevelAndNameLog  thrpt    3  5390.427 ± 35953.429  ops/s
[info] FilterBenchmarks.filterByLogLevelAndNameLog        thrpt    3  4323.591 ± 22497.521  ops/s
[info] FilterBenchmarks.handWrittenFilterLog              thrpt    3  2725.011 ±   348.357  ops/s
[info] FilterBenchmarks.noFilteringLog                    thrpt    3  1957.494 ±  1254.506  ops/s```

@ThijsBroersen ThijsBroersen changed the title Fixed #612 - implement LogFilter wildcard support Fixed #612 - implement LogFilter glob-like wildcard support Jan 8, 2023
@justcoon
Copy link
Contributor

justcoon commented Jan 8, 2023

thank you for contribution

@ThijsBroersen ThijsBroersen changed the title Fixed #612 - implement LogFilter glob-like wildcard support Feature #612 - implement LogFilter glob-like wildcard support Jan 8, 2023
@ThijsBroersen
Copy link
Contributor Author

Hmm, I see this library still needs to support older Scala versions for which the CI tests now fail 😞

@ThijsBroersen ThijsBroersen marked this pull request as draft January 8, 2023 22:47
@justcoon
Copy link
Contributor

justcoon commented Feb 3, 2023

hello @ThijsBroersen , hope you are doing well,

are you planning to continue to work on this issue?

thank you

@ThijsBroersen ThijsBroersen force-pushed the feature-612/support-wildcard-logfilters branch from 2695a28 to bd4a8c7 Compare March 13, 2023 07:26
@ThijsBroersen ThijsBroersen force-pushed the feature-612/support-wildcard-logfilters branch from bd4a8c7 to 0423f01 Compare March 24, 2023 20:34
@ThijsBroersen
Copy link
Contributor Author

ThijsBroersen commented Mar 25, 2023

PR

[info] Benchmark                                           Mode  Cnt      Score      Error  Units
[info] FilterBenchmarks.cachedFilterByLogLevelAndNameLog  thrpt    3  11807.296 ± 2000.280  ops/s
[info] FilterBenchmarks.filterByLogLevelAndNameLog        thrpt    3  11467.630 ± 5999.410  ops/s
[info] FilterBenchmarks.handWrittenFilterLog              thrpt    3   9390.290 ± 2512.218  ops/s
[info] FilterBenchmarks.noFilteringLog                    thrpt    3   8268.888 ± 1204.542  ops/s

Master

[info] Benchmark                                           Mode  Cnt      Score      Error  Units
[info] FilterBenchmarks.cachedFilterByLogLevelAndNameLog  thrpt    3  11525.222 ± 1802.478  ops/s
[info] FilterBenchmarks.filterByLogLevelAndNameLog        thrpt    3  11873.502 ± 2359.884  ops/s
[info] FilterBenchmarks.handWrittenFilterLog              thrpt    3   9522.591 ±  927.634  ops/s
[info] FilterBenchmarks.noFilteringLog                    thrpt    3   7957.346 ± 1085.727  ops/s

@ThijsBroersen ThijsBroersen marked this pull request as ready for review March 25, 2023 12:20
@ThijsBroersen ThijsBroersen marked this pull request as draft March 25, 2023 15:18
@ThijsBroersen ThijsBroersen marked this pull request as ready for review March 25, 2023 15:27
Copy link
Contributor

@justcoon justcoon left a comment

Choose a reason for hiding this comment

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

thank you very much for contribution

@justcoon justcoon merged commit 8469b9f into zio:master Mar 28, 2023
@ThijsBroersen ThijsBroersen deleted the feature-612/support-wildcard-logfilters branch March 28, 2023 17:37
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.

2 participants