[kalleep]

Adds encoding::negotiate so a scrape endpoint can answer in the format the scraper asks for.

let media_type = negotiate(accept, [Format::PrometheusText0_0_4, Format::OpenMetricsText1_0_0]);
let mut body = String::new();

match media_type.format() {
    Format::PrometheusText0_0_4 => prometheus_text::encode(&mut body, &registry).unwrap(),
    Format::OpenMetricsText1_0_0 => text::encode(&mut body, &registry).unwrap(),
    // `Format` is non-exhaustive, and only offered formats are returned.
    _ => unreachable!(),
}

// Respond with `body` and `media_type.content_type()` as the `Content-Type`.

negotiate returns a MediaType, which has the Format to encode in and the Content-Type to respond with. MediaType has private fields, so escaping can be added to it later without breaking anything.

Format is #[non_exhaustive] and has PrometheusText0_0_4 and OpenMetricsText1_0_0, plus PrometheusProtobuf with the prometheus_protobuf feature.

I tried to match the Go implementation as much as possible:

• A range matches a format when its type, subtype and parameters match. charset is ignored.
• A range without a version matches only when it names the media type. Wildcards like / match nothing and fall back, so browsers and curl get text.
• OpenMetrics also matches version=0.0.1, and is answered with 1.0.0.
• When nothing matches, the fallback is the text format if it is offered, otherwise the first offered format.

Fixes: #377

Notes

Added missing CONTENT_TYPE constants to text and prometheus_protobuf

[kalleep]

Hey @krisztianfekete I opened this pr in draft since I wanted to discuss a few things:

1. I included OpenMetricsProtobuf enven though it is deprecated, you want it included or should I remove it?
2. Maybe we should add another function to the returned format that I did not originally think:

impl Format {

    pub fn encode<W: io::Write>(self, writer: &mut W, registry: &Registry) -> io::Result<()> {
        match self {
            Format::OpenMetricsText => {
                let mut buffer = String::new();
                text::encode(&mut buffer, registry).map_err(io::Error::other)?;
                writer.write_all(buffer.as_bytes())
            }
            Format::PrometheusText => {
                let mut buffer = String::new();
                prometheus_text::encode(&mut buffer, registry).map_err(io::Error::other)?;
                writer.write_all(buffer.as_bytes())
            }
            // `encode_to_vec` writes each `MetricFamily` with a length prefix.
            #[cfg(feature = "prometheus_protobuf")]
            Format::PrometheusProtobuf => {
                let buffer = prometheus_protobuf::encode_to_vec(registry).map_err(io::Error::other)?;
                writer.write_all(&buffer)
            }
            #[cfg(feature = "openmetrics_protobuf")]
            Format::OpenMetricsProtobuf => {
                use prost::Message;
                let metric_set = openmetrics_protobuf::encode(registry).map_err(io::Error::other)?;
                writer.write_all(&metric_set.encode_to_vec())
            }
        }
    }
}

This way the caller would not have to think about handling the returned format differently. Also this is just some example code and maybe there is a way to get rid of intermediate buffers.

I did not update any example code yet since I wanted to discuss these points first. Also 2 could be done in a separate pr if you want it and to not make this pr too big.

[herin049]

Personally, I think we should make the negotiation mechanism more configurable that what is currently implemented, and make the returned type not be a format, but a media type (so we can later support escaping schemes). I was thinking something like the following:

#[derive(Clone, Debug)]
pub struct Negotiator {
    formats: Vec<Format>,
    escaping_schemes: Vec<EscapingScheme>,
    default_escaping: EscapingScheme,
    fallback: MediaType,
}

impl Negotiator {
    pub fn new() -> Self { Self::default() }
    pub fn with_formats(self, formats: impl IntoIterator<Item = Format>) -> Self;
    pub fn with_escaping_schemes(self, schemes: impl IntoIterator<Item = EscapingScheme>) -> Self;
    pub fn with_default_escaping(self, scheme: EscapingScheme) -> Self;
    pub fn with_fallback(self, media_type: MediaType) -> Self;
    pub fn negotiate(&self, accept: Option<&str>) -> MediaType;
}

pub enum Format {
    OpenMetricsText,
    PrometheusText0_0_4,
    PrometheusText1_0_0,
    #[cfg(feature = "prometheus_protobuf")]
    #[cfg_attr(docsrs, doc(cfg(feature = "prometheus_protobuf")))]
    PrometheusProtobuf,
}

pub struct MediaType {
    format: Format,
    escaping: EscapingScheme,
}

impl MediaType {
    pub const fn new(format: Format, escaping: EscapingScheme) -> Self;
    pub const fn format(&self) -> Format { self.format }
    pub const fn escaping(&self) -> EscapingScheme { self.escaping }
    pub fn encode<W: io::Write>(&self, registry: &Registry, writer: &mut W) -> Result<(), EncodeError>;
}

Also, +1 to adding an encoding helper which performs the encoding dispatching for us -- I think many users would find this useful.

