Skip to content
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.

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

Version information in text/protobuf or header only? #84

Closed
RichiH opened this issue Jun 8, 2018 · 7 comments
Closed

Version information in text/protobuf or header only? #84

RichiH opened this issue Jun 8, 2018 · 7 comments

Comments

@RichiH
Copy link
Member

RichiH commented Jun 8, 2018

fhemberger/prometheus-formatter#1 (comment) raises an interesting point: Not all systems may have access to the header information.

Conceivably, there might not even be any HTTP involved at all in some uses cases

Having version information in the body would generally make sense and be lightweight enough in relation to a usual payload.

@brian-brazil
Copy link
Contributor

I can see that causing confusion, as for example implementors may set that and not the HTTP header and break parsers. I think it's best to keep this to a single place, and I think out of band is fine for that.

@RichiH
Copy link
Member Author

RichiH commented Jun 8, 2018

The test suite would catch that.

We could also not have this information in the header, but I don't feel as if I have thought that possibility through, yet.

@brian-brazil
Copy link
Contributor

If you're using HTTP, the content type belongs in the header - and is needed if you're going to be doing sane content type negotiation.

I'm not sure we should be changing the protocol just to workaround a security feature in one browser.

@melkorm
Copy link

melkorm commented Aug 30, 2018

https://developer.chrome.com/extensions/webRequest#event-onHeadersReceived it is possible to examine headers in Chrome's plugin system.

What I've found is that just version in the header seems not enough. It would be great to have text/plain; version=0.0.4; prometheus or maybe text/plain-prometheus; version=0.0.4 or something to easily determinate what it is, as I can imagine other systems using version=x.y.z in their content types headers conflicting and making it harder with detecting proper content type.

@brian-brazil
Copy link
Contributor

The content type will include "openmterics".

@melkorm
Copy link

melkorm commented Aug 30, 2018

@brian-brazil Great! Than for me this issue can be closed 👍

@RichiH
Copy link
Member Author

RichiH commented Dec 1, 2020

As part of the RFC process, we did apply for application/openmetrics-text and application/openmetrics-proto. Please see https://github.com/OpenObservability/OpenMetrics/blob/master/specification/OpenMetrics.md#iana-considerations-security

As such, this is all out-of-band.

@RichiH RichiH closed this as completed Dec 1, 2020
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

No branches or pull requests

3 participants