Skip to content

Advanced Query Processing Architecture #7

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ShashankFC wants to merge 1 commit into from
Closed

Advanced Query Processing Architecture #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions public/app/plugins/datasource/loki/querySplitting.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,21 @@ describe('runSplitQuery()', () => {
});
});

test('Interpolates queries before execution', async () => {
const request = createRequest([{ expr: 'count_over_time({a="b"}[$__auto])', refId: 'A', step: '$step' }]);
datasource = createLokiDatasource({
replace: (input = '') => {
return input.replace('$__auto', '5m').replace('$step', '5m');
},
getVariables: () => [],
});
jest.spyOn(datasource, 'runQuery').mockReturnValue(of({ data: [] }));
await expect(runSplitQuery(datasource, request)).toEmitValuesWith(() => {
expect(jest.mocked(datasource.runQuery).mock.calls[0][0].targets[0].expr).toBe('count_over_time({a="b"}[5m])');
expect(jest.mocked(datasource.runQuery).mock.calls[0][0].targets[0].step).toBe('5m');
});
});

test('Skips partial updates as an option', async () => {
await expect(runSplitQuery(datasource, request, { skipPartialUpdates: true })).toEmitValuesWith((emitted) => {
// 3 days, 3 chunks, 3 requests.
Expand Down
5 changes: 4 additions & 1 deletion public/app/plugins/datasource/loki/querySplitting.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,10 @@ export function runSplitQuery(
request: DataQueryRequest<LokiQuery>,
options: QuerySplittingOptions = {}
) {
const queries = request.targets.filter((query) => !query.hide).filter((query) => query.expr);
const queries = request.targets
.filter((query) => !query.hide)
.filter((query) => query.expr)
.map((query) => datasource.applyTemplateVariables(query, request.scopedVars, request.filters));

@codeant-ai codeant-ai Bot Feb 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: The splitting logic now pre-applies applyTemplateVariables with request.filters, but datasource.runQuery (via DataSourceWithBackend.query) will apply applyTemplateVariables again with the same filters, causing ad hoc filters to be injected twice into the split Loki queries; this can lead to duplicated label filters and unintended query semantics, so the pre-interpolation used only for splitting decisions should not pass request.filters. [logic error]

Severity Level: Major ⚠️ 
- ❌ Split Loki queries send ad hoc filters twice.
- ⚠️ Shard-split queries pre-apply and re-apply ad hoc filters.
- ⚠️ Split and non-split query expressions become inconsistent.
Suggested change
.map((query) => datasource.applyTemplateVariables(query, request.scopedVars, request.filters));
.map((query) => datasource.applyTemplateVariables(query, request.scopedVars));
Steps of Reproduction ✅ 
1. In `public/app/features/query/state/PanelQueryRunner.ts:335`, a dashboard or panel
query against a Loki datasource with ad hoc filters sets `request.filters =
this.templateSrv.getAdhocFilters(ds.name, true);`, so `request.filters` contains one or
more `AdHocVariableFilter` entries.

2. The same request is passed to `LokiDatasource.query()` in
`public/app/plugins/datasource/loki/datasource.ts:299-344`, where, if
`config.featureToggles.lokiQuerySplitting` is enabled and
`requestSupportsSplitting(fixedRequest.targets)` is true, the code executes `return
runSplitQuery(this, fixedRequest);` (line ~338), using `fixedRequest` which still carries
`request.filters`.

3. Inside `runSplitQuery()` in
`public/app/plugins/datasource/loki/querySplitting.ts:291-303`, the code builds `queries`
as:

   `request.targets.filter(...).filter(...).map((query) =>
   datasource.applyTemplateVariables(query, request.scopedVars, request.filters));`

   so `LokiDatasource.applyTemplateVariables()` (defined in `datasource.ts:1110-1141`) is
   invoked once here with `adhocFilters = request.filters`, which appends those filters to
   `target.expr` via `addAdHocFilters()`.

4. `runSplitQuery()` then creates grouped `requests` whose `targets` are these
already-interpolated queries and calls `runSplitGroupedQueries()`, which in turn calls
`datasource.runQuery(subRequest)` (querySplitting.ts:180). `LokiDatasource.runQuery()` at
`datasource.ts:350-357` delegates to `super.query(fixedRequest)`. In
`DataSourceWithBackend.query()`
(`packages/grafana-runtime/src/utils/DataSourceWithBackend.ts:137-192`), each target is
transformed with:

   `...(shouldApplyTemplateVariables ? this.applyTemplateVariables(q, request.scopedVars,
   request.filters) : q)`, so `applyTemplateVariables()` is called a second time with the
   same `request.filters`. Because `applyTemplateVariables()` again calls
   `addAdHocFilters()` on an expression that already includes those filters, each ad hoc
   filter is injected twice into the final LogQL selector sent to the backend. This double
   injection only happens on the splitting path; non-splitting queries call
   `applyTemplateVariables()` exactly once via `DataSourceWithBackend.query()`.
Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** public/app/plugins/datasource/loki/querySplitting.ts
**Line:** 299:299
**Comment:**
	*Logic Error: The splitting logic now pre-applies `applyTemplateVariables` with `request.filters`, but `datasource.runQuery` (via `DataSourceWithBackend.query`) will apply `applyTemplateVariables` again with the same filters, causing ad hoc filters to be injected twice into the split Loki queries; this can lead to duplicated label filters and unintended query semantics, so the pre-interpolation used only for splitting decisions should not pass `request.filters`.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

const [nonSplittingQueries, normalQueries] = partition(queries, (query) => !querySupportsSplitting(query));
const [logQueries, metricQueries] = partition(normalQueries, (query) => isLogsQuery(query.expr));

Expand Down
37 changes: 26 additions & 11 deletions public/app/plugins/datasource/loki/shardQuerySplitting.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,9 @@ describe('runShardSplitQuery()', () => {
request = createRequest([{ expr: '$SELECTOR', refId: 'A', direction: LokiQueryDirection.Scan }]);
datasource = createLokiDatasource();
datasource.languageProvider.fetchLabelValues = jest.fn();
datasource.interpolateVariablesInQueries = jest.fn().mockImplementation((queries: LokiQuery[]) => {
return queries.map((query) => {
query.expr = query.expr.replace('$SELECTOR', '{a="b"}');
return query;
});
datasource.applyTemplateVariables = jest.fn().mockImplementation((query: LokiQuery) => {
query.expr = query.expr.replace('$SELECTOR', '{a="b"}');
return query;
});
jest.mocked(datasource.languageProvider.fetchLabelValues).mockResolvedValue(['1', '10', '2', '20', '3']);
const { metricFrameA } = getMockFrames();
Expand All @@ -83,6 +81,25 @@ describe('runShardSplitQuery()', () => {
});
});

test('Interpolates queries before execution', async () => {
const request = createRequest([{ expr: 'count_over_time({a="b"}[$__auto])', refId: 'A', step: '$step' }]);
datasource = createLokiDatasource({
replace: (input = '') => {
return input.replace('$__auto', '5m').replace('$step', '5m');
},
getVariables: () => [],
});
jest.spyOn(datasource, 'runQuery').mockReturnValue(of({ data: [] }));
datasource.languageProvider.fetchLabelValues = jest.fn();
jest.mocked(datasource.languageProvider.fetchLabelValues).mockResolvedValue(['1', '10', '2', '20', '3']);
await expect(runShardSplitQuery(datasource, request)).toEmitValuesWith(() => {
expect(jest.mocked(datasource.runQuery).mock.calls[0][0].targets[0].expr).toBe(
'count_over_time({a="b", __stream_shard__=~"20|10"} | drop __stream_shard__[5m])'
);
expect(jest.mocked(datasource.runQuery).mock.calls[0][0].targets[0].step).toBe('5m');
});
});

test('Users query splitting for querying over a day', async () => {
await expect(runShardSplitQuery(datasource, request)).toEmitValuesWith(() => {
// 5 shards, 3 groups + empty shard group, 4 requests
Expand All @@ -92,7 +109,7 @@ describe('runShardSplitQuery()', () => {

test('Interpolates queries before running', async () => {
await expect(runShardSplitQuery(datasource, request)).toEmitValuesWith(() => {
expect(datasource.interpolateVariablesInQueries).toHaveBeenCalledTimes(1);
expect(datasource.applyTemplateVariables).toHaveBeenCalledTimes(5);

expect(datasource.runQuery).toHaveBeenCalledWith({
intervalMs: expect.any(Number),
Expand Down Expand Up @@ -153,11 +170,9 @@ describe('runShardSplitQuery()', () => {
});

test('Sends the whole stream selector to fetch values', async () => {
datasource.interpolateVariablesInQueries = jest.fn().mockImplementation((queries: LokiQuery[]) => {
return queries.map((query) => {
query.expr = query.expr.replace('$SELECTOR', '{service_name="test", filter="true"}');
return query;
});
datasource.applyTemplateVariables = jest.fn().mockImplementation((query: LokiQuery) => {
query.expr = query.expr.replace('$SELECTOR', '{service_name="test", filter="true"}');
return query;
});

await expect(runShardSplitQuery(datasource, request)).toEmitValuesWith(() => {
Expand Down
6 changes: 3 additions & 3 deletions public/app/plugins/datasource/loki/shardQuerySplitting.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ import { LokiQuery } from './types';
*/

export function runShardSplitQuery(datasource: LokiDatasource, request: DataQueryRequest<LokiQuery>) {
const queries = datasource
.interpolateVariablesInQueries(request.targets, request.scopedVars)
const queries = request.targets
.filter((query) => query.expr)
.filter((query) => !query.hide);
.filter((query) => !query.hide)
.map((query) => datasource.applyTemplateVariables(query, request.scopedVars, request.filters));

@codeant-ai codeant-ai Bot Feb 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: Similar to time-based splitting, shard splitting now calls applyTemplateVariables with request.filters before delegating to runSplitQuery, but runSplitQuery's sub-requests are executed via datasource.runQuery, which applies applyTemplateVariables again with the same filters, resulting in duplicated ad hoc label filters on sharded queries; using filters only at execution time avoids double application while still letting shard lookup see an interpolated selector. [logic error]

Severity Level: Major ⚠️ 
- ⚠️ Loki shard-split queries duplicate ad hoc label filters.
- ⚠️ Affects `LokiDatasource.query` when sharding feature enabled.
- ⚠️ Extra filters may slow backend Loki query evaluation.
- ⚠️ Shard label lookups still function but queries are bloated.
Suggested change
.map((query) => datasource.applyTemplateVariables(query, request.scopedVars, request.filters));
.map((query) => datasource.applyTemplateVariables(query, request.scopedVars));
Steps of Reproduction ✅ 
1. Enable Loki shard splitting so `LokiDatasource.query()` routes through
`runShardSplitQuery()` when sharding is supported: see
`public/app/plugins/datasource/loki/datasource.ts:299-344`, especially the branch at
`datasource.ts:336-338` calling `runShardSplitQuery(this, fixedRequest)` when
`config.featureToggles.lokiShardSplitting` is true and
`requestSupportsSharding(fixedRequest.targets)` is satisfied.

2. Issue a Loki dashboard/explore query that (a) supports sharding and (b) has ad hoc
filters populated into `request.filters` (standard Grafana ad hoc UI). At runtime this
yields a `DataQueryRequest<LokiQuery>` where `filters` is non-empty, as exercised in
`datasource.test.ts` around the "When using adhoc filters" and `addAdHocFilters` tests
(`datasource.test.ts:229-307, 1232-1315`) and where `targets` contain a log or
metric-over-logs expression.

3. In `runShardSplitQuery()` (`shardQuerySplitting.ts:48-55`), the code builds `queries`
from `request.targets` and calls `datasource.applyTemplateVariables(query,
request.scopedVars, request.filters)` for each target (line 52).
`LokiDatasource.applyTemplateVariables()` (`datasource.ts:1110-1140`) internally calls
`addAdHocFilters()` (`datasource.ts:1073-1091`), which in turn uses `addLabelToQuery()`
(`modifyQuery.ts:147-213`) to splice ad hoc filters into the query expression. For log
queries with a parser (e.g. `{bar="baz"} | logfmt`) and ad hoc filters on labels not
already in the stream selector, `addLabelToQuery()` falls into the
`addFilterAsLabelFilter()` path (`modifyQuery.ts:185-195, 511-540`) which has no
deduplication, so the expression now contains a `| label op` pipeline segment for each ad
hoc filter.

4. Later in the same request, shard splitting creates per-shard sub-requests in
`splitQueriesByStreamShard()` (`shardQuerySplitting.ts:57-209`). For each group, it
computes `targets` from `groups[group].targets` (which are the already-templated
`queries`) and builds `subRequest = { ...request, targets:
interpolateShardingSelector(targets, shardsToQuery) }` at
`shardQuerySplitting.ts:152-155`. This `subRequest` is passed into
`runSplitQuery(datasource, subRequest, ...)` (`shardQuerySplitting.ts:163`), where
`runSplitQuery()` (`querySplitting.ts:291-300`) *again* maps `request.targets` through
`datasource.applyTemplateVariables(query, request.scopedVars, request.filters)`. Because
`subRequest.filters` is the original `request.filters` and the expressions already contain
ad hoc filters from step 3, this second call adds another identical set of label filters
via `addFilterAsLabelFilter()`. The resulting per-shard queries sent via
`datasource.runQuery(subRequest)` contain duplicated ad hoc pipeline filters (e.g. `... |
job=\`grafana\` | job=\`grafana\``), confirming that sharded queries can have ad hoc
filters applied twice in the current code.
Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** public/app/plugins/datasource/loki/shardQuerySplitting.ts
**Line:** 52:52
**Comment:**
	*Logic Error: Similar to time-based splitting, shard splitting now calls `applyTemplateVariables` with `request.filters` before delegating to `runSplitQuery`, but `runSplitQuery`'s sub-requests are executed via `datasource.runQuery`, which applies `applyTemplateVariables` again with the same filters, resulting in duplicated ad hoc label filters on sharded queries; using filters only at execution time avoids double application while still letting shard lookup see an interpolated selector.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎


return splitQueriesByStreamShard(datasource, request, queries);
}
Expand Down
Loading