feat(redisotel): truncate statement span attributes - #4114

Enforce the remaining statement budget per argument

When a command has a long but sub-limit key followed by a max-sized value (for example, SET with a 500-byte key and 512-byte value under the default), the pre-append check still passes and this call grants the value another full maxLen; the resulting db.statement is about 1 KiB (and binary data can be larger after hex encoding), so WithDBStatementMaxLen does not provide its documented overall statement cap. Pass only the remaining budget, accounting for encoded output and markers, when appending.

Useful? React with 👍 / 👎.

Bound pipeline buffer allocation by the limit

When tracing a large pipeline with truncation enabled, this allocation reserves 64 * len(cmds) bytes before the loop stops near the configured cap; a 100,000-command pipeline therefore creates a 6.4 MB temporary buffer to retain only roughly 512 bytes. This makes the instrumentation's memory and GC cost scale with the entire pipeline despite the new cap, so bound the initial capacity by the configured maximum.

Useful? React with 👍 / 👎.

Treat a zero binary budget as exhausted

With truncation enabled, the new remaining-budget path can pass limit == 1 here; integer division then sets byteLimit to zero, and the byteLimit > 0 guard skips truncation so the fallthrough hex-encodes the entire argument. For example, at the default 512-byte limit, SET plus a 506-byte key leaves one byte for a 1 MiB non-printable value and produces roughly 2 MiB of db.statement, so this still defeats the cap and recreates the large telemetry allocation; treat zero as an exhausted encoded budget rather than as unlimited.

Useful? React with 👍 / 👎.

Bound classification work to the retained prefix

When truncation is enabled for a large printable string, such as a multi-megabyte JSON value, isSimple(bytesVal) walks the entire argument before appendArgTruncated retains only the first remaining-budget bytes. Because this hook runs synchronously before the Redis operation, default tracing still adds O(payload size) work despite the 512-byte limit, so inspect only the retained prefix (or otherwise bound classification work) rather than the discarded suffix.

Useful? React with 👍 / 👎.

Avoid copying byte-slice rules before truncation

When ACL SETUSER receives a large rule as a []byte (for example, a multi-megabyte >password), secretArgs calls argString for every rule and this conversion copies the entire slice before the rule is redacted; the later truncation therefore keeps at most maxLen but the tracing hook still incurs a payload-sized allocation. Inspect the byte slice directly or compare it without converting the whole value to a string.

Useful? React with 👍 / 👎.

Guard the pipeline capacity addition from overflow

When a caller supplies WithDBStatementMaxLen a value above math.MaxInt-64, maxLen+64 wraps negative, this branch assigns a negative bufCap, and the subsequent make panics before the wrapped Redis hook is called. Compute this cap with overflow-safe or saturating arithmetic so a valid positive configuration cannot crash pipeline processing.

AGENTS.md reference: AGENTS.md:L117-L121

Useful? React with 👍 / 👎.

Avoid materializing custom arguments before truncation

When a valid command argument is a large custom value (for example, a named []byte implementing encoding.BinaryMarshaler, which internal/proto.Writer.WriteArg accepts), fmt.Sprint(v) constructs a representation of the entire payload before limit is applied. A 1 MiB binary-marshaler slice can therefore allocate a multi-megabyte decimal string synchronously in the tracing hook despite the default 512-byte setting, defeating the new allocation bound; handle supported custom values without formatting the discarded suffix.

Useful? React with 👍 / 👎.

Fixed in commit 59d9d7d: added explicit support for encoding.BinaryMarshaler, net.IP, and pointer arguments in appendArgTruncated, directly bounding the binary representation to the retained prefix without materializing full formatted strings via fmt.Sprint.

Match time values before BinaryMarshaler

