Ability to extend Logger class.

[bogdan]

What is the problem?

I am trying to extend winston logger with custom methods on typescript. Currently it can only be done via as any and assigning properties to the object manually. Moreover, methods like Logger#child require to be improved.

Here is how it looks like:

interface LoggerExtras {
  benchmark: Benchmark;
  child(options: Record<string, any>): this;
}
type Logger = winston.Logger & LoggerExtras;
const logger: Logger = createLogger() as any;

logger.benchmark = ...

export default logger;

What's the feature?

Ideally, I want to be able to extend Logger in a pretty OOP way without manual property assignment manipulations like:

class MyLogger extends winston.Logger {
  benchmark(...) {
    ....
  }
}
export default new MyLogger(options);

Is it possible to add such a functionality and ensure typescript is supported?

What problem is the feature intended to solve?

Make winston logger more extensible in a clear way that is common for OOP language.

Is the absence of this feature blocking you or your team? If so, how?

It is not a blockers as there are hacks to achieve that.

Is this feature similar to an existing feature in another tool?

Yes, this feature is common feature in any OOP friendly logger where you can just use inheritance as a pattern.
It is very unusual that making Logger is done through a factory method but not a constructor. Most loggers just expose a Logger class to you like so: https://ruby-doc.org/stdlib-2.4.0/libdoc/logger/rdoc/Logger.html

Is this a feature you're prepared to implement, with support from us?

Yes.

[edriwing]

That's a very good idea. I am currently trying to do the same :)

[Datzu712]

I found a not pretty solution ...

import winston from "winston"

// @ts-ignore
class Logger extends (createLogger as winston.Logger) {
    constructor(config: winston.LoggerOptions) {
        super(config)
        
    }
}

console.log(new Logger({}))

That works for me, but I don't like use @ts-ignore and extends by a function.
Note: If you remove @ts-ignore, you got an error...

[Faithfinder]

I found a not elegant solution ...

import winston from "winston"

// @ts-ignore
class Logger extends (createLogger as winston.Logger) {
    constructor(config: winston.LoggerOptions) {
        super(config)
        
    }
}

console.log(new Logger({}))

That works for me, but I don't like use @ts-ignore and extends by a function. Note: If you remove @ts-ignore, you got an error...

Unfortunately this breaks .child()

[wesamjabali]

Another non-elegant solution that doesn't break .child(), used in my project: https://github.com/wesamjabali/nest-graphql-starter/blob/main/src/core/logger/logger.service.ts

The important bits:

import {
  child,
  createLogger,
  format,
  LeveledLogMethod,
  Logger,
  transports,
} from 'winston';

export class LoggerService {
  private coreLogger: Logger;
  isErrorEnabled: () => boolean;
  isWarnEnabled: () => boolean;
  isInfoEnabled: () => boolean;
  isVerboseEnabled: () => boolean;
  isDebugEnabled: () => boolean;
  isSillyEnabled: () => boolean;
  error: LeveledLogMethod;
  warn: LeveledLogMethod;
  info: LeveledLogMethod;
  http: LeveledLogMethod;
  verbose: LeveledLogMethod;
  debug: LeveledLogMethod;
  silly: LeveledLogMethod;
  child: typeof child;
  log = (...message: string[]) => {
    this.coreLogger.defaultMeta = { service: message?.[1] ?? DEFAULT_SERVICE };
    this.coreLogger.log(LOG_LEVEL, message?.[0] ?? '');
  };

  constructor() {
    const winstonLogger = createLogger({...});
    Object.setPrototypeOf(this, Object.getPrototypeOf(winstonLogger));
    this.coreLogger = winstonLogger;
    for (const key of Object.keys(winstonLogger)) {
      this[key] = winstonLogger[key];
    }
  }

Hope this helps someone. Please let me know if you have any questions. You have to assign log to the corelogger's info otherwise it'll infinite loop.

[Faithfinder]

I'm just reaching for it deep into the package

import { transports, format, Logger } from 'winston';
//@ts-expect-error Winston doesn't export the Logger class but it's easy to reach for it. See https://github.com/winstonjs/winston/issues/2170
import HiddenLogger from 'winston/lib/winston/logger';
// Easy to reach for, but hard to type.
const LoggerClass = HiddenLogger as typeof Logger;

[marioplumbarius]

Any plans to implement this?

[marc-wilson]

+1

[wbt]

#2181 has failing tests and even I don't seem to have permissions to re-run them.

[wbt]

See also #2170

[marc-wilson]

@wbt, is there anyone you can contact on the core team to get this fixed? It looks like it has been sitting here for a few years now.

[wbt]

There's also been no funding for a few years now for anybody on the core team to author or review updates and extensions, so that's not a big surprise.

[wesamjabali]

Is winston dying then?On May 27, 2023, at 16:13, wbt ***@***.***> wrote: There's also been no funding for a few years now for anybody on the core team to author or review updates and extensions, so that's not a big surprise. —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: ***@***.***>

[wbt]

It's in a state of being maintained, but not actively expanding/growing. Folks who find it useful could still contribute skilled effort and/or funding to pay for it; some occasionally do make the first type of contribution.

[DABH]

This should be solved via #2181 and will go out in the next release

[marc-wilson]

Any hint at when the next release will be?

[DABH]

Published in 3.10.0

[marc-wilson]

This doesn't appear to be resolved. I have pulled down 3.10 and have this setup

import { Logger } from 'winston';
export class MyLogger extends Logger {
  constructor(options) {
     super(options);
  }
}

It works to a certain extent.

This works

const logger = new MyLogger(...);
logger.log('info', 'stuff');

This throws an error

const logger = new MyLogger(...);
logger.info('stuff');

TypeError: logger.info is not a function

It doesn't look like any of the leveled logging functions are getting defined when Logger is instantiated. If I just do new Logger(...), I get the same result. And of course, if I do createLogger(...) everything works as expected. But, createLogger is a function and not a class so I cannot extend it.

The hack mentioned above in the issue is still required unless i'm missing something.

[DABH]

@Faithfinder Any chance you would be able to investigate and possibly address @marc-wilson 's issue here?

[Faithfinder]

I'm not really ready to become a maintainer =P

createLogger is the function that defines the log functions from levels, so this is quite expected. It's a pity that index.d.ts defines these on the class itself, as this is a lie. But that's why I'm joking about a Typescript rewrite.

Though if I were serious about that, I'd argue to drop this feature anyway. Correctly defining types for properties/methods dynamically derived from options is some typescript wizardry. Probably possible, but I'd argue that not worth it.

[DABH]

Haha, you might already be the number one expert on extending the Logger class though ;) (Of course, @marc-wilson if you want to propose a solution, we would be extremely grateful for that as well!)

