Skip to content

app/vtselect: honor extra_filters in Jaeger and Tempo APIs#184

Open
immanuwell wants to merge 1 commit into
VictoriaMetrics:masterfrom
immanuwell:fix-extra-filters-trace-apis
Open

app/vtselect: honor extra_filters in Jaeger and Tempo APIs#184
immanuwell wants to merge 1 commit into
VictoriaMetrics:masterfrom
immanuwell:fix-extra-filters-trace-apis

Conversation

@immanuwell

@immanuwell immanuwell commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Describe Your Changes

Fixes #178

extra_filters worked on native LogsQL endpoints, but Jaeger and Tempo paths just dropped it. That is kinda bad in auth scoped setups, because vmauth can inject ?extra_filters=... in normal traffic.

This patch parses extra_filters in shared trace request params and applies them to the trace query builders, including search, tags, traces and Tempo metrics. Also adds regression tests for parse, merge and bad input.

Repro:

  1. Put VictoriaTraces behind vmauth, or call a Jaeger or Tempo endpoint with ?extra_filters={"env":"prod"}.
  2. Hit a trace API like /select/tempo/api/search or /select/tempo/api/v2/search/tags.
  3. Before this patch the filter is ignored, so results are wider than expected.
  4. After this patch the same constraint is applied, nice and boring.

Tests:
make test-full
make apptest

Checklist


Summary by cubic

Apply extra_filters to Jaeger and Tempo trace APIs so auth-scoped setups using vmauth get the same filtering as LogsQL. This prevents overly broad results and keeps queries within tenant constraints.

  • Bug Fixes
    • Parse extra_filters from requests and store in tracecommon.CommonParams (ExtraFilters).
    • Apply filters via CommonParams.ApplyExtraFilters across trace queries (search, tags, traces, service graph, metrics, exemplars).
    • Support both JSON object and LogsQL filter formats; merge multiple extra_filters values.
    • Add tests for applying, merging, and rejecting invalid filters.

Written for commit bce1244. Summary will update on new commits.

Review in cubic

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 6 files

Re-trigger cubic

Comment thread app/vtselect/traces/tracecommon/tracecommon.go Outdated
Comment thread app/vtselect/traces/query/query.go Outdated
@immanuwell
immanuwell force-pushed the fix-extra-filters-trace-apis branch from bce1244 to 18d19f5 Compare July 12, 2026 07:36
@immanuwell

Copy link
Copy Markdown
Contributor Author

@JayiceZ fixed both suggestions in the latest push, PTAL

@immanuwell
immanuwell requested a review from JayiceZ July 12, 2026 07:37

@JayiceZ JayiceZ left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

extra_filters should be only applied to the http query from users, not all the query in the codebase. Please check.

defer cancel()

cp.Query = q
cp.Query = cp.ApplyExtraFilters(q)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

findTraceIDTimeSplitTimeRange will still apply extra filter

q.AddPipeOffsetLimit(0, limit)

cp.Query = q
cp.Query = cp.ApplyExtraFilters(q)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

internal request should not apply extra filters.

GetServiceGraphTimeRange and GetServiceDBGraphTimeRange are called by a background task, not users request.

}

cp.Query = q
cp.Query = cp.ApplyExtraFilters(q)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't like executing this function everywhere. The extra filter should work just like hiddenFieldsFilters that live only in common path.

@immanuwell I'd like to rewrite it if you don't mind. And maybe you can talk an extra look after.

@jiekun

jiekun commented Jul 22, 2026

Copy link
Copy Markdown
Member

Generally the files under logsql should remain unchanged. The logsql folder is preserved in this repo to use the logsQL APIs exposed by VictoriaLogs. With the changes in this repo, it means every time we sync from the upstream, we have to do the same function deletion and refactoring. Please revert the changes in logsql and just move them to the tracecommon.

@jiekun jiekun self-assigned this Jul 22, 2026
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.

Jaeger and Tempo APIs do not support extra_filters query parameter

3 participants