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.

Sign up for GitHub

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

Jump to bottom

Encoder without EOF #153

Closed
amunra opened this issue Jul 11, 2023 · 5 comments · Fixed by #205
Closed

Encoder without EOF #153

amunra opened this issue Jul 11, 2023 · 5 comments · Fixed by #205

Comments

@amunra
Copy link
Contributor

amunra commented Jul 11, 2023

Hello!
I have a more uncommon use case where I'm using Rust in a multi-language setup.

The rest of the system is already serving requests to Prometheus and I'd like to slot in a few metrics for the parts of my application written in Rust.

I like that the client doesn't do HTTP serving and stick to the protocol, so it's 99% of what I need.

What causes me trouble is just a single line of code:

    writer.write_str("# EOF\n")?;

which prevents me from using this crate in a way that's composable with other output.

Would you consider a variant of the encode function that doesn't write out the # EOF?

Maybe called pub fn encode_metrics?
@amunra
Copy link
Contributor Author

amunra commented Jul 11, 2023

I'm happy to raise a PR, but if I am wondering if you'd have a better suggestion, especially in the context of these upcoming changes: #149.

@mxinden
Copy link
Member

mxinden commented Jul 12, 2023

I like that the client doesn't do HTTP serving and stick to the protocol, so it's 99% of what I need.

Good to hear.

What causes me trouble is just a single line of code:

    writer.write_str("# EOF\n")?;

:D

Would you consider a variant of the encode function that doesn't write out the # EOF?

Generally yes. I see the use-case of wanting to combine multiple encode outputs, whether from the same client library or different ones.

That said, in case performance is not an issue for you and you are fine with a small hack, why not encode into a String and then drop the last line?

@mxinden
Copy link
Member

mxinden commented Jul 12, 2023

I'm happy to raise a PR, but if I am wondering if you'd have a better suggestion, especially in the context of these upcoming changes: #149.

I don't see how #149 would help or effect your situation. You could implement your own Collector that collects the metrics from the parts written in a different language. Though that sounds very painful.

@amunra
Copy link
Contributor Author

amunra commented Jul 12, 2023 •
edited
Loading

Ours is a performance-sensitive application and I'd rather not have to double-buffer :-)

We embed Rust, rather than the other way around.

@amunra
Copy link
Contributor Author

amunra commented Jul 12, 2023 •
edited
Loading

The reason why I was mentioning PR #149 is that as I was looking at it I noticed the new encode is implemented as:

/// Encode the metrics registered with the provided [`Registry`] into the
/// provided [`Write`]r using the OpenMetrics text format.
pub fn encode<W>(writer: &mut W, registry: &Registry) -> Result<(), std::fmt::Error>
where
    W: Write,
{
    registry.encode(&mut DescriptorEncoder::new(writer).into())?;
    writer.write_str("# EOF\n")?;
    Ok(())
}

pub(crate) struct DescriptorEncoder<'a> {
    writer: &'a mut dyn Write,
    prefix: Option<&'a Prefix>,
    labels: &'a [(Cow<'static, str>, Cow<'static, str>)],
}

The alternative here might be to expose the DescriptorEncoder as pub rather than pub(crate). I think an independent function is cleaner (though I can't necessarily think of a good name).

If not, I'll wait for PR #149 to be merged and raise mine after (given that code is changing anyways).

@amunra amunra mentioned this issue Jul 13, 2023
Closed
tyrone-wu added a commit to tyrone-wu/client_rust that referenced this issue Jun 27, 2024
@tyrone-wu @amunra
feat(encoding): add functions for individually encoding registry & EOF
9f024f7 
All credits on the initial idea, implementation, and testing belong to
@amunra, who is the original author of this PR prometheus#154.

From the original PR description:

Adds new `encode_registry` and `encode_eof` functions to allow encoding
of parts of the response.

This is useful when there are multiple registries at play, or when
composing metrics for a process that embeds Rust as part of its logic
whilst serving metrics to Prometheus outside of Rust.

Fixes: prometheus#153
Refs: prometheus#154, prometheus#204

Co-authored-by: Adam Cimarosti <amunra@users.noreply.github.com>
Signed-off-by: Tyrone Wu <wudevelops@gmail.com>
@tyrone-wu tyrone-wu mentioned this issue Jun 27, 2024
Merged
tyrone-wu added a commit to tyrone-wu/client_rust that referenced this issue Jun 27, 2024
@tyrone-wu @amunra
feat(encoding): add functions for individually encoding registry & EOF
86d81e1 
All credits on the initial idea, implementation, and testing belong to
@amunra, who is the original author of this PR prometheus#154.

From the original PR description:

Adds new `encode_registry` and `encode_eof` functions to allow encoding
of parts of the response.

This is useful when there are multiple registries at play, or when
composing metrics for a process that embeds Rust as part of its logic
whilst serving metrics to Prometheus outside of Rust.

Fixes: prometheus#153
Refs: prometheus#154, prometheus#204

Co-authored-by: amunra <amunra@users.noreply.github.com>
Signed-off-by: tyrone-wu <wudevelops@gmail.com>
tyrone-wu added a commit to tyrone-wu/client_rust that referenced this issue Jun 27, 2024
@tyrone-wu @amunra
feat(encoding): add functions for individually encoding registry & EOF
b643b2e 
All credits on the initial idea, implementation, and testing belong to
@amunra, who is the original author of this PR prometheus#154.

From the original PR description:

Adds new `encode_registry` and `encode_eof` functions to allow encoding
of parts of the response.

This is useful when there are multiple registries at play, or when
composing metrics for a process that embeds Rust as part of its logic
whilst serving metrics to Prometheus outside of Rust.

Fixes: prometheus#153
Refs: prometheus#154, prometheus#204

Co-authored-by: amunra <cimarosti@gmail.com>
Signed-off-by: tyrone-wu <wudevelops@gmail.com>
tyrone-wu added a commit to tyrone-wu/client_rust that referenced this issue Jul 3, 2024
@tyrone-wu @amunra
feat(encoding): add functions for individually encoding registry & EOF
7b7f1fb 
All credits on the initial idea, implementation, and testing belong to
@amunra, who is the original author of this PR prometheus#154.

From the original PR description:

Adds new `encode_registry` and `encode_eof` functions to allow encoding
of parts of the response.

This is useful when there are multiple registries at play, or when
composing metrics for a process that embeds Rust as part of its logic
whilst serving metrics to Prometheus outside of Rust.

Fixes: prometheus#153
Refs: prometheus#154, prometheus#204

Co-authored-by: amunra <cimarosti@gmail.com>
Signed-off-by: tyrone-wu <wudevelops@gmail.com>
@mxinden mxinden closed this as completed in a914fc9 Jul 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@amunra @mxinden