UTF-8 Content Negotiation #1102

ywwg · 2025-04-08T21:29:40Z

Part of #1013

The remaining piece is configuration to force escaping even if a scraper does not request it

ywwg · 2025-04-10T13:59:14Z

prometheus_client/exposition.py

@@ -37,7 +37,7 @@
    'write_to_textfile',
 )

-CONTENT_TYPE_LATEST = 'text/plain; version=0.0.4; charset=utf-8'
+CONTENT_TYPE_PLAIN = 'text/plain; version=0.0.4; charset=utf-8'


text/plain v 0.0.4 is not the latest, so I thought it best to rename this

Though I agree with you, this will be a breaking change as other programs use CONTENT_TYPE_LATEST. Perhaps we should update this to version=1.0.0 instead?

Since this is needed for compatibility, but LATEST and PLAIN are both really different versions of PLAIN that is confusing for external users to use. I'd propose a couple options:

Keep CONTENT_TYPE_PLAIN internal, so rename it to _CONTENT_TYPE_PLAIN.

Support content types for each version, so have CONTENT_TYPE_PLAIN_0_4_0, CONTENT_TYPE_PLAIN_1_0_0, and then alias CONTENT_TYPE_LATEST to CONTENT_TYPE_PLAIN_1_0_0.

I like option 2

ywwg · 2025-04-10T14:38:33Z

TODO need more openmetrics/exposition tests

ywwg · 2025-04-10T15:09:52Z

prometheus_client/openmetrics/exposition.py



-def _escape(s: str) -> str:
+def _escape(s: str, escaping: str, is_labelname: bool) -> str:


I don't love how messy this function signature is getting...

Yeah, it is internal only so not the end of the world. Would it help at all to have a second function for specifically escaping label names?

I tried another approach that looks better

Signed-off-by: Owen Williams <owen.williams@grafana.com>

csmarchbanks · 2025-04-18T14:23:42Z

prometheus_client/exposition.py

@@ -37,7 +37,7 @@
    'write_to_textfile',
 )

-CONTENT_TYPE_LATEST = 'text/plain; version=0.0.4; charset=utf-8'
+CONTENT_TYPE_PLAIN = 'text/plain; version=0.0.4; charset=utf-8'


Though I agree with you, this will be a breaking change as other programs use CONTENT_TYPE_LATEST. Perhaps we should update this to version=1.0.0 instead?

csmarchbanks · 2025-04-18T14:25:12Z

prometheus_client/exposition.py

+            escaping = _get_escaping(toks)
+            # Only return an escaping header if we have a good version and
+            # mimetype.
+            if version == '1.0.0':


Should this test that the version is at least 1.0.0? I imagine at least anything in 1.x should be ok?

looks like there's a python lib for that!

lmk if I should just test for major version versus any >= version

(I think any greater version should be fine)

csmarchbanks · 2025-04-18T14:27:25Z

prometheus_client/openmetrics/exposition.py



-def _escape(s: str) -> str:
+def _escape(s: str, escaping: str, is_labelname: bool) -> str:


Yeah, it is internal only so not the end of the world. Would it help at all to have a second function for specifically escaping label names?

csmarchbanks · 2025-04-18T14:28:20Z

tools/simple_client.py

Is this used anywhere? Or just a nice example for testing locally?

just for testing locally

but is also pretty nice for people to get off the ground

ywwg · 2025-04-30T17:30:59Z

Though I agree with you, this will be a breaking change as other programs use CONTENT_TYPE_LATEST. Perhaps we should update this to version=1.0.0 instead?

we do still need a 0.0.4 constant as the fallback when all else fails. What I could do is update LATEST to be 1.0.0 and then create the new PLAIN constant for 0.0.4

csmarchbanks

Generally looks to be getting there! A few comments mostly around the content types still. CONTENT_TYPE_LATEST from just exposition.py (not open metrics) doesn't actually seem to be used?

prometheus_client/__init__.py

csmarchbanks · 2025-05-29T01:16:43Z

prometheus_client/exposition.py

@@ -37,7 +37,7 @@
    'write_to_textfile',
 )

-CONTENT_TYPE_LATEST = 'text/plain; version=0.0.4; charset=utf-8'
+CONTENT_TYPE_PLAIN = 'text/plain; version=0.0.4; charset=utf-8'


Since this is needed for compatibility, but LATEST and PLAIN are both really different versions of PLAIN that is confusing for external users to use. I'd propose a couple options:

Keep CONTENT_TYPE_PLAIN internal, so rename it to _CONTENT_TYPE_PLAIN.

Support content types for each version, so have CONTENT_TYPE_PLAIN_0_4_0, CONTENT_TYPE_PLAIN_1_0_0, and then alias CONTENT_TYPE_LATEST to CONTENT_TYPE_PLAIN_1_0_0.

prometheus_client/exposition.py

prometheus_client/openmetrics/exposition.py

prometheus_client/validation.py

ywwg commented Apr 10, 2025

View reviewed changes

ywwg marked this pull request as ready for review April 10, 2025 14:08

ywwg marked this pull request as draft April 10, 2025 14:17

ywwg force-pushed the owilliams/utf8-02 branch from 61e4a01 to 3240802 Compare April 10, 2025 14:24

ywwg force-pushed the owilliams/utf8-02 branch 2 times, most recently from 33ea581 to 630fb65 Compare April 10, 2025 14:42

ywwg marked this pull request as ready for review April 10, 2025 14:42

ywwg marked this pull request as draft April 10, 2025 14:42

ywwg force-pushed the owilliams/utf8-02 branch from 60097b9 to 769f7fb Compare April 10, 2025 15:00

ywwg marked this pull request as ready for review April 10, 2025 15:00

ywwg requested a review from vesari April 10, 2025 15:03

ywwg commented Apr 10, 2025

View reviewed changes

ywwg changed the title ~~initial work on content negotiation for client python~~ UTF-8 Content Negotiation Apr 10, 2025

UTF-8 Content Negotiation

80fa206

Signed-off-by: Owen Williams <owen.williams@grafana.com>

ywwg force-pushed the owilliams/utf8-02 branch from d7d7dc2 to 80fa206 Compare April 10, 2025 18:20

csmarchbanks reviewed Apr 18, 2025

View reviewed changes

address notes

5f5f196

ywwg requested a review from csmarchbanks April 30, 2025 18:25

lint

2e0d626

ywwg marked this pull request as draft April 30, 2025 18:31

fix tests

dc98d96

ywwg marked this pull request as ready for review April 30, 2025 18:49

csmarchbanks reviewed May 29, 2025

View reviewed changes

ywwg added 3 commits May 29, 2025 10:57

Merge remote-tracking branch 'origin/master' into owilliams/utf8-02

62aa038

notes and lint

bae52d1

fix import order

2bfa418

ywwg requested a review from csmarchbanks May 29, 2025 15:21



		def _escape(s: str) -> str:
		def _escape(s: str, escaping: str, is_labelname: bool) -> str:

UTF-8 Content Negotiation #1102

Are you sure you want to change the base?

UTF-8 Content Negotiation #1102

Uh oh!

Conversation

ywwg commented Apr 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ywwg Apr 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ywwg commented Apr 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ywwg commented Apr 30, 2025

Uh oh!

csmarchbanks left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ywwg commented Apr 8, 2025 •

edited

Loading

ywwg Apr 10, 2025 •

edited

Loading