Add a Proper Logging Framework

[dbudzins]

We are mostly just using the logDev method everywhere. We should really add a logging framework with an adjustable log level.

[ChristiaanScheermeijer]

@dbudzins @AntonLantukh @royschut @langemike @MelissaDTH

In the next sprint, we will implement a logging application, and I think this might help with that.

We've used Pino before, but this is mainly created for NodeJS applications and might be overkill.

Now that we have services, I am considering creating a LoggingService with a simple interface. For example:

interface ILoggingService {
  debug(namespace: string, ...args: unknown[]): void;
  info(namespace: string, ...args: unknown[]): void;
  warn(namespace: string, ...args: unknown[]): void;
  error(namespace: string, error: Error, ...args: unknown[]): void;
  fatal(namespace: string, error: Error, ...args: unknown[]): void;

  // optionally implement support for (custom) transporters, for example:
  // consoleTransporter, sentryTransporter, cloudWatchTransporter, ... 
  registerTransporter(transporter: (level: string, args: unknown[], error?: Error) => void): void;
}

I see two potential ways of adding a logging implementation.

Allow for registering custom transporters (see above)

• Pro: This allows for multiple transporters (easier)
• Pro: Less logic to re-implement (determine the log level and which logs to output)
• Con: More logic to implement initially

Override the default LoggingService with a custom implementation

• Pro: Full control
• Pro: Easier to maintain and implement
• Con: Custom implementations need to implement more

Although, a third solution is to override the existing LoggingService and only override a generic method that handles the actual logging. This might be even more simple:

class SentryLoggingService extends LoggingService {
  override log(level: string, namespace: string, args: unknown[], error?: Error) {
    // console logging
    super.log(level, namespace, args, error);

    // log to Sentry
    Sentry.captureMessage(`[${namespace}]: ${args.join(',')}`, level);
  }
}

Should we accept only one string and mark the rest as additional data (context) which may be useful debug data?

interface ILoggingService {
  debug(namespace: string, message: string, ...additionalData: unknown[]): void;
  // ...
}

What do you think?

[dbudzins]

I think the first option (a single service with configurable transports) is the cleanest. I'm not sure how it's more work initially, maybe it's just being careful about separation of concerns? Options 2 or 3 are then always available to anyone else who needs anything more complex.

[AntonLantukh]

We could probably stick to the approach we already have with Account / Checkout services:

1. Create LoggingService which is "abstract class" and required methods like "registerTransporter" and "log".
2. Create basic implementation.
3. When needed basic implementation can be replaced with a custom one.

This one looks like both first and second option?

[ChristiaanScheermeijer]

@dbudzins @AntonLantukh, thanks for the feedback!

I don't think it's needed for a mechanism like integrations because it's just one service instead of multiple services for one integration. We also don't apply this for the EntitlementService (yet).

If we go for option 1, I think the default console transporter should also be registered like custom transporters. The question is, where do we create and register this transporter?

In register.ts

// Logging
container.bind(LoggingService).toSelf();
container.get(LoggingService).registerTransporter(consoleTransporter);

We can also leverage the container with multiple services. This is an alternative solution to the registerTransporter method.

// Logging
container.bind(LogginService).toSelf();
container.bind(LOG_TRANSPORTER_TYPE).to(ConsoleTransporter);
container.bind(LOG_TRANSPORTER_TYPE).to(AnalyticsTransporter);
container.bind(LOG_TRANSPORTER_TYPE).to(SentryTransporter);

The LoggingService will just call the log method on all transporters and can be easily extended with more (custom) log transporters:

class LoggingService {
  log(...args) {
    container.getAll(LOG_TRANSPORTER_TYPE).map(transporter => transporter.log(...args));
  }
}

[dbudzins]

Console default makes sense for dev and test, but not prod or demo. Support for multiples is nice.

I'm assuming most transporters will need some configuration, too right? My first thought is to put these settings into the ini file because they are mostly set-and-forget.

[ChristiaanScheermeijer]

Yes, it would make sense to have some configuration available per transporter. I would also like to configure the log level per transporter as well.

