You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by ha...@apache.org on 2026/08/09 23:35:28 UTC
(skywalking-banyandb) 03/03: perf(tracesampler): decode escape-free tag rows without copying
This is an automated email from the ASF dual-hosted git repository.
hanahmily pushed a commit to branch perf/vararray-fast-path
in repository https://gitbox.apache.org/repos/asf/skywalking-banyandb.git
commit 219b274ee66a79c8e7732a183f1381f958de7075
Author: Hongtao Gao <ha...@gmail.com>
AuthorDate: Sun Aug 9 13:47:25 2026 +0000
perf(tracesampler): decode escape-free tag rows without copying
arrayEntries copied EVERY tag row into a scratch buffer before decoding it.
That copy exists for one reason: vararray.UnmarshalVarArray rewrites its input
in place to strip escapes, and the engine hands the SAME TraceBatch to every
link of a sampler chain, so decoding the block directly would make a rule's
verdict depend on its position in the chain.
But the rewrite only happens for a row that actually carries an escape. Trace
tag values rarely contain '|' or '\', so the common row was paying a full copy
to protect against a mutation that never occurred.
Gate the copy on a per-row escape check. A row without one now decodes straight
from the block, with the entries aliasing it. Two facts make that safe and both
are required: the decode performs no write without an escape to remove, and
banyand/trace/assembleTraceBlock already deep-copies every value into
block-owned storage "so the merge loop may recycle/overwrite those buffers while
an abandoned Decide goroutine still reads the block" — so the bytes outlive the
call even on the timeout path. The check is per ROW, not per column or schema,
because it is a property of the data.
Three further costs fall out of that split:
- An escape-free row no longer calls UnmarshalVarArray at all. Having proved no
escape is present, entries are exactly the delimiter-separated runs, so the
codec's own per-entry escape scan is redundant; splitting on the delimiter
directly removes it. This is the one place that assumes the encoding, and only
under that proof.
- The per-row copy buffer and the stable buffer backing the decoded strings are
now acquired lazily, on the first genuinely escaped row. A column of unescaped
rows takes no pooled buffer at all; Bounded.Put was 8.7% flat in the profile.
- matchEntries rebuilt r.TagKey + "=" on every call, once per rule per trace.
It is fixed at admission, so precompute it into the rule (runtime.concatstrings
was 4.3%).
Measured A/B/A against origin/main, 5 counts per arm:
Decide/sw/tagRulesOnly 68.40 → 24.36 µs -64.4%
Decide/zipkin/tagRulesOnly 68.61 → 24.34 µs -64.5%
Decide/zipkin/errorsOnly 69.43 → 24.51 µs -64.7%
Decide/sw/scenario 102.44 → 56.31 µs -45.0%
Decide/zipkin/scenario 129.61 → 83.91 µs -35.3%
Decide/sw/errorsOnly 8.18 → 8.00 µs -2.2% (dedicated error
column; control)
Two tests pin the contract. TestArrayEntries_LeavesBlockBytesIntact asserts the
decode leaves col.Values byte-identical across a column mixing escaped and
unescaped rows; deleting the escape check fails it and the pre-existing
TestDecide_EscapedEntriesSurviveChainedLinks. TestArrayEntries_SplitPathMatchesCodec
pins the hand-rolled split against the codec — same entries, same error — over
empty, single, multi, empty-entry and unterminated rows.
Also fixes three defects found while reading, none of them new:
- arrayEntries documented a nil-stableBuf fallback that could not execute: it
dereferenced the pointer before the guard that was supposed to protect it, so
the fallback branch was dead and a nil argument panicked. The buffer is now
owned by arrayEntries and returned to the caller, so the nil contract is gone
rather than merely documented.
- keepTrace's re-acquire guard keyed off entriesPtr, which arrayEntries returns
nil on error, so a second call site could decode twice and leak the first
stable buffer. It did not, but only because the fail-open ordering made the
second site unreachable. It now keys off an explicit decoded flag, which the
release path reads too.
- sampler_bench_ext_test.go cited docs/design/phase-9-sw-trace-sampler-bench.md,
which does not exist; point it at the Phase 9 workstream in the merge
optimization plan instead.
---
CHANGES.md | 2 +
.../skywalking/internal/tracesampler/sampler.go | 215 +++++++++++++++------
.../tracesampler/sampler_bench_ext_test.go | 7 +-
.../internal/tracesampler/sampler_test.go | 113 +++++++++++
4 files changed, 272 insertions(+), 65 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index 19d04847f..506414a9d 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -173,6 +173,8 @@ Release Notes.
- Bump Go and ui/mcp dependencies to clear Dependabot advisories: `golang.org/x/net` v0.52.0→v0.56.0 (CVE-2026-25680), `opencontainers/runc` v1.3.3→v1.3.6 (CVE-2026-41579). Refresh mcp/ui lockfiles and license attribution.
- Defer the sw-trace-sampler tag-array decode to first use: 60% per-trace cost reduction on the realistic-mix path (`duration+err+tag` 2,710 → 1,015 ns/trace, 2,380 → 664 allocs/op on the SkyWalking schema).
- Alias decoded entry strings via `pkg/convert.BytesToString`: 408 → 1 alloc/op on SkyWalking tag paths and 1,536 → 1 on Zipkin tag paths (the realistic 3-row × 8-entry trace); 24-40% ns/trace reduction on tag-using paths. Also fixes a pre-existing nil-deref in `keepTrace` when the array column is absent.
+- Decode escape-free trace tag rows without copying them. The sw/zipkin trace sampler copied every tag row before decoding, because the var-array decode rewrites its input in place and the engine hands the same batch to every link of a sampler chain. That copy is only needed for a row that actually contains an escape, so rows without one now decode straight from the block with the entries aliasing it — safe because `assembleTraceBlock` already deep-copies each value into block-owned stor [...]
+- Precompute each tag rule's `"tagKey="` entry prefix at admission instead of rebuilding it on every `matchEntries` call (once per rule per trace).
- Add an escape-free fast path to `vararray.UnmarshalVarArray`, the var-array decoder shared by sidx, trace query, index filtering, the dump tool and the sampler SDK. An entry containing no escape byte is located with two `bytes.IndexByte` scans instead of a per-byte loop, dropping that case from O(len) to O(1) (-91% at a 256-byte payload) and leaving the buffer untouched. An entry that does carry an escape resumes the in-place loop at the first escape rather than restarting, since the b [...]
## 0.10.0
diff --git a/plugins/skywalking/internal/tracesampler/sampler.go b/plugins/skywalking/internal/tracesampler/sampler.go
index cd3eb7a48..da16826a0 100644
--- a/plugins/skywalking/internal/tracesampler/sampler.go
+++ b/plugins/skywalking/internal/tracesampler/sampler.go
@@ -169,7 +169,13 @@ func (s Schema) firstClassColumn(tagKey, errorColumn string) string {
// rule is one sure-keep tag predicate. Exactly one matcher is honored, checked
// in the order exists, equals, in, regex.
type rule struct {
- re *regexp.Regexp
+ re *regexp.Regexp
+ // prefix is the "tagKey=" that an entry must carry for this rule to apply,
+ // precomputed at admission because matchEntries runs per rule per trace and
+ // the concatenation showed up as runtime.concatstrings in the tag-rule
+ // profile. Empty when the rule targets the array column itself, which is the
+ // case that matches a bare entry rather than a "key=value" one.
+ prefix string
Regex string `json:"regex"`
TagKey string `json:"tagKey"`
Equals string `json:"equals"`
@@ -400,7 +406,10 @@ func New(configJSON []byte, schema Schema) (sdk.Sampler, error) {
s.errorTagInArray = schema.ErrorTagInArray
// The in-array error signal is just an exists rule on the error key, evaluated
// by the same matcher as keepTagRules so the two can never diverge.
+ // Built here rather than through validateRules, so its prefix has to be
+ // set explicitly — matchEntries reads the precomputed field for every rule.
s.errorRule = rule{TagKey: s.errorTag, Exists: true}
+ s.errorRule.setPrefix(s.arrayColumn)
}
if c.DurationThresholdMs < 0 {
return nil, fmt.Errorf("tracesampler: durationThresholdMs must be >= 0, got %d", c.DurationThresholdMs)
@@ -484,10 +493,20 @@ func validateRules(rs []rule, schema Schema, errorColumn string) error {
}
r.re = re
}
+ r.setPrefix(schema.ArrayTagColumn)
}
return nil
}
+// setPrefix precomputes the rule's entry prefix against the flattened array
+// column. A rule naming the array column itself matches a bare entry, so it has
+// no prefix; every other rule matches "tagKey=value" entries.
+func (r *rule) setPrefix(arrayColumn string) {
+ if r.TagKey != arrayColumn {
+ r.prefix = r.TagKey + "="
+ }
+}
+
// Kind reports the sampler kind, satisfying the generic sdk.Plugin interface
// that sdk.Sampler embeds.
func (s *Sampler) Kind() sdk.Kind { return sdk.KindSampler }
@@ -566,26 +585,33 @@ func (s *Sampler) keepTrace(b *sdk.TraceBlock) bool {
// release) keeps this variable on the stack — the pool owns the backing
// storage, so taking the pointer's address is unnecessary.
//
- // stableBufPtr is the *[]byte that backs the per-row entry strings. The
- // strings alias its backing array via convert.BytesToString, so the array
- // must outlive every matchEntries call below. The lazy decode acquires
- // both pointers on the first call site that needs them; both are released
- // at the single exit below. A nil stableBufPtr means no entries were
- // decoded (skip path) and there is nothing to release.
+ // stableBufPtr is the *[]byte that backs the entry strings of any ESCAPED
+ // row, which arrayEntries acquires and hands back only when it needed one.
+ // Those strings alias its backing array via convert.BytesToString, so the
+ // array must outlive every matchEntries call below; it is released at the
+ // single exit. A nil stableBufPtr means no row needed unescaping (the
+ // common case) or nothing was decoded at all, and there is nothing to
+ // release.
+ //
+ // decoded, not a nil-check on entriesPtr, is what makes the decode fire
+ // once: arrayEntries returns a nil entriesPtr on error, so keying the
+ // second call site off entriesPtr would decode twice — and overwrite (leak)
+ // the stable buffer of the first attempt.
var (
entriesPtr *[]string
stableBufPtr *[]byte
entriesErr error
+ decoded bool
)
keep := false
// Error keep.
if s.keepErrors {
if s.errorTagInArray {
- if entriesPtr == nil {
- stableBufPtr = acquireStableBuf()
- entriesPtr, entriesErr = arrayEntries(b.Tag(s.arrayColumn), stableBufPtr)
+ if !decoded {
+ decoded = true
+ entriesPtr, stableBufPtr, entriesErr = arrayEntries(b.Tag(s.arrayColumn))
}
- if entriesErr != nil || matchEntries(*entriesPtr, &s.errorRule, s.arrayColumn) {
+ if entriesErr != nil || matchEntries(*entriesPtr, &s.errorRule) {
keep = true
}
} else {
@@ -598,15 +624,15 @@ func (s *Sampler) keepTrace(b *sdk.TraceBlock) bool {
// Sure-keep tag rules. The decode is hoisted out of the loop so the
// lazy fetch fires exactly once even when multiple rules are configured.
if !keep && len(s.rules) > 0 {
- if entriesPtr == nil {
- stableBufPtr = acquireStableBuf()
- entriesPtr, entriesErr = arrayEntries(b.Tag(s.arrayColumn), stableBufPtr)
+ if !decoded {
+ decoded = true
+ entriesPtr, stableBufPtr, entriesErr = arrayEntries(b.Tag(s.arrayColumn))
}
if entriesErr != nil {
keep = true
} else {
for i := range s.rules {
- if matchEntries(*entriesPtr, &s.rules[i], s.arrayColumn) {
+ if matchEntries(*entriesPtr, &s.rules[i]) {
keep = true
break
}
@@ -618,10 +644,11 @@ func (s *Sampler) keepTrace(b *sdk.TraceBlock) bool {
if !keep && s.healthySampleRate > 0 && sampleFraction(b.TraceID) < s.healthySampleRate {
keep = true
}
- if entriesPtr != nil {
+ // Release only what a decode acquired. Keying this off decoded rather than
+ // off the pointers is what keeps the flag honest: it is the same flag both
+ // call sites above test, so a future third site cannot quietly skip it.
+ if decoded {
releaseEntries(entriesPtr)
- }
- if stableBufPtr != nil {
releaseStableBuf(stableBufPtr)
}
return keep
@@ -826,25 +853,36 @@ func releaseStableBuf(b *[]byte) {
// the address of a []string local would impose. releaseEntries takes the
// pointer by value too.
//
-// The stableBuf pointer is the caller-owned backing buffer for the decoded
-// entry strings. Each entry's bytes are appended to *stableBuf and then
-// aliased via convert.BytesToString: the string header allocation that
-// string(buf[idx:end]) would impose is skipped, but only because *stableBuf
-// is never reset — only appended to — across the lifetime of the strings.
-// releaseStableBuf is the caller's responsibility, called after the strings
-// are no longer needed. arrayEntries returns without touching stableBuf's
-// pool lifecycle; passing nil for stableBuf falls back to the safe-but-
-// allocating string() conversion, which the tests rely on for the no-column
-// path that never produces any entry.
-func arrayEntries(col *sdk.TagColumn, stableBuf *[]byte) (*[]string, error) {
+// The second return value is the stable buffer backing the decoded entry
+// strings, or nil when no row needed one. Only an ESCAPED row needs it: its
+// bytes are unescaped into a scratch copy, so the resulting strings cannot alias
+// either that scratch (it is reused per row) or the block (it still holds the
+// escaped form), and are instead appended to the stable buffer and aliased via
+// convert.BytesToString — skipping the string header allocation that
+// string(buf[idx:end]) would impose. The buffer is only ever appended to, never
+// reset, so earlier strings stay valid. An escape-free row aliases the block
+// directly and needs no buffer at all, which is the common case, so the buffer
+// is acquired lazily and a whole column of unescaped rows returns nil here.
+//
+// The caller MUST releaseStableBuf whatever is returned, and only after the
+// strings are no longer read — they alias it.
+//
+// errInvalidVarArray mirrors the error vararray.UnmarshalVarArray returns for an
+// entry with no terminating delimiter. The escape-free split path reports the
+// same condition, so it must read identically in logs whichever path decoded
+// the row.
+var errInvalidVarArray = errors.New("invalid variable array")
+
+func arrayEntries(col *sdk.TagColumn) (*[]string, *[]byte, error) {
if col == nil {
// Always return a non-nil pool element so the caller can dereference
// without a nil check. matchEntries on an empty slice returns false,
// which matches the pre-pool behavior of "no entries" → no match.
- return acquireEntries(), nil
+ return acquireEntries(), nil, nil
}
- // Size the copy buffer to the longest row up front so it is allocated exactly once
- // and never grows mid-loop. This needs no decode — only the raw byte lengths.
+ // Early exit for a column whose every row is empty: it can yield no entry, so
+ // the row loop below would do nothing. This needs no decode — only the raw
+ // byte lengths.
widest := 0
for _, raw := range col.Values {
if len(raw) > widest {
@@ -852,7 +890,7 @@ func arrayEntries(col *sdk.TagColumn, stableBuf *[]byte) (*[]string, error) {
}
}
if widest == 0 {
- return acquireEntries(), nil
+ return acquireEntries(), nil, nil
}
// Acquire the result slice from the pool. The backing array is reused across
// calls; the length is reset to zero so stale entries from the previous decode
@@ -863,19 +901,74 @@ func arrayEntries(col *sdk.TagColumn, stableBuf *[]byte) (*[]string, error) {
// references it. The error path here releases locally because the slice
// never escapes to keepTrace.
out := acquireEntries()
- // Acquire the per-row copy buffer from the pool. If its cap is smaller than
- // the longest row of this trace, append will allocate a fresh backing array;
- // the pool then sees a larger buffer on release and the steady-state cap
- // converges to the widest row the schema produces.
- bufPtr := acquireCopyBuf()
- defer releaseCopyBuf(bufPtr)
- buf := *bufPtr
- stable := *stableBuf
+ // The per-row copy buffer is acquired LAZILY, on the first row that actually
+ // carries an escape. A column whose rows are all escape-free never needs one,
+ // and the pool checkout/release is not free: releaseCopyBuf's Bounded.Put was
+ // 8.7% flat in the tag-rule CPU profile. If its cap is smaller than the row
+ // being copied, append allocates a fresh backing array; the pool then sees a
+ // larger buffer on release and the steady-state cap converges to the widest
+ // escaped row the schema produces.
+ var (
+ bufPtr *[]byte
+ buf []byte
+ )
+ defer func() {
+ if bufPtr != nil {
+ // Persist any growth so the next caller sees the wider cap.
+ *bufPtr = buf
+ releaseCopyBuf(bufPtr)
+ }
+ }()
+ // The stable buffer is acquired lazily too, and only an escaped row needs it.
+ var (
+ stablePtr *[]byte
+ stable []byte
+ )
for row := range col.Values {
if col.Values[row] == nil {
continue
}
- // Decode a COPY of the row, never the engine's buffer. vararray.UnmarshalVarArray
+ // A row with no escape byte is decoded straight out of the block, with the
+ // entries aliasing it — no row copy, no stable-buffer append.
+ //
+ // Two facts make that safe, and BOTH are required:
+ //
+ // 1. No write. vararray.UnmarshalVarArray only shifts bytes when it
+ // consumes an escape; with none present it does not touch the buffer,
+ // so the corruption hazard described below cannot arise.
+ // 2. No recycling. assembleTraceBlock (banyand/trace/pipeline_chain.go)
+ // deep-copies every value into block-owned storage precisely "so the
+ // merge loop may recycle/overwrite those buffers while an abandoned
+ // Decide goroutine still reads the block". The bytes therefore outlive
+ // this call even on the timeout path, so aliasing them cannot observe
+ // recycled memory.
+ //
+ // The escape check is per row and must stay that way: it is a property of
+ // the DATA, not of the column or the schema.
+ if raw := col.Values[row]; bytes.IndexByte(raw, vararray.Escape) < 0 {
+ // The row is known escape-free, so entries are exactly the
+ // delimiter-separated runs and UnmarshalVarArray's own escape scan
+ // would be redundant work on every entry — it was 34.6% flat in the
+ // tag-rule profile. Split on the delimiter directly instead. This is
+ // the ONE place that may assume the encoding, and only because the
+ // check above proves no escape can be present: with an escape, a
+ // delimiter byte may be escaped and this loop would split mid-value.
+ idx := 0
+ for idx < len(raw) {
+ rel := bytes.IndexByte(raw[idx:], vararray.EntityDelimiter)
+ if rel < 0 {
+ // Same condition, and the same message, that
+ // UnmarshalVarArray reports for an unterminated entry.
+ releaseEntries(out)
+ releaseStableBuf(stablePtr)
+ return nil, nil, fmt.Errorf("str array: %w", errInvalidVarArray)
+ }
+ *out = append(*out, convert.BytesToString(raw[idx:idx+rel]))
+ idx += rel + 1
+ }
+ continue
+ }
+ // Escaped row: decode a COPY, never the block's buffer. vararray.UnmarshalVarArray
// shifts bytes left past every escape in place, TraceBlock slices are documented
// read-only, and the engine hands the SAME TraceBatch to every link of a chain
// (sdk.applyChainLink). Decoding in place would therefore corrupt the value for
@@ -889,33 +982,34 @@ func arrayEntries(col *sdk.TagColumn, stableBuf *[]byte) (*[]string, error) {
// (which is only ever appended to, never reset, so prior bytes stay valid)
// and then alias stable via convert.BytesToString — skipping the per-entry
// string-header allocation that string(buf[idx:end]) would impose.
+ if bufPtr == nil {
+ bufPtr = acquireCopyBuf()
+ buf = *bufPtr
+ stablePtr = acquireStableBuf()
+ stable = *stablePtr
+ }
buf = append(buf[:0], col.Values[row]...)
idx := 0
for idx < len(buf) {
end, next, err := vararray.UnmarshalVarArray(buf, idx)
if err != nil {
releaseEntries(out)
- return nil, fmt.Errorf("str array: %w", err)
+ releaseStableBuf(stablePtr)
+ return nil, nil, fmt.Errorf("str array: %w", err)
}
start := len(stable)
stable = append(stable, buf[idx:end]...)
- if stableBuf != nil {
- *out = append(*out, convert.BytesToString(stable[start:]))
- } else {
- // Defensive fallback: copy out of stable when the caller did not
- // provide a stable buffer. Tests pass nil for the no-column path,
- // which never reaches this branch.
- *out = append(*out, string(stable[start:]))
- }
+ *out = append(*out, convert.BytesToString(stable[start:]))
idx = next
}
}
- // Persist any growth of buf and stable into their pool pointers so the next
- // caller sees the wider cap. Without this the growth would be lost when the
- // local variables were reassigned only via append.
- *bufPtr = buf
- *stableBuf = stable
- return out, nil
+ // Persist any growth so the next caller sees the wider cap; without this the
+ // growth would be lost when the local was reassigned only via append. Both
+ // pointers stay nil when no row carried an escape, which is the common case.
+ if stablePtr != nil {
+ *stablePtr = stable
+ }
+ return out, stablePtr, nil
}
// errNoErrorColumn reports that the schema's error column is absent from the block.
@@ -969,11 +1063,8 @@ func (s *Sampler) hasErrorColumn(b *sdk.TraceBlock) (bool, error) {
// no "="). The comparison is exact, so a longer key such as "error_rate=0" never
// satisfies an exists rule on "error". keepErrors routes through here too, so the
// two cannot drift apart.
-func matchEntries(entries []string, r *rule, arrayColumn string) bool {
- prefix := ""
- if r.TagKey != arrayColumn {
- prefix = r.TagKey + "="
- }
+func matchEntries(entries []string, r *rule) bool {
+ prefix := r.prefix
for _, entry := range entries {
if r.Exists && entry == r.TagKey {
return true
diff --git a/plugins/skywalking/internal/tracesampler/sampler_bench_ext_test.go b/plugins/skywalking/internal/tracesampler/sampler_bench_ext_test.go
index 561783027..6b92d4cc4 100644
--- a/plugins/skywalking/internal/tracesampler/sampler_bench_ext_test.go
+++ b/plugins/skywalking/internal/tracesampler/sampler_bench_ext_test.go
@@ -20,9 +20,10 @@
// benchPlugins / benchEntries / benchBatch / mustSampler / runDecide / benchSink
// helpers are reused rather than re-declared.
//
-// Layer 1 only covers synthetic-data engine microbenchmarks. No production-ratio
-// claim, no calibrated fixture, no .so loading. See
-// docs/design/phase-9-sw-trace-sampler-bench.md §Layer 1.
+// These cover synthetic-data engine microbenchmarks only: no production-ratio
+// claim, no calibrated fixture, no .so loading. See the Phase 9 workstream in
+// docs/design/trace-pipeline-merge-optimization-plan.md, whose exit gate asks for
+// per-call-site attribution of the plugin-local costs these measure.
package tracesampler
import (
diff --git a/plugins/skywalking/internal/tracesampler/sampler_test.go b/plugins/skywalking/internal/tracesampler/sampler_test.go
index 6d82dd0c5..a8517256d 100644
--- a/plugins/skywalking/internal/tracesampler/sampler_test.go
+++ b/plugins/skywalking/internal/tracesampler/sampler_test.go
@@ -25,6 +25,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
+ "github.com/apache/skywalking-banyandb/pkg/encoding/vararray"
"github.com/apache/skywalking-banyandb/pkg/pb/v1/valuetype"
"github.com/apache/skywalking-banyandb/pkg/pipeline/sdk"
"github.com/apache/skywalking-banyandb/pkg/pipeline/sdk/sdktest"
@@ -718,6 +719,118 @@ func TestDecide_HealthySampleRate(t *testing.T) {
assert.Equal(t, []bool{false}, verdict.Keep)
}
+// TestArrayEntries_LeavesBlockBytesIntact pins the precondition that lets
+// arrayEntries alias the block's own bytes instead of copying every row: the
+// decode must leave col.Values byte-identical.
+//
+// An escape-free row is decoded in place and the entries alias it, which is only
+// sound because UnmarshalVarArray does not write when there is no escape to
+// remove. An escaped row is still copied first, because that decode DOES write.
+// If either half regresses, the engine hands the SAME batch to every link of a
+// chain, so a later rule would read rewritten bytes and its verdict would depend
+// on its position in the chain — silent and data-dependent.
+//
+// The mixed column is deliberate: the choice is per row, not per column.
+func TestArrayEntries_LeavesBlockBytesIntact(t *testing.T) {
+ rows := [][]string{
+ {"db.type=PostgreSQL", "http.method=GET"}, // escape-free
+ {`db.statement=SELECT a|b`, `url=C:\tmp`}, // both bytes needing an escape
+ {"plain"}, // escape-free, single entry
+ }
+ col := &sdk.TagColumn{Name: "tags", ValueType: valuetype.ValueTypeStrArr}
+ for _, entries := range rows {
+ var encoded []byte
+ for _, e := range entries {
+ encoded = vararray.MarshalVarArray(encoded, []byte(e))
+ }
+ col.Values = append(col.Values, encoded)
+ }
+ before := make([][]byte, len(col.Values))
+ for i, v := range col.Values {
+ before[i] = append([]byte(nil), v...)
+ }
+
+ entriesPtr, stablePtr, err := arrayEntries(col)
+ require.NoError(t, err)
+ defer releaseEntries(entriesPtr)
+ defer releaseStableBuf(stablePtr)
+
+ assert.Equal(t, []string{
+ "db.type=PostgreSQL", "http.method=GET",
+ `db.statement=SELECT a|b`, `url=C:\tmp`,
+ "plain",
+ }, *entriesPtr, "every row's entries must decode, escaped or not")
+
+ for i := range col.Values {
+ assert.Equal(t, before[i], col.Values[i],
+ "row %d was rewritten; a later chain link would read corrupted bytes", i)
+ }
+}
+
+// TestArrayEntries_SplitPathMatchesCodec pins the one place the sampler assumes
+// the var-array encoding. The escape-free branch splits on the delimiter itself
+// instead of calling UnmarshalVarArray, to skip that function's redundant
+// per-entry escape scan. It must therefore agree with the codec exactly — same
+// entries, same error — on every row shape, or a rule's verdict would depend on
+// which branch decoded the row.
+func TestArrayEntries_SplitPathMatchesCodec(t *testing.T) {
+ rows := []struct {
+ name string
+ encoded []byte
+ }{
+ {"empty", nil},
+ {"single entry", []byte("abc|")},
+ {"two entries", []byte("a|b|")},
+ {"empty entry first", []byte("|a|")},
+ {"empty entry last", []byte("a||")},
+ {"all empty entries", []byte("|||")},
+ {"unterminated", []byte("abc")},
+ {"unterminated after a good entry", []byte("a|bc")},
+ }
+ for _, row := range rows {
+ t.Run(row.name, func(t *testing.T) {
+ // Reference: decode via the codec, which is what the escaped branch does.
+ var (
+ want []string
+ wantErr error
+ )
+ ref := append([]byte(nil), row.encoded...)
+ for idx := 0; idx < len(ref); {
+ end, next, err := vararray.UnmarshalVarArray(ref, idx)
+ if err != nil {
+ wantErr = err
+ break
+ }
+ want = append(want, string(ref[idx:end]))
+ idx = next
+ }
+
+ col := &sdk.TagColumn{
+ Name: "tags", ValueType: valuetype.ValueTypeStrArr,
+ Values: [][]byte{append([]byte(nil), row.encoded...)},
+ }
+ got, stablePtr, err := arrayEntries(col)
+
+ if wantErr != nil {
+ require.Error(t, err, "codec rejected this row, so the split path must too")
+ assert.Contains(t, err.Error(), wantErr.Error(), "both paths must report the same condition")
+ return
+ }
+ require.NoError(t, err)
+ defer releaseEntries(got)
+ defer releaseStableBuf(stablePtr)
+ // Compared by content: a row yielding nothing gives the codec a nil
+ // slice and arrayEntries a zero-length pooled one, which is the same
+ // value to every caller (matchEntries iterates the length).
+ if len(want) == 0 {
+ assert.Empty(t, *got)
+ return
+ }
+ assert.Equal(t, want, *got)
+ })
+ }
+}
+
// TestDecide_FailOpenOnDecodeError proves a malformed tag value keeps the trace
// (fail open) rather than dropping it or erroring the whole batch: an is_error
// column whose raw int64 is not 8 bytes fails to decode, and keepErrors keeps