Fix panic in old tag-style search + integer dedicated columns + vp5 - #7135
Merged
Conversation
mdisibio
requested review from
carles-grafana,
electron0zero,
ie-pham,
javiermolinar,
mapno,
mattdurham,
oleg-kozlyuk-grafana,
ruslan-mikhailov,
stoewer,
yvrhdn and
zalegrala
as code owners
May 8, 2026 20:13
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a panic in vParquet5 tag-style search when a tag maps to an integer dedicated column (previously a string/substring predicate could be applied to an int column).
Changes:
- Add dedicated-column type handling in
makePipelineWithRowGroups, using an integer equality predicate when the dedicated column isintand the query value parses as an integer. - Update vParquet5 block search tests to include an integer dedicated resource attribute and a corresponding tag-style search case.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
tempodb/encoding/vparquet5/block_search.go |
Routes tag-style dedicated-column filters to int-vs-string predicates based on dedicated column type to avoid panics. |
tempodb/encoding/vparquet5/block_search_test.go |
Adds test data + a tag-style request exercising integer dedicated-column search. |
3 tasks
knylander-grafana
added a commit
that referenced
this pull request
May 20, 2026
* fix: add recover to livestore iterateblocks (#7134) (cherry picked from commit 7b250d1) * Fix panic in old tag-style search + integer dedicated columns + vp5 (#7135) * Fix old tag-style search handling of integer dedicated columns * Cleanup test test * Changelog * Review feedback, additional tests (cherry picked from commit 2f74ea8) * Tempo 3.0 migration: docker-compose example + doc tweaks (#7122) * docs: tighten Tempo 3.0 migration validation steps - Fix Vulture metric name: tempo_vulture_trace_mismatches_total -> tempo_vulture_trace_error_total (the former never existed). - Note that live-store readiness can take up to readiness_max_wait on a fresh deployment with an empty Kafka topic. - Replace vague "confirm all components are healthy" with an explicit pre-cutover checklist. * example: docker-compose for Tempo 2.x -> 3.0 microservices migration Self-contained reproduction of the migration playbook documented at docs/sources/tempo/set-up-for-tracing/setup-tempo/migrate-to-3.md. Profiles: v2 (Tempo 2.9.2 microservices), v3 (Tempo 3.0.0-rc.1 with Redpanda + Vulture), shared infra (MinIO, Prometheus, Grafana, Alloy as the trace router, k6 trace generator). Both deployments share the same MinIO bucket. Switch traffic with a single env-var change on Alloy. README walks through deploy -> validate -> switch -> drain -> decommission -> verify, mirroring the doc structure. * fix typo * example: use Grafana Explore for metric checks in README Replace curl + jq snippets with the equivalent PromQL queries to be run in Grafana Explore against the Prometheus datasource (already deployed). The trace-by-id validation in step 3 also moves to a Grafana flow: search in the Tempo 2.x datasource, copy a trace ID, paste it into Tempo 3.0. * example: tie step 3 wait time to max_block_duration The 6 min wait was unexplained. It is max_block_duration (5 min) plus ~1 min of flush overhead, which is the upper bound on when a freshly ingested trace is guaranteed to be in object storage. * Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Address remaining Copilot review comments - backend_scheduler_addr: 3200 -> 9095 (correct gRPC port; matches jsonnet and integration test configs). - Add tempo-v3-backend-scheduler to backend-worker depends_on. - Clarify RF wording in step 3 (this example uses RF=1). * docs: apply review suggestions from @knylander-grafana - Lead with "Validate..." instead of "It is highly recommended..." - Reword the pre-cutover guard as an imperative. * example: surface compaction_disabled check at top of step 2 Per @javiermolinar review: this is a safety check (concurrent compaction against shared storage corrupts blocks), so it should be the first thing the operator confirms after `docker compose up`, not after the health checks. * example: address two Copilot findings - Update tempo-v3.yaml header comment: block_builder uses ordinal partitions_per_instance, not explicit assigned_partitions. - Make redpanda-init idempotent by creating the topic with 2 partitions directly; previous version kept incrementing the partition count on every re-run via add-partitions. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> (cherry picked from commit 7b54a5d) * [DOC] Tempo 3.0 release notes (#7121) (cherry picked from commit 7ae4436) * chore: prepare changelog for v3.0.0 --------- Co-authored-by: Xiaoguang Zhang <shawn.zhang@grafana.com> Co-authored-by: Martin Disibio <martin.disibio@grafana.com> Co-authored-by: Mario <mapno0@proton.me> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Kim Nylander <104772500+knylander-grafana@users.noreply.github.com>
mattdurham
pushed a commit
to mattdurham/tempo
that referenced
this pull request
Jun 18, 2026
…rafana#7135) * Fix old tag-style search handling of integer dedicated columns * Cleanup test test * Changelog * Review feedback, additional tests
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.
What this PR does:
If you try to use the old tag-style search against a vParquet5 new integer dedicated column, it will panic, by trying to use a string predicate against an integer. For performance we don't check every individual value, the type checks are done much higher up. This fixes the tag-style search. This is new because vParquet4 didn't have dedicated columns for integers, only the hard-code ones which were handled as expected.
This is most easily triggered by searching
/api/search?http.status_code=500because it is a common integer attribute.Which issue(s) this PR fixes:
Fixes #
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]