fix(tmplexec): memory blowup in multiproto#6258
Conversation
Signed-off-by: Dwi Siswanto <git@dw1.io>
Co-authored-by: Nakul Bharti <knakul853@users.noreply.github.com> Signed-off-by: Dwi Siswanto <git@dw1.io>
|
""" WalkthroughThe changes refactor how event data is stored in the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Template Executor
participant Utils as utils.FillPreviousEvent
participant Map as previous Map
Caller->>Utils: FillPreviousEvent(ID, event, previous)
Utils->>Map: Insert event data with appropriate keys
Utils-->>Caller: Return
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pkg/tmplexec/utils/utils.go (1)
18-35: Minor micro-optimisations & robustness forFillPreviousEventThe function does its job, but two very small tweaks could tighten it further:
strings.Builderis allocated once but thenReset()is called every iteration; allocating inside the loop is a bit clearer and avoids holding onto a potentially large backing array between calls (edge-case memory win).- Ignoring the error from
previous.Setmakes sense today (it always returnsnil), but a short comment would future-proof the intent.- builder := &strings.Builder{} - for k, v := range event.InternalEvent { + for k, v := range event.InternalEvent { + var builder strings.Builder if reqTypeWithIndexRegex.MatchString(k) { _ = previous.Set(k, v) // always returns nil continue } - builder.WriteString(ID) - builder.WriteString("_") - builder.WriteString(k) - _ = previous.Set(builder.String(), v) - builder.Reset() + builder.WriteString(ID) + builder.WriteByte('_') + builder.WriteString(k) + _ = previous.Set(builder.String(), v) // nolint:errcheck reason: Set never errors today }Not critical, just polish.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
pkg/tmplexec/generic/exec.go(2 hunks)pkg/tmplexec/multiproto/multi.go(2 hunks)pkg/tmplexec/utils/utils.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
pkg/tmplexec/multiproto/multi.go (1)
pkg/tmplexec/utils/utils.go (1)
FillPreviousEvent(18-35)
pkg/tmplexec/generic/exec.go (1)
pkg/tmplexec/utils/utils.go (1)
FillPreviousEvent(18-35)
pkg/tmplexec/utils/utils.go (2)
pkg/templates/types/types.go (1)
SupportedProtocolsStrings(73-82)pkg/output/output.go (2)
InternalWrappedEvent(100-113)InternalEvent(93-93)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Lint
🔇 Additional comments (5)
pkg/tmplexec/utils/utils.go (1)
12-14: Regex stays frozen at init – remember to re-evaluate when adding new protocols
reqTypeWithIndexRegexis compiled once at package‐init time from the list returned bytypes.SupportedProtocolsStrings().
If a new protocol is introduced later in the codebase butSupportedProtocolsStrings()is not updated (or its output changes dynamically), the regex will silently miss the new prefix and keys will receive an unwantedID_prefix.Just worth keeping in mind for future protocol additions.
pkg/tmplexec/multiproto/multi.go (2)
12-12: Nice: unnecessarystringsimport removedImport list is slimmer after centralising the logic in
utils.FillPreviousEvent.
93-93: Centralised previous-event handling – good callReplacing the former inline key-mangling block with
utils.FillPreviousEventkeeps this file lean and eliminates duplicate logic.pkg/tmplexec/generic/exec.go (2)
10-10: Import change looks cleanDependency on
utilsintroduced, no other import fallout.
68-69: Refactor improves readabilityDelegating to
utils.FillPreviousEventremoves several lines of string handling and keeps behaviour consistent withmultiproto.
Signed-off-by: Dwi Siswanto <git@dw1.io>
tarunKoyalwar
left a comment
There was a problem hiding this comment.
FYI , we have this memory blow up issue in flow protocol templates as well , if you run any code protocol template like aws audit which has for loop it will exponentially stack all previous events as long as for loop runs
|
also just in case i think we should confirm this issue is not happening in flow protocol templates, flow protocol executor doesn't use previous event instead uses a cc: @dwisiswant0 |
The `FillPreviousEvent` func was modified to prevent overwriting/duplicating entries in the previous map. It now checks if a key `k` from `event.InternalEvent` already exists in the previous map. If it does, the key is skipped. This ensures that if `k` was already set (potentially w/o a prefix), it's not re-added with an `ID_` prefix. Additionally, keys in `event.InternalEvent` that already start with the current `ID_` prefix are also skipped to avoid redundant prefixing. This change simplifies the logic by removing the `reqTypeWithIndexRegex` and directly addresses the potential for duplicate / incorrectly prefixed keys when `event.InternalEvent` grows during protocol request execution. Signed-off-by: Dwi Siswanto <git@dw1.io>
Thanks for pointing that out, @tarunKoyalwar! Could you please re-review 8f5e2be? TODO:
|
Signed-off-by: Dwi Siswanto <git@dw1.io>
|
Current tests: basic-template-multiproto (w/ request ID)id: basic-template-multiproto
info:
name: Test Template Multiple Protocols
author: pdteam
severity: info
http:
- method: GET
id: first_iter_http
path:
- '{{BaseURL}}/1'
matchers:
- type: word
words:
- "Test is test matcher text"
- method: GET
path:
- '{{BaseURL}}/2'
matchers:
- type: word
words:
- "Test is test matcher text"
- method: GET
path:
- '{{BaseURL}}/3'
matchers:
- type: word
words:
- "Test is test matcher text"
- method: GET
path:
- '{{BaseURL}}/4'
matchers:
- type: word
words:
- "Test is test matcher text"
- method: GET
path:
- '{{BaseURL}}/5'
matchers:
- type: word
words:
- "Test is test matcher text"
- method: GET
path:
- '{{BaseURL}}/6'
matchers:
- type: word
words:
- "Test is test matcher text"
- method: GET
path:
- '{{BaseURL}}/7'
matchers:
- type: word
words:
- "Test is test matcher text"
- method: GET
path:
- '{{BaseURL}}/8'
matchers:
- type: word
words:
- "Test is test matcher text"
- method: GET
path:
- '{{BaseURL}}/9'
matchers:
- type: word
words:
- "Test is test matcher text"
- method: GET
path:
- '{{BaseURL}}/10'
matchers:
- type: word
words:
- "Test is test matcher text"
- method: GET
path:
- '{{BaseURL}}/11'
matchers:
- type: word
words:
- "Test is test matcher text"
- method: GET
path:
- '{{BaseURL}}/12'
matchers:
- type: word
words:
- "Test is test matcher text"
- method: GET
path:
- '{{BaseURL}}/13'
matchers:
- type: word
words:
- "Test is test matcher text"
- method: GET
path:
- '{{BaseURL}}/14'
matchers:
- type: word
words:
- "Test is test matcher text"
- method: GET
path:
- '{{BaseURL}}/15'
matchers:
- type: word
words:
- "Test is test matcher text"
- method: GET
path:
- '{{BaseURL}}/16'
matchers:
- type: word
words:
- "Test is test matcher text"
- method: GET
path:
- '{{BaseURL}}/17'
matchers:
- type: word
words:
- "Test is test matcher text"
- method: GET
path:
- '{{BaseURL}}/18'
matchers:
- type: word
words:
- "Test is test matcher text"basic-template-multiproto-rawid: basic-template-multiproto-raw
info:
name: Test Template Multiple Protocols RAW
author: pdteam
severity: info
http:
- raw:
- |
GET /1 HTTP/1.1
Host: {{Hostname}}
Origin: {{BaseURL}}
Connection: close
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Language: en-US,en;q=0.9
matchers:
- type: word
words:
- "Test is test matcher text"
- raw:
- |
GET /2 HTTP/1.1
Host: {{Hostname}}
Origin: {{BaseURL}}
Connection: close
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Language: en-US,en;q=0.9
matchers:
- type: word
words:
- "Test is test matcher text"
- raw:
- |
GET /3 HTTP/1.1
Host: {{Hostname}}
Origin: {{BaseURL}}
Connection: close
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Language: en-US,en;q=0.9
matchers:
- type: word
words:
- "Test is test matcher text"
- raw:
- |
GET /4 HTTP/1.1
Host: {{Hostname}}
Origin: {{BaseURL}}
Connection: close
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Language: en-US,en;q=0.9
matchers:
- type: word
words:
- "Test is test matcher text"
- raw:
- |
GET /5 HTTP/1.1
Host: {{Hostname}}
Origin: {{BaseURL}}
Connection: close
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Language: en-US,en;q=0.9
matchers:
- type: word
words:
- "Test is test matcher text"
- raw:
- |
GET /6 HTTP/1.1
Host: {{Hostname}}
Origin: {{BaseURL}}
Connection: close
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Language: en-US,en;q=0.9
matchers:
- type: word
words:
- "Test is test matcher text"
- raw:
- |
GET /7 HTTP/1.1
Host: {{Hostname}}
Origin: {{BaseURL}}
Connection: close
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Language: en-US,en;q=0.9
matchers:
- type: word
words:
- "Test is test matcher text"
- raw:
- |
GET /8 HTTP/1.1
Host: {{Hostname}}
Origin: {{BaseURL}}
Connection: close
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Language: en-US,en;q=0.9
matchers:
- type: word
words:
- "Test is test matcher text"
- raw:
- |
GET /9 HTTP/1.1
Host: {{Hostname}}
Origin: {{BaseURL}}
Connection: close
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Language: en-US,en;q=0.9
matchers:
- type: word
words:
- "Test is test matcher text"
- raw:
- |
GET /10 HTTP/1.1
Host: {{Hostname}}
Origin: {{BaseURL}}
Connection: close
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Language: en-US,en;q=0.9
matchers:
- type: word
words:
- "Test is test matcher text"
- raw:
- |
GET /11 HTTP/1.1
Host: {{Hostname}}
Origin: {{BaseURL}}
Connection: close
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Language: en-US,en;q=0.9
matchers:
- type: word
words:
- "Test is test matcher text"
- raw:
- |
GET /12 HTTP/1.1
Host: {{Hostname}}
Origin: {{BaseURL}}
Connection: close
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Language: en-US,en;q=0.9
matchers:
- type: word
words:
- "Test is test matcher text"
- raw:
- |
GET /13 HTTP/1.1
Host: {{Hostname}}
Origin: {{BaseURL}}
Connection: close
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Language: en-US,en;q=0.9
matchers:
- type: word
words:
- "Test is test matcher text"
- raw:
- |
GET /14 HTTP/1.1
Host: {{Hostname}}
Origin: {{BaseURL}}
Connection: close
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Language: en-US,en;q=0.9
matchers:
- type: word
words:
- "Test is test matcher text"
- raw:
- |
GET / HTTP/1.1
Host: {{Hostname}}
Origin: {{BaseURL}}
Connection: close
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Language: en-US,en;q=0.9
matchers:
- type: word
words:
- "Test is test matcher text"
- raw:
- |
GET /15 HTTP/1.1
Host: {{Hostname}}
Origin: {{BaseURL}}
Connection: close
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Language: en-US,en;q=0.9
matchers:
- type: word
words:
- "Test is test matcher text"
- raw:
- |
GET /16 HTTP/1.1
Host: {{Hostname}}
Origin: {{BaseURL}}
Connection: close
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Language: en-US,en;q=0.9
matchers:
- type: word
words:
- "Test is test matcher text"
- raw:
- |
GET /17 HTTP/1.1
Host: {{Hostname}}
Origin: {{BaseURL}}
Connection: close
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Language: en-US,en;q=0.9
matchers:
- type: word
words:
- "Test is test matcher text"
- raw:
- |
GET /18 HTTP/1.1
Host: {{Hostname}}
Origin: {{BaseURL}}
Connection: close
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Language: en-US,en;q=0.9
matchers:
- type: word
words:
- "Test is test matcher text"basic-template-multiproto-mixed (w/ request ID)id: basic-template-multiproto-mixed
info:
name: Test Template Multiple Protocols (Mixed)
author: pdteam
severity: info
http:
- method: GET
id: first_iter_http
path:
- '{{BaseURL}}/1'
matchers:
- type: word
words:
- "Test is test matcher text"
- method: GET
path:
- '{{BaseURL}}/2'
matchers:
- type: word
words:
- "Test is test matcher text"
- method: GET
path:
- '{{BaseURL}}/3'
matchers:
- type: word
words:
- "Test is test matcher text"
- method: GET
path:
- '{{BaseURL}}/4'
matchers:
- type: word
words:
- "Test is test matcher text"
- method: GET
path:
- '{{BaseURL}}/5'
matchers:
- type: word
words:
- "Test is test matcher text"
- method: GET
path:
- '{{BaseURL}}/6'
matchers:
- type: word
words:
- "Test is test matcher text"
- method: GET
path:
- '{{BaseURL}}/7'
matchers:
- type: word
words:
- "Test is test matcher text"
- method: GET
path:
- '{{BaseURL}}/8'
matchers:
- type: word
words:
- "Test is test matcher text"
- method: GET
path:
- '{{BaseURL}}/9'
matchers:
- type: word
words:
- "Test is test matcher text"
- raw:
- |
GET /10 HTTP/1.1
Host: {{Hostname}}
Origin: {{BaseURL}}
Connection: close
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Language: en-US,en;q=0.9
matchers:
- type: word
words:
- "Test is test matcher text"
- raw:
- |
GET /11 HTTP/1.1
Host: {{Hostname}}
Origin: {{BaseURL}}
Connection: close
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Language: en-US,en;q=0.9
matchers:
- type: word
words:
- "Test is test matcher text"
- raw:
- |
GET /12 HTTP/1.1
Host: {{Hostname}}
Origin: {{BaseURL}}
Connection: close
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Language: en-US,en;q=0.9
matchers:
- type: word
words:
- "Test is test matcher text"
- raw:
- |
GET /13 HTTP/1.1
Host: {{Hostname}}
Origin: {{BaseURL}}
Connection: close
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Language: en-US,en;q=0.9
matchers:
- type: word
words:
- "Test is test matcher text"
- raw:
- |
GET /14 HTTP/1.1
Host: {{Hostname}}
Origin: {{BaseURL}}
Connection: close
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Language: en-US,en;q=0.9
matchers:
- type: word
words:
- "Test is test matcher text"
- raw:
- |
GET / HTTP/1.1
Host: {{Hostname}}
Origin: {{BaseURL}}
Connection: close
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Language: en-US,en;q=0.9
matchers:
- type: word
words:
- "Test is test matcher text"
- raw:
- |
GET /15 HTTP/1.1
Host: {{Hostname}}
Origin: {{BaseURL}}
Connection: close
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Language: en-US,en;q=0.9
matchers:
- type: word
words:
- "Test is test matcher text"
- raw:
- |
GET /16 HTTP/1.1
Host: {{Hostname}}
Origin: {{BaseURL}}
Connection: close
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Language: en-US,en;q=0.9
matchers:
- type: word
words:
- "Test is test matcher text"
- raw:
- |
GET /17 HTTP/1.1
Host: {{Hostname}}
Origin: {{BaseURL}}
Connection: close
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Language: en-US,en;q=0.9
matchers:
- type: word
words:
- "Test is test matcher text"
- raw:
- |
GET /18 HTTP/1.1
Host: {{Hostname}}
Origin: {{BaseURL}}
Connection: close
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8
Accept-Language: en-US,en;q=0.9
matchers:
- type: word
words:
- "Test is test matcher text" |
Signed-off-by: Dwi Siswanto <git@dw1.io>
tarunKoyalwar
left a comment
There was a problem hiding this comment.
lgtm !
for context, similar issue that exists in flow but with different root cause is already being tracked in this issue #5352
Proposed changes
Fixes #6253
Checklist
Summary by CodeRabbit