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

Close connections held by appenders #217

Closed
dfrese opened this issue Feb 1, 2017 · 4 comments
Closed

Close connections held by appenders #217

dfrese opened this issue Feb 1, 2017 · 4 comments

Comments

@dfrese
Copy link

dfrese commented Feb 1, 2017

Many appenders (need to?) open files/sockets on demand as a side-effect of the first message (http, databases, etc.), but usually there seems to be no way to close them properly. Sometimes it's just a waste of resources, sometimes there can be non-daemon threads preventing jvm shutdown (riemann), sometimes they are kept in atoms preventing multiple different appenders of that type (mongodb, if I read it correctly).

Any plans to address this generally? An optional close/shutdown fn for appenders might help, which would then have to be called if an appender is removed due to set-config!, and maybe in a timbre-shutdown (or just do a (set-config! nil))?

@ptaoussanis
Copy link
Member

Hi there,

I'm generally inclined to suggest that this is something that appenders should handle for themselves at an appender-specific level. That's because different appenders might have different needs re: holding open resources, etc. - and different kinds of config that'd make sense for their specific use case.

Holding resources is usually something you want to be explicit about.

Trying to automate/generalize all this at the level of the logging library may lead to a significant loss of flexibility, significant increase in complexity, or both.

To be clear: I haven't given this enough thought to have actually come to any conclusions; just stating my first reaction to the idea :-)

Would be open to seeing concrete ideas (an API, etc.) if you or anyone else felt like proposing anything.

which would then have to be called if an appender is removed due to set-config!, and maybe in a timbre-shutdown (or just do a (set-config! nil))?

Wouldn't be keen on automatic side-effects triggered by config changes, so if we're talking about an API here - would need to be something that you'd specifically call into if you know you're using an appender (or appenders) that require shutdown.

Cheers :-)

@AlexanderMann
Copy link

This is something we've run into recently at CircleCI as well. A simple api change might be a :close-fn key present in the appender interface. Helpers and whatnot could then be built around collecting those lambdas out of the present config etc.

@ptaoussanis
Copy link
Member

@AlexanderMann Thanks for the feedback Alexander, apologies for the very long delay replying.

Would definitely be open to seeing a sketch PR, or more detailed description of what you have in mind, e.g.:

  • What API do you have in mind for a close-fn? (Notably fn sig and sketch docstring/contract)
  • What API do you have in mind for how/when these close-fns will be triggered?

The more concrete info someone can provide on this, the quicker it'll be to get something into Timbre.

In the meantime, note that appenders can be given additional keys/data (i.e. outside the standard Timbre API) - so it should be possible for custom appenders to already implement their own version of this kind of mechanism so long.

@ptaoussanis ptaoussanis self-assigned this Oct 20, 2022
@ptaoussanis ptaoussanis added this to the v6 milestone Oct 20, 2022
@ptaoussanis
Copy link
Member

Experimental implementation of this will be included in forthcoming Timbre release 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants