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

Support custom labels #68

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

sashayakovtseva
Copy link
Contributor

@sashayakovtseva sashayakovtseva commented Jun 17, 2021

Hello @slok,

I've added custom labels support for my project and now I see that this may be the solution for #47. Also did tiny tests cleanup during my work.

Custom labels are supported for prometheus as this is the collector I personally use. However, new functionality does not break previous implementation of Recorder and anyone can add custom labels support for any existing recorder.

FastHTTP example is also provided.

Hope this PR help someone.

@sashayakovtseva sashayakovtseva requested a review from slok as a code owner June 17, 2021 17:56
@sashayakovtseva
Copy link
Contributor Author

Hey @slok, would you take a look at this PR please?

@slok
Copy link
Owner

slok commented Feb 11, 2022

Hi @sashayakovtseva

Thanks for the PR, sorry I missed this one.

I don't know if we want to implement this in the library as today, dynamic labels are tricky and could have undesired side effects.

For now, I would like to park this feature and think a bit more.

Also, we have this other PR #74 that implements a similar feature.

@sashayakovtseva
Copy link
Contributor Author

This PR was opened before #74

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

Successfully merging this pull request may close these issues.

2 participants