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

Request metrics question #4

Closed
lilaja opened this issue Feb 3, 2021 · 3 comments
Closed

Request metrics question #4

lilaja opened this issue Feb 3, 2021 · 3 comments

Comments

@lilaja
Copy link

lilaja commented Feb 3, 2021

const timer = httpMetric.startTimer(labels);

@danopia are you sure this will be called before all other middlewares ( build in verdaccio ) ?
last time i checked it wasn't.

I mean if we start timers at the end of request processing will latencies be accurate?

@danopia
Copy link
Member

danopia commented Feb 3, 2021

The plugin is apparently placed after some built-in middlewares (CORS, etc) and before the built-in APIs.

I've previously mentioned this on the other thread @ verdaccio/verdaccio#1815 (comment):

I've found that middlewares are in front of the internal API in the express stack so the API requests (PUTs, etc) do show up. It seems possible to hack the express handler stack from within the plugin to put the metrics first in 4.x however I haven't seen that as super necessary so I haven't implemented that (meaning things like CORS requests are currently not reported).

So, if one wanted to make the timing metrics more accurate it could make sense to do that handler stack splicing to reorder our handler to the front of the line.

There's also the promise of "next major I was planning to add some sort of lifecycle so some actions can be triggered before and after" which would make it a bit cleaner; not sure if/when we'd see that from upstream though.

@lilaja
Copy link
Author

lilaja commented Feb 3, 2021

Oh sorry i missed that comment, your comment on the original issue.
Ok that make sense. Thanks!

@lilaja lilaja closed this as completed Feb 3, 2021
@danopia
Copy link
Member

danopia commented Feb 3, 2021

No worries. If for your usecase you want the 'most accurate' possible picture of request time, feel free to make a PR to do the splicing hack; something like:

// register middleware first, then:
const observer = app._router.stack.pop();
app._router.splice(0, 0, observer);

In our usecase here, we mostly want to know that package gets/puts are being processed reasonably well & the default stack order has been working well enough for us since most request time is spent within the API routes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants