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

Add early-exit to TracingChannel #10

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

Qard
Copy link

@Qard Qard commented Mar 8, 2024

Node.js 21.8.0 will add support for TracingChannel trace methods exiting early. I opted for just editing the exiting TracingChannel polyfill to fully replace the core TracingChannel up to 21.8.0 as the change required edits to every method anyway. Fully replacing it has better performance than adding a wrapping execution later unnecessarily.

@Qard
Copy link
Author

Qard commented Mar 20, 2024

Ah...I seem to have run into some bugs with the existing code. The bindStore(...) and unbindStore(...) methods do not swap to the active prototype. I'll have to figure out how to do that with the code for that living in a separate place. 🤔

@tlhunter tlhunter force-pushed the tracing-channel-early-exit branch from 3556072 to cfd0362 Compare May 7, 2024 19:39
tlhunter added a commit that referenced this pull request May 9, 2024
- some `diagnostics_channel` changes landed in Node.js in nodejs/node#51915
  - this PR adds the `TracingChannel#hasSubscribers` helper method available in Node.js v22 and v20.13
  - this PR _does not_ add the early exit functionality from that same commit
- since this PR only implements half of the functionality in that commit we aren't fully v22 and v20.13 compatible
  - for that reason I haven't updated the README to state that we track said versions
  - I did add a note to the README about this partial jump in functionality
- this PR also adds CI tests against Node.js v22.x
- partially obsoletes #10 which is a WIP that attempted to implement both
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant