Bristol icon indicating copy to clipboard operation
Bristol copied to clipboard

Allow targets to be automatically added to default Bristol instance, configured by env vars

Open TomFrost opened this issue 7 years ago • 3 comments

It's extremely common to implement Bristol (and most loggers) in a project by loading and configuring it in a JS file within a project, and then requiring that file throughout the project to make calls to the logger. This adds boilerplate code to every project, forcing the programmer to come up with their own scheme to map between configurations and the chained configuration interface of Bristol. This also puts a tedious blockade in 12Factor projects, where all configuration should be able to be changed by env vars.

In Cryptex, I implemented a system where the library could be transparently configured at load time, using zero code, just by setting the appropriate environment variables. I'd like Bristol to have a similar solution, recognizing that:

  • some features in Bristol require writing JavaScript functions and can never be fully replicated in env vars
  • ~it will likely remain a best practice to wrap Bristol in another JS file even if code-level configuration is not needed, in case it is later~ I think enough of this is solved that this may not be expressly needed for common use cases
  • ~it is infeasible (as in: possible but terribly ugly/unintuitive) to configure more than one target on more than one Bristol instance in this manner~ solved

Given the above, I still find a lot of worth in pre-written env var integration due to the amount of configuration mapping code that will reduce for the end-user, and paving the way for Bristol to take more control over its configuration options and processes in the future.

With that said, there is a significant challenge that comes from making this configuration style functional: custom targets and formatters need to be added by the library user in the code, but Bristol is loaded and will need to be configured before they are added.

And so, I propose adding a layer to the search chain for Bristol modules. If someone adds a target or formatter foo, Bristol will first attempt to load a built-in target or formatter named foo, and if that fails, it will attempt to require(process.env.BRISTOL_MODULES_DIR + '/foo), and if that fails, it will attempt to require('bristol-foo'). If someone tries adding a target or formatter that starts with a dot or slash, Bristol will assume it's a path and require() it with no modification.

This allows custom targets and formatters to be discovered without requiring code to "install" them before a new target is configured.

@MarkHerhold, I'm particularly interested in your thoughts on this as the maintainer of Palin. This would likely require publishing that formatter to a new NPM package name, but would reduce the code involved in configuring Bristol to use it.

TomFrost avatar Dec 19 '16 15:12 TomFrost

@TomFrost Neat idea!

Personally I'm a huge fan of config which lets me manage my configurations for multiple environments and regions easily in a hierarchical structure. That said, I can see your approach being useful for Bristol as well. I do wonder if this environment-variable-based-bootstrapping should just be implemented as a wrapper around Bristol though....

I have no problem moving either the Palin repository or npm package and will happily go along with whatever approach makes for a seamless integration in the Bristol ecosystem.

Have you thought about just using the fully-qualified package name? I am not sure I see the advantage to prepending bristol- before requiring a module. Also, I don't know how well this approach will work for scoped packages.

As a final thought, I have two targets in my application. One target logs to the console with warn and error severity levels where the logs are collected by systemd. The other target logs to a remote service using all levels. I'd be curious as to how this would be configured via an environment variable. A JSON string environment variable might be one possibility.

MarkHerhold avatar Dec 19 '16 18:12 MarkHerhold

Personally I'm a huge fan of config which lets me manage my configurations for multiple environments and regions easily in a hierarchical structure. That said, I can see your approach being useful for Bristol as well. I do wonder if this environment-variable-based-bootstrapping should just be implemented as a wrapper around Bristol though....

I used to be a huge config user :) Since then, I've been sold on the 12Factor approach which frowns on config files (though the config library itself also has some handy tooling to convert env vars into more complex config objects). At my company, we have a multitude of docker containers on a distributed infrastructure, so the 100% env-var-controlled config is nice for spinning environments up and down without changing code.

With that said, we're wrapping Bristol in a custom config system now to map env vars to Bristol config, and that turned into a pitfall. Not only would we have to edit that every time we want access to a new feature, it can be inconsistent between projects. Building that mapping into Bristol would solve that across the board.

Have you thought about just using the fully-qualified package name? I am not sure I see the advantage to prepending bristol- before requiring a module. Also, I don't know how well this approach will work for scoped packages.

This is a really excellent point. Perhaps I'll remove the bristol- prefix entirely, then, and just attempt to require() the target/formatter name if it's not built-in. You're right, that wouldn't work with scoped packages at all.

As a final thought, I have two targets in my application. One target logs to the console with warn and error severity levels where the logs are collected by systemd. The other target logs to a remote service using all levels. I'd be curious as to how this would be configured via an environment variable. A JSON string environment variable might be one possibility.

So the env vars would only be able to auto-configure one of these targets (and, as such, might not be a good fit for your use case at all). But it might look like this for the console target:

BRISTOL_TARGET=console
BRISTOL_FORMATTER=syslog
BRISTOL_SEVERITY_HIGH=error
BRISTOL_SEVERITY_LOW=warn

TomFrost avatar Dec 19 '16 19:12 TomFrost

...now that I'm thinking about it, I suppose it would be fairly easy to support multiple targets in env vars without it getting confusing. The above 4, plus these, would then configure what you have right now:

BRISTOL2_TARGET=file
BRISTOL2_TARGETOPT_FILE=/var/log/myapp.json
BRISTOL2_FORMATTER=json

TomFrost avatar Dec 19 '16 21:12 TomFrost