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

chore: Add a internal logger for debugging #2298

Open
maschad opened this issue May 14, 2024 · 6 comments · May be fixed by #2366
Open

chore: Add a internal logger for debugging #2298

maschad opened this issue May 14, 2024 · 6 comments · May be fixed by #2366
Assignees
Labels

Comments

@maschad
Copy link
Member

maschad commented May 14, 2024

Motivation

When completing the Utf8ToString implementation I realize we don't log some warnings.

We have deprecated @ethersproject/logger but it may still be useful to log some potential inconsistencies as warnings, debug or info logs.

Usage example

	import  { logger } from '@fuel-ts/logger'

	function warnUser( 
	  reason: string,
	  offset: number,
	  bytes: Uint8Array,
	) {
	
		logger.warn(`invalid codepoint at offset ${offset}; ${reason}, bytes: ${bytes}`);
		return offset
	}

	// Maximum code point
	if (res > 0x10ffff) {
      	i += warnUser('OUT_OF_RANGE', i - 1 - extraLength, bytes, result, res);
      	continue;
    }

Possible implementations

We could create a simple custom logger on top of a tool like debug

Related: #1360

@maschad maschad self-assigned this May 14, 2024
@maschad maschad added the chore label May 14, 2024
@arboleya
Copy link
Member

While I understand the motivation behind something more robust with multi-level logging (log, info, warn, etc.) I usually lean more towards simplicity, a territory where debug shines.

I don't have a hard opinion on this, but having a first proposal and execution plan could be good here to gather people's opinions before spending too much time on such a thing, as it can be time-consuming to apply across the whole codebase.

cc @Dhaiwat10 - since you mentioned this wrt to the fuels CLI.

@maschad maschad removed their assignment May 14, 2024
@maschad
Copy link
Member Author

maschad commented May 14, 2024

Right @arboleya so I my initial proposal is just create a logger package e.g. @fuel-ts/logger which would just have an interface which would allow us to have loggers per package.

export interface Logger {
  (formatter: any, ...args: any[]): void
  error(formatter: any, ...args: any[]): void
  warn(formatter: any, ...args: any[]): void
  info(formatter: any, ...args: any[]): void
  enabled: boolean
}

and this would have some default methods to create a logger per package e.g.:

/**
 * Creates a logger for the passed package name.
 *
 * @example
 *
 * ```TypeScript
 * import { logger } from '@fuel-ts/logger'
 *
 * const log = logger('account')
 * log.info('hello world')
 * // logs "account: hello world"
 * ```
 */
export function logger (name: string): Logger {
  // info logging is a no-op by default
  let info: debug.Debugger = createDisabledLogger(`${name}:info`)

  // look at all the debug names and see if info logging has explicitly been enabled
  if (debug.enabled(`${name}:info`) && debug.names.map(r => r.toString()).find(n => n.includes(':info')) != null) {
    info = debug(`${name}:info`)
  }

  return Object.assign(debug(name), {
    error: debug(`${name}:error`),
    info
  })
}

function createDisabledLogger (namespace: string): debug.Debugger {
  const logger = (): void => {}
  logger.enabled = false
  logger.color = ''
  logger.diff = 0
  logger.log = (): void => {}
  logger.namespace = namespace
  logger.destroy = () => true
  logger.extend = () => logger

  return logger
}

We could also have some useful default methods such as a wallet logger

/**
 * Create a component logger that will prefix any log messages with a truncated
 * wallet address.
 *
 * @example
 *
 * ```TypeScript
 * import { walletLogger } from '@fuel-ts/logger'
 * import {HDWallet} from 'fuels'
 *
 * const hdwallet = HDWallet.fromSeed('seed')
 * const logger = walletLogger(hdwallet)
 *
 * const log = logger.forComponent('account')
 * log.info('hello world')
 * // logs "0x…21243:account hello world"
 * ```
 */
export function walletLogger (HDWallet: wallet, options: Partial<WalletLoggerOptions> = {}): ComponentLogger {
  return prefixLogger(truncateWalletAddress(wallet, options))
}

and one per fuel node for instance.

/**
 * Create a component logger that will prefix any log messages with the passed
 * string.
 *
 * @example
 *
 * ```TypeScript
 * import { prefixLogger } from '@fuel-ts/logger'
 *
 * const logger = prefixLogger('my-fuel-node')
 *
 * const log = logger.forComponent('account')
 * log.info('hello world')
 * // logs "my-fuel-node:account hello world"
 * ```
 */
export function prefixLogger (prefix: string): ComponentLogger {
  return {
    forComponent (name: string) {
      return logger(`${prefix}:${name}`)
    }
  }
}

export interface ComponentLogger {
  forComponent(name: string): Logger
}

These are some initial ideas, open to feedback @nedsalk @petertonysmith94 @danielbate @Dhaiwat10 @Torres-ssf

@petertonysmith94
Copy link
Contributor

I like the idea of this!

I'd add that it would be nice to encapsulate our CLI logger, do you think this would come under the @fuel-ts/logger namespace?


export interface Logger {
  (formatter: any, ...args: any[]): void
  error(formatter: any, ...args: any[]): void
  warn(formatter: any, ...args: any[]): void
  info(formatter: any, ...args: any[]): void
  enabled: boolean
}

Out of curiosity, how did you envision the formatter field being used?

@danielbate
Copy link
Contributor

In the interest of reducing rather than increasing packages, could this go inside utils re #1517?

@maschad
Copy link
Member Author

maschad commented May 20, 2024

Out of curiosity, how did you envision the formatter field being used?

The formatter would be useful for displaying different types of debug info - for instance %b could be for displaying a b256 address, this is because generally you don't want to add extra compute to debug logs by default so essentially that formatting / conversion would only be done when logs are enabled (which would not be the case in prod of course).

In the interest of reducing rather than increasing packages, could this go inside utils re #1517?

I am open to that, my preference on having this as a separate package is that it decouples it from other utilities, I generally find that utils directories lend themselves to bloat as they tend to be too ambiguous.

@maschad maschad self-assigned this May 20, 2024
@maschad maschad linked a pull request May 22, 2024 that will close this issue
@petertonysmith94
Copy link
Contributor

The formatter would be useful for displaying different types of debug info - for instance %b could be for displaying a b256 address, this is because generally you don't want to add extra compute to debug logs by default so essentially that formatting / conversion would only be done when logs are enabled (which would not be the case in prod of course).

A wealth of knowledge, thanks for explaining 😄 One thing I'd also add would be I'd prefer unknown to any.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants