Skip to content

Conversation

@svanharmelen
Copy link
Contributor

This PR is somewhat related to #529, but it's not trying to achieve/solve the same thing.

For a few of our applications we get some label values (e.g. client ID, client type, IP address) during a protocol handshake. Once the connection is setup each connection has a session object that remains valid during the lifetime of the session. So we were looking for a way to store the connection specifiek values in either a map or a slice so we could use them with vec.with(&session.labels) or vec.with_label_values(&session.label_values).

But since the current implementation requires these maps or slices to only contain &str values, this currently isn't possible without some additional hacks or workarounds which makes the code less clean and less efficient.

Allowing values that implement AsRef<str> (so you can use HashMap<&str, String> maps or Vec<String> slices) solved this issue.

P.S. The second commit only contains a few LSP and Clippy related fixes to silence any warnings. Let me know if that is OK, if not I will drop that commit!

@ti-chi-bot
Copy link

ti-chi-bot bot commented Mar 3, 2025

Welcome @svanharmelen! It looks like this is your first PR to tikv/rust-prometheus 🎉

@svanharmelen
Copy link
Contributor Author

I noticed two checks failed, but it looks like its not related to my PR. Do you want me to try and fix it in my PR or will those be fixed in main instead?

@lucab
Copy link
Member

lucab commented Mar 12, 2025

Thanks for the PR.
Yes CI is currently red, the fix for that is at #539 and pending review.

P.S. The second commit only contains a few LSP and Clippy related fixes to silence any warnings. Let me know if that is OK, if not I will drop that commit!

That commit would better go into its own PR.

@svanharmelen
Copy link
Contributor Author

svanharmelen commented Mar 12, 2025

Thanks @lucab, removed the commit and created #540 for that 👍🏻

Signed-off-by: Sander van Harmelen <[email protected]>
@lucab lucab merged commit 4a0e282 into tikv:master Mar 19, 2025
8 checks passed
@lucab lucab mentioned this pull request Mar 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants