Skip to content

Remove rf1_after from query frontend and query paths#6969

Merged
mapno merged 2 commits intografana:mainfrom
mapno:remove-rf1-after-frontend
Apr 15, 2026
Merged

Remove rf1_after from query frontend and query paths#6969
mapno merged 2 commits intografana:mainfrom
mapno:remove-rf1-after-frontend

Conversation

@mapno
Copy link
Copy Markdown
Contributor

@mapno mapno commented Apr 15, 2026

What this PR does:

Simplifies the 2.x to 3.0 migration by removing the need to configure rf1_after. Non-metric query paths (search, tags, trace-by-ID) now inspect all blocks regardless of replication factor, with deduplication at query time. This means mixed RF3/RF1 block stores work without configuration.

Metric queries continue to select RF1 blocks only, as they require deduplicated data.

The rf1_after config field and proto fields are kept for backward compatibility, but the intention is to remove them before the v3.0 release.

Which issue(s) this PR fixes:
Fixes #6960

Checklist

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

Copilot AI review requested due to automatic review settings April 15, 2026 08:46
@mapno mapno force-pushed the remove-rf1-after-frontend branch from 6db175f to 3deda20 Compare April 15, 2026 08:48
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Removes rf1_after-based replication-factor filtering from non-metrics query paths so mixed RF3/RF1 block stores can be queried without special migration configuration, while keeping metrics queries restricted to RF1 blocks.

Changes:

  • Stop filtering backend blocks by replication factor for trace-by-ID, search, and tag/tag-values queries (query all blocks and dedupe at query time).
  • Remove rf1After HTTP query parameter handling and frontend propagation/sharder logic for non-metrics paths.
  • Mark rf1_after as deprecated/ignored in config and update examples/docs/tests accordingly (metrics path remains RF1-only).

Reviewed changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
tempodb/tempodb.go Drops RF-based inclusion logic for backend block selection in Find/compacted blocks.
tempodb/encoding/common/interfaces.go Removes RF1After from SearchOptions.
pkg/tempopb/tempo.proto Keeps RF1After proto fields but annotates as deprecated via comments.
pkg/tempopb/tempo.pb.go Regenerated output reflecting proto comment/format changes.
pkg/api/search_tags.go Removes parsing of rf1After from tag/tag-values HTTP request parsing.
pkg/api/http.go Removes rf1After URL param constant, parsing, and request-building propagation; updates trace-by-ID validation signature.
pkg/api/http_test.go Updates ValidateAndSanitizeRequest tests for new return signature (no rf1After).
modules/querier/querier.go Stops passing RF1After into store search options for trace-by-ID.
modules/querier/http.go Stops reading/logging/forwarding rf1After for trace-by-ID handlers.
modules/frontend/util.go Replaces rf1FilterFn with an acceptAllBlocks filter.
modules/frontend/traceid_sharder.go Stops propagating rf1After into sharded trace-by-ID backend requests.
modules/frontend/traceid_handlers.go Updates validation call sites for new signature (no rf1After).
modules/frontend/tag_sharder.go Removes rf1After from tag sharding interface and queries all blocks.
modules/frontend/tag_sharder_test.go Updates fake request type to match tag sharder interface change.
modules/frontend/search_sharder.go Removes RF1After config and queries all blocks in backend sharding.
modules/frontend/search_sharder_test.go Removes RF1After filtering test (no longer applicable).
modules/frontend/search_handlers_test.go Updates expected job/block metrics due to scanning more blocks.
modules/frontend/frontend.go Removes RF1After propagation wiring; clarifies filterFn is generic.
modules/frontend/config.go Marks rf1_after config as deprecated/ignored (still accepted).
integration/util/config-base.yaml Removes query_frontend.rf1_after from integration base config.
example/docker-compose/*/tempo.yaml Removes query_frontend.rf1_after from example configs.
docs/sources/tempo/configuration/manifest.md Removes rf1_after from the rendered config manifest snippet.
cmd/tempo-vulture/main.go Deprecates the vulture flag help text and removes rf1After query-param injection.

Comment thread cmd/tempo-vulture/main.go Outdated
Comment thread docs/sources/tempo/configuration/manifest.md
Comment thread modules/frontend/config.go Outdated
Comment thread modules/frontend/config.go Outdated
Comment thread pkg/tempopb/tempo.proto
@mapno mapno force-pushed the remove-rf1-after-frontend branch from 3deda20 to dd83352 Compare April 15, 2026 09:06
Copilot AI review requested due to automatic review settings April 15, 2026 11:18
@mapno mapno force-pushed the remove-rf1-after-frontend branch from dd83352 to 5debdea Compare April 15, 2026 11:18
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 1 comment.

Comment thread tempodb/tempodb.go
@mapno mapno force-pushed the remove-rf1-after-frontend branch from 5debdea to 90e44c3 Compare April 15, 2026 11:25
Query all blocks regardless of replication factor for non-metrics paths
(search, tags, trace-by-ID). Metrics queries continue to filter RF1-only.

The rf1_after config field is kept but deprecated for YAML backward
compatibility. Proto fields are marked deprecated for wire compatibility.

Fixes grafana#6960
Copilot AI review requested due to automatic review settings April 15, 2026 11:40
@mapno mapno force-pushed the remove-rf1-after-frontend branch from 90e44c3 to 3fccc56 Compare April 15, 2026 11:40
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread tempodb/encoding/common/interfaces.go
Comment thread pkg/api/http.go
Comment thread cmd/tempo-vulture/main.go
@mapno mapno marked this pull request as ready for review April 15, 2026 13:12
Copilot AI review requested due to automatic review settings April 15, 2026 13:12
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread pkg/api/http.go
URLParamRF1After = "rf1After"
urlMaxSeries = "maxSeries"
urlInstant = "instant"

Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

URLParamRF1After was an exported constant in pkg/api and is now removed. Even if the query param is deprecated/ignored, removing the symbol is a Go API breaking change for any downstream code importing this module (including external tooling). Consider keeping the constant (deprecated in a comment) so existing code still compiles, while leaving the parsing behavior as-is (ignored).

Suggested change
// Deprecated: URLParamRF1After is kept for source compatibility only.
// The corresponding query parameter is deprecated and ignored.
URLParamRF1After = "rf1_after"

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine

@@ -41,7 +41,7 @@ type Config struct {
AllowedHeaders []string `yaml:"allowed_headers,omitempty"`

// RF1After specifies the time after which RF1 logic is applied.
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The field-level comment for RF1After still says it "specifies the time after which RF1 logic is applied", but the same line now marks it as deprecated/ignored. Can we update the comment to avoid implying it still affects behavior (and maybe point users to the new behavior: non-metrics query paths ignore RF entirely)?

Suggested change
// RF1After specifies the time after which RF1 logic is applied.
// RF1After is deprecated and ignored. Non-metrics query paths ignore RF entirely.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's marked as deprecated just below.

Comment thread cmd/tempo/app/config.go
Copy link
Copy Markdown
Contributor

@mattdurham mattdurham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm minor comment

@mapno mapno merged commit 0c5f841 into grafana:main Apr 15, 2026
32 checks passed
@mapno mapno deleted the remove-rf1-after-frontend branch April 15, 2026 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Query all block replication factors without rf1_after

3 participants