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

nodejs_version_info empty after registry reset #238

Closed
ssube opened this issue Jan 4, 2019 · 4 comments · Fixed by #338
Closed

nodejs_version_info empty after registry reset #238

ssube opened this issue Jan 4, 2019 · 4 comments · Fixed by #338
Assignees
Labels

Comments

@ssube
Copy link

ssube commented Jan 4, 2019

I've been using this library in a small bot with great results, and recently implemented a feature where SIGINT and SIGHUP will reload the config file and reset any stateful counters (for development). I'm using collectDefaultMetrics({ registry }) to collect general Node metrics and call registry.resetMetrics upon receiving the signals.

After calling resetMetrics and waiting for the next interval tick, most metrics re-appear, but not all of them:

# HELP nodejs_heap_space_size_available_bytes Process heap space size available from node.js in bytes.
# TYPE nodejs_heap_space_size_available_bytes gauge
nodejs_heap_space_size_available_bytes{space="new"} 658168 1546562858531
nodejs_heap_space_size_available_bytes{space="old"} 1583448 1546562858531
nodejs_heap_space_size_available_bytes{space="code"} 456544 1546562858531
nodejs_heap_space_size_available_bytes{space="map"} 514160 1546562858531
nodejs_heap_space_size_available_bytes{space="large_object"} 1397517824 1546562858531

# HELP nodejs_version_info Node.js version info.
# TYPE nodejs_version_info gauge

It would make sense that nodejs_version_info is only collected once, as it is very unlikely to change, but it seems to get lost after the registry is reset.

What's the best way to make sure that metric is always populated? It would be pretty easy for me to clear the interval and call collectDefaultMetrics again, if that makes sense.

Edit: reading further, it looks like this may be a non-issue if #180 is implemented as discussed, since my code would call that each time rather than set and reset an interval. Would that be the most correct solution?

@siimon
Copy link
Owner

siimon commented Jan 11, 2019

Yeah I would say that fixing it as suggested in #180 would be the way to fix this problem as well!

@sam-github
Copy link
Contributor

@ssube was this fixed by #180?

@ssube
Copy link
Author

ssube commented Feb 25, 2020

Not quite - after resetting the registry, the nodejs_version_info gauge remains empty forever. The other metrics are being updated every sample, especially visible with memory.

