feat(metrics): Add metrics buffering and sampling #821
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
flub
reviewed
Oct 27, 2020
flub
approved these changes
Oct 28, 2020
jan-auer
approved these changes
Oct 28, 2020
| @@ -75,6 +83,8 @@ pub struct MetricsClient { | |||
| pub statsd_client: StatsdClient, | |||
| /// Default tags to apply to every metric | |||
| pub default_tags: BTreeMap<String, String>, | |||
| /// Global sample rate | |||
| pub sample_rate: f32, | |||
There was a problem hiding this comment.
Note: After merging, this needs to be added to docs as well.
| @@ -16,7 +16,7 @@ clean: | |||
| # Builds | |||
|
|
|||
| build: setup-git | |||
| @cargo +stable build --all-features | |||
| cargo +stable build --all-features | |||
There was a problem hiding this comment.
This change is unrelated, can we revert?
There was a problem hiding this comment.
I found it useful to see what we're actually building, it's the same behavior we have for test and lint targets.
| @@ -140,15 +177,51 @@ pub fn configure_statsd<A: ToSocketAddrs>( | |||
| prefix: &str, | |||
| host: A, | |||
| default_tags: BTreeMap<String, String>, | |||
| buffering: bool, | |||
| sample_rate: f32, | |||
There was a problem hiding this comment.
This function is now sufficiently complex to become a builder. I'm happy to make the changes in a follow-up, since I'd like to move all metrics and logging related concerns from relay-common into a separate crate anyway.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds the following features:
metrics.buffering). If enabled, metrics will be queued and buffered in a separate thread. Set totrueby default.Caveat:
BufferedUdpMetricSinkdoesn't flush until the buffer is full, so theoretically, if relay doesn't emit any metrics for a while (or if the sampling rate is set to something low), there could be delays in sending the metrics. But some metrics relay sends (e.g. about open/reused connections) are emitted regularly, so it shouldn't be an issue.metrics.sample_rate). Set to 1.0 by default.Note: the sampling implementation is pretty naive now, so for example we might simply ignore some calls to counters without scaling it back.