-
Notifications
You must be signed in to change notification settings - Fork 94
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
Discussion around winston-papertrail 2.x #84
Comments
I'm no longer a maintainer nor using it personally, but @johlym or @markdascher may know the current status. |
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. |
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. |
More on my use case: I'm using the /* Inside my Logger class constructor */
const logFormatter = (level, message) => {
return `[${level}] ${this.metadataString} ${message}`;
}; The 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:
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! |
It seems like the winston {
"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. |
@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 I'll use this issue to post updates. |
@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. |
@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. |
@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. |
@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. |
Hey all! v2 branch is up: https://github.com/kenperkins/winston-papertrail/tree/v2 The only change at the moment is 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. |
@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! |
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. |
@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 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. |
We would like to switch from |
@benjaminlaib There's a working 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 |
It seems this project is no longer maintained. Is that the case? @kenperkins @troy
The text was updated successfully, but these errors were encountered: