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

Exposed get values on Event to access logged param while handling hooks #706

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

SaravananSai07
Copy link

No description provided.

@rs
Copy link
Owner

rs commented Jan 17, 2025

This method won't work until the event is written as the JSON in the buffer won't be completed/valid. What is the use-case of this?

@SaravananSai07
Copy link
Author

SaravananSai07 commented Jan 17, 2025

What is the use-case of this?

Selectively publish logs to another destination based on keys passed in the log. And this happens in a hook.

@rs
Copy link
Owner

rs commented Jan 17, 2025

This approach is highly inefficient and involves unnecessary allocations. Exposing such a function could encourage its use and create the misleading impression that it is an efficient solution. In my opinion, using a logging library as an event system is not the right design choice. A more effective approach would be to introduce a dedicated layer between your emitters and the logging library. This layer can handle event dispatching to other systems as needed, ensuring a cleaner and more efficient separation of concerns.

@SaravananSai07
Copy link
Author

SaravananSai07 commented Jan 18, 2025

This approach is highly inefficient and involves unnecessary allocations. Exposing such a function could encourage its use and create the misleading impression that it is an efficient solution. In my opinion, using a logging library as an event system is not the right design choice. A more effective approach would be to introduce a dedicated layer between your emitters and the logging library. This layer can handle event dispatching to other systems as needed, ensuring a cleaner and more efficient separation of concerns.

Allocations are an issue but it is made sure that it happens only if someone needs the props. Not to be defensive, if the library provide hooks and let the clients act based on level and message, why not other params in the log?
Will drop the PR if this is not the design choice that we want to make. Just curious about the question.

@rs
Copy link
Owner

rs commented Jan 18, 2025

Ideally, you could route or filter messages based on any field, not just levels. However, due to the performance-oriented design of this library, this is not possible without parsing the buffer you just built. I’d say that if you need to do that, you’d be better off with a library that isn’t designed with zero allocation and performance as priorities. Such a library would likely end up being more optimized for your use case than parsing the buffer.

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

Successfully merging this pull request may close these issues.

2 participants