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

Improve logging and document approach & configuration clearly #39

Open
ches opened this issue Feb 25, 2016 · 1 comment
Open

Improve logging and document approach & configuration clearly #39

ches opened this issue Feb 25, 2016 · 1 comment

Comments

@ches
Copy link
Contributor

ches commented Feb 25, 2016

On #38 a logger object was added for the package, but then removed in favor of considering it later where it could receive focused discussion. This issue is a reminder to do that!

The library currently has a dependency on grizzled-slf4j and it's being imported, but I only see it being used for one debug call currently 😄 Grizzled appears to be a simple wrapper with some nice properties, though, and using its trait support is very non-intrusive, almost leaves nothing to discuss code-wise.

I think we should:

  • Affirm that grizzled-slf4j suffices and maintainers are happy with it.
  • Judiciously add some more logging where it is helpful. The Grizzled wrapper makes levels that are turned off inexpensive.
  • Document configuration for users in the README. This is essentially delegated to standard SLF4J means (providing your own "binding", concrete logger implementation), but we should briefly summarize and give pointers to how that works.

We're currently including the slf4j-simple binding as a dependency—this is probably overly opinionated/limiting since it only logs to stderr and at INFO level and above. Operators usually want more choice than that: any believer in 12-Factor app tenets will want stdout, many orgs have log aggregation conventions based on syslog, or files with collectors, etc.

Generally the idea with SLF4J is that libraries don't ship any binding, they just use the facade and leave it to applications to provide a chosen, configured binding, then all their SLF4J-enabled libraries will use it. It defaults to no-op if none is provided. So, the Keen client probably shouldn't have slf4j-simple as a dependency.

@ches
Copy link
Contributor Author

ches commented Feb 25, 2016

Just realized the slf4j-simple dependency is test-only, so that's not a problem. 🙈

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

No branches or pull requests

1 participant