-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
pkg/store: Expose metric about empty stream responses in proxy store #2030
Conversation
pkg/store/proxy.go
Outdated
@@ -348,6 +374,12 @@ func startStreamSeriesSet( | |||
defer wg.Done() | |||
defer close(s.recvCh) | |||
|
|||
i := 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like a more descriptive name like numResponses
would be better. I am way too used to seeing i
as a placeholder variable in for-loops for the index 😄 Here we do an infinite loop so it probably doesn't fit as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed
d572ae8
to
10e2f6e
Compare
There was an attempt to add a similar thing here as well: #1789 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a build error, but otherwise it looks good (:
Good start: observe how often we hit this situation!
Giedrius, not really connected to other issues, it's not about slow stores but rather stores that were asked for data that they don't have.
10e2f6e
to
5a8026e
Compare
Signed-off-by: Frederic Branczyk <fbranczyk@gmail.com>
5a8026e
to
63c2d53
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Hence the word 'similar' 😄 |
Changes
This adds a metric to track how often we do requests that end up in empty responses. Related to #1611.
Verification
Did queries that stores didn't have data for, and verified that indeed the
/metrics
endpoint reflects this.@bwplotka @squat @GiedriusS