[vParquet5] Fix buffer reuse bug with event-level dedicated columns by mdisibio · Pull Request #6914 · grafana/tempo

mdisibio · 2026-04-09T16:48:18Z

What this PR does:
There is a bug with buffer reuse in event-level dedicated columns, for event attributes. The buffer was not sufficiently cleared, leading to duplicated attributes across events.

Extended the roundtrip/tempodb level tests to fully populate all options and types now (like integers and blobs), to test holistically, and not just in the encoding itself. Verified they exhibit the bug before fixing.

I updated vulture with more randomness too, but it wasn't able to trigger the bug. I think it requires a higher write volume.

Which issue(s) this PR fixes:
Fixes #

Checklist

Tests updated
Documentation added
CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Copilot

Pull request overview

Extends Tempodb/vParquet round-trip and query tests to better reproduce and prevent a buffer reuse bug affecting event-level dedicated columns (especially for blob/int dedicated attributes).

Changes:

Add dedicated column assignments to tempodb round-trip tests and populate traces with random dedicated attributes (including event-level).
Update vParquet3/4/5 tests to treat dedicated.span.5 as a blob-backed string and use a consistent fixed blob payload for assertions.
Add utilities in pkg/util/test to generate dedicated attributes (including blob-sized payloads).

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`tempodb/tempodb_test.go`	Configures blocks with dedicated columns and enriches traces with dedicated attributes for end-to-end round-trip verification.
`tempodb/encoding/vparquet5/schema_test.go`	Updates expected dedicated attribute values to use the blob test payload.
`tempodb/encoding/vparquet5/block_traceql_test.go`	Updates TraceQL match cases for blob-backed dedicated span attribute.
`tempodb/encoding/vparquet5/block_search_test.go`	Updates search tests to use blob payload for dedicated span attribute.
`tempodb/encoding/vparquet5/block_findtracebyid_test.go`	Strengthens find-by-id test to round-trip fully populated traces and explicitly exercises buffer reuse.
`tempodb/encoding/vparquet4/schema_test.go`	Aligns vParquet4 schema tests with blob payload expectations for `dedicated.span.5`.
`tempodb/encoding/vparquet4/block_traceql_test.go`	Aligns vParquet4 TraceQL tests with blob payload expectations.
`tempodb/encoding/vparquet4/block_search_test.go`	Aligns vParquet4 search tests with blob payload expectations.
`tempodb/encoding/vparquet3/schema_test.go`	Aligns vParquet3 schema tests with blob payload expectations.
`tempodb/encoding/vparquet3/block_traceql_test.go`	Aligns vParquet3 TraceQL tests with blob payload expectations.
`tempodb/encoding/vparquet3/block_search_test.go`	Aligns vParquet3 search tests with blob payload expectations.
`pkg/util/trace_info.go`	Modifies vulture trace tag generation (randomizes presence of some “fixed” tags and well-known attributes).
`pkg/util/test/req.go`	Adds helpers/constants for dedicated column generation, including blob-sized payloads and random dedicated attribute population.

Copilot

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.

Copilot · 2026-04-09T21:50:56Z

+		var (
+			id  = test.ValidTraceID(nil)
+			req = test.MakeTrace(rand.Int()%10, id)
+		)
+
+		// Populate data using the same dedicated columns as configured on the block above.
+		test.AddRandomDedicatedAttributes(req)
+


These tests now rely on randomized dedicated attributes (including per-event subsets) to reproduce the buffer-reuse bug. Because TestCompleteBlock runs many subtests with t.Parallel() and uses the global math/rand, the exact attribute patterns can vary with scheduling, which can make this regression test less deterministic (e.g., in the unlikely case all events happen to set all dedicated columns). What do you think about making the pattern deterministic (fixed seed/local rand, or explicitly constructing at least one event where a dedicated column is omitted) so the bug is guaranteed to be exercised?

Unfortunately I don't think this will work. A static seed that triggers the bug now, could easily stop triggering the bug after innocuous changes and go unnoticed. The randomness has been serving us well for several years, we'll keep it for now.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

