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

Add relabeling setting to add label only to response count metric #128

Merged

Conversation

stempler
Copy link
Contributor

Dynamic relabeling can potentially result in a lot of time series being collected, especially related to the response and upstreams time histograms.
This change is intended to allow collecting more detailed information on response count without the side effect of adding more dimensions to the other metrics.

I hope this is something that you may find useful. Basically my first time writing any Go code, so you are very welcome to propose improvements to the code.

@helmich-bot
Copy link
Collaborator

There has not been any activity to this pull request in the last 30 days. It will automatically be closed after 7 more days. Remove the stale label to prevent this.

@martin-helmich
Copy link
Owner

No, not stale. This is still on my to-do list, @stempler -- sorry about the wait.

Copy link
Owner

@martin-helmich martin-helmich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finally getting to this, sorry for the long wait 🙄

👍 in general from me. I've made a suggestion for a tiny possible optimization. Also, could you possibly add some tests for this feature? I personally don't have any use case for this feature and probably won't notice if I accidentally break it in future releases. 🤷

main.go Outdated Show resolved Hide resolved
main.go Show resolved Hide resolved
Dynamic relabeling can potentially result in a lot of time series being
collected, especially related to the response and upstreams time
histograms.
This change is intended to allow collecting more detailed information on
response count without the side effect of adding more dimensions to the
other metrics.
@codeclimate
Copy link

codeclimate bot commented Jul 13, 2020

Code Climate has analyzed commit 3c90389 and detected 0 issues on this pull request.

View more on Code Climate.

@helmich-bot
Copy link
Collaborator

There has not been any activity to this pull request in the last 30 days. It will automatically be closed after 7 more days. Remove the stale label to prevent this.

@martin-helmich
Copy link
Owner

Sorry for the lack of attention. I'll give this PR another look shortly.

@martin-helmich martin-helmich self-assigned this Dec 13, 2020
@martin-helmich martin-helmich merged commit f88faa0 into martin-helmich:master Jan 5, 2021
@martin-helmich
Copy link
Owner

Finally got around to have a detailed look at this -- merged 🎉 (I pushed a few minor nitpicks on top of your branch myself). 👍 Thanks for the contribution and again sorry for the wait.

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.

3 participants