gremo/nest-winston

The context maybe better to set protect

mostafa8026 opened this issue ยท 5 comments

I aim to create my logger service, but I can't set context because it is protected. I think it is better to set it protected.
take a look at my implementation:

import { Injectable, Scope, Inject, LoggerService as NestLoggerService } from '@nestjs/common';
import { WinstonLogger, WINSTON_MODULE_PROVIDER } from 'nest-winston';
import { Logger } from 'winston';

@Injectable({
  scope: Scope.TRANSIENT,
})
export class LoggerService extends WinstonLogger implements NestLoggerService {

  constructor(@Inject(WINSTON_MODULE_PROVIDER) private readonly _logger: Logger) {
    super(_logger);
  }

  public stat(message: any, context?: string) {
    context = context || this.context;

    if ('object' === typeof message) {
      const { message: msg, ...meta } = message;

      return this._logger.log({
        level: 'stat',
        message: msg as string,
        context,
        ...meta,
      });
    }

    return this._logger.log({
      level: 'stat',
      message: message,
      context,
    });
  }

}

Do you accept PR?

Any news?

gremo commented

I'm sorry for the very late response, I'm quite busy recently with my personal and working life. I'll try to follow more this library that, honestly, went so far and has been used by many. Didn't expected that ๐Ÿ˜„

Anyways, is there something that prevents you to call setContext on the logger class itself?

I created my own LoggerService that utilizes your useful class. I have two option:

1- Extends from the WinstonLogger and then add my own logging functions, as I've mentioned it before.
2- Associate your class as an instance inside my LoggerService like this:

@Injectable({
  scope: Scope.TRANSIENT,
})
export class LoggerService implements NestLoggerService, TypeOrmLogger {

  protected tags: StatTag[] = [];
  protected TYPEORM_CONTEXT_NAME = 'TypeORM';
  private context: string;

  constructor(
    @Inject(WINSTON_MODULE_NEST_PROVIDER) private readonly _logger: WinstonLogger,
    private _sharedSentryService: SharedSentrySingletonService) {
    this._logger.setContext(this.context);
  }

  private setContext(context: string) {
    this.context = context;
  }

  private getSentryBreadCrumb(level: Severity, message: any, context: string, type = 'debug') {
    let sentryMessage = '';
    if (typeof message === 'string') {
      sentryMessage = message;
    } else {
      sentryMessage = message?.message || 'empty message';
    }

    return {
      message: sentryMessage,
      level: level,
      type: type,
      category: context ?? 'logger',
      data: message,
    };
  }

  public log(message: any, context?: string): any {
    this._sharedSentryService.instance().addBreadcrumb(
      this.getSentryBreadCrumb(Severity.Info, message, context),
    );
    return this._logger.log(message, context);
  }

  /**
   * You can set default logs in your class in order to not pass them everytime for stat.
   * @param tags
   */
  public setTags(...tags: StatTag[]) {
    tags.forEach(_item => {
      if (!this.tags.includes(_item)) {
        this.tags.push(_item);
      }
    });
  }

  /**
   * Whenever you want to trace any tag stats you can use this method
   * @param tags this tags help you to generate diagrams
   * @param message the main message that describe your stat purpose
   * @param meta any related objects to this stat
   * @param context currently this context does not work, issue in progress
   */
  public stat(tags: StatTag[], message: string, meta: Record<string, any>, context?: string) {
    context = context || this.context;

    this.tags.forEach(_item => {
      if (!tags.includes(_item)) {
        tags.unshift(_item);
      }
    });

    this._sharedSentryService.instance().addBreadcrumb(
      this.getSentryBreadCrumb(Severity.Log, message, context),
    );

    return this._logger.log({
      time: new Date(),
      level: 'stat',
      tags,
      message: message,
      context,
      ...meta,
    });
  }

And this association works fine for now, But I had to create the setContext functions that you've already written, and it can be removed if you make the context public (or protected) ๐Ÿ˜‰

Anyways, I think current implementation is ok for now (Do you have any idea?).