Add excludedPrefixes option and improve performance #3
Conversation
Add a new excludedPrefixes option to the logger configuration, which allows users to specify prefixes that should not be logged. This can be useful in some cases where excluding prefixes is easier than including them. Change the method of getting the calling assembly from GetCallingAssembly to also use GetEntryAssembly, which fixes the problem of logging the wrong assembly name when the logger setup assembly is separated from the actual app assembly. Use a HashSet instead of a List to store and check the allowed assemblies, which improves the performance of the logger by reducing the number of comparisons.
|
Hi, thank you so much for the PR. It's great to know that there is a use for it. I really like the idea of excluding prefixes, and also great about the entry assembly. I'll have to test a bit myself to see if it's working as I originally intended. I'm not a big fan of the clean file name, but I see why it could be useful. It should definitely be an option if it is included, as my use case requires the full path to be logged. I can also see that you work with the file paths using string manipulation. This is platform-dependent and probably won't work the same way on Linux or Mac. If you can, please use the Path and Directory classes to ensure cross-platform support. I can also see that the tests now take ~5 seconds to run on my machine, while they took ~25 ms before. I think it is due to the changes you made in I'm more than happy to merge the PR once those issues are resolved! :) I'm unfortunately very busy myself at the moment, so don't have much time to work on this myself. |
FilePath can now be customized to desired depth and will show full path by default and is platform agnostic returned to previous performance by not loading referenced assemblies of assemblies that are not included
|
Thank you for your valuable feedback. I have improved the clean file method by using Path and Directory classes, and I have modified it to output the full file path by default. However, it can also be configured to filter the path up to a certain depth if needed. Regarding the performance issue, I think I have restored the previous speed by avoiding loading the assemblies that are only referenced by an assembly that is not part of the desired assemblies. I am not aware of any scenario where this approach might miss a required assembly that is one or two references deep, but I could not reproduce such a case in my tests. The performance difference between 20 milliseconds and more than 4 seconds is significant, and I don’t think it is worth sacrificing for a hypothetical situation. But if you have any example of such a case, I would be happy to work on a solution for it. I appreciate your time and attention, and I look forward to hearing from you again. |
|
Hi again, thank you for the changes, they look really great! :) I merged the PR now and have added some minor changes and some more tests for the new options as well. I will create a new release with the changes now. Thanks for your contribution, it is really appreciated! |
Hello, I have made some changes to the serilog-enrichers-callerinfo project and I would like to request a pull request. Here are the main features of my changes:
Add a new excludedPrefixes option to the logger configuration, which allows users to specify prefixes that should not be logged. This can be useful in some cases where excluding prefixes is easier than including them.
Change the method of getting the calling assembly from GetCallingAssembly to also use GetEntryAssembly, which fixes the problem of logging the wrong assembly name when the logger setup assembly is separated from the actual app assembly.
Use a HashSet instead of a List to store and check the allowed assemblies, which improves the performance of the logger by reducing the number of comparisons.
I have also added a test and checked if all previous tests are passing. I hope you find these changes useful and acceptable. Please let me know if you have any feedback or questions. Thank you for your time and consideration.