allow BatchTimeout to be overriden on the libhoney Transmission #509
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
MikeGoldsmith
added
type: enhancement
MikeGoldsmith
requested changes
Sep 12, 2022
There was a problem hiding this comment.
Looks great, thanks @leviwilson 🎉
Only thing missing is adding an example to the config_config.toml to show it being overriden.
MikeGoldsmith
added
the
status: revision needed
done |
|
Thanks @leviwilson ! @MikeGoldsmith fyi someday there's a more interesting problem to solve here, of refinery noticing Honeycomb's rate-limiting errors and adapting. This change will go a long way toward letting people solve this problem themselves now. |
MikeGoldsmith
approved these changes
Sep 14, 2022
kentquirk
removed
the
status: revision needed
ghost
pushed a commit
to opsramp/tracing-proxy
that referenced
this pull request
Jul 5, 2024
…ycombio#509) This PR allows for the BatchTimeout to be configured for refinery rather than explicitly using DefaultBatchTimeout from libhoney * allow BatchTimeout to be overridden on the libhoney Transmission * add an example of modifying BatchTimeout in config_complete.toml
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?
We have an issue where our ALB traffic has a
~5mlatency because of the cadence in which the CloudWatch logs are sent. Because the ALB represents our top-level traces, this means that all web traffic is delayed by that same amount.Decreasing the
TraceTimeoutvalue assists with this by spreading out the traffic, however this makes our rules a moot point as things we are intentionally trying to downsample do not get downsampled.What we really need is a manner to control how frequently events are sent upstream to Honeycomb so we can control how frequently we send things. https://github.com/honeycombio/libhoney-go/blob/8af08e456cfe1f786247ffdc755855999add2d76/libhoney.go#L136 indicates that
SendFrequencycan override how often batches are sent.Short description of the changes
This PR allows for the
BatchTimeoutto be configured for refinery rather than explicitly usingDefaultBatchTimeout. I realizeSendFrequencyis what was mentioned above, but refinery callsNewClientrather thanInitand you can see that allSendFrequencydoes is driveBatchTimeouton theTransmissionanyway.