Skip to content
This repository has been archived by the owner on Apr 18, 2023. It is now read-only.

Allow limiting grpc codes tracked in histrogram #96

Open
brancz opened this issue Dec 30, 2020 · 3 comments
Open

Allow limiting grpc codes tracked in histrogram #96

brancz opened this issue Dec 30, 2020 · 3 comments
Labels

Comments

@brancz
Copy link
Collaborator

brancz commented Dec 30, 2020

Latency alerts tend to be most focused on succeeding calls, and since errors can already be accurately I would like to allow to ignore failing requests from the histogram metrics.

I am a bit torn though on what to do:

  1. Make this configurable, I don't really like this because suddenly the same histogram metric of different processes may behave totally different from the next.
  2. Change the default, this might cause unexpected behavior for users that are already using the library today.

Since I'm unable to ignore failed requests from the histogram at all today, I'm going to mark this as a bug, as it's essentially of no use as it is for alerting purposes.

Wdyt @bwplotka ?

@brancz brancz added the bug label Dec 30, 2020
@bwplotka
Copy link
Collaborator

Totally +100 to this. I was thinking about this as well. In fact I think we should care about user error, system error etc. Histograms per grpc code would be bit too cardinal IMO, but is also an option.

I have a proposition:

WDYT?

@brancz
Copy link
Collaborator Author

brancz commented Dec 30, 2020

  1. would be totally possible as a major version breaking change, so that sounds good to me. What's left to "finalize"? Happy to help out!

@bwplotka
Copy link
Collaborator

bwplotka commented Dec 30, 2020

Here is a plan: https://github.com/grpc-ecosystem/go-grpc-middleware/issues?q=is%3Aissue+is%3Aopen+label%3Av2

Literally, at any point now, we could release v2 officially. We already have release candidates. But I would love to have metrics in first so grpc-ecosystem/go-grpc-middleware#346

Plus I think the last thing that has to be finalized is moving to newest protobuf Go bindings which I kind of stopped at some point doing, so help wanted: grpc-ecosystem/go-grpc-middleware#321

Once grpc-ecosystem/go-grpc-middleware#346 and grpc-ecosystem/go-grpc-middleware#321 are in I believe we can ship v2. Plus users can already use v2 it's quite stable (except proto and grpc deps update). Both blockers are not connected as well you can help move this repo if you want 🤗

Maybe I will find time this or next week for grpc-ecosystem/go-grpc-middleware#321

cc @johanbrandhorst

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants