dnlup / doc Goto Github PK

Because a Sampler instance can be stopped, it would be nice if all the metrics supported starting and stopping/destroying on demand without creating a new Sampler.

Using this https://gist.github.com/treecy/437791c02e9edfa2e1006f1da9d34e10

check if our listing process has anything that can improved/fixed.

A new API has popped out on Node 12:

https://nodejs.org/dist/latest-v12.x/docs/api/process.html#process_process_resourceusage

It might be useful in this module.

Let's try to see if we can reuse Node types in the declarations.

Consider letting the user calling sample manually.

I must have done something wrong during a rebase. From version 2.0.2 to version 3.0.1, I found out that I am carrying duplicate commits. I have fixed that on the branch next, which is now the repo's default branch, but I cannot fix it in the already published tags. That would require unpublishing those versions from npm, rewriting the changelog, and making new tags. A lot of things could go wrong in the process. I'll do better next time. I should have known better. I sincerely apologize.

/cc @acheronfail

I'll keep this open for a bit as a reminder.

As cited in the documentation the observer should be disconnected once the callback is called for perf reason.

Keeping this field is starting to be cumbersome. Once the set of options for the event loop delay gives the user enough flexibility I think it should be removed.

Benchmarks result could be automatically parsed and validated using JSON. Now, they still have to be manually inspected.

We should fix this deprecation notice:

(node:53274) [DEP0152] DeprecationWarning: Custom PerformanceEntry accessors are deprecated. Please use the detail property.

Node diagnostic channels are useful to instrument code and I think it would be useful to support them.

Use Node diagnostics channels to gather tcp socket count.

Add benchmarks to keep the overhead in check. The manual tests I have done so far are using a barebone http server to see if its req/sec throughput is affected, which is not so far. It will be useful for the future as we add new metrics to be collected.

Consider adding garbage collection metrics.

Hello,

I've been seeing some intermittent failures with this error.

The value of "val" is out of range. It must be a safe integer greater than 0. Received 0

Based on the stack trace it's coming from here.

https://github.com/dnlup/doc/blob/next/lib/gc.js#L71

The code I have that should be throwing the error is simplified here.

const doc = require('@dnlup/doc');

const sampler = doc({
  collect: {
    gc: true,
    cpu: false,
    memory: false,
    resourceUsage: false,
    eventLoopDelay: false,
    eventLoopUtilization: false,
    activeHandles: false,
  },
});

sampler.on('sample', () => {
  // Other functionality here
});

Is there something on my end that could be wrong and causing this error to be thrown intermittently? It's not easily reproducible.

Thank you.

Node API as a reference:

• nodejs/node#35664

This also opens up a question: should this module be worker_thread aware? Meaning, if it's running in a worker thread, should it forward metrics to the main thread?

Like _getActiveHandles, process also has _getActiveRequests. It might be a useful addition.

As titled. To avoid clashes if eventLoopUtilization will define its own options.

Tests are flaky right now.

The only platform where they run consistently is Linux. On one side, I would prefer to don't limit the checks on metrics values on just being greater than zero; on the other, I should find a good balance to avoid these random errors.

Public a simple website to document this module.

I am opening this to remember to use the native Histogram when it will be supported on all Node LTS versions. As of now it's supported only on 16 (see here).

Unfortunately, tests are getting flaky again. This needs to be looked into.

Sometimes it's just a failed npm install and there's nothing I can do about it, but most of the time it's a failed check on a value, which means that this approach is not super reliable in a CI environment.

The current (not published yet) implementation causes a lot of inline caches to get megamorphic. Let's try to avoid that, if possible.

Consider adding an entry point that can be used with --require

Add process active handles. Since this uses an undocumented private API of the process, it should be optional, and the docs should report this.

As an example:

https://github.com/nearform/node-clinic-doctor/blob/501e12386aade69b2aca41b75b7272cb9f69b504/collect/process-stat.js#L45

Due to a limitation in standard types are ignored when linting.

Ref:

• https://standardjs.com/#typescript
• standard/standard#1283

I think it might be useful to track the total time and pauses taken by the gc. Right now the user has to do this computation.

As an example here's how datadog exposes it:

https://docs.datadoghq.com/tracing/runtime_metrics/nodejs/

Dependabot encountered the following error when parsing your .dependabot/config.yml:

The property '#/version' value 2 did not match one of the following values: 1
The property '#/update_configs/0/package_manager' value "npm" did not match one of the following values: javascript, ruby:bundler, php:composer, java:maven, elixir:hex, rust:cargo, java:gradle, dotnet:nuget, go:dep, go:modules, docker, elm, submodules, github_actions, python, terraform

Please update the config file to conform with Dependabot's specification using our docs and online validator.

It is time to start testing/adding esm.

Sometimes timer-based tests fail. Maybe using a fake timer would solve this.

To give a better picture of the GC work happening in the process, I think it should be included in the metric the count for each PerformanceEntry kind and, potentially, flag.

As the title says, I think doc should expose helpers to allow the user to check if a metric is supported.

In Node 14 there's this new API that should be available here:

https://nodejs.org/api/perf_hooks.html#perf_hooks_performance_eventlooputilization_util1_util2

There should be options that let the user select which metrics the package should collect.

While it is nice to have a JSON schema for defining the configuration options, the generated validate function is not small. Consider changing this approach.

Following the suggestion made by @acheronfail in #103 (comment), let's try to refactor how the default export is handled to allow TS to catch errors at compile time and not at runtime as it is now with the current definitions.

It might be useful to have a streaming api also.

This might introduce a breaking change because I am already using the data event.

https://github.com/ahmadnassri/action-dependabot-auto-merge

Work on checks in tests to decrease flakiness.