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

Allow multiple File transports for a logger #149

Closed
wants to merge 4 commits into from

Conversation

pdobrev
Copy link

@pdobrev pdobrev commented Jun 29, 2012

Makes it possible to use several File transports for one logger:

var logger = new (winston.Logger)({
    transports: [
        new (winston.transports.File) ({ 
            level: 'info',
            filename: 'log/access.log',
            json: true
        }),
        new (winston.transports.File) ({ 
            level: 'error',
            filename: 'log/error.log',
            json: true
        })
    ]
});

@domenic
Copy link
Contributor

domenic commented Jun 29, 2012

Why only file?

@chjj
Copy link
Member

chjj commented Jun 29, 2012

Adding a special exception for file is just a workaround. Doing something like that kind of defeats the point of having different extensible transports.

In order to allow multiple of the same transport (which does sound like it might be a decent idea), a lot more has to change. The logger management has to change, and aside from allowing multiple of each transport type, things like querying and streaming have to be changed as well, as they assume there is only one of a particular transport.

@pdobrev
Copy link
Author

pdobrev commented Jul 2, 2012

@chjj, you're right, it's a workaround for file. It specifically addresses this issue: #101. I don't think it defeats the purpose of different extensible transports, though, it just allows you to have more instances of the same type of transport (file, in this case).

You're right there's more that has to change - for example, this changeset breaks a couple of test cases that expect the transport to have the name 'file', whereas it is now called 'file' + filename. The rest seems to be working fine, at least for my usage, since transports are usually iterated over and not queried by name.

I don't know if adding support for multiple transports of the same kind is on your list, and if it is, I would be happy to use it when it's implemented, but until then I'll just have to stick with the workaround.

@indexzero
Copy link
Member

Transport instances should have uid properties and the keys in logger.transports should reflect that. Lets not do this half-way.

@indexzero indexzero closed this Jul 8, 2012
@indexzero
Copy link
Member

@pdobrev This is actually a pretty easy fix. Just set this.uid = /* unique id */ in the base Transport constructor function and use it in the Logger.

@pdobrev
Copy link
Author

pdobrev commented Jul 14, 2012

Thanks, @indexzero, I'll have a look when I get more time on my hands and will try to implement this the right way, not just as a half-assed workaround.

@ghost
Copy link

ghost commented Jun 18, 2013

Did this pull request ever make its way into the repository? A stack-overflow post mentioned it was merged into this forked repo, which works, but the current flatiron/winston doesn't support this feature.
https://github.com/pdobrev/winston

@ash211
Copy link
Contributor

ash211 commented Aug 16, 2013

I'm still getting this error also with winston 0.7.2

@ash211 ash211 mentioned this pull request Aug 16, 2013
@phresnel
Copy link

Too bad it isn't as easy as just

    new winston.transports.File({
        filename: __dirname + '/debug.log', 
        json: false
    }),
    new winston.transports.File({
        filename: __dirname + '/debug.colored.log', 
        json: false,
        colorize: true
    })

My use case would be that e.g. with cat debug.colored.log, you could get a fancy version which is nice to the eyes, but also have a <editor-of-choice> debug.log as the portable version that will also work in an internet café during a holidy trip where there is only Windows' edit.

@wilzbach
Copy link
Contributor

would be great to see this finally arriving in the master :/

@indexzero
Copy link
Member

@greenify this has been in master for a long time. Please take the time to read the documentation before commenting on issues 😈

https://github.com/winstonjs/winston#multiple-transports-of-the-same-type

@winstonjs winstonjs locked and limited conversation to collaborators Nov 29, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants