dbfannin / ngx-logger

Angular logger
MIT License
428 stars 110 forks source link

NGXLogger support for non ng components / classes #36

Closed IsNull closed 6 years ago

IsNull commented 6 years ago

As the logger currently must be injected, it is cumbersome if you have a complex domain model in the web app consisting of plain and simple classes.

The way I know loggers from .NET / Java is that they can be used by static references. The question is if it makes sense to apply this pattern here to?

A middle ground probably could be a global, static holder:

class Foo {
    private static logger = NgxLoggerHolder.instance;
}

Optionally, depending on required information, we could also allow passing the current class, giving the opportunity to include the class name in the log.

class Foo {
    private static logger = NgxLoggerHolder.forClass(Foo);
}

The way I see it this system could co-exist with the current implementation and basically proxy to the configured instance once ready.

What are your thoughts on this?

dbfannin commented 6 years ago

I've been putting some thought into this for a while. Here are the big problems I would like to solve.

So my thought is this doing something like this...

// manages the loggers
class NGXLogger {
  private loggers: {};

  constructor(public http?: HttpClient) {
    this.createLogger()
  }

  createLogger(loggerId: string = 'default', config: LoggerConfig) {
    const logger = this.getLogger(loggerId);

    if (logger) {
      return logger;
    }

    this.loggers[loggerId] = new Logger(this.http, config);

    return this.loggers[loggerId];
  }

  getLogger(loggerId: string = 'default') {
    return this.loggers[loggerId];
  }
}

// All of the logger functionality
class Logger {

  constructor(private http: HttpClient, private config: LoggerConfig) {
  }

  trace() {
  }

  debug() {
  }
}
IsNull commented 6 years ago

This module is built for angular, but if I have non-angular classes in my project, I would like to use the same logger

To achieve this, I think one approach would be:

common-typescript-library

ngx-logger

One important point is that a constructor is not part of the API, so it will be transparent to the consumers what is required by the logger and also where the logs will go.

I would like to only specify the config at the time of creating a logger instance, not on importing of the module

Well, usually you want to have the same config for the logger per project. The interesting part is that you want to config a logger implementation which you do not know in advance (i.e. . the generic logger builder API) - so i would go with a simple key/value approach.

A logger implementation will have access to all properties and will respect the ones he can.

However - maybe we should consider using an existing solution like this https://github.com/mreuvers/typescript-logging and provide additional features. Such as angular integration and server-side support.

dbfannin commented 6 years ago

@IsNull, sorry I'm just now getting back around to this. With the updates in 2.1.0, you can create a custom logger that is separate from the standard provided logger. I wonder if this gets us one step closer to your proposal.

We let the app manage creating the custom logger, and supplying it's own non-angular modules with that logger in whatever capacity the developer chooses. An improvement on that, would be to establish our own pattern, and support it by default, or update the readme with a section on how we suggest others do it.

Again, sorry I'm just now getting back to this thread. Thanks!

IsNull commented 6 years ago

I ended up creating a new logger library based solely on TypeScript, supporting global configuration and configuration of custom appenders and formatters: @elderbyte/ts-logger This was required as we already have lots of different TS libs and need a global log system.

If you are interested, we could merge our efforts and this ngx-logger could use the generic ts-logger under the hoods.

dbfannin commented 6 years ago

I've been giving this some thought. I think for now we'll keep our current logging functionality. If we get to a point that it feels right to separate, maybe we can chat then about consolidating our efforts. As of now, I don't see any benefits for NGX Logger adding TS Logger as a dependency.

I'll go ahead and close this issue for now, since the logger now supports a way to do this.

Thanks!