-
Notifications
You must be signed in to change notification settings - Fork 30k
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
diagnostics_channel: capture console messages #56292
base: main
Are you sure you want to change the base?
Conversation
6ba11e5
to
07fbcd4
Compare
07fbcd4
to
8094626
Compare
8094626
to
b3830bf
Compare
I lack context on this, so I might be missing some point. Can this use the Inspector protocol instead of adding hooks directly into console object. In-process inspector API can be configured to watch for the messages? This way there will be less coupling. |
The Inspector API is substantially more expensive. This is meant for being able to rewrite potentially all log messages in production, so it needs to be fast. |
Thanks for the reply. |
Maybe worth to extend test to verify adding/removing/replacing elements, Main problem I see with mutating diagnostics channels is the undefined sequence if there are more subscribers. |
|
||
* `args` {any\[]} | ||
|
||
Emitted when `console.log()` is called. Receives and array of the arguments |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe describe that this is the arguments array and therefore mutation is possible.
I've added diagnostics_channel support to capture and modify inputs to the main console methods. This enables some useful things like APMs injecting tracing data into log messages to correlate log messages to the requests from which they originated. You can also replace the args list entirely, so you could apply your own custom formatting to produce JSON logs instead or things like that.
cc @nodejs/diagnostics @nodejs/console