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 status code label to handled_latency_seconds histogram #50

Open
ashamukov opened this issue Oct 28, 2021 · 5 comments
Open

Add status code label to handled_latency_seconds histogram #50

ashamukov opened this issue Oct 28, 2021 · 5 comments

Comments

@ashamukov
Copy link
Contributor

Hello!
I'd like to add status code label to the handled_latency_seconds histogram, to distinguish timings by this criteria.
To activate this feature, a boolean flag could be added to Configuration.
Looks like pretty obvious enhancement, but may be you see some drawbacks? What do you think?

If this is good idea, could you also please tell, how much time is required to publish a new release (after potential commit)?
Thanks!

@dinowernli
Copy link
Member

This sounds reasonable to me. The only concern would be the potential blowup of the amount of data due to the cardinality of the code enum combined with the already quite-large histogram metric. That said, if it's opt-in (as you're proposing), I don't see any issues. Feel free to send me a PR.

As for releasing, I'm maintaining this library in my free time so releases tend to happen on-demand and subject to my available bandwidth. That said, the amount of work required for each release has recently dropped significantly, so I'd be happy to cut a new one once the feature has landed.

@ashamukov
Copy link
Contributor Author

Hi Dino! Thanks for the answer! Sounds great, hope I'll come back soon with PR! 🙏

@ashamukov
Copy link
Contributor Author

Hi Dino! It's me again) Just would like to say, that if some of release-cutting work is "forkable", I could also help)

@andreweacott
Copy link

Hi, I was looking at this issue and realised the PR was merged but a new release wasn't cut.
I'd also be happy to help out if there's anything you want to offload.

@dinowernli
Copy link
Member

I've just cut release 0.6.0 with this change and have pushed it to maven. According to the docs, it should be available in the next 24h.

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

No branches or pull requests

3 participants