LOG4J2-3256: Reduce ignored package scope of KafkaAppender #640
Conversation
….apache.kafka.[common,clients]
There was a problem hiding this comment.
@dongjinleekr
Thank you for your PR.
This feels pretty brittle. What about putting this package list in a text file in the resource folder?
Note that this PR is for master but our current work is based on the branch release-2.x
|
@garydgregory So, you mean...
Do I understand correctly? |
@dongjinleekr |
|
@garydgregory Great, with more maintainability! 👍 |
|
Hi @garydgregory, After some trial and error, I concluded that maintaining the ignored package list in an array would be better than a file-based approach. Here is why:
I also updated the PR to follow this way. How do you think? Is there any specific reason for the file-based approach? |
| @@ -136,6 +136,18 @@ public B setRetryCount(final String retryCount) { | |||
| return new Builder<B>().asBuilder(); | |||
| } | |||
|
|
|||
| private static final String[] KAFKA_CLIENT_PACKAGES = new String[] { "org.apache.kafka.common", "org.apache.kafka.clients" }; | |||
There was a problem hiding this comment.
If you use a static init block, you can sort the array and then bsearch() it in the lookup method.
There was a problem hiding this comment.
It seems like we are checking the package prefix, not the exact match. Is there any way to do prefix match with binary search? (Or, using regex would be better?)
There was a problem hiding this comment.
You are correct, I forgot that this is just a prefix match, discard my last comment. I'll review in the morning unless someone else does first.
Modified GitHub patch #640 from Lee Dongjin/dongjinleekr/dongjin@apache.org.
Modified GitHub patch #640 from Lee Dongjin/dongjinleekr/dongjin@apache.org.
Modified GitHub patch #640 from Lee Dongjin/dongjinleekr/dongjin@apache.org.
|
I modified GitHub patch #640 and pushed patches to branches You are credited in Thank you for your help! |
No description provided.