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

Child logger sinks don't override parent ones #15

Closed
M0ns1gn0r opened this issue Sep 17, 2024 · 5 comments
Closed

Child logger sinks don't override parent ones #15

M0ns1gn0r opened this issue Sep 17, 2024 · 5 comments
Assignees
Labels
enhancement New feature or request

Comments

@M0ns1gn0r
Copy link

I have the following configuration:

  {
    category: category('App'),
    level: 'info',
    sinks: ['console', 'backend'],
  },
  {
    category: category('App', 'Screen1'),
    level: 'debug',
    sinks: ['console'],
  },

When I log something with the Screen1 logger, the message is written twice to the console and sent once to the backend. What I want is to exclude specific categories from the sinks configured by default.

I think it is more logical to resort to the parent sinks only if the child sinks are not configured.

@dahlia
Copy link
Owner

dahlia commented Sep 19, 2024

Yes, we should give users the option to choose how they want to handle the parent logger's sinks or filters, e.g.:

[
  {
    category: ["App"],
    level: "info",
    sinks: ["console", "backend"],
  },
  {
    category: ["App", "Screen1"],
    level: "debug",
    sinks: ["console"],
    parentSinks: "override",  // "inherit" by default
  },
]

How do you see an API like the one above?

@dahlia dahlia self-assigned this Sep 19, 2024
@dahlia dahlia added the enhancement New feature or request label Sep 19, 2024
@M0ns1gn0r
Copy link
Author

In my mind, the more convenient API would be to apply only the sinks that were explicitly specified:

  {
    category: category('App'),
    level: 'info',
    sinks: ['console', 'backend'],
  },
  {
    category: category('App', 'Screen1'),
    level: 'debug',
    sinks: ['console'], // overwrites the sinks
  },
  {
    category: category('App', 'Screen2'),
    level: 'debug',
   // sinks not specified - keeps the parent sinks as is
  },

I think that's OK because the number of sinks should be relatively small. It shouldn't be a problem to duplicate a subset of the parent's sinks if needed. An additional benefit is the complete transparency of what sinks are going to be applied, without the need to mentally go through all parents and combine them.

@dahlia
Copy link
Owner

dahlia commented Sep 19, 2024

That makes sense, but there are two main considerations for me:

  1. If overriding the parent logger's sinks becomes the default behavior, it breaks backward compatibility.

  2. If you have a relatively large number of categories, it's cumbersome to manually copy the parent logger's sinks, and it's easy to make mistakes when adding or removing sinks.

Therefore, I think it should be a choice whether to inherit or override the parent logger's sinks, and the default should be inherit.

@dahlia dahlia closed this as completed in 51f9ff3 Sep 23, 2024
@dahlia
Copy link
Owner

dahlia commented Sep 23, 2024

It's implemented and documented, and will be shipped with v0.6.0, the next release. Until then, you can give it a try in advance with v0.6.0-dev.82+51f9ff32 (JSR & npm).

@dahlia
Copy link
Owner

dahlia commented Sep 24, 2024

This is shipped with LogTape v0.6.0 (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

No branches or pull requests

2 participants