Fix stale goroutine label, validate UTF-8 on label strings#1454
Open
gnurizen wants to merge 1 commit into
Open
Fix stale goroutine label, validate UTF-8 on label strings#1454gnurizen wants to merge 1 commit into
gnurizen wants to merge 1 commit into
Conversation
19372b1 to
108290a
Compare
This regressed in 40dd9d9 ("ebpf: simplify get_pristine_per_cpu_record" open-telemetry#1091), which removed an explicit zeroing of trace->custom_labels.labels on the assumption that the slots were only read after a successful write that fully populated them. The slots are populated via bpf_probe_read_user, which only writes the bytes it reads, so a key/value shorter than one previously written to the same per-CPU slot inherits the trailing bytes from the prior trace and produces a corrupted label. The eBPF side now writes a single NUL after each bpf_probe_read_user; userspace stops at the first NUL, so one terminator is sufficient. On the Go side, label keys and values are validated as UTF-8 (required by OTLP/pprof), with deliberately asymmetric strictness: - Keys are strict: any invalid byte (including a single split-rune continuation at the end) drops the whole label. A corrupted key would silently group unrelated samples under a garbage name. - Values are lenient: on fixed-width truncation that splits a multi-byte rune we salvage the longest valid UTF-8 prefix rather than drop the label, so a clipped request_id or customer_name still arrives with everything up to the broken rune intact. Two metrics count labels dropped for each reason. Comm is left as best-effort: it's kernel-supplied, almost always ASCII, and we don't have a useful fallback if it isn't.
108290a to
b1d5bac
Compare
This was referenced May 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This regressed in 40dd9d9 ("ebpf: simplify
get_pristine_per_cpu_record" #1091), which removed the explicit zeroing oftrace->custom_labels.labelson the assumption that the slots were only read after a successful write that fully populated them. The slots are populated viabpf_probe_read_user, which only writes the bytes it reads, so a key/value shorter than one previously written to the same per-CPU slot inherits trailing bytes from the prior trace and produces a corrupted label.bpf_probe_read_user.request_idorcustomer_namestill arrives with everything up to the broken rune intact.IDGoLabelsDroppedInvalidName,IDGoLabelsDroppedInvalidValue) count labels dropped for each reason.commis left as best-effort: kernel-supplied, almost always ASCII, no useful fallback if it isn't.Supersedes #1453.