kenperkins / winston-papertrail

Papertrail transport for Winston
MIT License
203 stars 94 forks source link

Discussion around winston-papertrail 2.x #84

Open ffxsam opened 6 years ago

ffxsam commented 6 years ago

It seems this project is no longer maintained. Is that the case? @kenperkins @troy

troy commented 6 years ago

I'm no longer a maintainer nor using it personally, but @johlym or @markdascher may know the current status.

ffxsam commented 6 years ago

I'm considering starting a fork. I have a need to have separate/multiple log formatters in a single transport, or at least a way to internally share a single connection so that multiple instances don't open new connections.

But if I can open a PR, that would be preferable. I just want to make sure it could actually get merged by a maintainer.

kenperkins commented 6 years ago

I'm amenable to moving the repo if the current maintainers are unavailable. I'm also willing to take new maintainers. I'm just unable at present to commit any time and am far behind on the current state.

ffxsam commented 6 years ago

More on my use case:

I'm using the logFormat option to add in my own metadata next to the log level, e..g:

/* Inside my Logger class constructor */
    const logFormatter = (level, message) => {
      return `[${level}] ${this.metadataString} ${message}`;
    };

The Logger class constructor starts off with this:

class Logger {
  constructor(programName, metadata) {
    this.programName = programName;
    this.metadataString = metadata
      ? Object.keys(metadata)
          .filter(key => !!metadata[key])
          .map(key => `[${key}:${COLOR_RED}${metadata[key]}${COLOR_END}]`)
          .join(' ')
      : '';

So basically, in Papertrail I get log lines that look like:

Nov 09 08:43:22 ec2-async-handler-main async-worker-staging:  [info] [pubId:3xc7KB9QyAR3KF4CT] Calling getcountasync 

because I can instantiate a logger like so:

const logger = new Logger('async-worker-staging', {
  pubId: '3xc7KB9QyAR3KF4CT',
});

The thing is, we're working with massive amounts of data, and we go through a huge loop where that pubId will be different every time (meaning, a separate log formatter function every time). I've built my Logger class in a way where it maintains an internal map of programs, and only creates one transport per program name, so as not to open potentially hundreds of connections to Papertrail. Of course, this also means a single log formatter per transport, which doesn't work for this use case. And I can't log the pubId in the usual metadata arg:

logger.log('info', 'Blah', { pubId: '3xc7KB9QyAR3KF4CT' })

because there's dozens of lines of other metadata, and I need that pubId on every single log line. If it's in the metadata as shown above, it only shows up on one line.

I don't know how much of that makes sense, it's hard for me to explain (code is better shown than talked about). 😉 If there's some easy way to do this, I'm all ears because I'd rather not spend much time on it if I don't have to!

ffxsam commented 6 years ago

It seems like the winston Papertrail class could be modified to store a reference to the socket connection statically on the class, and so future instantiations of winston.transports.Papertrail could check and reuse the existing connection (if the host and port match, of course). So an internal map of sockets would exist, e.g.

{
  "logsX.papertrail.com:12345": conn1,
  "logsX.papertrail.com:98765": conn2
}

I dunno if that would work or is a good idea. Admittedly I'm rather new to both winston & Papertrail.

ffxsam commented 6 years ago

@kenperkins @johlym @markdascher I'm working on a rewrite of the Papertrail transport. It'll be a new major version since it introduces breaking changes (built for winston 3.x). Right now this is just for my own internal use, but once I have a chance to polish up the code, document it, write tests (it's gonna be a while), I can open a PR and have some folks look at it.

One big departure in functionality will be the ability to have multiple transports use a single connection, e.g.:

const connection = new PapertrailConnection({
  host: 'logsX.papertrailapp.com',
  port: 12345,
});
const papertrailTransport = new PapertrailTransport({
  connection,
  colorize: true,
  program: 'test',
  logFormat: logFormatter,
});

This will allow people to use different log formatting and different program parameters, without creating multiple connections to Papertrail.

I'll use this issue to post updates.

ffxsam commented 6 years ago

@kenperkins Hi Ken, could you please add me as a maintainer?

Also, does anyone have any spare cycles to review the code once I've opened a PR for 2.x? It's a drastic departure from 1.x, and I'm not entirely sure how I can force socket errors to test & ensure all of that works correctly.

ZacharyDraper commented 5 years ago

@ffxsam I am interested in your rebuild. Tried this today with Winston 3.1.0 and while it works, it has a number of problems. Would love to help test what you are working on.

ffxsam commented 5 years ago

@ZacharyDraper Great, I could use the help as I'm swamped lately! The code isn't pushed yet, it's on my Mac which is not hooked up at the moment. I'll post here when it's up so people can contribute.

lcersly commented 5 years ago

@ffxsam Also interested in the rebuild. Especially the part about single connection but multiple loggers. In Winston 3.0 they are using something called categories, and it sound like something I would like to use if it's supported by Papertrail.

As such, I would like to help with either reviewing or rewriting if need be.

ffxsam commented 5 years ago

Hey all! v2 branch is up: https://github.com/kenperkins/winston-papertrail/tree/v2

The only change at the moment is lib/winston-papertrail.js. There's still a bit that needs to be done here, such as handling failed connections and such. But on a basic level, it works. Here's an example:

const connection = new PapertrailConnection({
  host: 'logsX.papertrailapp.com',
  port: 123,
});
const papertrailTransport = new PapertrailTransport({
  connection,
  colorize: true,
  program: 'hello',
  logFormat: (level, message) => `[${level}] ${message}`,
});

const logger = winston.createLogger({
  transports: [papertrailTransport],
  format: winston.format.colorize(),
});

And then:

logger.log({
  level: 'info',
  message: 'Log THIS!',
});

Please feel free to open PRs against the new v2 branch. Please make sure all ESLint rules pass.

ZacharyDraper commented 5 years ago

@ffxsam I did some testing on this today and it works great! It solves all of the problems that were keeping us from using this package. Thank you!

ffxsam commented 5 years ago

No prob! Keep in mind it's still incomplete as it doesn't handle network errors gracefully (or at all). If anyone can help contribute, that'd be awesome.

ZacharyDraper commented 5 years ago

@ffxsam What ideas do you have on what should happen in the event a connection cannot be made to Papertrail? The following code at the bottom of your connect() method would at least be able to detect the problem. For now, I have it just printing a console message.

    socket.on('error', function(e){
        console.log('Unable to connect to Papertrail');
    });

In testing this, I've found that Papertrail seems to become unavailable every night for a short time and that when the transport is unable to connect it simply stops running. Even when Papertrail is available, the transport will not send log messages until it is restarted. Will continue to investigate and update this post.

draperunner commented 5 years ago

@ffxsam I opened #86 to enable the inlineMeta option, which was not set in the constructor, therefore always ignored.

b5l commented 5 years ago

We would like to switch from log4js to winston 3 in our software, however we need winston-papertrail for it. Releasing a version to our clients with a git repository in its package.json is not an option. Can we expect version 2 of winston-papertrail to be released soon? What has to be done before it can be released? I might be able to contribute.

ffxsam commented 5 years ago

@benjaminlaib There's a working v2 branch, you're welcome to use it and work on it. It needs more comprehensive error-handling, I think. Other than that, it works.

Personally, it turns out I won't be using Papertrail in the future, so I won't be contributing. But I'll be around for any PRs into the v2 branch.