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

feat: add support for custom labels in the counter metric #62

Merged
merged 4 commits into from
Nov 27, 2023

Conversation

sralloza
Copy link
Contributor

Add support to custom labels for in the counter metric.

Closes #61

The test passes if you launch it alone, but fails when launching all with this message:
panic: a previously registered descriptor with the same fully-qualified name as...

I don't have enough experience with golang and the source code to fix it.

go.mod Outdated Show resolved Hide resolved
@depado
Copy link
Owner

depado commented Nov 15, 2023

Amazing, thanks for your PR, let me have a look at the testing part, maybe I can figure out what's wrong 😄

@depado
Copy link
Owner

depado commented Nov 27, 2023

Alright I think I figured it out. Turns out, unregistering is not sufficient as stated in the library documentation:

// Note that even after unregistering, it will not be possible to
// register a new Collector that is inconsistent with the unregistered
// Collector, e.g. a Collector collecting metrics with the same name but
// a different help string. The rationale here is that the same registry
// instance must only collect consistent metrics throughout its
// lifetime.

This is why when executed individually, the test passes, and why it fails when executed with others.

prom_test.go Outdated Show resolved Hide resolved
Co-authored-by: Depado <Depado@users.noreply.github.com>
Copy link

codecov bot commented Nov 27, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (0f5f022) 100.00% compared to head (d1c63c2) 100.00%.

Additional details and impacted files
@@            Coverage Diff            @@
##            master       #62   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            3         3           
  Lines          247       257   +10     
=========================================
+ Hits           247       257   +10     

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

@depado depado merged commit 5b6bce4 into depado:master Nov 27, 2023
3 checks passed
@depado
Copy link
Owner

depado commented Nov 27, 2023

Thanks a lot for your PR 👍
Sorry it took me so long to figure out what was going on in the tests.

@depado
Copy link
Owner

depado commented Nov 27, 2023

v1.8.0 was released and includes your feature 👍

@sralloza sralloza deleted the feat/custom-counter-labels branch November 28, 2023 10:45
@sralloza
Copy link
Contributor Author

Thanks for the review and for the release!

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.

Feature request: custom metric labels
2 participants