When tracing an HSET struct with a nil *time.Time field (a value supported by the command writer), the pointer satisfies encoding.BinaryMarshaler and reaches this call before the nil-aware *time.Time branch, so Time.MarshalBinary panics even though internal/proto/writer.go:189-210 would serialize it as the zero time; non-nil time values are likewise logged as binary hex instead of the RFC3339 value sent to Redis. Move the time cases before the interface dispatch so instrumentation does not crash or misreport otherwise valid commands.

AGENTS.md reference: AGENTS.md:L117-L123

Useful? React with 👍 / 👎.

Fixed in commit a2dd0ee: moved time.Time, *time.Time, time.Duration, and *time.Duration before any custom type interface dispatch. Non-nil timestamps format as RFC3339Nano matching proto.Writer, and nil *time.Time pointers safely format as <nil> without dereferencing or panicking.

Avoid fully marshaling truncated custom values

When tracing a large encoding.BinaryMarshaler whose method builds a fresh result, this call constructs the entire payload before appendBytesTruncated can apply the 512-byte budget, and internal/proto/writer.go:205-210 invokes it again to send the request. The fresh evidence after the earlier custom-argument fix is that MarshalBinary accepts no limit and returns the complete byte slice, so a multi-megabyte value still adds a payload-sized tracing allocation; use a bounded representation that does not fully marshal it in the hook.

Useful? React with 👍 / 👎.

Fixed in commit a2dd0ee: custom types now inspect underlying []byte / string slices directly via reflection to avoid payload-sized allocations in tracing hooks.

Preserve redaction supplied by named Stringer arguments

When a named string argument implements both fmt.Stringer (returning a redacted value) and encoding.BinaryMarshaler (so internal/proto.Writer.WriteArg accepts it), this reflection branch reads the underlying string directly instead of using the safe String result. The parent path through rediscmd.CmdString used fmt.Sprint for such named types, so the default max-length path now exports secrets that were previously masked in db.statement; keep a bounded representation without exposing the underlying value.

Useful? React with 👍 / 👎.

Bound formatting of struct-backed custom arguments

When a valid argument is a struct-backed encoding.BinaryMarshaler containing a large byte slice, internal/proto.Writer.WriteArg accepts it but this fallback renders the entire struct and slice before appendStringTruncated applies the 512-byte budget. The fresh evidence after the earlier custom-argument fixes is that the new reflection branches only bound underlying string/slice kinds; a 1 MiB marshaler struct still creates a multi-megabyte decimal string synchronously in tracing, so use a bounded placeholder or representation instead of formatting the whole value.

Useful? React with 👍 / 👎.

Truncate command errors within the statement budget

When a command enters Process with Err() already set to a long message (callers can do this through the public Cmder.SetErr API), this append bypasses all argument budgeting. A 1 MiB pre-set error therefore still creates and copies a 1 MiB db.statement under the default 512-byte limit, and pipelines likewise grow beyond their cap; append only the error prefix that fits the remaining budget.

Useful? React with 👍 / 👎.

Truncate scalar arguments to the remaining budget

When WithDBStatementMaxLen is smaller than a scalar's textual form, the numeric, time, and bool branches append the entire value and ignore limit; for example, INCRBY key math.MaxInt64 with maxLen == 16 has five bytes left here but emits all 19 digits, producing a 30-byte statement with no truncation marker. The caller now passes the correct remaining budget, but these branches still bypass it, so scalar formatting should also use a bounded append path.

Useful? React with 👍 / 👎.

secretArgs, secretConfigParams, equalFoldArg, argString, appendUTF8String, isSimple and the number cases of appendArgTruncated copy extra/rediscmd/rediscmd.go. Please move the limit into rediscmd and call it from here, so redaction lives in one place. hasSecretRulePrefix and the pointer cases can move with it. argString can then use the existing zero-copy String(v) instead of string(v), which also covers the open Codex thread on line 164.

reflect.Value.String() on a Kind() == String value returns the underlying string and skips the type's own String() method. With type Secret string that has String() string { return "***" } (plus MarshalBinary, so WriteArg accepts it), the default path writes the secret. The []byte-kind branch on line 409 has the same problem. When truncation is on, a fmt.Stringer should go through appendStringTruncated(b, v.String(), limit). Only a string/[]byte kind with no methods should use the kind fast path. Please add a test with a redacting Stringer.