I had to make a few other changes for v12.0 (ssube/isolex#837), but have gotten it working with a hard-coded (for the moment) timeout of 10 (https://github.com/ssube/isolex/pull/837/files#diff-5a28ddf4a6b3741f1800b6fcc801c9b1R42):

    this.collector({
      register: this.metrics,
      timeout: 10,
    });

Metrics:

# HELP nodejs_heap_space_size_available_bytes Process heap space size available from Node.js in bytes.
# TYPE nodejs_heap_space_size_available_bytes gauge
nodejs_heap_space_size_available_bytes{space="read_only"} 229576
nodejs_heap_space_size_available_bytes{space="new"} 415016
nodejs_heap_space_size_available_bytes{space="old"} 1980992
nodejs_heap_space_size_available_bytes{space="code"} 28416
nodejs_heap_space_size_available_bytes{space="map"} 1101440
nodejs_heap_space_size_available_bytes{space="large_object"} 0
nodejs_heap_space_size_available_bytes{space="code_large_object"} 0
nodejs_heap_space_size_available_bytes{space="new_large_object"} 1047488

# HELP nodejs_version_info Node.js version info.
# TYPE nodejs_version_info gauge

# HELP nodejs_gc_duration_seconds Garbage collection duration by kind, one of major, minor, incremental or weakcb.
# TYPE nodejs_gc_duration_seconds histogram
nodejs_gc_duration_seconds_bucket{le="0.001",kind="minor"} 7
nodejs_gc_duration_seconds_bucket{le="0.01",kind="minor"} 7
nodejs_gc_duration_seconds_bucket{le="0.1",kind="minor"} 7
nodejs_gc_duration_seconds_bucket{le="1",kind="minor"} 7
nodejs_gc_duration_seconds_bucket{le="2",kind="minor"} 7
nodejs_gc_duration_seconds_bucket{le="5",kind="minor"} 7
nodejs_gc_duration_seconds_bucket{le="+Inf",kind="minor"} 7
nodejs_gc_duration_seconds_sum{kind="minor"} 0.0019990769999999997
nodejs_gc_duration_seconds_count{kind="minor"} 7

Not sure if this is something I'm doing wrong or related to how the version info is being collected.

@sam-github
Copy link
Contributor

Probably the version gauge needs to return a collect function. It didn't before, and doesn't now, because the values never change. I guess the interaction with reset wasn't considered.

zbjornson added a commit to zbjornson/prom-client that referenced this issue Mar 29, 2020
* Adds a `collect` fn, which may be async, to each metric type. This provides a cleaner interface than registering collectors, and allows the collector to do asynchronous work. (Previously it would be one scrape behind if it had to do async work.)

* In turn, makes `registry.metrics()` and `registry.get...` async, and removes the `collectors`/`registerCollector` functions.

* Fixes `process_max_fds` and `process_start_time_seconds` so that they have values after the registry is reset (same vein as siimon#238)

* Cleans up the readme a bit (removes some out-of-date info)
zbjornson added a commit to zbjornson/prom-client that referenced this issue May 9, 2020
* Adds a `collect` fn, which may be async, to each metric type. This provides a cleaner interface than registering collectors, and allows the collector to do asynchronous work. (Previously it would be one scrape behind if it had to do async work.)

* In turn, makes `registry.metrics()` and `registry.get...` async, and removes the `collectors`/`registerCollector` functions.

* Fixes `process_max_fds` and `process_start_time_seconds` so that they have values after the registry is reset (same vein as siimon#238)

* Cleans up the readme a bit (removes some out-of-date info)
zbjornson added a commit to zbjornson/prom-client that referenced this issue May 16, 2020
* Adds a `collect` fn, which may be async, to each metric type. This provides a cleaner interface than registering collectors, and allows the collector to do asynchronous work. (Previously it would be one scrape behind if it had to do async work.)

* In turn, makes `registry.metrics()` and `registry.get...` async, and removes the `collectors`/`registerCollector` functions.

* Fixes `process_max_fds` and `process_start_time_seconds` so that they have values after the registry is reset (same vein as siimon#238)

* Cleans up the readme a bit (removes some out-of-date info)
zbjornson added a commit to zbjornson/prom-client that referenced this issue May 16, 2020
* Adds a `collect` fn, which may be async, to each metric type. This provides a cleaner interface than registering collectors, and allows the collector to do asynchronous work. (Previously it would be one scrape behind if it had to do async work.)

* In turn, makes `registry.metrics()` and `registry.get...` async, and removes the `collectors`/`registerCollector` functions.

* Fixes `process_max_fds` and `process_start_time_seconds` so that they have values after the registry is reset (same vein as siimon#238)

* Cleans up the readme a bit (removes some out-of-date info)
zbjornson added a commit to zbjornson/prom-client that referenced this issue Jul 4, 2020
* Adds a `collect` fn, which may be async, to each metric type. This provides a cleaner interface than registering collectors, and allows the collector to do asynchronous work. (Previously it would be one scrape behind if it had to do async work.)

* In turn, makes `registry.metrics()` and `registry.get...` async, and removes the `collectors`/`registerCollector` functions.

* Fixes `process_max_fds` and `process_start_time_seconds` so that they have values after the registry is reset (same vein as siimon#238)

* Cleans up the readme a bit (removes some out-of-date info)
zbjornson added a commit that referenced this issue Jul 4, 2020
* Adds a `collect` fn, which may be async, to each metric type. This provides a cleaner interface than registering collectors, and allows the collector to do asynchronous work. (Previously it would be one scrape behind if it had to do async work.)

* In turn, makes `registry.metrics()` and `registry.get...` async, and removes the `collectors`/`registerCollector` functions.

* Fixes `process_max_fds` and `process_start_time_seconds` so that they have values after the registry is reset (same vein as #238)

* Cleans up the readme a bit (removes some out-of-date info)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants