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

loggly config file inconsistency between winston and node-loggly #7

Closed
marksoper opened this issue May 10, 2011 · 4 comments · May be fixed by guypod/winston#3
Closed

loggly config file inconsistency between winston and node-loggly #7

marksoper opened this issue May 10, 2011 · 4 comments · May be fixed by guypod/winston#3

Comments

@marksoper
Copy link

This may just be a documentation issue, or perhaps it makes sense to fix:

The format for the config file in node-loggly

{
"subdomain": "your-subdomain",
"auth": {
"username": "your-username",
"password": "your-password"
},
"inputs": [{
"token": "your-really-long-token-you-got-when-you-created-an-http-input",
"id": 000 // ID of this input
}]
}

The format for loggly config in winston:

{
"transports": {
"loggly": {
"subdomain": "your-subdomain",
"inputToken": "really-long-token-you-got-from-loggly",
"auth": {
"username": "your-username",
"password": "your-password"
}
}
}
}

Why not modify winston to use the node-loggly config file format?

@indexzero
Copy link
Member

So I think there is some confusion here. That format is for running the tests and not for use in code. In the case of node-loggly:

  var loggly = require('loggly');
  var config = {
    subdomain: "your-subdomain",
    auth: {
      username: "your-username",
      password: "your-password"
    }
  };
  var client = loggly.createClient(config);

Whereas in winston:

var logger = new (winston.Logger)({
    transports: [
      new (winston.transports.Console)(),
      new (winston.transports.Loggly)({
        "subdomain": "your-subdomain",
        "inputToken": "really-long-token-you-got-from-loggly",
        "auth": {
          "username": "your-username",
          "password": "your-password"
        }
      })
    ]
  });

Let me know if you run into anymore problems.

@marksoper
Copy link
Author

I do think this is more about the core of how winston uses node-loggly,
rather than just an issue of test config files, so I've included what is
hopefully a much more clear writeup of the issue below.

This may not be a battle worth fighting right now, may be a nitpick about
organization style rather than a critical issue at this point, as it doesn't
hinder the basic functionality of either winston or node-loggly. I'm
bringing it up because I found it confusing when I first tried to set up
winston/loggly, but that may also be a function of being still somewhat new
to node and node packages.

In the case of node-loggly:

Only subdomain is required to create a Loggly client, with auth creds
required later to call most methods.
Once created, the getInputs method can be called to get a list of inputs, so
the client can now log to any input you want on the fly.

from loggly/lib/loggly/core.js

//
// function createClient (options)
//   Creates a new instance of a Loggly client.
//
exports.createClient = function (options) {
  return new Loggly(config.createConfig(options));
};

from loggly/lib/loggly/config.js

//
// Config (defaults)
//   Constructor for the Config object
//
var Config = function (defaults) {
  if (!defaults.subdomain) throw new Error('Subdomain is required to create
an instance of Config');

  this.subdomain = defaults.subdomain;
  this.auth = defaults.auth || null;
};

In the case of winston:

Winston's Loggly Transport is tied to a specific input, and that input
information is required to create the transport.
If the design goal is that these transports act as winston-consistent
wrappers around node-loggly and other logging mechanisms, they should
present a client constructor interface as close as possible to the original
(why can't a transport be created with just subdomain and auth to give me
access to the Loggly client through the winston transport) and surface
whatever is needed from the original so that using the original outside of
the transport isn't necessary (as in the current model, where if I want to
log to multiple inputs on the fly, I need to use node-loggly independently
of winston to call getInputs and then create and manage multiple winston
transports for each of them).

//
// function Loggly (options)
//   Constructor for the Loggly transport object.
//
var Loggly = exports.Loggly = function (options) {
  options = options || {};
  if (!options.subdomain)  throw new Error('Loggly Subdomain is required');
  if (!options.inputToken && !options.inputName) throw new Error('Target
input token or name is required.');
  if (!options.auth && options.inputName)        throw new Error('Loggly
authentication is required');

  this.name = 'loggly';
  this.level = options.level || 'info';
  this.logBuffer = [];

  this.client = loggly.createClient({
    subdomain: options.subdomain,
    auth: options.auth || null
  });

@indexzero
Copy link
Member

@marksoper Thanks for the detailed feedback here. The API design is centered around the fact that once an input is created it can be logged to. If a Loggly input is added without a valid input name or input token then logging messages could be potentially lost. Since you're already planning on using node-loggly independently I would do something like this: https://gist.github.com/973904

@marksoper
Copy link
Author

Makes sense. Thanks for the clarification. I've been working on some
analytics extensions to winston and will send an update on that soon.
Mark

On Sun, May 15, 2011 at 11:51 PM, indexzero <
reply@reply.github.com>wrote:

@marksoper Thanks for the detailed feedback here. The API design is
centered around the fact that once an input is created it can be logged to.
If a Loggly input is added without a valid input name or input token then
logging messages could be potentially lost. Since you're already planning on
using node-loggly independently I would do something like this:
https://gist.github.com/973904

Reply to this email directly or view it on GitHub:
https://github.com/indexzero/winston/issues/7#comment_1170414

617-999-3047
masoper@gmail.com
http://blog.marksoper.net
http://twitter.com/marksoper
http://facebook.com/masoper
http://linkedin.com/in/marksoper
https://github.com/marksoper

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

Successfully merging a pull request may close this issue.

2 participants