RuntimeTools / appmetrics-dash

A data visualizer that uses " Node Application Metrics" (appmetrics) to monitor and display Node.js application data as a html web application.
Other
452 stars 55 forks source link

HTTP incoming requests not showing #63

Open tobespc opened 7 years ago

tobespc commented 7 years ago

this is for https://github.com/RuntimeTools/appmetrics/issues/408

only happens when using .attach() method. Seems we need to require appmetics first then pass it into the constructor for the dashboard

var appmetrics = require('appmetrics') var dash = require('appmetrics-dash') var express = require("express"); var app = express(); dash.attach({appmetrics:appmetrics});

docs change required

sam-github commented 7 years ago

That's interesting, I would expect it to happen for attach() or monitor() (since attach just calls monitor).

Perhap the difference is between monitor's two modes, the "create an internal express/http Server" mode, and the "replace an http Server's even listeners" mode? Attach only uses the latter, because it attaches to a pre-existing http Server, it doesn't create its own, but the example code uses the "open a second server on a second port" mode.

sam-github commented 7 years ago

Also, appmetrics is generally expected to be the first module required, which makes it awkward when its required internally by appmetrics-dash, and appmetrics-dash allows an http server to be injected, which means it supports being called to late, doesn't it?

sjanuary commented 7 years ago

This is because of how the probes work (they hook into 'require' calls). With monitor we call require('http') after monitor is called (https://github.com/RuntimeTools/appmetrics-dash/blob/master/lib/appmetrics-dash.js#L115) in order to create our own http server, so yes if you pass your own server into monitor you would see the same problem and would need to work around it in the same way.

@tobespc the docs update needs to also cover calling monitor and passing in a server

sjanuary commented 6 years ago

Fixed by #72