Increase default Jaeger queue size for store-gateways and queriers #7068
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
…eriers when using Helm
charleskorn
force-pushed
the
charleskorn/jaeger-queue-size
branch
from
fcebfc7
to
e3cdb70
Compare
…elm / Jsonnet comparison
charleskorn
force-pushed
the
charleskorn/jaeger-queue-size
branch
from
947c57e
to
5456a48
Compare
56quarters
approved these changes
Jan 9, 2024
| @@ -61,6 +61,8 @@ | |||
| ), | |||
| // Dynamically set GOMEMLIMIT based on memory request. | |||
| GOMEMLIMIT: std.toString(std.floor($.util.siToBytes($.store_gateway_container.resources.requests.memory))), | |||
|
|
|||
| JAEGER_REPORTER_MAX_QUEUE_SIZE: '1000', | |||
There was a problem hiding this comment.
Do we want to make this a configuration setting in jsonnet for parity with Helm?
There was a problem hiding this comment.
Marco suggested not doing that in https://github.com/grafana/mimir/pull/6764/files#diff-f3070297971f928d50b986208d8f23344d5c0a1d37f2c1422eed86feea6bb31a, so I'm following the pattern established there. I'd be happy either way.
I went for a different approach in Helm because I believe it's a harder (or even impossible) to extend the default value of something, so it'd be tricky for someone to extend our default set of environment variables that includes JAEGER_REPORTER_MAX_QUEUE_SIZE with additional variables. Might be wrong though.
2 tasks
charleskorn
added a commit
that referenced
this pull request
Jan 10, 2024
charleskorn
added a commit
that referenced
this pull request
Jan 10, 2024
… size for all components configurable in Helm chart (#7086) * Set JAEGER_REPORTER_MAX_QUEUE_SIZE for write and backend containers deployed by Jsonnet. * Upstream default values of JAEGER_REPORTER_MAX_QUEUE_SIZE used for query-frontends, ingesters and rulers at Grafana Labs * Add support for setting Jaeger reporter max queue size to Helm chart * Update default values in Helm chart for ruler, ingester and query-frontends * Add changelog entries, and fix location of Helm chart changelog entry added in #7068.
7 tasks
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.
What this PR does
This PR upstreams a change we've made internally at Grafana Labs which reduces the number of trace spans dropped due to the Jaeger client queue becoming full.
It increases the value of Jaeger's queue size for store-gateways and queriers when using Helm and Jsonnet.
Note that the increase for queriers when using Jsonnet was already applied in https://github.com/grafana/mimir/pull/6764/files#diff-b51f5403fbd4f68fb4ad925c7fdd104a50c4165085b19f75b54405472f74b8f0.
Note to reviewers: if the pattern of adding a
jaegerReporterMaxQueueSizeHelm value for each component looks good, I'll follow up this PR with another to add this for all components.Which issue(s) this PR fixes or relates to
(none)
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX].about-versioning.mdupdated with experimental features.