This is still unbounded for struct-backed BinaryMarshaler values: fmt.Sprint formats the whole value and the result is cut afterwards. Master does the same, but the PR description says large payloads are not copied. A bounded option is a type placeholder such as <main.Blob> when truncation is on, with fmt.Sprint kept only for maxLen <= 0.

The limit is one budget for the whole statement (one command, or all commands of a pipeline), and the marker and the command error text can go past it. Suggested wording:

Contain Stringer panics inside statement rendering

When an otherwise valid encoding.BinaryMarshaler also implements fmt.Stringer and its String method panics for a particular value, this direct call lets the panic escape the built-in tracing hook before next is called; the Redis writer would only call MarshalBinary, and the previous fmt.Sprint path recovered formatter panics. Use a panic-safe rendering path so telemetry cannot abort an otherwise serializable Redis operation.

AGENTS.md reference: AGENTS.md:L117-L122

Useful? React with 👍 / 👎.

Build pipeline summaries without rendering arguments

When ProcessPipelineHook sees a pipeline containing a large value, this call passes 0, so CmdsStringLimit renders the entire pipeline into the discarded second return value before buildPipelineStatement performs its bounded render. The fresh evidence after the earlier pipeline-capacity fix is this new unbounded summary path: a 1 MiB SET is still copied, and large pipelines are fully formatted despite the default cap; derive the summary directly from command names instead.

Useful? React with 👍 / 👎.

Bound non-struct custom argument formatting

When a valid command argument implements encoding.BinaryMarshaler on a large map, array, or pointer to an array, it reaches this fallback because the bounded branches only cover strings, byte slices, and struct-backed values. The fresh evidence after the struct-backed custom-argument fix is that a type Blob [1<<20]byte with MarshalBinary still makes fmt.Sprint allocate a multi-megabyte decimal representation before the 512-byte truncation is applied; use a bounded placeholder for the remaining custom backing kinds.

Useful? React with 👍 / 👎.

String() runs here with no recover. A value whose String() panics now crashes the tracing hook, and the hook contract (AGENTS.md) says hooks must not panic. Master's fmt.Sprint recovered from it. Call through a helper:

func safeString(s fmt.Stringer) (out string) {
	defer func() {
		if recover() != nil {
			out = fmt.Sprint(s) // fmt recovers the panic itself
		}
	}()
	return s.String()
}

Then use appendStringTruncated(b, safeString(s), limit).

This renders every argument, including 1 MiB values, and discards the statement. It runs on every pipeline, even with db.statement off. The span name only needs the unique command names (up to 10). Add a rediscmd helper that returns those names and use it here, so the summary never renders arguments.

Preserve error-backed redaction before reflecting strings

When a valid argument is a named string that implements both error and encoding.BinaryMarshaler, the old fmt.Sprint path used its Error() representation, but this reflection branch now reads the backing string directly. For a secret wrapper whose Error() returns *** while MarshalBinary() returns hunter2, both default redisotel tracing and existing rediscmd.CmdString callers now export hunter2; honor panic-safe error formatting before exposing the underlying value.

Useful? React with 👍 / 👎.

Guard the recovery fallback against nested formatter panics

When a valid encoding.BinaryMarshaler's String() panics with a value whose own String or Error formatter also panics, this recovery handler calls fmt.Sprint(s), which re-enters String() and Go's nested-panic guard re-panics while formatting the panic value; the panic therefore still escapes ProcessHook before next runs. Fresh evidence after the simple Stringer recovery is this nested formatter path; recover the fallback too or emit a fixed placeholder.

AGENTS.md reference: AGENTS.md:L117-L122

Useful? React with 👍 / 👎.

Avoid materializing full Stringer output before truncation