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

Registering a metric after creating RestrictedRegistry with its name does not add the metric to RestrictedRegistry #674

Closed
pavel-lexyr opened this issue Jun 24, 2021 · 1 comment · Fixed by #675

Comments

@pavel-lexyr
Copy link
Contributor

The method restricted_registry works by filtering all metrics once at the time of its creation, as one can see from looking at the file registry.py.

If you register a metric after creating the restriction for it, it is not added to the restricted registry - while the docstring could be interpreted as claiming otherwise.

Solution 1: the class could be documented in a way that makes its behaviour explicit;
Solution 2: the class could be expanded to recollect metrics from its parent on every collect() call.

@csmarchbanks
Copy link
Member

I agree, this could be confusing, though the "Intended usage" is hinting that it should be ephemeral to a scrape/output generation. Personally, I would lean slightly towards solution 2 as it is more intuitive and would not require reading the docstring.

csmarchbanks pushed a commit that referenced this issue Jul 1, 2021
* Made the registry restriction handle new metrics.
* Made `restricted_metric()` private.

Signed-off-by: Pavel <pavel@lexyr.com>
csmarchbanks pushed a commit that referenced this issue Jul 6, 2021
…registry collections (#680)

* Made restricted registries call collect() only on relevant collections.
* Added a skip-if for a test that won't run on Python 2.7.
* Moved yielding target_info out of the lock.
* Fixed style and a race condition.

Signed-off-by: Pavel <pavel@lexyr.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants