[#674] Made the registry restriction handle newly added metrics. #675
Conversation
Signed-off-by: Pavel <pavel@lexyr.com>
prometheus_client/metrics_core.py
Outdated
| @@ -58,6 +58,15 @@ def __repr__(self): | |||
| self.samples, | |||
| ) | |||
|
|
|||
| def restricted_metric(self, names): | |||
There was a problem hiding this comment.
Do you think there is a good use case for using this function externally? If not perhaps it should be _restricted_metric so we could easily change/remove it later.
There was a problem hiding this comment.
Fixed. Good catch!
Signed-off-by: Pavel <pavel@lexyr.com>
| for name in names: | ||
| if name in self._names_to_collectors: | ||
| collectors.add(self._names_to_collectors[name]) | ||
| for collector in collectors: |
There was a problem hiding this comment.
Removing this code has caused a serious performance regression. The whole point of this feature is not to collect from collectors that aren't needed, so that it's very cheap to access just one or two collectors.
There was a problem hiding this comment.
OK, thank you for the context.
@pavel-lexyr perhaps let's approach this by improving the documentation, would you be willing to open a PR to change this back but with more complete documentation?
There was a problem hiding this comment.
You don't need to undo the whole change (even though it is a behaviour change as to when it binds), but the collector lookup is important.
There was a problem hiding this comment.
Addressed your concerns in #680. Let me know if you find the implementation agreeable.
…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>
Fixes #674.