[kalleep]

Yes I agree that it should probably be a bit more configurable and you have a good point about escaping scheme. I also just found https://prometheus.io/docs/instrumenting/content_negotiation/ so need to make sure we follow that.

[herin049]

Yes I agree that it should probably be a bit more configurable and you have a good point about escaping scheme. I also just found https://prometheus.io/docs/instrumenting/content_negotiation/ so need to make sure we follow that.

I think we can defer adding the escaping scheme support to content negotiation for now, but it's good to keep in mind. I have a WIP PR for adding this and can integrate the changes from this PR once it's merged.

[krisztianfekete]

Thanks both for the discussion on the proposal!

Just to keep this one focused:

1. Let's drop OpenMetricsProtobuf. Go doesn't negotiate it, Prometheus never requests it, and it's deprecated here.
2. +1 on Format::encode, but let's do that in a follow-up PR.
3. Let's keep the free negotiate(accept, offered) -> Format mirroring NegotiateAccept for now. We don't support escaping yet, and Format is #[non_exhaustive], so we can extend it when as per @herin049's escaping work in a follow-up.

Small things to align with Go's matchFormat:

• Wildcards (*/*, type/*) shouldn't satisfy the version param, so they fall through to the text fallback. Browsers and curl then get text/plain even when OpenMetrics is offered first. See the wildcard and browser cases in expfmt/encode_test.go. This is how a project using this library has worked this around: metrics: better negotiation of content type agentgateway/agentgateway#3746
• OpenMetrics with version=0.0.1 (or no version) should match OpenMetricsText.

Once that's in and the examples are updated, feel free to mark it ready for review!

[kalleep]

Thanks for the feedback!

Let's keep the free negotiate(accept, offered) -> Format mirroring NegotiateAccept for now. We don't support escaping yet, and Format is #[non_exhaustive], so we can extend it when as per @herin049's escaping work in a follow-up.

Not sure how this would work in a nice way since we just return the Format enum. If we add escpaing support later we would have to update that enum with a separate type that includes escaping info to not break existing users e.g.

Current:

pub enum Format {
    PrometheusText0_0_4,
    OpenMetricsText1_0_0,
    PrometheusProtobuf,
}

Updated:

pub enum Format {
    PrometheusText0_0_4,
    OpenMetricsText1_0_0,
    OpenMetricsText1_0_0Escaped(EscapingScheme),
    PrometheusProtobuf,
}

So I like the idea that @herin049 had about returning a structure, from his example MediaType, since we can extend that one with escaping scheme without breaking users.

[krisztianfekete]

Thanks for the feedback!

Let's keep the free negotiate(accept, offered) -> Format mirroring NegotiateAccept for now. We don't support escaping yet, and Format is #[non_exhaustive], so we can extend it when as per @herin049's escaping work in a follow-up.

Not sure how this would work in a nice way since we just return the Format enum. If we add escpaing support later we would have to update that enum with a separate type that includes escaping info to not break existing users e.g.

Current:

pub enum Format {
    PrometheusText0_0_4,
    OpenMetricsText1_0_0,
    PrometheusProtobuf,
}

Updated:

pub enum Format {
    PrometheusText0_0_4,
    OpenMetricsText1_0_0,
    OpenMetricsText1_0_0Escaped(EscapingScheme),
    PrometheusProtobuf,
}

So I like the idea that @herin049 had about returning a structure, from his example MediaType, since we can extend that one with escaping scheme without breaking users.

Fair point! Since escaping ends up in the Content-Type, let's have negotiate return a MediaType with private fields, and only format() and content_type() for now, so escaping can be added later without breaking anything. I'd still skip the Negotiator builder and EscapingScheme until they're needed.

[krisztianfekete]

Thanks, this is close! Added a few small comments inline, + signature fix is needed, see CI failure.

[krisztianfekete]

Please also set Vary: Accept here and in the other examples, like promhttp does.

[krisztianfekete]

q=0 means "not acceptable", so let's skip those ranges, and treat q values outside 0.0..=1.0 (e.g. NaN) as invalid.

[krisztianfekete]

Looks good! Could you add a few test cases for q=0, q=NaN, and an uppercase media type, so this doesn't regress?

[kalleep]

Added a couple of more test for it

[krisztianfekete]

Can you please add test cases for a browser Accept header and Prometheus's default scrape Accept header as well?

[krisztianfekete]

Nit: media types are case-insensitive, so eq_ignore_ascii_case would be safer here.

[krisztianfekete]

Nit: let's derive Hash too, to match Format.

[kalleep]

Thanks, addressed all comment 👍

Will follow up with the encode helper after this one is merged!

[krisztianfekete]

Let's still change this to offered: &[Format]. With impl AsRef, calling it as negotiate(accept, &offered) like the doc example does triggers clippy::needless_borrows_for_generic_args in users' code, the same lint that failed CI here.

[kalleep]

Done

[krisztianfekete]

Awesome, thanks!