Repository navigation
Conversation
Info.info() accepted any key or value except None. A non-string such as
{'build_number': 42} was stored, and every later scrape of the registry
then failed with AttributeError ("'int' object has no attribute
'replace'"), so one bad call hid all metrics on the endpoint.
Check the types where the value is set, as was done for None in prometheus#1012,
so the caller gets a ValueError and the registry stays scrapeable. The
docs already say both keys and values must be strings.
Refs prometheus#1046
Signed-off-by: Sonica-B <shreyaa.boyane@gmail.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Anyone who passes a number (or any other non-string) to
Info.info()loses their whole/metricsendpoint: the call succeeds, and every later scrape of that registry fails withAttributeError: 'int' object has no attribute 'replace', so all metrics, not only the Info one, disappear. #1046 reports this with a float temperature; #521, #583 and #1011 hit the same failure earlier. The docs already say "valmust be adict[str, str]— both keys and values must be strings", butinfo()only rejectsNone(#1012).This change makes
info()raiseValueErrorfor any key or value that is not astr, at the call that caused it, so the registry stays scrapeable. It does not coerce values to strings, following the comment on #1046 that enforcing strings lets users correct the mistake.Refs #1046
flowchart LR subgraph Before A1["Info.info({'build_number': 42})"] --> B1["stored in Info._value"] B1 --> C1["generate_latest() / openmetrics generate_latest()"] C1 --> D1["AttributeError: whole scrape fails"] end subgraph After A2["Info.info({'build_number': 42})"] --> B2["ValueError at the call site"] B2 --> C2["previous value kept; scrape returns all metrics"] endBefore and after
Before (
e2f2bf9):After:
A non-string key (
{1: 'x'}) failed the same way before (TypeError: object of type 'int' has no len()at scrape time) and is now rejected by the same check.Changes
prometheus_client/metrics.py:Info.info()checks that every key and value is astrbefore storing the dict. The existingNonecheck and its message are unchanged.tests/test_core.py:TestInfo.test_non_string_keys_and_valuescovers an int, a float, a non-string key and a list, for an Info with and without labels, and checks that the previous value survives and that both the text and the OpenMetrics output still render. It fails onmasterand passes with the change.Testing
Windows 11, Python 3.11.9:
pytest tests -q: 440 passed, 14 skippedflake8 prometheus_client/ tests/: cleanisort --check prometheus_client/ tests/: cleanmypy prometheus_client/ tests/: the same 9 missing-optional-dependency errors as onmaster, none new