Skip to content

Coerce label values to strings in MetricFamily add_metric - #1221

Open
cpruijsen wants to merge 2 commits into
prometheus:masterfrom
cpruijsen:fix/issue-270
Open

cpruijsen wants to merge 2 commits into
prometheus:masterfrom
cpruijsen:fix/issue-270

Conversation

@cpruijsen

Copy link
Copy Markdown

@csmarchbanks

Fixes #270.

Custom collectors could store integer label values in add_metric(), then fail during exposition with AttributeError: 'int' object has no attribute 'replace'.
This converts the labels sequence to strings in all eight metric families, consistent with Metric.labels().

Chosen: convert at add_metric() entry. The alternative was conversion during exposition.
Entry conversion gives every sample consumer string labels and leaves the escaping code unchanged.

The regression test checks string labels across all eight families and successful Prometheus exposition.
It fails on the baseline with AssertionError: 2 != None.

Validation:

  • Python 3.14 full suite: 427 passed, 3 optional dependency skips.
  • tests/test_core.py: 115 passed under both Python 3.9 and 3.14.
  • Flake8, isort, and the pinned mypy check pass.
  • The local Python 3.9 full suite has six TLS 1.3 failures that also occur on the baseline.

The custom collector MetricFamily classes stored label values verbatim,
so non-string values such as ints crashed exposition with
AttributeError: 'int' object has no attribute 'replace'. Coerce them to
str on entry, matching what Metric.labels() already does for the
standard metric types.

Signed-off-by: Christopher Pruijsen <[email protected]>

@csmarchbanks csmarchbanks left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

I think we need updates for InfoMetricFamily and StateSetMetricFamily as well which can pass in ints and such via dicts.

Should we also update the type hints to be Sequence[object] as we just need things to be stringable now as well?

InfoMetricFamily label values passed in the value dict and
StateSetMetricFamily state names were still stored verbatim, so ints
there produced samples with non-string labels. Coerce them to str too.

Since add_metric now accepts any stringable label value, type the label
values as Sequence[object], the info dict as Mapping[str, object] and
the stateset dict as Mapping[Any, bool].

Signed-off-by: Christopher Pruijsen <[email protected]>
@cpruijsen

Copy link
Copy Markdown
Author

Info now converts the values in its dict to str, and StateSet converts the state names. I also widened the hints where values get stringified: the add_metric labels are Sequence[object], the info dict is Mapping[str, object] and the stateset dict is Mapping[Any, bool]. I used Mapping because Dict is invariant, so with Dict mypy would reject existing callers that pass Dict[str, str].

This branch has not been deployed

No deployments
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.

Non-stringy label values in MetricFamily metrics cause export-time errors

2 participants