-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
refactor(metrics): move to core #12386
Conversation
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.
Nice to see 100 lines removed. How's the performance?
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.
Can you stabilize typing for Deno.OpMetrics
in this PR as well?
Before (main):
After (#12386):
|
Avoids overhead of wrapping ops, also deprecates byte fields (legacy of previous buffer-based op-layer)
Can deprecate in a separate PR
8337b97
to
963449b
Compare
The type declarations are still not updated for |
Should we stabilize them after stripping the byte fields (and other changes I would like to make) or stabilize |
I'm fine with both approaches as there aren't many known consumers of this API. Your call. |
Lets stabilize as is and break in follow-up changes, we can break for 2.0 since the changes would mostly be cleanups instead of imperative improvements |
Avoids overhead of wrapping ops (extra alloc due to reboxing futures for async ops) also stabilizes
Deno.metrics().ops
.I'll deprecate byte fields (legacy of previous buffer-based op-layer) in a follow-up PR