Skip to content

Commit ed1d7c0

Browse files
committed
fix(venom): user executors performance issues
Signed-off-by: alexGNX <[email protected]>
1 parent f3548d1 commit ed1d7c0

File tree

9 files changed

+195
-155
lines changed

9 files changed

+195
-155
lines changed

dump.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ func WithFormatterLowerFirstKey() dump.KeyFormatterFunc {
9595
return func(s string, level int) string {
9696
if level == 0 && strings.Contains(s, ".") {
9797
pos := strings.Index(s, ".")
98-
return strings.ToLower(s[0:pos])+s[pos:]
98+
return strings.ToLower(s[0:pos]) + s[pos:]
9999
}
100100
if level == 0 {
101101
return strings.ToLower(f(s, level))

executors/exec/exec.go

+31-24
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package exec
22

33
import (
4-
"bufio"
54
"context"
65
"fmt"
76
"os"
@@ -161,42 +160,50 @@ func (Executor) Run(ctx context.Context, step venom.TestStep) (interface{}, erro
161160
return nil, fmt.Errorf("runScriptAction: Cannot get stderr pipe: %s", err)
162161
}
163162

164-
stdoutreader := bufio.NewReader(stdout)
165-
stderrreader := bufio.NewReader(stderr)
166-
167163
result := Result{}
168164
outchan := make(chan bool)
165+
169166
go func() {
167+
var sb strings.Builder
168+
sb.Grow(1024 * 1024) // Pre-allocate 1MB
169+
170+
// For efficiency, read in larger chunks
171+
buf := make([]byte, 64*1024) // 64KB buffer
170172
for {
171-
line, errs := stdoutreader.ReadString('\n')
172-
if errs != nil {
173-
// ReadString returns what has been read even though an error was encountered
174-
// ie. capture outputs with no '\n' at the end
175-
result.Systemout += line
176-
stdout.Close()
177-
close(outchan)
178-
return
173+
n, err := stdout.Read(buf)
174+
if n > 0 {
175+
sb.Write(buf[:n])
176+
}
177+
if err != nil {
178+
break
179179
}
180-
result.Systemout += line
181-
venom.Debug(ctx, venom.HideSensitive(ctx, line))
182180
}
181+
182+
result.Systemout = sb.String()
183+
close(outchan)
183184
}()
184185

185186
errchan := make(chan bool)
186187
go func() {
188+
var sb strings.Builder
189+
sb.Grow(64 * 1024) // Pre-allocate 64KB for stderr
190+
191+
buf := make([]byte, 8*1024) // 8KB buffer for stderr
187192
for {
188-
line, errs := stderrreader.ReadString('\n')
189-
if errs != nil {
190-
// ReadString returns what has been read even though an error was encountered
191-
// ie. capture outputs with no '\n' at the end
192-
result.Systemerr += line
193-
stderr.Close()
194-
close(errchan)
195-
return
193+
n, err := stderr.Read(buf)
194+
if n > 0 {
195+
chunk := buf[:n]
196+
sb.Write(chunk)
197+
venom.Debug(ctx, venom.HideSensitive(ctx, string(chunk)))
198+
}
199+
if err != nil {
200+
break
196201
}
197-
result.Systemerr += line
198-
venom.Debug(ctx, venom.HideSensitive(ctx, line))
199202
}
203+
204+
result.Systemerr = sb.String()
205+
stderr.Close()
206+
close(errchan)
200207
}()
201208

202209
if err := cmd.Start(); err != nil {

executors/grpc/grpc.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ func (Executor) Run(ctx context.Context, step venom.TestStep) (interface{}, erro
265265
// invoke the gRPC
266266
err = grpcurl.InvokeRPC(ctx, descSource, cc, e.Service+"/"+e.Method, headers, &handle, rf.Next)
267267
if err != nil {
268-
return nil, err
268+
return nil, fmt.Errorf("grpcurl.InvokeRPC() failed.\nUrl: %q\nService: %q\nMethod: %q\nData:%v: %v", e.URL, e.Service, e.Method, e.Data, err)
269269
}
270270

271271
elapsed := time.Since(start)

log.go

+16-8
Original file line numberDiff line numberDiff line change
@@ -44,17 +44,25 @@ func asJsonString(i interface{}) string {
4444
// HideSensitive replace the value with __hidden__
4545
func HideSensitive(ctx context.Context, arg interface{}) string {
4646
s := ctx.Value(ContextKey("secrets"))
47+
48+
// Fast path: if no secrets to hide, avoid unnecessary string conversion
49+
if s == nil {
50+
if str, ok := arg.(string); ok {
51+
return str
52+
}
53+
return fmt.Sprint(arg)
54+
}
4755
cleanVars := fmt.Sprint(arg)
48-
if s != nil {
49-
switch reflect.TypeOf(s).Kind() {
50-
case reflect.Slice:
51-
secrets := reflect.ValueOf(s)
52-
for i := 0; i < secrets.Len(); i++ {
53-
secret := fmt.Sprint(secrets.Index(i).Interface())
54-
cleanVars = strings.ReplaceAll(cleanVars, secret, "__hidden__")
55-
}
56+
57+
switch reflect.TypeOf(s).Kind() {
58+
case reflect.Slice:
59+
secrets := reflect.ValueOf(s)
60+
for i := 0; i < secrets.Len(); i++ {
61+
secret := fmt.Sprint(secrets.Index(i).Interface())
62+
cleanVars = strings.ReplaceAll(cleanVars, secret, "__hidden__")
5663
}
5764
}
65+
5866
return cleanVars
5967
}
6068

process.go

+5
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,11 @@ func (v *Venom) Parse(ctx context.Context, path []string) error {
8989
return err
9090
}
9191

92+
err = v.registerUserExecutors(ctx)
93+
if err != nil {
94+
return errors.Wrapf(err, "unable to register user executors")
95+
}
96+
9297
missingVars := []string{}
9398
extractedVars := []string{}
9499
for i := range v.Tests.TestSuites {

read_partial.go

+3-18
Original file line numberDiff line numberDiff line change
@@ -11,24 +11,6 @@ import (
1111
"github.com/rockbears/yaml"
1212
)
1313

14-
func getUserExecutorInputYML(ctx context.Context, btesIn []byte) (H, error) {
15-
btes := readPartialYML(btesIn, "input")
16-
17-
var result = map[string]interface{}{}
18-
var tmpResult = map[string]interface{}{}
19-
20-
if len(btes) > 0 {
21-
if err := yaml.Unmarshal([]byte(btes), &tmpResult); err != nil {
22-
return nil, err
23-
}
24-
}
25-
for k, v := range tmpResult {
26-
result[k] = v
27-
}
28-
29-
return result, nil
30-
}
31-
3214
func getVarFromPartialYML(ctx context.Context, btesIn []byte) (H, error) {
3315
btes := readPartialYML(btesIn, "vars")
3416
type partialVars struct {
@@ -52,6 +34,9 @@ func readPartialYML(btes []byte, attribute string) string {
5234
var record bool
5335
for scanner.Scan() {
5436
line := scanner.Text()
37+
line = strings.TrimFunc(line, func(r rune) bool {
38+
return !unicode.IsGraphic(r)
39+
})
5540
if strings.HasPrefix(line, attribute+":") {
5641
record = true
5742
} else if len(line) > 0 {

types.go

+2-3
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"encoding/json"
66
"encoding/xml"
77
"fmt"
8+
"maps"
89
"strings"
910
"time"
1011
"unicode"
@@ -25,9 +26,7 @@ const (
2526
type H map[string]interface{}
2627

2728
func (h H) Clone() H {
28-
var h2 = make(H, len(h))
29-
h2.AddAll(h)
30-
return h2
29+
return maps.Clone(h)
3130
}
3231

3332
func (h *H) Add(k string, v interface{}) {

types_executor.go

+71-24
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"encoding/json"
66
"fmt"
77
"reflect"
8+
"regexp"
89
"strings"
910

1011
"github.com/gosimple/slug"
@@ -168,11 +169,13 @@ func GetExecutorResult(r interface{}) map[string]interface{} {
168169
}
169170

170171
type UserExecutor struct {
171-
Executor string `json:"executor" yaml:"executor"`
172-
Input H `json:"input" yaml:"input"`
173-
RawTestSteps []json.RawMessage `json:"steps" yaml:"steps"`
174-
Output json.RawMessage `json:"output" yaml:"output"`
175-
Filename string `json:"-" yaml:"-"`
172+
Executor string
173+
Input H `json:"input" yaml:"input"`
174+
TestSteps []json.RawMessage `json:"steps" yaml:"steps"`
175+
Raw []byte `json:"-" yaml:"-"` // the raw file content of the executor
176+
RawInputs []byte `json:"-" yaml:"-"`
177+
Filename string `json:"-" yaml:"-"`
178+
Output json.RawMessage `json:"output" yaml:"output"`
176179
}
177180

178181
// Run is not implemented on user executor
@@ -202,34 +205,66 @@ func (ux UserExecutor) ZeroValueResult() interface{} {
202205

203206
func (v *Venom) RunUserExecutor(ctx context.Context, runner ExecutorRunner, tcIn *TestCase, tsIn *TestStepResult, step TestStep) (interface{}, error) {
204207
vrs := tcIn.TestSuiteVars.Clone()
205-
uxIn := runner.GetExecutor().(UserExecutor)
206-
207-
for k, va := range uxIn.Input {
208-
if strings.HasPrefix(k, "input.") {
209-
// do not reinject input.vars from parent user executor if exists
210-
continue
211-
} else if !strings.HasPrefix(k, "venom") {
212-
if vl, ok := step[k]; ok && vl != "" { // value from step
213-
vrs.AddWithPrefix("input", k, vl)
214-
} else { // default value from executor
215-
vrs.AddWithPrefix("input", k, va)
208+
ux := runner.GetExecutor().(UserExecutor)
209+
var tsVars map[string]string
210+
newUX := UserExecutor{}
211+
var err error
212+
213+
// process inputs
214+
if len(ux.RawInputs) != 0 {
215+
tsVars, err = DumpString(vrs)
216+
if err != nil {
217+
return nil, errors.Wrapf(err, "error processing executor inputs: unable to dump testsuite vars")
218+
}
219+
220+
interpolatedInput, err := interpolate.Do(string(ux.RawInputs), tsVars)
221+
if err != nil {
222+
return nil, errors.Wrapf(err, "unable to interpolate executor inputs %q", ux.Executor)
223+
}
224+
225+
err = yaml.Unmarshal([]byte(interpolatedInput), &newUX)
226+
if err != nil {
227+
return nil, errors.Wrapf(err, "unable to unmarshal inputs for executor %q - raw interpolated:\n%v", ux.Executor, string(interpolatedInput))
228+
}
229+
230+
for k, va := range newUX.Input {
231+
if strings.HasPrefix(k, "input.") {
232+
// do not reinject input.vars from parent user executor if exists
233+
continue
234+
} else if !strings.HasPrefix(k, "venom") {
235+
if vl, ok := step[k]; ok && vl != "" { // value from step
236+
vrs.AddWithPrefix("input", k, vl)
237+
} else { // default value from executor
238+
vrs.AddWithPrefix("input", k, va)
239+
}
240+
} else {
241+
vrs.Add(k, va)
216242
}
217-
} else {
218-
vrs.Add(k, va)
243+
}
244+
tsVars, err = DumpString(vrs)
245+
if err != nil {
246+
return nil, errors.Wrapf(err, "error processing executor inputs: unable to dump testsuite vars")
219247
}
220248
}
221-
// reload the user executor with the interpolated vars
222-
_, exe, err := v.GetExecutorRunner(ctx, step, vrs)
249+
250+
interpolatedFull, err := interpolate.Do(string(ux.Raw), tsVars)
251+
if err != nil {
252+
return nil, errors.Wrapf(err, "unable to interpolate executor %q", ux.Executor)
253+
}
254+
// quote any remaining template expressions to ensure proper YAML parsing
255+
sanitized := quoteTemplateExpressions([]byte(interpolatedFull))
256+
257+
err = yaml.Unmarshal([]byte(sanitized), &newUX)
223258
if err != nil {
224-
return nil, errors.Wrapf(err, "unable to reload executor")
259+
return nil, errors.Wrapf(err, "unable to unmarshal executor %q - raw interpolated :\n%v", ux.Executor, string(sanitized))
225260
}
226-
ux := exe.GetExecutor().(UserExecutor)
261+
ux.Output = newUX.Output
227262

228263
tc := &TestCase{
229264
TestCaseInput: TestCaseInput{
230265
Name: ux.Executor,
231-
RawTestSteps: ux.RawTestSteps,
232266
Vars: vrs,
267+
RawTestSteps: newUX.TestSteps,
233268
},
234269
number: tcIn.number,
235270
TestSuiteVars: tcIn.TestSuiteVars,
@@ -283,7 +318,7 @@ func (v *Venom) RunUserExecutor(ctx context.Context, runner ExecutorRunner, tcIn
283318
}
284319

285320
if len(tsIn.Errors) > 0 {
286-
return outputResult, fmt.Errorf("failed")
321+
return outputResult, fmt.Errorf("executor %q failed - raw interpolated:\n%v", ux.Executor, string(sanitized))
287322
}
288323

289324
// here, we have the user executor results.
@@ -332,3 +367,15 @@ func (v *Venom) RunUserExecutor(ctx context.Context, runner ExecutorRunner, tcIn
332367
}
333368
return result, nil
334369
}
370+
371+
// quoteTemplateExpressions adds double quotes around template expressions in YAML content.
372+
// It specifically targets expressions that follow a colon and whitespace like 'key: {{.variable}}'
373+
// and are not already enclosed in quotes. This ensures proper YAML parsing of template variables.
374+
func quoteTemplateExpressions(content []byte) []byte {
375+
// First capture group matches everything up to the colon, checking the last non-whitespace
376+
// character isn't a quote (to skip JSON keys)
377+
re := regexp.MustCompile(`(?m)(^.*[^"\s][\s]*)(:\s+)({{.*?}})(.*?)(?:\s*)$`)
378+
379+
// Put quotes around the template expression and what follows it
380+
return re.ReplaceAll(content, []byte(`$1$2"$3$4"`))
381+
}

0 commit comments

Comments
 (0)