feat: Update Honeycomb logger to use EMAThroughput sampler #1328
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
|
Will this also aid #1222? |
Yes, it probably 👍🏻 |
kentquirk
approved these changes
Sep 16, 2024
logger/honeycomb.go
Outdated
| PerKeyThroughputPerSec: loggerConfig.SamplerThroughput, | ||
| MaxKeys: 1000, | ||
| h.sampler = &dynsampler.EMAThroughput{ | ||
| AdjustmentInterval: 10 * time.Second, |
There was a problem hiding this comment.
Let's lengthen this interval -- maybe to 30s, please?
VinozzZ
reviewed
Sep 16, 2024
|
We already sample refinery log messages; this change is just give burst protection when a sudden increase increase happens. The throughput samplers ensure any unique log message is sent at least once per window. |
TylerHelmuth
pushed a commit
that referenced
this pull request
Oct 16, 2024
## Which problem is this PR solving? Updates the Honeycomb logger to use the [EMAThroughput](https://github.com/honeycombio/dynsampler-go/blob/main/emathroughput.go#L77) sampler. This sampler has built-in support for burst protection that the [PerKeyThroughput](https://github.com/honeycombio/dynsampler-go/blob/main/perkeythroughput.go#L17) sampler does not. Burst protection would be useful when a cluster node does down unexpectedly because the other nodes in the cluster will fail to make peer requests until it is removed from the cluster. This can result in a very high number of "failed to send" log messages in a very small window, faster than what the PerKeyThroughput sampler can adjust the sample rate which results in all log messages being sent. The burst protection from the EMAThroughput sampler would help here as it will schedule an update to sample rates if a high number of events are received very quickly, allowing it to react quicker. This is a like for like replacement, and such I haven't added any additional configuration options the new sampler supports. We can add more later if desired. ## Short description of the changes - Replace the Honeycomb logger sampler with the EMAThroughput sampler.
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.
Which problem is this PR solving?
Updates the Honeycomb logger to use the EMAThroughput sampler. This sampler has built-in support for burst protection that the PerKeyThroughput sampler does not.
Burst protection would be useful when a cluster node does down unexpectedly because the other nodes in the cluster will fail to make peer requests until it is removed from the cluster. This can result in a very high number of "failed to send" log messages in a very small window, faster than what the PerKeyThroughput sampler can adjust the sample rate which results in all log messages being sent. The burst protection from the EMAThroughput sampler would help here as it will schedule an update to sample rates if a high number of events are received very quickly, allowing it to react quicker.
This is a like for like replacement, and such I haven't added any additional configuration options the new sampler supports. We can add more later if desired.
Short description of the changes