This repository has been archived by the owner on Mar 28, 2019. It is now read-only.
Sample successful delivery and connections stats at 10% #217
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
ghost
added this to the 
ghost
force-pushed
the
feature/sample-rate
branch
from
93ae105
to
04ea63a
Compare
ghost
force-pushed
the
feature/sample-rate
branch
from
04ea63a
to
a3cf377
Compare
ghost
force-pushed
the
feature/sample-rate
branch
2 times, most recently
from
8c85f6a
to
603c84d
Compare
| @@ -426,7 +426,7 @@ func (b *EtcdBalancer) Fetch() (peers *EtcdPeers, err error) { | |||
| b.metrics.Increment("balancer.fetch.error") | |||
| return nil, err | |||
| } | |||
| b.metrics.Increment("balancer.fetch.success") | |||
| b.metrics.IncrementByRate("balancer.fetch.success", 1, 0.1) | |||
There was a problem hiding this comment.
Not sure if this needs a low sample rate...we're only polling etcd every 10 seconds by default, and updating every minute. Even with both rates reduced to a second, that seems like a trickle compared to the storm of delivery metrics.
added 4 commits
March 3, 2015 09:19
* Ensure `IncrementByRate()` passes the absolute value of the delta to `StatsClient.Dec()`. * Replace `TestMetrics` with a `StatsClient` interface for `Metrics`. * Lazily initialize `Metrics` snapshots. * Remove remaining logged metrics. * Add tests. Closes #177.
`{IncrementBy, Timer}Rate()` and `GaugeDelta()` no longer call the
underlying `StatsClient` methods for zero values.
ghost
force-pushed
the
feature/sample-rate
branch
from
603c84d
to
74fdb69
Compare
|
Looks fine, except the sampling rate shouldn't be hard-coded, it should be a config value so we can determine it when we deploy. |
|
@bbangert Sounds good. Do you think a single config value for all metrics should be sufficient? Does the sampling logic look sound? |
|
@kitcambridge I think a single value for the heaviest usage ones is fine. Sampling logic looks sound. I am curious about how much faster the stats might be if they stopped doing all that inefficient string concat. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Metrics.IncrementByRate()andTimerByRate()for specifying the sampling rate.IncrementByRate()would callStatsClient.Dec()with a negative value.TestMetricswith aStatsClientinterface.Closes #177.