npm / npm-registry-client

http://npm.im/npm-registry-client
ISC License
264 stars 108 forks source link

Add support for cli statistics reporting #138

Closed iarna closed 7 years ago

iarna commented 8 years ago

Usage should look like:

client.sendAnonymousCLIMetrics(uri, params, cb)

It is important that calls to this function not hold the event loop open. That is, if the program would naturally exit before this completes, we shouldn't block that exit. Ordinarily you would achieve this by unrefing any sockets/timers involved. This may not be feasible with request.


The end point is:

PUT /-/npm/anon-metrics/v1/:metricId

Where metricId would be a uniqueid to this set of stats. If the server sees the same uniqueid more than once then the latter version will supersede the former. This means that if one PUT fails, we can try again later with newer data and not end up double counting.

This end point expects a JSON object and ignores any keys it doesn't understand. The initial keys would be:

from: 'W3CDTF',
to: 'W3CDTF',
successfulInstalls: #,
failedInstalls: #

The cli issue for this: https://github.com/npm/npm/issues/12529

The registry issue for this: https://github.com/npm/registry/issues/11

othiym23 commented 8 years ago

The current name of the method lacks a verb, which is inconsistent with the rest of the API – maybe sendAnonymousCLIStats?

iarna commented 8 years ago

sounds good

iarna commented 8 years ago

You used the term metrics in other discussion of this and I like that better than stats, so I've switched over to that instead.

iarna commented 8 years ago

Hey @lazywithclass, I think this is ready. An initial version that does not try to implement the "don't make the program wait" semantics would be fine. That is, if you want to do a version of this that just uses request directly, that would be quite helpful as it'd give our registry team code to work against and would let us proof-of-concept.

iarna commented 8 years ago

(Well I thoguht the don't make it wait would involve unrefing the socket, but it seems that doesn't apply while connecting, so we may actually end up shunting calls to this off to a separate process in the npm portion of this code.)

lazywithclass commented 8 years ago

Thanks for this @iarna, I will dive into the details.

A couple questions: