Winston with classes - creating new instance for each file

Question

I am not sure, but I feel like something is wrong with this code. Suppose I have a winston class where a new instance is created every time the logger is used, which is every file.

Is there anything wrong with this logic?

// In logger.js
const winston = require('winston');

class Winston {
    constructor(errorPath) {
        const format = winston.format;
        const customFormatter = format((info) => {
            return Object.assign({
                timestamp: info.timestamp
            }, info);
        });

        let settings = {
            level: 'silly',
            format: winston.format.json(),
            transports: [
                new (winston.transports.File)({
                    filename: errorPath + '/error.log',
                    level: 'error',
                    handleExceptions: true,
                    stack: true,
                    format: format.combine(
                        format.timestamp(),
                        customFormatter(),
                        format.json()
                    )
                }),
                new (winston.transports.File)({
                    filename: errorPath + '/general.log',
                    format: format.combine(
                        format.timestamp(),
                        customFormatter(),
                        format.json()
                    )
                })
            ],
            exitOnError: false
        };
        settings.transports.push(new (winston.transports.Console)());
        return new winston.createLogger(settings);
    }
}

module.exports = Winston;

Then we suppose we have other files.

a.js
const Loger = require('./logger');
const logger = new Loger('somepath');
logger.error('Some error');
logger.debug('Some debug');

b.js
const Loger = require('./logger');
const logger = new Loger('somepath');
logger.error('Some error');
logger.debug('Some debug');

c.js
const Loger = require('./logger');
const logger = new Loger('somepath');
logger.error('Some error');
logger.debug('Some debug');

Will this create unnecessary overhead?

Answer 1

Suppose I have a winston class

That's the first problem. You should not use a class here. The only class Winston should be inside the winston library itself. And your class is in fact a factory function only, like createLogger: it doesn't have any methods, and the constructor returns an object.

a new instance is created every time the logger is used, which is every file

Unless you create a new logger every time you log a single message, it is fine to create one logger per file and then use that logger multiple times within the file.

somepath is the same in each file

That might actually be problematic. There should be a single transport per output file, otherwise you might get race conditions when two loggers try to write to the same file at the same time.

And yes, it's just wasteful to create multiple logger instances if they all share the same configuration. Why not just create the logger directly in logger.js and share this single instance?

// logger.js
const winston = require('winston');

const errorPath = 'somepath';
const format = winston.format;
const customFormatter = format((info) => {
    return Object.assign({
        timestamp: info.timestamp
    }, info);
});

let settings = {
    level: 'silly',
    format: winston.format.json(),
    transports: [
        new (winston.transports.File)({
            filename: errorPath + '/error.log',
            level: 'error',
            handleExceptions: true,
            stack: true,
            format: format.combine(
                format.timestamp(),
                customFormatter(),
                format.json()
            )
        }),
        new (winston.transports.File)({
            filename: errorPath + '/general.log',
            format: format.combine(
                format.timestamp(),
                customFormatter(),
                format.json()
            )
        })
    ],
    exitOnError: false
};
settings.transports.push(new (winston.transports.Console)());

module.exports = new winston.createLogger(settings);

// a-module.js
const logger = require('./logger');
logger.error('Some error');
logger.debug('Some debug');