-
Notifications
You must be signed in to change notification settings - Fork 30.3k
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
Feature Request: Performance hooks entry for http #28445
Comments
@jasnell i believe you worked a lot on the |
Yes, APMs do monkey-patching of http but we have more needs then the data provided by |
@Flarna I totally agree, it's a large task to offer all the functionality of an APM in the core. |
It is certainly possible. One of the challenges with the http implementation currently is that it is fairly brittle performance wise... we would need to be careful about where we introduce data collection and how. The actual emitting of the perf entries is done at the C++ layer because there is integration into the trace events mechanism but that's a fairly minor detail. Bottom line, this is certainly doable and I'm happy to help mentor someone through what is needed. A good first step, however, would be to detail which metrics are most helpful. |
Agreed that we need to take a look at the benchmark before/after.
I suppose you were referring to the http2 implementation but i didn't find any place on the code where the pref entries are linked to the trace events mechanism (i found your old PR that was never merged there though: #18809). I would be interested to understand the connection between both
Based from the specification of OpenCensus (which i believe people studied which metrics were useful for end users when making a choice) and the already-implemented http2 metrics, i think we could target:
What do you think ? |
```js const { PerformanceObserver, performance } = require('perf_hooks'); const http = require('http'); const obs = new PerformanceObserver((items) => { const entry = items.getEntries()[0]; console.log(entry.name, entry.duration); }); obs.observe({ entryTypes: ['http'] }); const server = http.Server(function(req, res) { server.close(); res.writeHead(200); res.end('hello world\n'); }); server.listen(0, function() { const req = http.request({ port: this.address().port, path: '/', method: 'POST' }).end(); }); ```
```js const { PerformanceObserver, performance } = require('perf_hooks'); const http = require('http'); const obs = new PerformanceObserver((items) => { const entry = items.getEntries()[0]; console.log(entry.name, entry.duration); }); obs.observe({ entryTypes: ['http'] }); const server = http.Server(function(req, res) { server.close(); res.writeHead(200); res.end('hello world\n'); }); server.listen(0, function() { const req = http.request({ port: this.address().port, path: '/', method: 'POST' }).end(); }); ``` PR-URL: nodejs#28486 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Closing it landed on master |
```js const { PerformanceObserver, performance } = require('perf_hooks'); const http = require('http'); const obs = new PerformanceObserver((items) => { const entry = items.getEntries()[0]; console.log(entry.name, entry.duration); }); obs.observe({ entryTypes: ['http'] }); const server = http.Server(function(req, res) { server.close(); res.writeHead(200); res.end('hello world\n'); }); server.listen(0, function() { const req = http.request({ port: this.address().port, path: '/', method: 'POST' }).end(); }); ``` PR-URL: #28486 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Is your feature request related to a problem? Please describe.
Retrieve metrics from a HTTP server.
Describe the solution you'd like
Pretty much the same as the current implementation between
http2
andperf_hooks
: https://github.com/nodejs/node/blob/master/test/parallel/test-http2-perf_hooks.jsSince the
http
server is only js-land, i believe we could just implement by hooking to theperf_hooks
js api.I've looked a little bit at the code, we could add a stats object in the request state and emit them when the request is ended (of course only if the entry has an observer).
I'm really interested on implementing it so i'm opening it an issue to have some feedback before coding anything.
Describe alternatives you've considered
Currently i believe all APM vendors are forced to monkey-patch the event-emitter on
http.Server.prototype
and then listen on request event, add event listener to know when the response is sent and compute some metrics about both the request and the response.The text was updated successfully, but these errors were encountered: