8000 Add target_info to registries by brian-brazil · Pull Request #453 · prometheus/client_python · GitHub
[go: up one dir, main page]
Skip to content

Add target_info to registries #453

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Sep 25, 2019
Merged

Add target_info to registries #453

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 31 additions & 2 deletions prometheus_client/registry.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@ class CollectorRegistry(object):
exposition formats.
"""

def __init__(self, auto_describe=False):
def __init__(self, auto_describe=False, target_info=None):
self._collector_to_names = {}
self._names_to_collectors = {}
self._auto_describe = auto_describe
self._lock = Lock()
self._target_info = {}
self.set_target_info(target_info)

def register(self, collector):
"""Add a collector to the registry."""
Expand Down Expand Up @@ -69,8 +71,13 @@ def _get_names(self, collector):
def collect(self):
"""Yields metrics from the collectors in the registry."""
collectors = None
ti = None
with self._lock:
collectors = copy.copy(self._collector_to_names)
if self._target_info:
8000 ti = self._target_info_metric()
if ti:
yield ti
for collector in collectors:
for metric in collector.collect():
yield metric
Expand All @@ -87,11 +94,14 @@ def restricted_registry(self, names):
Experimental."""
names = set(names)
collectors = set()
metrics = []
with self._lock:
if 'target_info' in names and self._target_info:
metrics.append(self._target_info_metric())
names.remove('target_info')
for name in names:
if name in self._names_to_collectors:
collectors.add(self._names_to_collectors[name])
metrics = []
for collector in collectors:
for metric in collector.collect():
samples = [s for s in metric.samples if s[0] in names]
Expand All @@ -106,6 +116,25 @@ def collect(self):

return RestrictedRegistry()

def set_target_info(self, labels):
with self._lock:
if labels:
if not self._target_info and 'target_info' in self._names_to_collectors:
raise ValueError('CollectorRegistry already contains a target_info metric')
self._names_to_collectors['target_info'] = None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little confused about this one, from my rudimentary understanding (or extreme rudimentary understanding should I say) - every value in _names_to_collectors should be a collector yeah?

If None is a value in _names_to_collectors and then it gets added to collectors var in restricted_registry:

        names = set(names)
        collectors = set()
        metrics = []
        with self._lock:
            for name in names:
                if name in self._names_to_collectors:
                    collectors.add(self._names_to_collectors[name])
            if 'target_info' in names and self._target_info:
                metrics.append(self._target_info_metric())
        for collector in collect
8000
ors:
            for metric in collector.collect():
                samples = [s for s in metric.samples if s[0] in names]
                if samples:
                    m = Metric(metric.name, metric.documentation, metric.type)
                    m.samples = samples
                    metrics.append(m)

Wouldn't the collector.collect() be a call to None from the line for metric in collector.collect() if names happens to contain target_info?

elif self._target_info:
self._names_to_collectors.pop('target_info', None)
self._target_info = labels

def get_target_info(self):
with self._lock:
return self._target_info

def _target_info_metric(self):
m = Metric('target', 'Target metadata', 'info')
m.add_sample('target_info', self._target_info, 1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, would it be required to also emit an actual value (i.e. in this case 1) or could we avoid that perhaps and special case it in the parser that metrics without a value are info metrics?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a standard info metric, so the value of 1 is required.

return m

def get_sample_value(self, name, labels=None):
"""Returns the sample value, or None if not found.

Expand Down
31 changes: 31 additions & 0 deletions tests/test_core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -664,6 +664,9 @@ def test_duplicate_metrics_raises(self):
# The name of the histogram itself isn't taken.
Gauge('h', 'help', registry=registry)

Info('i', 'help', registry=registry)
self.assertRaises(ValueError, Gauge, 'i_info', 'help', registry=registry)

def test_unregister_works(self):
registry = CollectorRegistry()
s = Summary('s', 'help', registry=registry)
Expand Down Expand Up @@ -696,6 +699,34 @@ def test_restricted_registry(self):
m.samples = [Sample('s_sum', {}, 7)]
self.assertEquals([m], registry.restricted_registry(['s_sum']).collect())

def test_target_info_injected(self):
registry = CollectorRegistry(target_info={'foo': 'bar'})
self.assertEqual(1, registry.get_sample_value('target_info', {'foo': 'bar'}))

def test_target_info_duplicate_detected(self):
registry = CollectorRegistry(target_info={'foo': 'bar'})
self.assertRaises(ValueError, Info, 'target', 'help', registry=registry)

registry.set_target_info({})
i = Info('target', 'help', registry=registry)
registry.set_target_info({})
self.assertRaises(ValueError, Info, 'target', 'help', registry=registry)
self.assertRaises(ValueError, registry.set_target_info, {'foo': 'bar'})
registry.unregister(i)
registry.set_target_info({'foo': 'bar'})

def test_target_info_restricted_registry(self):
registry = CollectorRegistry(target_info={'foo': 'bar'})
Summary('s', 'help', registry=registry).observe(7)

m = Metric('s', 'help', 'summary')
m.samples = [Sample('s_sum', {}, 7)]
self.assertEquals([m], registry.restricted_registry(['s_sum']).collect())

m = Metric('target', 'Target metadata', 'info')
m.samples = [Sample('target_info', {'foo': 'bar'}, 1)]
self.assertEquals([m], registry.restricted_registry(['target_info']).collect())


if __name__ == '__main__':
unittest.main()
0