Add all Go runtime metrics

[dongjiang1989]

expose all Go runtime metrics like GC stats, memory stats etc.

[oliver006]

Thanks for the PR!

Looking at the docs (and at the metrics that the exporter currently exports), I think thee two are already included in the default registry:

From:
https://github.com/prometheus/client_golang/blob/96a5ad6ef56e0d41d3bf06ba0478b34a0c718a92/prometheus/doc.go#L185-L188

// Also note that the DefaultRegisterer comes registered with a Collector for Go
// runtime metrics (via NewGoCollector) and a Collector for process metrics (via
// NewProcessCollector). With a custom registry, you are in control and decide
// yourself about the Collectors to register.

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.

Project coverage is 80.46%. Comparing base (b21204e) to head (85a8b25).
Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
main.go	0.00%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #997      +/-   ##
==========================================
- Coverage   80.59%   80.46%   -0.14%     
==========================================
  Files          19       19              
  Lines        2948     2953       +5     
==========================================
  Hits         2376     2376              
- Misses        460      465       +5     
  Partials      112      112              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[coveralls]

Pull Request Test Coverage Report for Build 15504448026

Details

• 0 of 6 (0.0%) changed or added relevant lines in 1 file are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage decreased (-0.1%) to 84.301%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
main.go	0	6	0.0%

Files with Coverage Reduction	New Missed Lines	%
main.go	1	0.0%

Totals
Change from base Build 15504439518:	-0.1%
Covered Lines:	2497
Relevant Lines:	2962

---

💛 - Coveralls

[dongjiang1989]

Looking at the docs (and at the metrics that the exporter currently exports), I think thee two are already included in the default registry:

Thanks @oliver006 review

Differences:

MustRegister(NewGoCollector())

change to

MustRegister(collectors.NewGoCollector(collectors.WithGoCollectorRuntimeMetrics(collectors.MetricsAll)))

ref: https://github.com/prometheus/client_golang/blob/main/prometheus/registry.go#L60-L63

[oliver006]

MustRegister(collectors.NewGoCollector(collectors.WithGoCollectorRuntimeMetrics(collectors.MetricsAll)))

Thanks for highlighting the diff.

What does that effectively mean? What metrics are we getting with that ^^^ that we're currently missing out on.

On the one hand I like being explicit about initializing the Registry, on the other hand I like using the DefaultRegistry because it means I'm going with the defaults and if something new is added to the defaults then I'll automagically pull it in when upgrading the client_golang dependency.

Thoughts?

[oliver006]

I had another look at this and I think this is fine.

Thanks for the PR and sorry for the delay!

[nantiferov]

From my observations, this change adds ~150 new metrics per each scrape, which is not huge, but after this change go internal metrics are responsible for 1/6 of all metrics coming from exporter.

It's possible to drop all of them with flag.Bool("redis-only-metrics", getEnvBool("REDIS_EXPORTER_REDIS_ONLY_METRICS", false), "Whether to also export go runtime metrics"), but in this case there'll be no runtime specific metrics, like CPU/memory usage or start time, as well as exporter_build_info metric.

I think it might make sense to have additional flag to provide only mimimal runtime metrics with only first option, which adds only 7 additional metrics VS 209 now.

// expose process metrics like CPU, Memory, file descriptor usage etc.
			collectors.NewProcessCollector(collectors.ProcessCollectorOpts{}),

Metrics:

process_cpu_seconds_total 0.08
process_max_fds 1.048575e+06
process_open_fds 9
process_resident_memory_bytes 2.0578304e+07
process_start_time_seconds 1.7528392889e+09
process_virtual_memory_bytes 1.26984192e+09
process_virtual_memory_max_bytes 1.8446744073709552e+19

I can prepare PR for this if this sounds good.

[oliver006]

I think that makes sense.
And I'd say most people don't need all the 209 metrics and it's cumbersome to drop them in the Prometheus server on scraping.

So why not go the other way around and add a flag "extended go metrics" (or similar) which defaults to false and that would enable the full set of 209 metrics.
What do you think?

[nantiferov]

So why not go the other way around and add a flag "extended go metrics" (or similar) which defaults to false and that would enable the full set of 209 metrics.

Yep, I think that would the best option.

[nantiferov]

Created issue to track it and PR with fix:

• Add separate cmdline flag to include all Go runtime metrics #1034
• Feature: add include-go-runtime-metrics flag to enable Go runtime metrics #1035

Please review when have time.