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

feat: add context to the logger #7

Closed
okikio opened this issue Aug 17, 2024 · 14 comments · Fixed by #8
Closed

feat: add context to the logger #7

okikio opened this issue Aug 17, 2024 · 14 comments · Fixed by #8
Assignees
Labels
enhancement New feature or request

Comments

@okikio
Copy link

okikio commented Aug 17, 2024

Right now LogTape has some support for what I describe as "context". When I say context, I'm referring to a way to externalized LogTape log function parameters.

Right now you can do

const logger = getLogger(["package", "module"])
logger.debug `This is a debug message with ${value}.`;
logger.fatal("This is a fatal message with {value}.", { value });logger.debug(l => l`This is a debug message with ${computeValue()}.`);
logger.debug("Or you can use a function call: {value}.", () => {
  return { value: computeValue() };
});

This is awesome but it has a 2 key weaknesses

  1. All your logs have to be strings, unlike console.log where you can log almost anything.
  2. You can't add properties when using tagged template functions.

The goal is to add context to classes such that these workflows can be supported.

// Logs to the general logger by default
logger.info`General Log ${5}`;

// Logs to the init logger with a custom template
logger.with({ logger: "init", message: "start {time}" }).warn({ time: Date.now() });

// Logs an error to the init logger
logger.with({ logger: "init" }).error("error {message}", { message: "Error message" });

// Logs an error to the init logger (with context)
logger.with({ logger: "init" }).context({ message: "Error message" }).error("error {message}");

// Logs an error and warning to the init logger (with context no. 2)
logger.with({ logger: "init", context: { message: "Error message" } }).error("error {message}").warn("warning {message}");

// Shortcut for init logger
logger.init.error("Initialization failed: {error}", { error: new Error("cool") });

// Logs to the custom logger
logger.customLogger.info("Custom logger in action");

// Shortcut for build logger
logger.build.fatal`A fatally cool message`;

// This will now cause a TypeScript error, as 'nonExistentLogger' is not registered
// logger.nonExistentLogger.info("This should not compile");

What makes this workflow somewhat interesting is that you can define the properties the log functions should use via .with({...}) but on each call line the properties are reset to their defaults. Allowing for some pretty cool use cases.

In terms of dealing with the first weakness, with the addition of a .with(...) method you would be able to just log anything not just strings, the .with(...) will also help with tagged template functions because you can just use .with(...) to store the properties.

The inspiration for this idea comes from https://jsr.io/@libs/logger/doc/~/Logger which you might find interesting.

@dahlia
Copy link
Owner

dahlia commented Aug 17, 2024

Okay, now I roughly understand what you mean by context, thanks for the explanation! Here are some questions:

  • What are these names like init and customLogger? Do they differ from categories (which already exist in LogTape)?

  • Would shortcuts be useful? In my opinion, shortcuts are difficult to implement type-safely in TypeScript.

@okikio
Copy link
Author

okikio commented Aug 17, 2024

init and customLogger refer to loggers, they would build on LogTapes pre-existing categories but make it easier to send logs to each category.

E.g. a function that wraps the various category loggers

export const initLogger = getLogger(["package", "init"]);
export const buildLogger = getLogger(["package", "build"]);
export const generalLogger = getLogger(["package", "general"]);

// Interface to extend Logger with custom loggers
export type LoggerWithCustomLoggers<L extends string> = Logger<L> & {
  [P in L]: Logger<L>;
}

// Function to create a logger wrapper with typed custom loggers
export function createWrapper<L extends string>(options: LoggerOptions<L> = {}): LoggerWithCustomLoggers<L> {
  const { logger = "general" as L, template, context = {}, loggers = {} } = options;
  const loggers = {
    general: generalLogger,
  } as Record<L | "general", LogTapeLogger>;
  let currentLogger = logger;  
  
  const result: LoggerWithCustomLoggers<L> = {
    with(options: LoggerOptions<L> = {}) {
      if (options.logger) {
        currentLogger = options.logger;
        if (!loggers[options.logger]) {
          throw new Error(`Logger "${options.logger}" is not registered.`);
        }
      }
      if (options.template) {
        template = options.template;
      }
      if (options.context) {
        Object.assign(context, options.context);
      }
  
      return loggers[options.logger].with({ template, context });
    }
  }

  // Dynamically create getters for custom loggers
  for (const [name, _logger] of Object.entries<LogTapeLogger>(loggers)) {
    loggers[name as L] = _logger;    
    Object.defineProperty(result, name as L, {
      get() {
        currentLogger = name;
        return result;
      },
      configurable: true,
      enumerable: true,
    });
  }
  
  return result;
}

