newrelic/node-newrelic — pull request #149 by joscarsson (2014-06-07 22:19:30+00:00)
Title: Configure newrelic using custom config object (discussion)

I'm opening this pull request mainly to start a discussion. I could have opened an issue, but in this case I think it is more constructive to actually provide some proof-of-concept code for what I am after.

So, in our application we have multiple processes/servers running from the same source code tree (so there will be several `app.js` run using different executions of the `node` binary, but only one `node_modules` folder. That means placing a `newrelic.js` in the root of the application is not "good enough" in our case, because we want to provide different application names for the different applications.

In `node-newrelic/lib/config.js` the following code block is found:

``` javascript
var CONFIG_FILE_LOCATIONS = [
process.cwd(),
process.env.NEW_RELIC_HOME,
process.env.HOME,
path.join(__dirname, '..', '..', '..') // above node_modules
];
```

That means we could make sure our `node` executions have their working directory set to where `app.js` resides, and place one `newrelic.js` at each of these locations (because of the `process.cwd()` above). Although I guess it will work, I haven't tried it, because it still does not satisfy all our requirements. We also want to be able to change `app_name` depending on which environment we run on (for example, stage environment should report to **App Name (Stage)** whereas production should report to **App Name**).

So what we are actually after is to be able to pass our own config object into newrelic directly, without using the `newrelic.js` file or environment variables. This gives several advantages:

1. We can utilize our existing configuration object which already have support for environment specific settings and so on, and just add a new subobject for newrelic settings.
2. We can dynamically during runtime edit the configuration before we pass it into newrelic (*not* after, but before).
3. We won't need any new configuration files on the file system.

In our `app.js`, it will look like this:

```
var ourAppConfig = require('./ourAppConfig');
var newrelic = require('newrelic/bootstrap')(ourAppConfig.newrelic);
```

The commits in this pull request enable this functionality. It wraps the code in `node-newrelic/index.js` in a function in `node-newrelic/bootstrap.js`. The `index.js` directly calls this new `bootstrap.js` function with an empty argument, meaning it will not use a custom configuration object so existing code will continue to function without changes.

You actually had this somewhat prepared in `node-newrelic/lib/config.js`:

```
function initialize(config) {
// removed code for display purposes

if (config) return new Config(config);
// more removed code for display purposes
```

and

```
/**
* Preserve the legacy initializer, but also allow consumers to manage their
* own configuration if they choose.
*/
Config.initialize = initialize;
```

The code in the new `bootstrap.js` utilizes this `initialize(config)` function.

The problem with this solution is the circular dependency between `logger` and `config`. To get this to work, I had to disable the `require()` of `config` in `logger`. This dependency seems a bit strange as you have noticed yourselves too, in `node-newrelic/lib/config.js`:

```
/**
* Because this module and logger depend on each other, the logger needs
* a way to inject the actual logger instance once it's constructed.
* It's kind of a Rube Goldberg device, but it works.
*
* @param {Logger} bootstrapped The actual, configured logger.
*/
Config.prototype.setLogger = function setLogger(bootstrapped) {
```

And this comment in `node-newrelic/index.js` (`bootstrap.js` in this pull request) is actually not correct any longer:

```
/* Loading the configuration can throw if a configuration file isn't found and
* the environment variable NEW_RELIC_NO_CONFIG_FILE isn't set.
*/
var config = require(path.join(__dirname, 'lib', 'config.js')).initialize(customConfig);
```

The row above will never throw, as it will already have thrown when requiring the `logger` (as that depends on `config`) on the first line in the file `logger = require(path.join(__dirname, 'lib', 'logger.js'))`. Is this intended?

If the circular dependency was removed somehow, the pull request would pretty much work as is. As I understand it, the "only" thing configured in `logger` from `config` is loglevel and filepath if logging to file. Perhaps there could be defaults for this instead, or only allow these options to be configured from environment variables? Anything removing the dependency on `config` would work.

**Or am I doing this whole thing wrong? Is there an easy solution for what I am after? If not, are you planning to support anything similar to the above? We really like newrelic, and this is the last piece for us to be able to use it in this system.**