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

allow exposing meta information for registered metrics #61

Merged
merged 7 commits into from
Dec 19, 2023

Conversation

hagen1778
Copy link
Contributor

New public method ExposeMetadata allows enabling exposition of dummy meta-info for all exposed metrics across all Sets.

This feature is needed to improve compatibility
with 3rd-party scrapers that require meta information to be present.

This commit doesn't update exposition of default system/process metrics to keep the list of changes small. This change should be added in a follow-up commit.

#48

New public method `ExposeMetadata` allows enabling exposition
of dummy meta-info for all exposed metrics across all Sets.

This feature is needed to improve compatibility
with 3rd-party scrapers that require meta information to be present.

This commit doesn't update exposition of default system/process
metrics to keep the list of changes small. This change should
be added in a follow-up commit.

#48
@hagen1778 hagen1778 requested review from valyala and f41gh7 December 15, 2023 14:06
Copy link

codecov bot commented Dec 15, 2023

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (cd448dd) 85.89% compared to head (8949879) 89.30%.
Report is 7 commits behind head on master.

❗ Current head 8949879 differs from pull request most recent head 4fc879e. Consider uploading reports for the commit 4fc879e to get more accurate results

Files Patch % Lines
floatcounter.go 0.00% 3 Missing ⚠️
summary.go 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #61      +/-   ##
==========================================
+ Coverage   85.89%   89.30%   +3.40%     
==========================================
  Files          11       12       +1     
  Lines         936      991      +55     
==========================================
+ Hits          804      885      +81     
+ Misses        109       76      -33     
- Partials       23       30       +7     

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

@hagen1778
Copy link
Contributor Author

@valyala one thing I didn't account for is using counters as gauges, as you do here:

	relabelConfigSuccess      = metrics.NewCounter(`vmagent_relabel_config_last_reload_successful`)

I think, this makes this PR useless when used like this.

@valyala valyala merged commit 9dc7358 into master Dec 19, 2023
2 checks passed
@valyala valyala deleted the add-help-type branch December 19, 2023 00:36
@valyala
Copy link
Contributor

valyala commented Dec 19, 2023

@hagen1778 , thanks for the pull request!

@valyala
Copy link
Contributor

valyala commented Dec 20, 2023

one thing I didn't account for is using counters as gauges, as you do here:
relabelConfigSuccess = metrics.NewCounter(vmagent_relabel_config_last_reload_successful)
I think, this makes this PR useless when used like this.

The commit b23fdf5 adds support for Gauge.Set() method, which can be used for setting gauge value without the need to pass callback to NewGauge(). This commit is included in the tag v1.29.0. So now the gauge value can be updated with the following code:

// Create a gauge with nil callback
g := metrics.NewGauge(gaugeName, nil)

// Set newValue to the gauge
g.Set(newValue)

valyala added a commit to VictoriaMetrics/VictoriaMetrics that referenced this pull request Dec 20, 2023
…l` metrics

This allows exposing the correct TYPE metadata for these labels when the app runs with -metrics.exposeMetadata command-line flag.
See VictoriaMetrics/metrics#61 (comment) for more details.

This is follow-up for 326a77c
valyala added a commit to VictoriaMetrics/VictoriaMetrics that referenced this pull request Dec 20, 2023
…l` metrics

This allows exposing the correct TYPE metadata for these labels when the app runs with -metrics.exposeMetadata command-line flag.
See VictoriaMetrics/metrics#61 (comment) for more details.

This is follow-up for 326a77c
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

Successfully merging this pull request may close these issues.

2 participants