Ini files may sound great, but the downside is that logging is only initialized when the ini file is loaded. This is too late to capture errors with Sentry (for example, when a JS or the ini file fails to load).

I don't have an alternative solution (yet) though...

[ChristiaanScheermeijer]

These are the current options:

1) Using an environment variable (works only when running build). For example: APP_LOGGING=console:info,sentry:error,gtm:off 2) Configure the log settings in the index.html using a script tag 3) Configure the log settings in the ini settings file 4) Configure the log settings in the app config (not preferred)

Do you see alternatives?

[ChristiaanScheermeijer]

I have drafted a quick proposal. This does require editing the source to modify the logging behavior, but I assume this will be the case anyways when adding custom logging frameworks. I think the repo should only provide the ConsoleTransporter configured to log everything when running in dev mode.

Let me know what you think.

enum LogLevel {
  DEBUG,
  INFO,
  WARN,
  ERROR,
  FATAL,
  SILENT,
}

interface LoggingTransporter {
  logLevel: LogLevel;
  log(level: LogLevel, scope: string, message: string, extra?: Record<string, unknown>, error?: Error): void;
}

const LOG_TRANSPORTER_TYPE = Symbol('LOG_TRANSPORTER_TYPE');

class LoggingService {
  private log(logLevel: LogLevel, scope: string, message: string, extra?: Record<string, unknown>, error?: Error) {
    container.getAll<LoggingTransporter>(LOG_TRANSPORTER_TYPE).forEach((transporter) => {
      // preventing the call here does limit options like adding breadcrumbs (which might be inferred from info logs) to Sentry calls
      if (logLevel >= transporter.logLevel) {
        transporter.log(logLevel, scope, message, extra, error);
      }
    });
  }

  debug = (scope: string, message: string, extra?: Record<string, unknown>) => this.log(LogLevel.DEBUG, scope, message, extra);
  info = (scope: string, message: string, extra?: Record<string, unknown>) => this.log(LogLevel.INFO, scope, message, extra);
  warn = (scope: string, message: string, extra?: Record<string, unknown>) => this.log(LogLevel.WARN, scope, message, extra);
  error = (scope: string, message: string, error: Error, extra?: Record<string, unknown>) => this.log(LogLevel.ERROR, scope, message, extra, error);
  fatal = (scope: string, message: string, error: Error, extra?: Record<string, unknown>) => this.log(LogLevel.FATAL, scope, message, extra, error);
}

class ConsoleTransporter implements LoggingTransporter {
  logLevel: LogLevel;

  constructor({ logLevel }: { logLevel: LogLevel }) {
    this.logLevel = logLevel;
  }

  log(level: LogLevel, scope: string, message: string, extra?: Record<string, unknown> | undefined, error?: Error | undefined): void {
    if (this.logLevel === LogLevel.SILENT) return;

    console.info(`[${level}] ${scope}: ${message}`);

    if (level === LogLevel.ERROR || level === LogLevel.FATAL) {
      console.error(error);
    }

    if (extra) {
      // eslint-disable-next-line no-console
      console.table(extra);
    }
  }
}

// Configure the generic LoggingService in the common package
container.bind(LoggingService).toSelf();

// Configure the transporter in web/src/modules/register.ts
container.bind<LoggingTransporter>(LOG_TRANSPORTER_TYPE).toDynamicValue(
  () =>
    new ConsoleTransporter({
      logLevel: import.meta.env.DEV ? LogLevel.DEBUG : LogLevel.SILENT,
    }),
);

// This is a custom transporter added by the publisher
container.bind<LoggingTransporter>(LOG_TRANSPORTER_TYPE).toDynamicValue(
  () =>
    new SentryTransporter({
      logLevel: import.meta.env.DEV ? LogLevel.SILENT : LogLevel.ERROR, // log ERROR and FATAL to Sentry on production
      includeBreadcrumbs: true, // include breadcrumbs example configuration
    }),
);

[ChristiaanScheermeijer]

I think we can rename everything to LogService and LogTransporter.