const logger = createWrapper({
  loggers: {
    init: initLogger,
    build: buildLogger,
    customLogger: getLogger(["package", "custom"]),
  },
});

^ This is psuedo-code there are probably a number of edge cases it doesn't take into account, but hopefully it helps paint a picture of what I'm think of.

Just for clarity when you say shortcuts are you referring to the .customLogger, .init, etc...?

@dahlia
Copy link
Owner

dahlia commented Aug 17, 2024

Just for clarity when you say shortcuts are you referring to the .customLogger, .init, etc…?

Yeah, they were what I referred to. By the way, I'd like to keep our focus on contexts first. Could we deal with shortcuts in a separate issue? 😅 In the meantime, we could simply use variables and getChild() method.

Coming back to contexts, I think the two methods, with() and context(), have quite a bit of overlap in purpose. If I understand them correctly, with() configures both a category and properties, whereas context() configures only properties, right? In my opinion, we need only the context() method because there's already the getChild() method.

const logger = getLogger("package");
logger.getChild("init").error("error {message}", { message: "Error message" });
logger.getChild("init").context({ message: "Error message" }).error("error {message}");

@okikio
Copy link
Author

okikio commented Aug 17, 2024

Ok, that should work, we can discuss the shortcuts in a separate issue. All in all, you get most of what I'm imaging. There is something to note though, we'd probably also want a .message(...) method, in order to tackle logs that aren't strings, so LogTape can properly be used in place of console.log.

E.g.

function fn() {
  return 10;
} 

const logger = getLogger("package");
logger.getChild("init").error("error {message}", { message: "Error message" });
logger.getChild("init").context({ message: "Error message" }).message("error {message}").error();

logger.getChild("init").context({ message: "Error message" }).message(fn).error(); 
// This should stringify `fn` and not apply any magic, ideally we'd let the sink's handle how to stringify none string values
// We'd still keep the context and emit said context as `properties` in the log record

@dahlia
Copy link
Owner

dahlia commented Aug 17, 2024

Hmm, okay. That's basically a syntactic sugar for the below invocation, right?

lgoger.getChild("init").context({ message: "Error message" }).error("{fn}", { fn });

@okikio
Copy link
Author

okikio commented Aug 17, 2024

No, not quite since we'd want the sinks themselves to handle the serialization. I used the example of a function, but a more apt example would be the DOM. On a browser you can log a DOM element to the console, if you were using the console sink you'd want the actual element logged to the console, so you can view it, get live updates, etc... But for the file sink you'd want the DOM element to be a string, so it can be viewed. Basically we'd want the sink to handle the serialization.

In this example, by doing this we'd be manually coercing the type of the function into a string before the message is emitted as a log record.

logger.getChild("init").context({ message: "Error message" }).error("{fn}", { fn });

To enable what I'm describing we'd ideally want

logger.getChild("init").context({ message: "Error message" }).message(fn).error();
// Where `fn` is the message that's passed directly to the log record (with no modifications)

Hopefully that makes sense.

@dahlia
Copy link
Owner

dahlia commented Aug 17, 2024

There seems to be a misunderstanding of one fact! Properties are not serialized when it gets into a LogRecord. Here's a demo:

const element = document.getElementById("element");
console.info(element);
getLogger("test").info("{element}", { element });

Developer Console

@dahlia dahlia added the enhancement New feature or request label Aug 17, 2024
@okikio
Copy link
Author

okikio commented Aug 18, 2024

Oh, I did not know about that...that is pretty cool

@dahlia
Copy link
Owner

dahlia commented Aug 19, 2024

About a method name: Is context() okay as a method name? Or should it be with()?

@okikio
Copy link
Author

okikio commented Aug 19, 2024

I personally prefer with()

@dahlia dahlia self-assigned this Aug 19, 2024
@dahlia
Copy link
Owner

dahlia commented Aug 19, 2024

Okay, let's go with with() then. I'll try to implement it within the week.

@okikio
Copy link
Author

okikio commented Aug 19, 2024

Thanks for taking the time 👍🏽

dahlia added a commit that referenced this issue Aug 19, 2024
Close #7
@dahlia dahlia mentioned this issue Aug 19, 2024
@dahlia
Copy link
Owner

dahlia commented Aug 19, 2024

@okikio I implemented it. Could you take a look on #8?

@dahlia dahlia closed this as completed in #8 Aug 20, 2024
@dahlia
Copy link
Owner

dahlia commented Aug 20, 2024

Contexts will be shipped in the next release (0.5.0), and are available now at 0.5.0-dev.60+f819929c (JSR & npm).

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

Successfully merging a pull request may close this issue.

2 participants