Add items and bytes limits to baggage extraction#11265
Add items and bytes limits to baggage extraction#11265gh-worker-dd-mergequeue-cf854d[bot] merged 1 commit intomasterfrom
Conversation
PerfectSlayer
added
type: enhancement
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a0f2ed289
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| boolean limitReached = baggage.size() >= maxItems || end > maxBytes; | ||
| if (limitReached) { |
There was a problem hiding this comment.
When an incoming baggage header exceeds trace.baggage.max.items or trace.baggage.max.bytes, this new limit branch truncates the extracted baggage but never calls BAGGAGE_METRICS.onBaggageTruncatedByItemLimit() or onBaggageTruncatedByByteLimit(). In those scenarios extract() still reports a successful extraction, so telemetry consumers lose the context_header.truncated signal that injection already emits for the same configured limits.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Those metrics are used for context injection, not extraction… and the team in charge of this code part and metrics already ready the patch.
| int kvSeparatorInd = input.indexOf(KEY_VALUE_SEPARATOR); | ||
| while (kvSeparatorInd != -1) { | ||
| int end = pairSeparatorInd; | ||
| boolean limitReached = baggage.size() >= maxItems || end > maxBytes; |
There was a problem hiding this comment.
btw I think that this is an approximation since end works with nb of chars and not bytes
There was a problem hiding this comment.
Yes: byte limit is enforced against String char (UTF-16 code unit) length: exact for ASCII baggage, conservative on memory for multi-byte UTF-8 input.
Using the exact byte limit would have allocated memory which we try to prevent with this fix.
PerfectSlayer
force-pushed
the
bbujon/baggage
branch
from
8a0f2ed to
4900d76
Compare
|
I think you need to rebase from master, there is a missing gitlab stage that is present on master branch now |
PerfectSlayer
force-pushed
the
bbujon/baggage
branch
from
4900d76 to
deeecf6
Compare
|
/merge |
|
View all feedbacks in Devflow UI.
This pull request is not mergeable according to GitHub. Common reasons include pending required checks, missing approvals, or merge conflicts — but it could also be blocked by other repository rules or settings.
The expected merge time in
|
gh-worker-dd-mergequeue-cf854d
Bot
merged commit 16c6a5f
into
master
3 participants
What Does This Do
This PR ensure baggage extraction applies baggage items and bytes limits.
Motivation
Keeping too many bagage items or too big baggage items can increase memory usage for the time the related extracted context is active.
Additional Notes
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]
Note: Once your PR is ready to merge, add it to the merge queue by commenting
/merge./merge -ccancels the queue request./merge -f --reason "reason"skips all merge queue checks; please use this judiciously, as some checks do not run at the PR-level. For more information, see this doc.