app/vtselect: apply extra_filters to the Jaeger and Tempo APIs

[Vandit1604]

Describe Your Changes

The Jaeger and Tempo APIs ignored the extra_filters and extra_stream_filters query args.

Only getCommonParams in app/vtselect/logsql/logsql.go read these args. That function serves the native LogsQL endpoints. Jaeger and Tempo use tracecommon.GetCommonParams, which did not read them.

vmauth injects these args to keep a user inside its own data. When they are dropped, that user sees other services.

This is master. Two services, alpha and beta, and extra_filters={"resource_attr:service.name":"alpha"}:

GET /select/logsql/query?query=*&extra_filters=...    -> alpha only        applied
GET /select/jaeger/api/services?extra_filters=...     -> ["alpha","beta"]  ignored
GET /select/jaeger/api/services/beta/operations?...   -> ["op-beta"]       ignored
GET /select/tempo/api/v2/search/tag/.../values?...    -> alpha and beta    ignored

After the change the last three return ["alpha"], [] and alpha.

How

Every Jaeger and Tempo path sets cp.Query and then calls cp.NewQueryContext. So GetCommonParams parses the args once, and NewQueryContext applies them. Every query endpoint of both APIs goes through that point.

parseExtraFilters and the two functions next to it moved from app/vtselect/logsql into a new app/vtselect/extrafilters package, because both sides need them now. The bodies do not change. Their tests moved with them.

The background service graph task builds its own CommonParams from a tenant ID. It has no HTTP request, so it stays unfiltered.

One other change

A malformed extra_filters now returns a 4xx from the Jaeger and Tempo APIs. Before, the arg was ignored. The LogsQL endpoints already behave this way.

Tests

apptest/tests/extra_filters_test.go ingests two services. It checks Jaeger services, Jaeger operations and the Tempo tag values API. I added a small TempoAPITagValues helper, because apptest had no Tempo support.

app/vtselect/traces/tracecommon/tracecommon_test.go covers the parsing, repeated args, malformed input, and that NewQueryContext applies the filters.

I removed the fix and ran the tests. All three end to end checks failed, and the unit test failed.

Checklist

• Tests added
• Changelog entry added
• make vet fmt pass, full test suite passes
• Commits are signed

Fixes #178

[cubic-dev-ai]

1 issue found across 8 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="app/vtselect/traces/tracecommon/tracecommon.go">

<violation number="1" location="app/vtselect/traces/tracecommon/tracecommon.go:72">
P1: When extra_filters is set, the Jaeger/Tempo trace-by-ID lookup (`/api/traces/<trace_id>`) returns empty for every trace, even ones inside the allowed tenant. NewQueryContext applies the extra filter to cp.Query before every query runs, including the internal trace-ID index lookup in findTraceIDTimeSplitTimeRange. Index rows only carry trace_id_idx, trace_id, start_time, end_time and _time (never resource_attr:service.name or user attributes), so a field filter like `"resource_attr:service.name":=alpha` matches none of them, the lookup fails with ErrOutOfRetention and GetTrace returns nil. Apply the extra filters only to the span-data queries (e.g. the traceID-list and span queries), not to the trace_id index-stream lookup, or the added e2e test never covers this path.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

P1: When extra_filters is set, the Jaeger/Tempo trace-by-ID lookup (/api/traces/<trace_id>) returns empty for every trace, even ones inside the allowed tenant. NewQueryContext applies the extra filter to cp.Query before every query runs, including the internal trace-ID index lookup in findTraceIDTimeSplitTimeRange. Index rows only carry trace_id_idx, trace_id, start_time, end_time and _time (never resource_attr:service.name or user attributes), so a field filter like "resource_attr:service.name":=alpha matches none of them, the lookup fails with ErrOutOfRetention and GetTrace returns nil. Apply the extra filters only to the span-data queries (e.g. the traceID-list and span queries), not to the trace_id index-stream lookup, or the added e2e test never covers this path.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At app/vtselect/traces/tracecommon/tracecommon.go, line 72:

<comment>When extra_filters is set, the Jaeger/Tempo trace-by-ID lookup (`/api/traces/<trace_id>`) returns empty for every trace, even ones inside the allowed tenant. NewQueryContext applies the extra filter to cp.Query before every query runs, including the internal trace-ID index lookup in findTraceIDTimeSplitTimeRange. Index rows only carry trace_id_idx, trace_id, start_time, end_time and _time (never resource_attr:service.name or user attributes), so a field filter like `"resource_attr:service.name":=alpha` matches none of them, the lookup fails with ErrOutOfRetention and GetTrace returns nil. Apply the extra filters only to the span-data queries (e.g. the traceID-list and span queries), not to the trace_id index-stream lookup, or the added e2e test never covers this path.</comment>

<file context>
@@ -57,11 +58,21 @@ type CommonParams struct {
 func (cp *CommonParams) NewQueryContext(ctx context.Context) *logstorage.QueryContext {
+	// Every traces API builds its own Query and then calls this, so applying the extra filters
+	// here covers all of them. AddExtraFilters ANDs the filters in, so a repeated call is harmless.
+	for _, f := range cp.ExtraFilters {
+		cp.Query.AddExtraFilters(f)
+	}
</file context>

[Vandit1604]

Closing this. I missed #184, which does the same thing and has been open since June.

Sorry @immanuwell, that one is yours. I should have checked the open PRs on the issue before starting.