Copilot · 2026-04-09T21:56:43Z

 // generateFixedAttributesWithPrefix returns the fixed attributes with a prefix. Keys are lowercase with a hyphen before the numeric suffix (e.g. string-01, int-01, blob-01).
 func (t *TraceInfo) generateFixedAttributesWithPrefix(prefix string) []*jaeger.Tag {
-	return []*jaeger.Tag{
-		{Key: fmt.Sprintf("%s-string-01", prefix), VType: jaeger.TagType_STRING, VStr: stringPtr(t.generateRandomString())},
-		{Key: fmt.Sprintf("%s-string-02", prefix), VType: jaeger.TagType_STRING, VStr: stringPtr(t.generateRandomString())},
-		{Key: fmt.Sprintf("%s-string-03", prefix), VType: jaeger.TagType_STRING, VStr: stringPtr(t.generateRandomString())},
-		{Key: fmt.Sprintf("%s-string-04", prefix), VType: jaeger.TagType_STRING, VStr: stringPtr(t.generateRandomString())},
-		{Key: fmt.Sprintf("%s-string-05", prefix), VType: jaeger.TagType_STRING, VStr: stringPtr(t.generateRandomString())},
-		{Key: fmt.Sprintf("%s-blob-01", prefix), VType: jaeger.TagType_STRING, VStr: stringPtr(t.generateRandomBlob(vultureBlobSize))},
-		{Key: fmt.Sprintf("%s-int-01", prefix), VType: jaeger.TagType_LONG, VLong: int64Ptr(t.generateRandomInt(1, 1000000))},
-		{Key: fmt.Sprintf("%s-int-02", prefix), VType: jaeger.TagType_LONG, VLong: int64Ptr(t.generateRandomInt(1, 1000000))},
-		{Key: fmt.Sprintf("%s-int-03", prefix), VType: jaeger.TagType_LONG, VLong: int64Ptr(t.generateRandomInt(1, 1000000))},
-		{Key: fmt.Sprintf("%s-int-04", prefix), VType: jaeger.TagType_LONG, VLong: int64Ptr(t.generateRandomInt(1, 1000000))},
-		{Key: fmt.Sprintf("%s-int-05", prefix), VType: jaeger.TagType_LONG, VLong: int64Ptr(t.generateRandomInt(1, 1000000))},
+	var (
+		numStrings = t.r.Intn(6)
+		numBlobs   = t.r.Intn(2)
+		numInts    = t.r.Intn(6)
+		tags       = make([]*jaeger.Tag, 0, numStrings+numBlobs+numInts)
+	)


generateFixedAttributesWithPrefix (and its comment) now returns a random count of tags with random values, so it’s no longer “fixed”. This is confusing for callers/readers and makes the docstring inaccurate. Could we either rename the helper (and/or update the comment) to reflect the new randomized behavior?

…6914) * Add failing tests at multiple levels that trigger the bug * Update tests, lint, and fix vp5 actual bug * review feedback * changelog * Update CHANGELOG.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update tempodb/tempodb_test.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Add failing tests at multiple levels that trigger the bug

3e96386

Copilot AI review requested due to automatic review settings April 9, 2026 16:48

mdisibio requested review from carles-grafana, electron0zero, ie-pham, javiermolinar, mapno, mattdurham, oleg-kozlyuk-grafana, ruslan-mikhailov, stoewer, yvrhdn and zalegrala as code owners April 9, 2026 16:48

mdisibio marked this pull request as draft April 9, 2026 16:48

Copilot started reviewing on behalf of mdisibio April 9, 2026 16:49 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

Comment thread pkg/util/test/req.go Outdated

Comment thread pkg/util/trace_info.go Outdated

Comment thread pkg/util/trace_info.go Outdated

mdisibio added 3 commits April 9, 2026 13:28

Update tests, lint, and fix vp5 actual bug

0d58ec8

review feedback

98af431

changelog

42dc8a3

Copilot AI review requested due to automatic review settings April 9, 2026 21:46

Copilot started reviewing on behalf of mdisibio April 9, 2026 21:46 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

Update CHANGELOG.md

f89234b

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings April 9, 2026 21:52

Copilot started reviewing on behalf of mdisibio April 9, 2026 21:53 View session

Update tempodb/tempodb_test.go

702c097

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI reviewed Apr 9, 2026

View reviewed changes

mdisibio marked this pull request as ready for review April 9, 2026 22:07

javiermolinar approved these changes Apr 10, 2026

View reviewed changes

mapno approved these changes Apr 10, 2026

View reviewed changes

mapno mentioned this pull request Apr 10, 2026

Fix stale event DedicatedAttributes when Trace buffer is reused #6878

Closed

3 tasks

mdisibio changed the title ~~WIP: [vParquet5] Fix buffer reuse bug with event-level dedicated columns~~ [vParquet5] Fix buffer reuse bug with event-level dedicated columns Apr 10, 2026

mdisibio merged commit 14d0452 into grafana:main Apr 10, 2026
27 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[vParquet5] Fix buffer reuse bug with event-level dedicated columns#6914

[vParquet5] Fix buffer reuse bug with event-level dedicated columns#6914
mdisibio merged 6 commits intografana:mainfrom
mdisibio:vp5-event-buffer-reuse-fix

mdisibio commented Apr 9, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

mdisibio Apr 9, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

mdisibio commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

mdisibio Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mdisibio commented Apr 9, 2026 •

edited

Loading