Skip to content

Commit e8f8bae

Browse files
authored
fix(multiprocess): avoid double-building child metric names (#1035) (#1146)
* fix(multiprocess): avoid double-building child metric names (#1035) Signed-off-by: hazel-shen <[email protected]> * test: ensure child metrics retain parent namespace/subsystem/unit Signed-off-by: hazel-shen <[email protected]> --------- Signed-off-by: hazel-shen <[email protected]>
1 parent 1783ca8 commit e8f8bae

File tree

2 files changed

+129
-3
lines changed

2 files changed

+129
-3
lines changed

prometheus_client/metrics.py

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,10 @@ def __init__(self: T,
109109
registry: Optional[CollectorRegistry] = REGISTRY,
110110
_labelvalues: Optional[Sequence[str]] = None,
111111
) -> None:
112+
113+
self._original_name = name
114+
self._namespace = namespace
115+
self._subsystem = subsystem
112116
self._name = _build_full_name(self._type, name, namespace, subsystem, unit)
113117
self._labelnames = _validate_labelnames(self, labelnames)
114118
self._labelvalues = tuple(_labelvalues or ())
@@ -176,13 +180,25 @@ def labels(self: T, *labelvalues: Any, **labelkwargs: Any) -> T:
176180
labelvalues = tuple(str(l) for l in labelvalues)
177181
with self._lock:
178182
if labelvalues not in self._metrics:
183+
184+
original_name = getattr(self, '_original_name', self._name)
185+
namespace = getattr(self, '_namespace', '')
186+
subsystem = getattr(self, '_subsystem', '')
187+
unit = getattr(self, '_unit', '')
188+
189+
child_kwargs = dict(self._kwargs) if self._kwargs else {}
190+
for k in ('namespace', 'subsystem', 'unit'):
191+
child_kwargs.pop(k, None)
192+
179193
self._metrics[labelvalues] = self.__class__(
180-
self._name,
194+
original_name,
181195
documentation=self._documentation,
182196
labelnames=self._labelnames,
183-
unit=self._unit,
197+
namespace=namespace,
198+
subsystem=subsystem,
199+
unit=unit,
184200
_labelvalues=labelvalues,
185-
**self._kwargs
201+
**child_kwargs
186202
)
187203
return self._metrics[labelvalues]
188204

tests/test_multiprocess.py

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,116 @@ def test_remove_clear_warning(self):
396396
assert "Removal of labels has not been implemented" in str(w[0].message)
397397
assert issubclass(w[-1].category, UserWarning)
398398
assert "Clearing labels has not been implemented" in str(w[-1].message)
399+
400+
def test_child_name_is_built_once_with_namespace_subsystem_unit(self):
401+
"""
402+
Repro for #1035:
403+
In multiprocess mode, child metrics must NOT rebuild the full name
404+
(namespace/subsystem/unit) a second time. The exported family name should
405+
be built once, and Counter samples should use "<family>_total".
406+
"""
407+
from prometheus_client import Counter
408+
409+
class CustomCounter(Counter):
410+
def __init__(
411+
self,
412+
name,
413+
documentation,
414+
labelnames=(),
415+
namespace="mydefaultnamespace",
416+
subsystem="mydefaultsubsystem",
417+
unit="",
418+
registry=None,
419+
_labelvalues=None
420+
):
421+
# Intentionally provide non-empty defaults to trigger the bug path.
422+
super().__init__(
423+
name=name,
424+
documentation=documentation,
425+
labelnames=labelnames,
426+
namespace=namespace,
427+
subsystem=subsystem,
428+
unit=unit,
429+
registry=registry,
430+
_labelvalues=_labelvalues)
431+
432+
# Create a Counter with explicit namespace/subsystem/unit
433+
c = CustomCounter(
434+
name='m',
435+
documentation='help',
436+
labelnames=('status', 'method'),
437+
namespace='ns',
438+
subsystem='ss',
439+
unit='seconds', # avoid '_total_total' confusion
440+
registry=None, # not registered in local registry in multiprocess mode
441+
)
442+
443+
# Create two labeled children
444+
c.labels(status='200', method='GET').inc()
445+
c.labels(status='404', method='POST').inc()
446+
447+
# Collect from the multiprocess collector initialized in setUp()
448+
metrics = {m.name: m for m in self.collector.collect()}
449+
450+
# Family name should be built once (no '_total' in family name)
451+
expected_family = 'ns_ss_m_seconds'
452+
self.assertIn(expected_family, metrics, f"missing family {expected_family}")
453+
454+
# Counter samples must use '<family>_total'
455+
mf = metrics[expected_family]
456+
sample_names = {s.name for s in mf.samples}
457+
self.assertTrue(
458+
all(name == expected_family + '_total' for name in sample_names),
459+
f"unexpected sample names: {sample_names}"
460+
)
461+
462+
# Ensure no double-built prefix sneaks in (the original bug)
463+
bad_prefix = 'mydefaultnamespace_mydefaultsubsystem_'
464+
all_names = {mf.name, *sample_names}
465+
self.assertTrue(
466+
all(not n.startswith(bad_prefix) for n in all_names),
467+
f"found double-built name(s): {[n for n in all_names if n.startswith(bad_prefix)]}"
468+
)
469+
470+
def test_child_preserves_parent_context_for_subclasses(self):
471+
"""
472+
Ensure child metrics preserve parent's namespace/subsystem/unit information
473+
so that subclasses can correctly use these parameters in their logic.
474+
"""
475+
class ContextAwareCounter(Counter):
476+
def __init__(self,
477+
name,
478+
documentation,
479+
labelnames=(),
480+
namespace="",
481+
subsystem="",
482+
unit="",
483+
**kwargs):
484+
self.context = {
485+
'namespace': namespace,
486+
'subsystem': subsystem,
487+
'unit': unit
488+
}
489+
super().__init__(name, documentation,
490+
labelnames=labelnames,
491+
namespace=namespace,
492+
subsystem=subsystem,
493+
unit=unit,
494+
**kwargs)
495+
496+
parent = ContextAwareCounter('m', 'help',
497+
labelnames=['status'],
498+
namespace='prod',
499+
subsystem='api',
500+
unit='seconds',
501+
registry=None)
502+
503+
child = parent.labels(status='200')
504+
505+
# Verify that child retains parent's context
506+
self.assertEqual(child.context['namespace'], 'prod')
507+
self.assertEqual(child.context['subsystem'], 'api')
508+
self.assertEqual(child.context['unit'], 'seconds')
399509

400510

401511
class TestMmapedDict(unittest.TestCase):

0 commit comments

Comments
 (0)