Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

string-based log-level filtering #14788

Closed
1 task done
y-nk opened this issue Mar 17, 2025 · 1 comment
Closed
1 task done

string-based log-level filtering #14788

y-nk opened this issue Mar 17, 2025 · 1 comment
Labels
needs triage This issue has not been looked into type: enhancement 🐺

Comments

@y-nk
Copy link

y-nk commented Mar 17, 2025

Is there an existing issue that is already proposing this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe it

I'm looking to manage log level filtering by env var but currently it's not so smart. it is good enough for basic use but would be much nicer if it was capable of more without a lot of userland logic.

Describe the solution you'd like

I'd like to be able to keep the concise writing of await NestFactory.create(AppModule, { logger: process.env.LOG_LEVEL }) while being able to selecting multiple levels of logs with a simple parseable format.

The currently expected format is string[] (as stated in the docs ['warn', 'error']) which could be solved with a simple ?.split(','), but as you can feel, userland code would grow as it would need sanitization and maybe more capabilities.

i was wondering if a format such as >=${log_level} would be beneficial.

it could be implemented as a filterLogLevel(parsableFormat: string): string[] next to the logger class so that it's shipped with, but totally opt-in ; or a static function of Logger so that we avoid having more imports.

for the strategy itself, i would consider a parser such as:

const KNOWN_LOG_LEVELS = ['debug', 'log', 'warn', 'error', 'fatal']

function filterLogLevel(parsableString = '>=debug') {
  const str = parsableString.replaceAll(' ', '').toLowerCase()

  if (str.at(0) === '>') {
    const orEqual = str.at(1) === '='

    const logLevelIndex = KNOWN_LOG_LEVELS.indexOf(
      str.substring(orEqual ? 2 : 1)
    )

    if (logLevelIndex === -1) {
      throw new Error(`parse error (unknown log level): ${str}`)
    }

    return KNOWN_LOG_LEVELS.slice(orEqual ? logLevelIndex : logLevelIndex + 1)
  } else if (str.includes(',')) {
    return str.split(',').filter(val => KNOWN_LOG_LEVELS.includes(val))
  }

  return str
}

or something similar. i don't think it's worth implementing ranges such as < or <> since usually log filtering works best in escalation.

Teachability, documentation, adoption, migration strategy

When you want to filter log levels, a few possibilities are available:

You could hardcode it:

const app = await NestFactory.create(AppModule, {
  logger: ['error', 'fatal'],
})

Or you could make it dependent of NODE_ENV:

const app = await NestFactory.create(AppModule, {
  logger: process.env.NODE_ENV === 'production'
    ? ['error', 'fatal']
    : true,
})

You can also use our new format which proposes the following syntaxes:

  1. as string list: error,fatal
  2. as range (inclusive): >=error
  3. as range (exclusive): >log
const app = await NestFactory.create(AppModule, {
  logger: Logger.filterLogLevels(`>=error`)
})

would produce the same output as above. This would become beneficial when driving your log filtering strategy from env vars:

const app = await NestFactory.create(AppModule, {
  logger: Logger.filterLogLevels(process.env.LOG_LEVEL)
})

What is the motivation / use case for changing the behavior?

i could keep the whole code in userland or make a separate npm package for this, but it feels right to have it in the Logger since tightly coupled to the logger class. I'm sure it'd be widely used.

@y-nk y-nk added needs triage This issue has not been looked into type: enhancement 🐺 labels Mar 17, 2025
@y-nk y-nk changed the title log level string based selection string-based log-level filtering Mar 17, 2025
@kamilmysliwiec
Copy link
Member

Let's track this here #14793

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage This issue has not been looked into type: enhancement 🐺
Projects
None yet
Development

No branches or pull requests

2 participants