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

wip: start toying with wrapping requests withRedFmetrics #222

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ggilmore
Copy link
Contributor

In addition to the metrics covered by RED, Google's SRE's "Monitoring Distributed Systems" recommends also capturing the the duration of failed operations as well. Tracking that separately seems important:

  • the duration of failed operations can have wildly different timings to successful ones, so we shouldn't conflate the two in an average
  • failed operations that are slow can still cause a bad UX

Zoekt seems to collect some info on the duration of failed requests, but it seems like it'd be nice to standardize that.


For now, this PR is toying with implementing the same pattern for bundling the RED metrics (+ the failed operation latency) that sourcegraph/sourcegraph's metrics package and influxDB uses.

It's not attempting to tie tracing/logging in the same module like sourcegraph/sourcegraph's metrics package - I thought it be better to keep it simpler for now.

Copy link
Member

@keegancsmith keegancsmith left a comment

Choose a reason for hiding this comment

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

cool stuff. Looks good to me so far. I'd also look into applying this to meteredSearcher in the sourcegraph codebase.

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