Agree on createLogger, but I also agree with Marc that const logger = new MyLogger(...); logger.info('stuff'); should work. I suspect this is just a TypeScript issue, right? Because @marc-wilson if you cast your logger to any then the leveled log methods work, right? So in that case we just need help in index.d.ts?

[Faithfinder]

Nonono, it's not. The methods don't exist on the class itself. Typescript just falsely says that they're there.

This is the specific line that does it.

winston/lib/winston/create-logger.js

Line 53 in 19ac9d8

Object.keys(opts.levels).forEach(function (level) {

[marc-wilson]

Fair enough, @Faithfinder

I would suggest (respectfully) reopening this issue since the PR doesn't address what's called out in the issue; therefore, the feature/issue is still outstanding.

I can certainly still make things work since the fix @Faithfinder did addressed a big portion of the root issue. I would just rather be able to do .info instead .log('info', ...). But, it's minor. At the very least, I would remove those methods form the typescript file so they don't pop up and people think they can use them (because they will fail).

The last few comments does bring me back to this comment, though:

#1902 (comment)

Is winston dying then?On May 27, 2023, at 16:13, wbt @.> wrote: There's also been no funding for a few years now for anybody on the core team to author or review updates and extensions, so that's not a big surprise. —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: @.>

[Faithfinder]

I can certainly still make things work since the fix @Faithfinder did addressed a big portion of the root issue. I would just rather be able to do .info instead .log('info', ...).

It's easy enough to do this manually with the added benefit of additional typesafety - you can define meta type more strongly and demand that some stuff must be logged, for example.

class CustomLogger extends Logger {
   /* other stuff */
   info = (message: string, meta: any): void => {
        this.log({ level: 'info', message, ...meta });
    };
}

At the very least, I would remove those methods form the typescript file so they don't pop up and people think they can use them (because they will fail).

Oh, but it's always been true. If you pass your levels in manually, the types don't know about it. Even if you don't -

   createLogger().emerg('message')

currently will pass typecheck, but fail at runtime.

I consider extending the Logger class a "secret advanced" feature, to use at your own risk - hence why I didn't update any docs in my PR. Changing the type definitions would be very much a breaking change for many people. You could get creative with Typescript instead and define special type for createLogger return, I guess, but the scope can get infinite here.

[DABH]

There is indeed no funding but occasionally the maintainers are still able to do reviews :) For new features and improvements there is more reliance on the community though.

That being said, I reopened the issue per the above discussion, but if somebody wants to make a PR that actually adds support for the leveled log methods for extended classes, I’d be more than happy to review and do a release. Let me know what sounds like the best option for you.

[1christianhall]

Can someone quickly jot an example of how this extension now works? The createLogger will create the derived Winston Logger, right? Not quite sure how to get an extended logger to get built in the right spot.

We need custom metadata on a per log call and so being able to construct child loggers that have an overridden log call is what we're trying to do. So createLogger.child() returning a custom instance.

Or, just to hack wrappers to make it all work.

I've been trying to just extend Logger to override the log method (since that is what I want to tweak)...and then provide my own createLogger that just takes a base logger and returns a child (which would be of this new type).

I can't get the TypeScript to work for the log method signature.

/* ideally, some way to do this sort of thing: 
    const rootLogger: MyLogger = createLogger({ 
    level: process.env.LOG_LEVEL ?? 'info',
    transports: [new transports.Console()],
    format: getFormat(),
});
(using Winston's createLogger)
*/

// but for now: 
const rootLogger: MyLogger = new MyLogger(); // etc. on config

export const createLogger = (//stuff): MyLogger => {
    // enrich stuff
    return rootLogger.child(//enrichedStuff);
};

export class MyLogger extends WinstonLogger {
    constructor(options: LoggerOptions) {
        super(options);
    }

    // have tried all sorts of flavors of trying to make this compile/work
    log(level: string, message: any, ...) {
        return this.log(level, message, //stuff);
    }
}