-
Notifications
You must be signed in to change notification settings - Fork 453
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
Added Logger which emits a log based on a condition #4488
Conversation
I added a more general comment on #4409, While this may be useful in some cases, I'm not sure it solves the original issue. Mainly it seems that this would be hard to use when there is unique information in the message that should be passed along. If we need to treat "issue tablet x" and "issue tablet y" as different messages and not as repeat occurrences of the same message, this seems to want to treat them the same. |
In f679cd3 I modified the Logger to be based on a generic condition and added a time based implementation that uses a configuration property for the time interval. Like @EdColeman mentioned, not sure how useful this is, but I can envision wanting to log things in a server when some condition is true. |
Would there be any benefit of allowing the property to be dynamic and stored in ZooKeeper? That would also require a watcher to react to updates - and that is likely to make using this class much harder, basically a server context would likely be required to get the instance / zoo reader / zoo watcher. It would really complicate the code, but would allow things to be adjusted without restarting services. The other draw-back would be because this is a general property, setting it will change all instances and it may be really hard to know what they are. One alternative could be to use log4j and isLevelEnabled to toggle between messages and or message frequency. Something like the following using a count, or could be modified to use a time.
Then, using named loggers and standard configuration, the level for that class could be dynamically adjusted at runtime on a named logger basis? |
yeah, this is a problem. Currently there is no way to modify the properties of a single server at runtime for debug purposes. I think the standard way to do this is using JMX. With JShell, manipulating JMX beans becomes easier I think. |
I had an issue with the implementation regarding AbstractLogger implementing Serializable. I reworked the implementation and removed the Property. I also added @keith-turner's message deduplication implementation to this. |
These changes were merged into #4558 |
Was having an issue in PR apache#4488 with an earlier version of the code complaining about Serializable. This seems to be resolved in this version of the code.
These changes were merged as part of #4558 |
Fixes #4409